mudhorn/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2025-01-11 04:47:29 -08:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

fix(editor): sending data as query on DELETE method (#3972)

Browse source 
* 🐛 fix sending data as query on DELETE method

* 👕 add type to imports

* 💪 enhance test
This commit is contained in:
Ben Hesseldieck 2022-08-30 11:54:50 +02:00 committed by GitHub
parent c2e97a89f9
commit fc2ff35c41
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 89 additions and 115 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
packages/cli/src/UserManagement/routes/users.ts
View file

@ -1,13 +1,17 @@
/* eslint-disable no-restricted-syntax */
/* eslint-disable import/no-cycle */
import { Response } from 'express';
import { LoggerProxy as Logger } from 'n8n-workflow';
import { In } from 'typeorm';
import validator from 'validator';
import { LoggerProxy as Logger } from 'n8n-workflow';
import { Db, InternalHooksManager, ITelemetryUserDeletionData, ResponseHelper } from '../..';
import { N8nApp, PublicUser } from '../Interfaces';
import { SharedCredentials } from '../../databases/entities/SharedCredentials';
import { SharedWorkflow } from '../../databases/entities/SharedWorkflow';
import { User } from '../../databases/entities/User';
import { UserRequest } from '../../requests';
import * as UserManagementMailer from '../email/UserManagementMailer';
import { N8nApp, PublicUser } from '../Interfaces';
import {
getInstanceBaseUrl,
hashPassword,
@ -16,10 +20,6 @@ import {
sanitizeUser,
validatePassword,
} from '../UserManagementHelper';
import { User } from '../../databases/entities/User';
import { SharedWorkflow } from '../../databases/entities/SharedWorkflow';
import { SharedCredentials } from '../../databases/entities/SharedCredentials';
import * as UserManagementMailer from '../email/UserManagementMailer';
import * as config from '../../../config';
import { issueCookie } from '../auth/jwt';

32
packages/cli/src/api/nodes.api.ts
View file

@ -2,36 +2,36 @@
import express from 'express';
import { PublicInstalledPackage } from 'n8n-workflow';
import { InternalHooksManager, LoadNodesAndCredentials, Push, ResponseHelper } from '..';
import config from '../../config';
import { ResponseHelper, LoadNodesAndCredentials, Push, InternalHooksManager } from '..';
import {
RESPONSE_ERROR_MESSAGES,
UNKNOWN_FAILURE_REASON,
STARTER_TEMPLATE_NAME,
} from '../constants';
import {
checkNpmPackageStatus,
executeCommand,
hasPackageLoaded,
isClientError,
isNpmError,
matchMissingPackages,
matchPackagesWithUpdates,
executeCommand,
checkNpmPackageStatus,
hasPackageLoaded,
removePackageFromMissingList,
parseNpmPackageName,
isClientError,
removePackageFromMissingList,
sanitizeNpmPackageName,
isNpmError,
} from '../CommunityNodes/helpers';
import {
getAllInstalledPackages,
findInstalledPackage,
getAllInstalledPackages,
isPackageInstalled,
} from '../CommunityNodes/packageModel';
import {
RESPONSE_ERROR_MESSAGES,
STARTER_TEMPLATE_NAME,
UNKNOWN_FAILURE_REASON,
} from '../constants';
import { isAuthenticatedRequest } from '../UserManagement/UserManagementHelper';
import type { NodeRequest } from '../requests';
import type { CommunityPackages } from '../Interfaces';
import { InstalledPackages } from '../databases/entities/InstalledPackages';
import type { CommunityPackages } from '../Interfaces';
import type { NodeRequest } from '../requests';
const { PACKAGE_NOT_INSTALLED, PACKAGE_NAME_NOT_PROVIDED } = RESPONSE_ERROR_MESSAGES;
@ -223,7 +223,7 @@ nodesController.get(
nodesController.delete(
'/',
ResponseHelper.send(async (req: NodeRequest.Delete) => {
const { name } = req.body;
const { name } = req.query;
if (!name) {
throw new ResponseHelper.ResponseError(PACKAGE_NAME_NOT_PROVIDED, undefined, 400);

8
packages/cli/src/requests.d.ts vendored
View file

@ -11,11 +11,11 @@ import {
IWorkflowSettings,
} from 'n8n-workflow';
import { User } from './databases/entities/User';
import { Role } from './databases/entities/Role';
import type { IExecutionDeleteFilter, IWorkflowDb } from '.';
import type { PublicUser } from './UserManagement/Interfaces';
import type { Role } from './databases/entities/Role';
import type { User } from './databases/entities/User';
import * as UserManagementMailer from './UserManagement/email/UserManagementMailer';
import type { PublicUser } from './UserManagement/Interfaces';
export type AuthlessRequest<
RouteParams = {},
@ -302,7 +302,7 @@ export declare namespace NodeRequest {
type Post = AuthenticatedRequest<{}, {}, { name?: string }>;
type Delete = Post;
type Delete = AuthenticatedRequest<{}, {}, {}, { name: string }>;
type Update = Post;
}

65
packages/cli/test/integration/credentials.api.test.ts
View file

@ -1,7 +1,7 @@
import express from 'express';
import { UserSettings } from 'n8n-core';
import { Db } from '../../src';
import { randomName, randomString } from './shared/random';
import { randomCredentialPayload, randomName, randomString } from './shared/random';
import * as utils from './shared/utils';
import type { CredentialPayload, SaveCredentialFunction } from './shared/types';
import type { Role } from '../../src/databases/entities/Role';
@ -49,7 +49,7 @@ test('POST /credentials should create cred', async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
const payload = credentialPayload();
const payload = randomCredentialPayload();
const response = await authOwnerAgent.post('/credentials').send(payload);
@ -97,7 +97,7 @@ test('POST /credentials should fail with missing encryption key', async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
const response = await authOwnerAgent.post('/credentials').send(credentialPayload());
const response = await authOwnerAgent.post('/credentials').send(randomCredentialPayload());
expect(response.statusCode).toBe(500);
@ -110,13 +110,13 @@ test('POST /credentials should ignore ID in payload', async () => {
const firstResponse = await authOwnerAgent
.post('/credentials')
.send({ id: '8', ...credentialPayload() });
.send({ id: '8', ...randomCredentialPayload() });
expect(firstResponse.body.data.id).not.toBe('8');
const secondResponse = await authOwnerAgent
.post('/credentials')
.send({ id: 8, ...credentialPayload() });
.send({ id: 8, ...randomCredentialPayload() });
expect(secondResponse.body.data.id).not.toBe(8);
});
@ -124,7 +124,7 @@ test('POST /credentials should ignore ID in payload', async () => {
test('DELETE /credentials/:id should delete owned cred for owner', async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
const savedCredential = await saveCredential(credentialPayload(), { user: ownerShell });
const savedCredential = await saveCredential(randomCredentialPayload(), { user: ownerShell });
const response = await authOwnerAgent.delete(`/credentials/${savedCredential.id}`);
@ -144,7 +144,7 @@ test('DELETE /credentials/:id should delete non-owned cred for owner', async ()
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
const member = await testDb.createUser({ globalRole: globalMemberRole });
const savedCredential = await saveCredential(credentialPayload(), { user: member });
const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });
const response = await authOwnerAgent.delete(`/credentials/${savedCredential.id}`);
@ -163,7 +163,7 @@ test('DELETE /credentials/:id should delete non-owned cred for owner', async ()
test('DELETE /credentials/:id should delete owned cred for member', async () => {
const member = await testDb.createUser({ globalRole: globalMemberRole });
const authMemberAgent = utils.createAgent(app, { auth: true, user: member });
const savedCredential = await saveCredential(credentialPayload(), { user: member });
const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });
const response = await authMemberAgent.delete(`/credentials/${savedCredential.id}`);
@ -183,7 +183,7 @@ test('DELETE /credentials/:id should not delete non-owned cred for member', asyn
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const member = await testDb.createUser({ globalRole: globalMemberRole });
const authMemberAgent = utils.createAgent(app, { auth: true, user: member });
const savedCredential = await saveCredential(credentialPayload(), { user: ownerShell });
const savedCredential = await saveCredential(randomCredentialPayload(), { user: ownerShell });
const response = await authMemberAgent.delete(`/credentials/${savedCredential.id}`);
@ -210,8 +210,8 @@ test('DELETE /credentials/:id should fail if cred not found', async () => {
test('PATCH /credentials/:id should update owned cred for owner', async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
const savedCredential = await saveCredential(credentialPayload(), { user: ownerShell });
const patchPayload = credentialPayload();
const savedCredential = await saveCredential(randomCredentialPayload(), { user: ownerShell });
const patchPayload = randomCredentialPayload();
const response = await authOwnerAgent
.patch(`/credentials/${savedCredential.id}`)
@ -245,8 +245,8 @@ test('PATCH /credentials/:id should update non-owned cred for owner', async () =
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
const member = await testDb.createUser({ globalRole: globalMemberRole });
const savedCredential = await saveCredential(credentialPayload(), { user: member });
const patchPayload = credentialPayload();
const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });
const patchPayload = randomCredentialPayload();
const response = await authOwnerAgent
.patch(`/credentials/${savedCredential.id}`)
@ -279,8 +279,8 @@ test('PATCH /credentials/:id should update non-owned cred for owner', async () =
test('PATCH /credentials/:id should update owned cred for member', async () => {
const member = await testDb.createUser({ globalRole: globalMemberRole });
const authMemberAgent = utils.createAgent(app, { auth: true, user: member });
const savedCredential = await saveCredential(credentialPayload(), { user: member });
const patchPayload = credentialPayload();
const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });
const patchPayload = randomCredentialPayload();
const response = await authMemberAgent
.patch(`/credentials/${savedCredential.id}`)
@ -314,8 +314,8 @@ test('PATCH /credentials/:id should not update non-owned cred for member', async
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const member = await testDb.createUser({ globalRole: globalMemberRole });
const authMemberAgent = utils.createAgent(app, { auth: true, user: member });
const savedCredential = await saveCredential(credentialPayload(), { user: ownerShell });
const patchPayload = credentialPayload();
const savedCredential = await saveCredential(randomCredentialPayload(), { user: ownerShell });
const patchPayload = randomCredentialPayload();
const response = await authMemberAgent
.patch(`/credentials/${savedCredential.id}`)
@ -331,7 +331,7 @@ test('PATCH /credentials/:id should not update non-owned cred for member', async
test('PATCH /credentials/:id should fail with invalid inputs', async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
const savedCredential = await saveCredential(credentialPayload(), { user: ownerShell });
const savedCredential = await saveCredential(randomCredentialPayload(), { user: ownerShell });
await Promise.all(
INVALID_PAYLOADS.map(async (invalidPayload) => {
@ -348,7 +348,7 @@ test('PATCH /credentials/:id should fail if cred not found', async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
const response = await authOwnerAgent.patch('/credentials/123').send(credentialPayload());
const response = await authOwnerAgent.patch('/credentials/123').send(randomCredentialPayload());
expect(response.statusCode).toBe(404);
});
@ -357,11 +357,10 @@ test('PATCH /credentials/:id should fail with missing encryption key', async ()
const mock = jest.spyOn(UserSettings, 'getEncryptionKey');
mock.mockRejectedValue(new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY));
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
const response = await authOwnerAgent.post('/credentials').send(credentialPayload());
const response = await authOwnerAgent.post('/credentials').send(randomCredentialPayload());
expect(response.statusCode).toBe(500);
@ -373,12 +372,12 @@ test('GET /credentials should retrieve all creds for owner', async () => {
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
for (let i = 0; i < 3; i++) {
await saveCredential(credentialPayload(), { user: ownerShell });
await saveCredential(randomCredentialPayload(), { user: ownerShell });
}
const member = await testDb.createUser({ globalRole: globalMemberRole });
await saveCredential(credentialPayload(), { user: member });
await saveCredential(randomCredentialPayload(), { user: member });
const response = await authOwnerAgent.get('/credentials');
@ -402,7 +401,7 @@ test('GET /credentials should retrieve owned creds for member', async () => {
const authMemberAgent = utils.createAgent(app, { auth: true, user: member });
for (let i = 0; i < 3; i++) {
await saveCredential(credentialPayload(), { user: member });
await saveCredential(randomCredentialPayload(), { user: member });
}
const response = await authMemberAgent.get('/credentials');
@ -428,7 +427,7 @@ test('GET /credentials should not retrieve non-owned creds for member', async ()
const authMemberAgent = utils.createAgent(app, { auth: true, user: member });
for (let i = 0; i < 3; i++) {
await saveCredential(credentialPayload(), { user: ownerShell });
await saveCredential(randomCredentialPayload(), { user: ownerShell });
}
const response = await authMemberAgent.get('/credentials');
@ -440,7 +439,7 @@ test('GET /credentials should not retrieve non-owned creds for member', async ()
test('GET /credentials/:id should retrieve owned cred for owner', async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
const savedCredential = await saveCredential(credentialPayload(), { user: ownerShell });
const savedCredential = await saveCredential(randomCredentialPayload(), { user: ownerShell });
const firstResponse = await authOwnerAgent.get(`/credentials/${savedCredential.id}`);
@ -465,7 +464,7 @@ test('GET /credentials/:id should retrieve owned cred for owner', async () => {
test('GET /credentials/:id should retrieve owned cred for member', async () => {
const member = await testDb.createUser({ globalRole: globalMemberRole });
const authMemberAgent = utils.createAgent(app, { auth: true, user: member });
const savedCredential = await saveCredential(credentialPayload(), { user: member });
const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });
const firstResponse = await authMemberAgent.get(`/credentials/${savedCredential.id}`);
@ -492,7 +491,7 @@ test('GET /credentials/:id should not retrieve non-owned cred for member', async
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const member = await testDb.createUser({ globalRole: globalMemberRole });
const authMemberAgent = utils.createAgent(app, { auth: true, user: member });
const savedCredential = await saveCredential(credentialPayload(), { user: ownerShell });
const savedCredential = await saveCredential(randomCredentialPayload(), { user: ownerShell });
const response = await authMemberAgent.get(`/credentials/${savedCredential.id}`);
@ -503,12 +502,11 @@ test('GET /credentials/:id should not retrieve non-owned cred for member', async
test('GET /credentials/:id should fail with missing encryption key', async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
const savedCredential = await saveCredential(credentialPayload(), { user: ownerShell });
const savedCredential = await saveCredential(randomCredentialPayload(), { user: ownerShell });
const mock = jest.spyOn(UserSettings, 'getEncryptionKey');
mock.mockRejectedValue(new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY));
const response = await authOwnerAgent
.get(`/credentials/${savedCredential.id}`)
.query({ includeData: true });
@ -527,13 +525,6 @@ test('GET /credentials/:id should return 404 if cred not found', async () => {
expect(response.statusCode).toBe(404);
});
const credentialPayload = () => ({
name: randomName(),
type: randomName(),
nodesAccess: [{ nodeType: randomName() }],
data: { accessToken: randomString(6, 16) },
});
const INVALID_PAYLOADS = [
{
type: randomName(),

4
packages/cli/test/integration/nodes.api.test.ts
View file

@ -265,7 +265,7 @@ test('DELETE /nodes should reject if package is not installed', async () => {
const {
statusCode,
body: { message },
} = await authAgent(ownerShell).delete('/nodes').send({
} = await authAgent(ownerShell).delete('/nodes').query({
name: utils.installedPackagePayload().packageName,
});
@ -282,7 +282,7 @@ test('DELETE /nodes should uninstall package', async () => {
mocked(findInstalledPackage).mockImplementationOnce(mockedEmptyPackage);
const { statusCode } = await authAgent(ownerShell).delete('/nodes').send({
const { statusCode } = await authAgent(ownerShell).delete('/nodes').query({
name: utils.installedPackagePayload().packageName,
});

9
packages/cli/test/integration/shared/random.ts
View file

@ -45,3 +45,12 @@ const POPULAR_TOP_LEVEL_DOMAINS = ['com', 'org', 'net', 'io', 'edu'];
const randomTopLevelDomain = () => chooseRandomly(POPULAR_TOP_LEVEL_DOMAINS);
export const randomName = () => randomString(4, 8);
export function randomCredentialPayload() {
return {
name: randomName(),
type: randomName(),
nodesAccess: [{ nodeType: randomName() }],
data: { accessToken: randomString(6, 16) },
};
}

11
packages/cli/test/integration/shared/testDb.ts
View file

@ -13,7 +13,14 @@ import {
MAPPING_TABLES_TO_CLEAR,
} from './constants';
import { DatabaseType, Db, ICredentialsDb } from '../../../src';
import { randomApiKey, randomEmail, randomName, randomString, randomValidPassword } from './random';
import {
randomApiKey,
randomCredentialPayload,
randomEmail,
randomName,
randomString,
randomValidPassword,
} from './random';
import { CredentialsEntity } from '../../../src/databases/entities/CredentialsEntity';
import { hashPassword } from '../../../src/UserManagement/UserManagementHelper';
import { entities } from '../../../src/databases/entities';
@ -287,7 +294,7 @@ function toTableName(sourceName: CollectionName | MappingName) {
* Save a credential to the test DB, sharing it with a user.
*/
export async function saveCredential(
credentialPayload: CredentialPayload,
credentialPayload: CredentialPayload = randomCredentialPayload(),
{ user, role }: { user: User; role: Role },
) {
const newCredential = new CredentialsEntity();

53
packages/cli/test/integration/users.api.test.ts
View file

@ -10,6 +10,7 @@ import {
randomValidPassword,
randomName,
randomInvalidPassword,
randomCredentialPayload,
} from './shared/random';
import { CredentialsEntity } from '../../src/databases/entities/CredentialsEntity';
import { WorkflowEntity } from '../../src/databases/entities/WorkflowEntity';
@ -208,49 +209,13 @@ test('DELETE /users/:id with transferId should perform transfer', async () => {
const owner = await testDb.createUser({ globalRole: globalOwnerRole });
const authOwnerAgent = utils.createAgent(app, { auth: true, user: owner });
const userToDelete = await Db.collections.User.save({
id: uuid(),
email: randomEmail(),
password: randomValidPassword(),
firstName: randomName(),
lastName: randomName(),
createdAt: new Date(),
updatedAt: new Date(),
globalRole: workflowOwnerRole,
});
const userToDelete = await testDb.createUser({ globalRole: globalMemberRole });
const newWorkflow = new WorkflowEntity();
const savedWorkflow = await testDb.createWorkflow(undefined, userToDelete);
Object.assign(newWorkflow, {
name: randomName(),
active: false,
connections: {},
nodes: [],
});
const savedWorkflow = await Db.collections.Workflow.save(newWorkflow);
await Db.collections.SharedWorkflow.save({
role: workflowOwnerRole,
const savedCredential = await testDb.saveCredential(undefined, {
user: userToDelete,
workflow: savedWorkflow,
});
const newCredential = new CredentialsEntity();
Object.assign(newCredential, {
name: randomName(),
data: '',
type: '',
nodesAccess: [],
});
const savedCredential = await Db.collections.Credentials.save(newCredential);
await Db.collections.SharedCredentials.save({
role: credentialOwnerRole,
user: userToDelete,
credentials: savedCredential,
});
const response = await authOwnerAgent.delete(`/users/${userToDelete.id}`).query({
@ -260,19 +225,25 @@ test('DELETE /users/:id with transferId should perform transfer', async () => {
expect(response.statusCode).toBe(200);
const sharedWorkflow = await Db.collections.SharedWorkflow.findOneOrFail({
relations: ['user'],
relations: ['workflow', 'user'],
where: { user: owner },
});
const sharedCredential = await Db.collections.SharedCredentials.findOneOrFail({
relations: ['user'],
relations: ['credentials', 'user'],
where: { user: owner },
});
const deletedUser = await Db.collections.User.findOne(userToDelete);
expect(sharedWorkflow.user.id).toBe(owner.id);
expect(sharedWorkflow.workflow).toBeDefined();
expect(sharedWorkflow.workflow.id).toBe(savedWorkflow.id);
expect(sharedCredential.user.id).toBe(owner.id);
expect(sharedCredential.credentials).toBeDefined();
expect(sharedCredential.credentials.id).toBe(savedCredential.id);
expect(deletedUser).toBeUndefined();
});

10
packages/editor-ui/src/api/helpers.ts
View file

@ -1,10 +1,6 @@
import axios, { AxiosRequestConfig, Method } from 'axios';
import {
IDataObject,
} from 'n8n-workflow';
import {
IRestApiContext,
} from '../Interface';
import { IDataObject } from 'n8n-workflow';
import type { IRestApiContext } from '../Interface';
class ResponseError extends Error {
// The HTTP status code of response
@ -52,7 +48,7 @@ async function request(config: {method: Method, baseURL: string, endpoint: strin
if (process.env.NODE_ENV !== 'production' && !baseURL.includes('api.n8n.io') ) {
options.withCredentials = true;
}
if (['PATCH', 'POST', 'PUT', 'DELETE'].includes(method)) {
if (['POST', 'PATCH', 'PUT'].includes(method)) {
options.data = data;
} else {
options.params = data;