From e06e147f8b3be98a31015f9aae6235679540b696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 10:48:31 +0100 Subject: [PATCH 01/37] :test_tube: Fix failing test --- packages/cli/test/integration/passwordReset.endpoints.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/test/integration/passwordReset.endpoints.test.ts b/packages/cli/test/integration/passwordReset.endpoints.test.ts index 748eb5c54770d..0953cde193bba 100644 --- a/packages/cli/test/integration/passwordReset.endpoints.test.ts +++ b/packages/cli/test/integration/passwordReset.endpoints.test.ts @@ -123,7 +123,7 @@ test('POST /forgot-password should fail if user is not found', async () => { const response = await authlessAgent.post('/forgot-password').send({ email: randomEmail() }); - expect(response.statusCode).toBe(404); + expect(response.statusCode).toBe(200); }); test('GET /resolve-password-token should succeed with valid inputs', async () => { From 013aab0f28ed25eab0e48f152fb810d1dcd3b395 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 10:54:59 +0100 Subject: [PATCH 02/37] :blue_book: Improve `createAgent` signature --- .../cli/test/integration/auth.endpoints.test.ts | 2 +- packages/cli/test/integration/shared/utils.ts | 13 ++++++------- .../cli/test/integration/users.endpoints.test.ts | 6 +++--- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/cli/test/integration/auth.endpoints.test.ts b/packages/cli/test/integration/auth.endpoints.test.ts index 784a1d2a16127..2fe1b256e5d3c 100644 --- a/packages/cli/test/integration/auth.endpoints.test.ts +++ b/packages/cli/test/integration/auth.endpoints.test.ts @@ -54,7 +54,7 @@ describe('auth endpoints', () => { }); test('POST /login should log user in', async () => { - const authlessAgent = await utils.createAgent(app, { auth: false }); + const authlessAgent = await utils.createAgent(app); const response = await authlessAgent.post('/login').send({ email: TEST_USER.email, diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index b099d584af360..d5779b290344f 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -183,19 +183,18 @@ export function getAllRoles() { // request agent // ---------------------------------- +/** + * Create a request agent, optionally with an auth cookie. + */ export async function createAgent( app: express.Application, - { auth, user }: { auth: boolean; user?: User } = { auth: false }, + options?: { auth: true; user: User }, ) { const agent = request.agent(app); agent.use(prefix(REST_PATH_SEGMENT)); - if (auth && !user) { - throw new Error('User required for auth agent creation'); - } - - if (auth && user) { - const { token } = await issueJWT(user); + if (options?.auth && options?.user) { + const { token } = await issueJWT(options.user); agent.jar.setCookie(`n8n-auth=${token}`); } diff --git a/packages/cli/test/integration/users.endpoints.test.ts b/packages/cli/test/integration/users.endpoints.test.ts index 1f46d55497e72..b43f0b7f457fa 100644 --- a/packages/cli/test/integration/users.endpoints.test.ts +++ b/packages/cli/test/integration/users.endpoints.test.ts @@ -327,7 +327,7 @@ test('GET /resolve-signup-token should fail with invalid inputs', async () => { }); test('POST /users/:id should fill out a user shell', async () => { - const authlessAgent = await utils.createAgent(app, { auth: false }); + const authlessAgent = await utils.createAgent(app); const userToFillOut = await Db.collections.User!.save({ email: randomEmail(), @@ -373,7 +373,7 @@ test('POST /users/:id should fill out a user shell', async () => { }); test('POST /users/:id should fail with invalid inputs', async () => { - const authlessAgent = await utils.createAgent(app, { auth: false }); + const authlessAgent = await utils.createAgent(app); const emailToStore = randomEmail(); @@ -394,7 +394,7 @@ test('POST /users/:id should fail with invalid inputs', async () => { }); test('POST /users/:id should fail with already accepted invite', async () => { - const authlessAgent = await utils.createAgent(app, { auth: false }); + const authlessAgent = await utils.createAgent(app); const globalMemberRole = await Db.collections.Role!.findOneOrFail({ name: 'member', From 2af92ab9612abf8b0aff4ed42e78f81a80d9605e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 11:04:22 +0100 Subject: [PATCH 03/37] :truck: Fix `LoggerProxy` import --- packages/cli/src/UserManagement/routes/users.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/cli/src/UserManagement/routes/users.ts b/packages/cli/src/UserManagement/routes/users.ts index b576c48e23340..2bbeb1efd00fc 100644 --- a/packages/cli/src/UserManagement/routes/users.ts +++ b/packages/cli/src/UserManagement/routes/users.ts @@ -4,6 +4,7 @@ import { Response } from 'express'; import { getConnection, In } from 'typeorm'; import { genSaltSync, hashSync } from 'bcryptjs'; import validator from 'validator'; +import { LoggerProxy } from 'n8n-workflow'; import { Db, ResponseHelper } from '../..'; import { N8nApp } from '../Interfaces'; @@ -18,9 +19,7 @@ import { User } from '../../databases/entities/User'; import { SharedWorkflow } from '../../databases/entities/SharedWorkflow'; import { SharedCredentials } from '../../databases/entities/SharedCredentials'; import { getInstance } from '../email/UserManagementMailer'; - import config = require('../../../config'); -import { LoggerProxy } from '../../../../workflow/dist/src'; import { issueCookie } from '../auth/jwt'; export function usersNamespace(this: N8nApp): void { From 491ac27b89f21fbade34f3711583b7b666f49b4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 11:40:14 +0100 Subject: [PATCH 04/37] :sparkles: Create credentials endpoints namespace --- packages/cli/src/Server.ts | 345 +--------------- packages/cli/src/UserManagement/Interfaces.ts | 5 +- .../src/endpoints/credentials.endpoints.ts | 370 ++++++++++++++++++ packages/cli/src/requests.d.ts | 10 +- 4 files changed, 383 insertions(+), 347 deletions(-) create mode 100644 packages/cli/src/endpoints/credentials.endpoints.ts diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index c1acd9b038713..cf8b86c72ee57 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -74,8 +74,6 @@ import { IDataObject, INodeCredentials, INodeCredentialsDetails, - INodeCredentialTestRequest, - INodeCredentialTestResult, INodeParameters, INodePropertyOptions, INodeType, @@ -112,10 +110,7 @@ import { GenericHelpers, ICredentialsDb, ICredentialsOverwrite, - ICredentialsResponse, ICustomRequest, - IExecutionDeleteFilter, - IExecutionFlatted, IExecutionFlattedDb, IExecutionFlattedResponse, IExecutionPushResponse, @@ -130,7 +125,6 @@ import { ITagWithCountDb, IWorkflowExecutionDataProcess, IWorkflowResponse, - IPersonalizationSurveyAnswers, NodeTypes, Push, ResponseHelper, @@ -169,8 +163,8 @@ import type { import { DEFAULT_EXECUTIONS_GET_ALL_LIMIT, validateEntity } from './GenericHelpers'; import { ExecutionEntity } from './databases/entities/ExecutionEntity'; import { SharedWorkflow } from './databases/entities/SharedWorkflow'; -import { SharedCredentials } from './databases/entities/SharedCredentials'; import { RESPONSE_ERROR_MESSAGES } from './constants'; +import { credentialsEndpoints } from './endpoints/credentials.endpoints'; require('body-parser-xml')(bodyParser); @@ -1557,342 +1551,7 @@ class App { // Credentials // ---------------------------------------- - this.app.get( - `/${this.restEndpoint}/credentials/new`, - ResponseHelper.send(async (req: WorkflowRequest.NewName): Promise<{ name: string }> => { - const requestedName = - req.query.name && req.query.name !== '' ? req.query.name : this.defaultCredentialsName; - - return await GenericHelpers.generateUniqueName(requestedName, 'credentials'); - }), - ); - - // Deletes a specific credential - this.app.delete( - `/${this.restEndpoint}/credentials/:id`, - ResponseHelper.send(async (req: CredentialRequest.Delete) => { - const { id: credentialId } = req.params; - - const shared = await Db.collections.SharedCredentials!.findOne({ - relations: ['credentials'], - where: whereClause({ - user: req.user, - entityType: 'credentials', - entityId: credentialId, - }), - }); - - if (!shared) { - throw new ResponseHelper.ResponseError( - `Credential with ID "${credentialId}" could not be found to be deleted.`, - undefined, - 404, - ); - } - - await this.externalHooks.run('credentials.delete', [credentialId]); - - await Db.collections.Credentials!.delete(credentialId); - - return true; - }), - ); - - // Creates new credentials - this.app.post( - `/${this.restEndpoint}/credentials`, - ResponseHelper.send(async (req: CredentialRequest.Create) => { - delete req.body.id; // delete if sent - - const newCredential = new CredentialsEntity(); - - Object.assign(newCredential, req.body); - - await validateEntity(newCredential); - - // Add the added date for node access permissions - for (const nodeAccess of newCredential.nodesAccess) { - nodeAccess.date = this.getCurrentDate(); - } - - const encryptionKey = await UserSettings.getEncryptionKey(); - - if (!encryptionKey) { - throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); - } - - // Encrypt the data - const coreCredential = new Credentials( - { id: null, name: newCredential.name }, - newCredential.type, - newCredential.nodesAccess, - ); - - // @ts-ignore - coreCredential.setData(newCredential.data, encryptionKey); - - const encryptedData = coreCredential.getDataToSave() as ICredentialsDb; - - Object.assign(newCredential, encryptedData); - - await this.externalHooks.run('credentials.create', [encryptedData]); - - const role = await Db.collections.Role!.findOneOrFail({ - name: 'owner', - scope: 'credential', - }); - - const savedCredential = await getConnection().transaction(async (transactionManager) => { - const savedCredential = await transactionManager.save(newCredential); - - savedCredential.data = newCredential.data; - - const newSharedCredential = new SharedCredentials(); - - Object.assign(newSharedCredential, { - role, - user: req.user, - credentials: savedCredential, - }); - - await transactionManager.save(newSharedCredential); - - return savedCredential; - }); - - const { id, ...rest } = savedCredential; - - return { id: id.toString(), ...rest }; - }), - ); - - // Test credentials - this.app.post( - `/${this.restEndpoint}/credentials-test`, - ResponseHelper.send( - async (req: express.Request, res: express.Response): Promise => { - const incomingData = req.body as INodeCredentialTestRequest; - - const encryptionKey = await UserSettings.getEncryptionKey(); - if (encryptionKey === undefined) { - return { - status: 'Error', - message: 'No encryption key got found to decrypt the credentials!', - }; - } - - const credentialsHelper = new CredentialsHelper(encryptionKey); - - const credentialType = incomingData.credentials.type; - return credentialsHelper.testCredentials( - credentialType, - incomingData.credentials, - incomingData.nodeToTestWith, - ); - }, - ), - ); - - // Updates existing credentials - this.app.patch( - `/${this.restEndpoint}/credentials/:id`, - ResponseHelper.send(async (req: CredentialRequest.Update): Promise => { - const { id: credentialId } = req.params; - - const updateData = new CredentialsEntity(); - Object.assign(updateData, req.body); - - const shared = await Db.collections.SharedCredentials!.findOne({ - relations: ['credentials'], - where: whereClause({ - user: req.user, - entityType: 'credentials', - entityId: credentialId, - }), - }); - - if (!shared) { - throw new ResponseHelper.ResponseError( - `Credential with ID "${credentialId}" could not be found to be updated.`, - undefined, - 404, - ); - } - - const { credentials: credential } = shared; - - // Add the date for newly added node access permissions - for (const nodeAccess of updateData.nodesAccess) { - if (!nodeAccess.date) { - nodeAccess.date = this.getCurrentDate(); - } - } - - const encryptionKey = await UserSettings.getEncryptionKey(); - - if (!encryptionKey) { - throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); - } - - const coreCredential = new Credentials( - { id: credential.id.toString(), name: credential.name }, - credential.type, - credential.nodesAccess, - credential.data, - ); - - const decryptedData = coreCredential.getData(encryptionKey); - - // Do not overwrite the oauth data else data like the access or refresh token would get lost - // everytime anybody changes anything on the credentials even if it is just the name. - if (decryptedData.oauthTokenData) { - // @ts-ignore - updateData.data.oauthTokenData = decryptedData.oauthTokenData; - } - - // Encrypt the data - const credentials = new Credentials( - { id: credentialId, name: updateData.name }, - updateData.type, - updateData.nodesAccess, - ); - - // @ts-ignore - credentials.setData(updateData.data, encryptionKey); - - const newCredentialData = credentials.getDataToSave() as ICredentialsDb; - - // Add special database related data - newCredentialData.updatedAt = this.getCurrentDate(); - - await this.externalHooks.run('credentials.update', [newCredentialData]); - - // Update the credentials in DB - await Db.collections.Credentials!.update(credentialId, newCredentialData); - - // We sadly get nothing back from "update". Neither if it updated a record - // nor the new value. So query now the hopefully updated entry. - const responseData = await Db.collections.Credentials!.findOne(credentialId); - - if (responseData === undefined) { - throw new ResponseHelper.ResponseError( - `Credential with ID "${credentialId}" could not be found to be updated.`, - undefined, - 400, - ); - } - - // Remove the encrypted data as it is not needed in the frontend - const { id, data, ...rest } = responseData; - - return { - id: id.toString(), - ...rest, - }; - }), - ); - - // Returns specific credentials - this.app.get( - `/${this.restEndpoint}/credentials/:id`, - ResponseHelper.send(async (req: CredentialRequest.Get) => { - const { id: credentialId } = req.params; - - const shared = await Db.collections.SharedCredentials!.findOne({ - relations: ['credentials'], - where: whereClause({ - user: req.user, - entityType: 'credentials', - entityId: credentialId, - }), - }); - - if (!shared) return {}; - - const { credentials: credential } = shared; - - if (req.query.includeData !== 'true') { - const { data, id, ...rest } = credential; - - return { - id: id.toString(), - ...rest, - }; - } - - const { data, id, ...rest } = credential; - - const encryptionKey = await UserSettings.getEncryptionKey(); - - if (!encryptionKey) { - throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); - } - - const coreCredential = new Credentials( - { id: credential.id.toString(), name: credential.name }, - credential.type, - credential.nodesAccess, - credential.data, - ); - - return { - id: id.toString(), - data: coreCredential.getData(encryptionKey), - ...rest, - }; - }), - ); - - // Returns all the saved credentials - this.app.get( - `/${this.restEndpoint}/credentials`, - ResponseHelper.send( - async (req: CredentialRequest.GetAll): Promise => { - let credentials: ICredentialsDb[] = []; - - const filter: Record = req.query.filter - ? JSON.parse(req.query.filter) - : {}; - - if (req.user.globalRole.name === 'owner') { - credentials = await Db.collections.Credentials!.find({ - select: ['id', 'name', 'type', 'nodesAccess', 'createdAt', 'updatedAt'], - where: filter, - }); - } else { - const shared = await Db.collections.SharedCredentials!.find({ - relations: ['credentials'], - where: whereClause({ - user: req.user, - entityType: 'credentials', - }), - }); - - if (!shared.length) return []; - - credentials = await Db.collections.Credentials!.find({ - select: ['id', 'name', 'type', 'nodesAccess', 'createdAt', 'updatedAt'], - where: { - id: In(shared.map(({ credentials }) => credentials.id)), - ...filter, - }, - }); - } - - let encryptionKey; - - if (req.query.includeData === 'true') { - encryptionKey = await UserSettings.getEncryptionKey(); - - if (!encryptionKey) { - throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); - } - } - - return credentials.map(({ id, ...rest }) => ({ id: id.toString(), ...rest })); - }, - ), - ); + credentialsEndpoints.apply(this); // ---------------------------------------- // Credential-Types diff --git a/packages/cli/src/UserManagement/Interfaces.ts b/packages/cli/src/UserManagement/Interfaces.ts index 7df3a450470c0..e14db144396fd 100644 --- a/packages/cli/src/UserManagement/Interfaces.ts +++ b/packages/cli/src/UserManagement/Interfaces.ts @@ -1,7 +1,7 @@ /* eslint-disable import/no-cycle */ import { Application } from 'express'; import { JwtFromRequestFunction } from 'passport-jwt'; -import { IPersonalizationSurveyAnswers } from '../Interfaces'; +import type { IExternalHooksClass, IPersonalizationSurveyAnswers } from '../Interfaces'; export interface JwtToken { token: string; @@ -32,4 +32,7 @@ export interface PublicUser { export interface N8nApp { app: Application; restEndpoint: string; + externalHooks: IExternalHooksClass; + defaultCredentialsName: string; + getCurrentDate(): Date; } diff --git a/packages/cli/src/endpoints/credentials.endpoints.ts b/packages/cli/src/endpoints/credentials.endpoints.ts new file mode 100644 index 0000000000000..0519dbd3a0023 --- /dev/null +++ b/packages/cli/src/endpoints/credentials.endpoints.ts @@ -0,0 +1,370 @@ +/* eslint-disable @typescript-eslint/no-shadow */ +/* eslint-disable @typescript-eslint/no-unused-vars */ +/* eslint-disable no-restricted-syntax */ +/* eslint-disable @typescript-eslint/no-non-null-assertion */ +/* eslint-disable import/no-cycle */ +import { getConnection, In } from 'typeorm'; +import { UserSettings, Credentials } from 'n8n-core'; +import { INodeCredentialTestResult } from 'n8n-workflow'; + +import { + CredentialsHelper, + Db, + GenericHelpers, + ICredentialsDb, + ICredentialsResponse, + ResponseHelper, + whereClause, +} from '..'; +import { RESPONSE_ERROR_MESSAGES } from '../constants'; +import { CredentialsEntity } from '../databases/entities/CredentialsEntity'; +import { SharedCredentials } from '../databases/entities/SharedCredentials'; +import { validateEntity } from '../GenericHelpers'; +import type { CredentialRequest } from '../requests'; +import type { N8nApp } from '../UserManagement/Interfaces'; + +export function credentialsEndpoints(this: N8nApp): void { + /** + * Return a unique credential name. + */ + this.app.get( + `/${this.restEndpoint}/credentials/new`, + ResponseHelper.send(async (req: CredentialRequest.NewName): Promise<{ name: string }> => { + const { name: newName } = req.query; + + return GenericHelpers.generateUniqueName( + newName ?? this.defaultCredentialsName, + 'credentials', + ); + }), + ); + + /** + * Delete a credential. + */ + this.app.delete( + `/${this.restEndpoint}/credentials/:id`, + ResponseHelper.send(async (req: CredentialRequest.Delete) => { + const { id: credentialId } = req.params; + + const shared = await Db.collections.SharedCredentials!.findOne({ + relations: ['credentials'], + where: whereClause({ + user: req.user, + entityType: 'credentials', + entityId: credentialId, + }), + }); + + if (!shared) { + throw new ResponseHelper.ResponseError( + `Credential with ID "${credentialId}" could not be found to be deleted.`, + undefined, + 404, + ); + } + + await this.externalHooks.run('credentials.delete', [credentialId]); + + await Db.collections.Credentials!.delete(credentialId); + + return true; + }), + ); + + /** + * Create a credential. + */ + this.app.post( + `/${this.restEndpoint}/credentials`, + ResponseHelper.send(async (req: CredentialRequest.Create) => { + delete req.body.id; // delete if sent + + const newCredential = new CredentialsEntity(); + + Object.assign(newCredential, req.body); + + await validateEntity(newCredential); + + // Add the added date for node access permissions + for (const nodeAccess of newCredential.nodesAccess) { + nodeAccess.date = this.getCurrentDate(); + } + + const encryptionKey = await UserSettings.getEncryptionKey(); + + if (!encryptionKey) { + throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); + } + + // Encrypt the data + const coreCredential = new Credentials( + { id: null, name: newCredential.name }, + newCredential.type, + newCredential.nodesAccess, + ); + + // @ts-ignore + coreCredential.setData(newCredential.data, encryptionKey); + + const encryptedData = coreCredential.getDataToSave() as ICredentialsDb; + + Object.assign(newCredential, encryptedData); + + await this.externalHooks.run('credentials.create', [encryptedData]); + + const role = await Db.collections.Role!.findOneOrFail({ + name: 'owner', + scope: 'credential', + }); + + const { id, ...rest } = await getConnection().transaction(async (transactionManager) => { + const savedCredential = await transactionManager.save(newCredential); + + savedCredential.data = newCredential.data; + + const newSharedCredential = new SharedCredentials(); + + Object.assign(newSharedCredential, { + role, + user: req.user, + credentials: savedCredential, + }); + + await transactionManager.save(newSharedCredential); + + return savedCredential; + }); + + return { id: id.toString(), ...rest }; + }), + ); + + /** + * Test a credential. + */ + this.app.post( + `/${this.restEndpoint}/credentials-test`, + ResponseHelper.send(async (req: CredentialRequest.Test): Promise => { + const { credentials, nodeToTestWith } = req.body; + + const encryptionKey = await UserSettings.getEncryptionKey(); + + if (!encryptionKey) { + return { + status: 'Error', + message: RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, + }; + } + + const helper = new CredentialsHelper(encryptionKey); + + return helper.testCredentials(credentials.type, credentials, nodeToTestWith); + }), + ); + + /** + * Update a credential. + */ + this.app.patch( + `/${this.restEndpoint}/credentials/:id`, + ResponseHelper.send(async (req: CredentialRequest.Update): Promise => { + const { id: credentialId } = req.params; + + const updateData = new CredentialsEntity(); + Object.assign(updateData, req.body); + + const shared = await Db.collections.SharedCredentials!.findOne({ + relations: ['credentials'], + where: whereClause({ + user: req.user, + entityType: 'credentials', + entityId: credentialId, + }), + }); + + if (!shared) { + throw new ResponseHelper.ResponseError( + `Credential with ID "${credentialId}" could not be found to be updated.`, + undefined, + 404, + ); + } + + const { credentials: credential } = shared; + + // Add the date for newly added node access permissions + for (const nodeAccess of updateData.nodesAccess) { + if (!nodeAccess.date) { + nodeAccess.date = this.getCurrentDate(); + } + } + + const encryptionKey = await UserSettings.getEncryptionKey(); + + if (!encryptionKey) { + throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); + } + + const coreCredential = new Credentials( + { id: credential.id.toString(), name: credential.name }, + credential.type, + credential.nodesAccess, + credential.data, + ); + + const decryptedData = coreCredential.getData(encryptionKey); + + // Do not overwrite the oauth data else data like the access or refresh token would get lost + // everytime anybody changes anything on the credentials even if it is just the name. + if (decryptedData.oauthTokenData) { + // @ts-ignore + updateData.data.oauthTokenData = decryptedData.oauthTokenData; + } + + // Encrypt the data + const credentials = new Credentials( + { id: credentialId, name: updateData.name }, + updateData.type, + updateData.nodesAccess, + ); + + // @ts-ignore + credentials.setData(updateData.data, encryptionKey); + + const newCredentialData = credentials.getDataToSave() as ICredentialsDb; + + // Add special database related data + newCredentialData.updatedAt = this.getCurrentDate(); + + await this.externalHooks.run('credentials.update', [newCredentialData]); + + // Update the credentials in DB + await Db.collections.Credentials!.update(credentialId, newCredentialData); + + // We sadly get nothing back from "update". Neither if it updated a record + // nor the new value. So query now the hopefully updated entry. + const responseData = await Db.collections.Credentials!.findOne(credentialId); + + if (responseData === undefined) { + throw new ResponseHelper.ResponseError( + `Credential ID "${credentialId}" could not be found to be updated.`, + undefined, + 400, + ); + } + + // Remove the encrypted data as it is not needed in the frontend + const { id, data, ...rest } = responseData; + + return { + id: id.toString(), + ...rest, + }; + }), + ); + + /** + * Retrieve a credential. + */ + this.app.get( + `/${this.restEndpoint}/credentials/:id`, + ResponseHelper.send(async (req: CredentialRequest.Get) => { + const { id: credentialId } = req.params; + + const shared = await Db.collections.SharedCredentials!.findOne({ + relations: ['credentials'], + where: whereClause({ + user: req.user, + entityType: 'credentials', + entityId: credentialId, + }), + }); + + if (!shared) return {}; + + const { credentials: credential } = shared; + + if (req.query.includeData !== 'true') { + const { data, id, ...rest } = credential; + + return { + id: id.toString(), + ...rest, + }; + } + + const { data, id, ...rest } = credential; + + const encryptionKey = await UserSettings.getEncryptionKey(); + + if (!encryptionKey) { + throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); + } + + const coreCredential = new Credentials( + { id: credential.id.toString(), name: credential.name }, + credential.type, + credential.nodesAccess, + credential.data, + ); + + return { + id: id.toString(), + data: coreCredential.getData(encryptionKey), + ...rest, + }; + }), + ); + + /** + * Retrieve all credentials. + */ + this.app.get( + `/${this.restEndpoint}/credentials`, + ResponseHelper.send(async (req: CredentialRequest.GetAll): Promise => { + let credentials: ICredentialsDb[] = []; + + const filter = req.query.filter + ? (JSON.parse(req.query.filter) as Record) + : {}; + + if (req.user.globalRole.name === 'owner') { + credentials = await Db.collections.Credentials!.find({ + select: ['id', 'name', 'type', 'nodesAccess', 'createdAt', 'updatedAt'], + where: filter, + }); + } else { + const shared = await Db.collections.SharedCredentials!.find({ + relations: ['credentials'], + where: whereClause({ + user: req.user, + entityType: 'credentials', + }), + }); + + if (!shared.length) return []; + + credentials = await Db.collections.Credentials!.find({ + select: ['id', 'name', 'type', 'nodesAccess', 'createdAt', 'updatedAt'], + where: { + id: In(shared.map(({ credentials }) => credentials.id)), + ...filter, + }, + }); + } + + let encryptionKey; + + if (req.query.includeData === 'true') { + encryptionKey = await UserSettings.getEncryptionKey(); + + if (!encryptionKey) { + throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); + } + } + + return credentials.map(({ id, ...rest }) => ({ id: id.toString(), ...rest })); + }), + ); +} diff --git a/packages/cli/src/requests.d.ts b/packages/cli/src/requests.d.ts index 1fffb7475de7d..4d076701df8ff 100644 --- a/packages/cli/src/requests.d.ts +++ b/packages/cli/src/requests.d.ts @@ -1,14 +1,16 @@ /* eslint-disable import/no-cycle */ import express = require('express'); -import { User } from './databases/entities/User'; -import { IExecutionDeleteFilter } from '.'; import { + INodeCredentialTestRequest, IConnections, ICredentialDataDecryptedObject, ICredentialNodeAccess, INode, IWorkflowSettings, -} from '../../workflow/dist/src'; +} from 'n8n-workflow'; + +import { User } from './databases/entities/User'; +import type { IExecutionDeleteFilter } from '.'; import type { PublicUser } from './UserManagement/Interfaces'; export type AuthenticatedRequest< @@ -74,6 +76,8 @@ export declare namespace CredentialRequest { type Update = AuthenticatedRequest<{ id: string }, {}, RequestBody>; type NewName = WorkflowRequest.NewName; + + type Test = AuthenticatedRequest<{}, {}, INodeCredentialTestRequest>; } // ---------------------------------- From 011ed907e0c51451db1e83ed7720ea0f3a3a3df3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 12:23:24 +0100 Subject: [PATCH 05/37] :test_tube: Set up initial tests --- packages/cli/src/UserManagement/Interfaces.ts | 1 - .../src/endpoints/credentials.endpoints.ts | 12 ++- .../integration/credentials.endpoints.test.ts | 102 ++++++++++++++++++ .../cli/test/integration/shared/types.d.ts | 2 +- packages/cli/test/integration/shared/utils.ts | 13 +-- 5 files changed, 118 insertions(+), 12 deletions(-) create mode 100644 packages/cli/test/integration/credentials.endpoints.test.ts diff --git a/packages/cli/src/UserManagement/Interfaces.ts b/packages/cli/src/UserManagement/Interfaces.ts index e14db144396fd..7b12e4260979d 100644 --- a/packages/cli/src/UserManagement/Interfaces.ts +++ b/packages/cli/src/UserManagement/Interfaces.ts @@ -34,5 +34,4 @@ export interface N8nApp { restEndpoint: string; externalHooks: IExternalHooksClass; defaultCredentialsName: string; - getCurrentDate(): Date; } diff --git a/packages/cli/src/endpoints/credentials.endpoints.ts b/packages/cli/src/endpoints/credentials.endpoints.ts index 0519dbd3a0023..fe35227bc9012 100644 --- a/packages/cli/src/endpoints/credentials.endpoints.ts +++ b/packages/cli/src/endpoints/credentials.endpoints.ts @@ -88,13 +88,17 @@ export function credentialsEndpoints(this: N8nApp): void { // Add the added date for node access permissions for (const nodeAccess of newCredential.nodesAccess) { - nodeAccess.date = this.getCurrentDate(); + nodeAccess.date = new Date(); } const encryptionKey = await UserSettings.getEncryptionKey(); if (!encryptionKey) { - throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); + throw new ResponseHelper.ResponseError( + RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, + undefined, + 500, + ); } // Encrypt the data @@ -196,7 +200,7 @@ export function credentialsEndpoints(this: N8nApp): void { // Add the date for newly added node access permissions for (const nodeAccess of updateData.nodesAccess) { if (!nodeAccess.date) { - nodeAccess.date = this.getCurrentDate(); + nodeAccess.date = new Date(); } } @@ -235,7 +239,7 @@ export function credentialsEndpoints(this: N8nApp): void { const newCredentialData = credentials.getDataToSave() as ICredentialsDb; // Add special database related data - newCredentialData.updatedAt = this.getCurrentDate(); + newCredentialData.updatedAt = new Date(); await this.externalHooks.run('credentials.update', [newCredentialData]); diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts new file mode 100644 index 0000000000000..6f18a721b14ee --- /dev/null +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -0,0 +1,102 @@ +import express = require('express'); +import { getConnection } from 'typeorm'; + +import { Db } from '../../src'; +import { randomName, randomString } from './shared/random'; +import * as utils from './shared/utils'; +import * as core from 'n8n-core'; + +let app: express.Application; + +beforeAll(async () => { + app = utils.initTestServer({ + namespaces: ['credentials'], + applyAuth: true, + externalHooks: true, + }); + await utils.initTestDb(); +}); + +beforeEach(async () => { + await utils.createOwnerShell(); +}); + +afterEach(async () => { + await utils.truncate(['User', 'Credentials']); +}); + +afterAll(() => { + return getConnection().close(); +}); + +test('POST /credentials should create a credential', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const response = await authShellAgent.post('/credentials').send(CREATE_CRED_PAYLOAD); + + expect(response.statusCode).toBe(200); + + const { id, name, type, nodesAccess, data: hashedData } = response.body.data; + + expect(id).toBe('1'); + expect(name).toBe(CREATE_CRED_PAYLOAD.name); + expect(type).toBe(CREATE_CRED_PAYLOAD.type); + expect(nodesAccess[0].nodeType).toBe(CREATE_CRED_PAYLOAD.nodesAccess[0].nodeType); + expect(hashedData).not.toBe(CREATE_CRED_PAYLOAD.data); + + const credential = await Db.collections.Credentials!.findOneOrFail(id); + + expect(credential.id).toBe(1); + expect(credential.name).toBe(CREATE_CRED_PAYLOAD.name); + expect(credential.type).toBe(CREATE_CRED_PAYLOAD.type); + expect(credential.nodesAccess[0].nodeType).toBe(CREATE_CRED_PAYLOAD.nodesAccess[0].nodeType); + expect(credential.data).not.toBe(CREATE_CRED_PAYLOAD.data); + + const sharedCredential = await Db.collections.SharedCredentials!.findOneOrFail({ + relations: ['user', 'credentials'], + where: { credentials: credential }, + }); + + expect(sharedCredential.user.id).toBe(shell.id); + expect(sharedCredential.credentials.name).toBe(CREATE_CRED_PAYLOAD.name); +}); + +test('POST /credentials should fail due to missing encryption key', async () => { + const { UserSettings } = require('n8n-core'); + const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); + mock.mockReturnValue(undefined); + + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const response = await authShellAgent.post('/credentials').send(CREATE_CRED_PAYLOAD); + + expect(response.statusCode).toBe(500); + + mock.mockRestore(); +}); + +test('POST /credentials should ignore ID in payload', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const firstResponse = await authShellAgent + .post('/credentials') + .send({ id: '8', ...CREATE_CRED_PAYLOAD }); + + expect(firstResponse.body.data.id).not.toBe('8'); + + const secondResponse = await authShellAgent + .post('/credentials') + .send({ id: 8, ...CREATE_CRED_PAYLOAD }); + + expect(secondResponse.body.data.id).not.toBe(8); +}); + +const CREATE_CRED_PAYLOAD = { + name: randomName(), + type: randomName(), + nodesAccess: [{ nodeType: randomName() }], + data: { accessToken: randomString(5, 15) }, +}; diff --git a/packages/cli/test/integration/shared/types.d.ts b/packages/cli/test/integration/shared/types.d.ts index be577afde2895..26c60f15e960f 100644 --- a/packages/cli/test/integration/shared/types.d.ts +++ b/packages/cli/test/integration/shared/types.d.ts @@ -10,6 +10,6 @@ export type SmtpTestAccount = { }; }; -export type EndpointNamespace = 'me' | 'users' | 'auth' | 'owner' | 'passwordReset'; +export type EndpointNamespace = 'me' | 'users' | 'auth' | 'owner' | 'passwordReset' | 'credentials'; export type NamespacesMap = Readonly void>>; diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index d5779b290344f..b8396bc748512 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -11,13 +11,14 @@ import { LoggerProxy } from 'n8n-workflow'; import config = require('../../../config'); import { AUTHLESS_ENDPOINTS, REST_PATH_SEGMENT } from './constants'; import { addRoutes as authMiddleware } from '../../../src/UserManagement/routes'; -import { Db, IDatabaseCollections } from '../../../src'; +import { Db, ExternalHooks, IDatabaseCollections } from '../../../src'; import { User } from '../../../src/databases/entities/User'; import { meNamespace as meEndpoints } from '../../../src/UserManagement/routes/me'; import { usersNamespace as usersEndpoints } from '../../../src/UserManagement/routes/users'; import { authenticationMethods as authEndpoints } from '../../../src/UserManagement/routes/auth'; import { ownerNamespace as ownerEndpoints } from '../../../src/UserManagement/routes/owner'; import { passwordResetNamespace as passwordResetEndpoints } from '../../../src/UserManagement/routes/passwordReset'; +import { credentialsEndpoints } from '../../../src/endpoints/credentials.endpoints'; import { getConnection } from 'typeorm'; import { issueJWT } from '../../../src/UserManagement/auth/jwt'; import { randomEmail, randomValidPassword, randomName } from './random'; @@ -43,13 +44,16 @@ export const initLogger = () => { export function initTestServer({ applyAuth, namespaces, + externalHooks, }: { applyAuth: boolean; + externalHooks?: true; namespaces?: EndpointNamespace[]; }) { const testServer = { app: express(), restEndpoint: REST_PATH_SEGMENT, + ...(externalHooks ? { externalHooks: ExternalHooks() } : {}), }; testServer.app.use(bodyParser.json()); @@ -69,6 +73,7 @@ export function initTestServer({ auth: authEndpoints, owner: ownerEndpoints, passwordReset: passwordResetEndpoints, + credentials: credentialsEndpoints, }; for (const namespace of namespaces) { @@ -178,7 +183,6 @@ export function getAllRoles() { ]); } - // ---------------------------------- // request agent // ---------------------------------- @@ -186,10 +190,7 @@ export function getAllRoles() { /** * Create a request agent, optionally with an auth cookie. */ -export async function createAgent( - app: express.Application, - options?: { auth: true; user: User }, -) { +export async function createAgent(app: express.Application, options?: { auth: true; user: User }) { const agent = request.agent(app); agent.use(prefix(REST_PATH_SEGMENT)); From 8ca92f4d11bfae1b63f66e51c682be8f20a9a54e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 12:54:34 +0100 Subject: [PATCH 06/37] :zap: Add validation to model --- packages/cli/src/databases/entities/CredentialsEntity.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/databases/entities/CredentialsEntity.ts b/packages/cli/src/databases/entities/CredentialsEntity.ts index a6d2796e0d32c..dd9375ab167fb 100644 --- a/packages/cli/src/databases/entities/CredentialsEntity.ts +++ b/packages/cli/src/databases/entities/CredentialsEntity.ts @@ -13,7 +13,7 @@ import { UpdateDateColumn, } from 'typeorm'; -import { Length } from 'class-validator'; +import { IsString, Length } from 'class-validator'; import config = require('../../../config'); import { DatabaseType, ICredentialsDb } from '../..'; import { SharedCredentials } from './SharedCredentials'; @@ -55,15 +55,18 @@ export class CredentialsEntity implements ICredentialsDb { id: number; @Column({ length: 128 }) + @IsString({ message: 'Credential `name` must be of type string.' }) @Length(3, 128, { message: 'Credential name must be $constraint1 to $constraint2 characters long.', }) name: string; @Column('text') + @IsString({ message: 'Credential `data` must be of type string.' }) data: string; @Index() + @IsString({ message: 'Credential `type` must be of type string.' }) @Column({ length: 32 }) type: string; From a21ff8c372e1331dcd4cbbc5ea21ccc541e8bc6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 13:00:58 +0100 Subject: [PATCH 07/37] :zap: Adjust validation --- packages/cli/src/databases/entities/CredentialsEntity.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/databases/entities/CredentialsEntity.ts b/packages/cli/src/databases/entities/CredentialsEntity.ts index dd9375ab167fb..e6518e6c4b1c7 100644 --- a/packages/cli/src/databases/entities/CredentialsEntity.ts +++ b/packages/cli/src/databases/entities/CredentialsEntity.ts @@ -13,7 +13,7 @@ import { UpdateDateColumn, } from 'typeorm'; -import { IsString, Length } from 'class-validator'; +import { IsArray, IsObject, IsString, Length } from 'class-validator'; import config = require('../../../config'); import { DatabaseType, ICredentialsDb } from '../..'; import { SharedCredentials } from './SharedCredentials'; @@ -62,7 +62,7 @@ export class CredentialsEntity implements ICredentialsDb { name: string; @Column('text') - @IsString({ message: 'Credential `data` must be of type string.' }) + @IsObject() data: string; @Index() @@ -74,6 +74,7 @@ export class CredentialsEntity implements ICredentialsDb { shared: SharedCredentials[]; @Column(resolveDataType('json')) + @IsArray() nodesAccess: ICredentialNodeAccess[]; @CreateDateColumn({ precision: 3, default: () => getTimestampSyntax() }) From 217c149a45f079dd91b69d1f883ae6df3782b3c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 13:01:05 +0100 Subject: [PATCH 08/37] :test_tube: Add test --- .../integration/credentials.endpoints.test.ts | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 6f18a721b14ee..e3e84c90d3719 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -4,7 +4,6 @@ import { getConnection } from 'typeorm'; import { Db } from '../../src'; import { randomName, randomString } from './shared/random'; import * as utils from './shared/utils'; -import * as core from 'n8n-core'; let app: express.Application; @@ -62,7 +61,43 @@ test('POST /credentials should create a credential', async () => { expect(sharedCredential.credentials.name).toBe(CREATE_CRED_PAYLOAD.name); }); -test('POST /credentials should fail due to missing encryption key', async () => { +test('POST /credentials should fail with invalid inputs', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const invalidPayloads = [ + { + type: randomName(), + nodesAccess: [{ nodeType: randomName() }], + data: { accessToken: randomString(5, 15) }, + }, + { + name: randomName(), + nodesAccess: [{ nodeType: randomName() }], + data: { accessToken: randomString(5, 15) }, + }, + { + name: randomName(), + type: randomName(), + data: { accessToken: randomString(5, 15) }, + }, + { + name: randomName(), + type: randomName(), + nodesAccess: [{ nodeType: randomName() }], + }, + {}, + [], + undefined, + ]; + + for (const invalidPayload of invalidPayloads) { + const response = await authShellAgent.post('/credentials').send(invalidPayload); + expect(response.statusCode).toBe(400); + } +}); + +test('POST /credentials should fail with missing encryption key', async () => { const { UserSettings } = require('n8n-core'); const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); mock.mockReturnValue(undefined); From e73afad046783c801f25d56b3fd378a8d1b7e71b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 13:09:04 +0100 Subject: [PATCH 09/37] :truck: Sort creds endpoints --- .../src/endpoints/credentials.endpoints.ts | 74 +++++++++---------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/packages/cli/src/endpoints/credentials.endpoints.ts b/packages/cli/src/endpoints/credentials.endpoints.ts index fe35227bc9012..a3d2e73221df6 100644 --- a/packages/cli/src/endpoints/credentials.endpoints.ts +++ b/packages/cli/src/endpoints/credentials.endpoints.ts @@ -25,7 +25,7 @@ import type { N8nApp } from '../UserManagement/Interfaces'; export function credentialsEndpoints(this: N8nApp): void { /** - * Return a unique credential name. + * Generate a unique credential name. */ this.app.get( `/${this.restEndpoint}/credentials/new`, @@ -40,35 +40,25 @@ export function credentialsEndpoints(this: N8nApp): void { ); /** - * Delete a credential. + * Test a credential. */ - this.app.delete( - `/${this.restEndpoint}/credentials/:id`, - ResponseHelper.send(async (req: CredentialRequest.Delete) => { - const { id: credentialId } = req.params; + this.app.post( + `/${this.restEndpoint}/credentials-test`, + ResponseHelper.send(async (req: CredentialRequest.Test): Promise => { + const { credentials, nodeToTestWith } = req.body; - const shared = await Db.collections.SharedCredentials!.findOne({ - relations: ['credentials'], - where: whereClause({ - user: req.user, - entityType: 'credentials', - entityId: credentialId, - }), - }); + const encryptionKey = await UserSettings.getEncryptionKey(); - if (!shared) { - throw new ResponseHelper.ResponseError( - `Credential with ID "${credentialId}" could not be found to be deleted.`, - undefined, - 404, - ); + if (!encryptionKey) { + return { + status: 'Error', + message: RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, + }; } - await this.externalHooks.run('credentials.delete', [credentialId]); - - await Db.collections.Credentials!.delete(credentialId); + const helper = new CredentialsHelper(encryptionKey); - return true; + return helper.testCredentials(credentials.type, credentials, nodeToTestWith); }), ); @@ -145,25 +135,35 @@ export function credentialsEndpoints(this: N8nApp): void { ); /** - * Test a credential. + * Delete a credential. */ - this.app.post( - `/${this.restEndpoint}/credentials-test`, - ResponseHelper.send(async (req: CredentialRequest.Test): Promise => { - const { credentials, nodeToTestWith } = req.body; + this.app.delete( + `/${this.restEndpoint}/credentials/:id`, + ResponseHelper.send(async (req: CredentialRequest.Delete) => { + const { id: credentialId } = req.params; - const encryptionKey = await UserSettings.getEncryptionKey(); + const shared = await Db.collections.SharedCredentials!.findOne({ + relations: ['credentials'], + where: whereClause({ + user: req.user, + entityType: 'credentials', + entityId: credentialId, + }), + }); - if (!encryptionKey) { - return { - status: 'Error', - message: RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, - }; + if (!shared) { + throw new ResponseHelper.ResponseError( + `Credential with ID "${credentialId}" could not be found to be deleted.`, + undefined, + 404, + ); } - const helper = new CredentialsHelper(encryptionKey); + await this.externalHooks.run('credentials.delete', [credentialId]); - return helper.testCredentials(credentials.type, credentials, nodeToTestWith); + await Db.collections.Credentials!.delete(credentialId); + + return true; }), ); From 4a424719f3aaaa165860dac14416b07890119644 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 13:26:07 +0100 Subject: [PATCH 10/37] :pencil2: Plan out pending tests --- .../integration/credentials.endpoints.test.ts | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index e3e84c90d3719..39b02244a5f0e 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -28,7 +28,7 @@ afterAll(() => { return getConnection().close(); }); -test('POST /credentials should create a credential', async () => { +test('POST /credentials should create cred', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); @@ -129,6 +129,30 @@ test('POST /credentials should ignore ID in payload', async () => { expect(secondResponse.body.data.id).not.toBe(8); }); +// DELETE /credentials/:id should delete cred for owner +// DELETE /credentials/:id should delete cred for owning member +// DELETE /credentials/:id should fail to delete cred for non-owning member +// DELETE /credentials/:id should fail if credential not found + +// PATCH /credentials/:id should update cred for owner +// PATCH /credentials/:id should update cred for owning member +// PATCH /credentials/:id should fail to update cred for non-owning member +// PATCH /credentials/:id should fail with invalid inputs +// PATCH /credentials/:id should fail if credential not found +// PATCH /credentials/:id should fail with missing encryption key + +// GET /credentials/:id should retrieve cred for owner +// GET /credentials/:id should retrieve cred for owning member +// GET /credentials/:id should return empty for non-owning member +// GET /credentials/:id should fail with missing encryption key +// GET /credentials/:id with includeData should retrieve cred and data + +// GET /credentials should retrieve all creds for owner +// GET /credentials should retrieve owned creds for member +// GET /credentials should not return non-owned creds for member +// GET /credentials should fail with missing encryption key +// GET /credentials with includeData should retrieve cred and data + const CREATE_CRED_PAYLOAD = { name: randomName(), type: randomName(), From 73ae460b8cbdd9c28c50e4b0f35926fd3f84ad63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 15:43:30 +0100 Subject: [PATCH 11/37] :test_tube: Add deletion tests --- .../integration/credentials.endpoints.test.ts | 84 ++++++++++++++++--- packages/cli/test/integration/shared/utils.ts | 16 ++++ 2 files changed, 88 insertions(+), 12 deletions(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 39b02244a5f0e..1fb095e7ae102 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -1,11 +1,17 @@ import express = require('express'); import { getConnection } from 'typeorm'; +import { Credentials } from '../../../core/dist/src'; import { Db } from '../../src'; +import { CredentialsEntity } from '../../src/databases/entities/CredentialsEntity'; +import { Role } from '../../src/databases/entities/Role'; +import { SharedCredentials } from '../../src/databases/entities/SharedCredentials'; import { randomName, randomString } from './shared/random'; import * as utils from './shared/utils'; +import { getCredentialOwnerRole } from './shared/utils'; let app: express.Application; +let credentialOwnerRole: Role; beforeAll(async () => { app = utils.initTestServer({ @@ -14,6 +20,7 @@ beforeAll(async () => { externalHooks: true, }); await utils.initTestDb(); + credentialOwnerRole = await getCredentialOwnerRole(); }); beforeEach(async () => { @@ -129,29 +136,82 @@ test('POST /credentials should ignore ID in payload', async () => { expect(secondResponse.body.data.id).not.toBe(8); }); -// DELETE /credentials/:id should delete cred for owner -// DELETE /credentials/:id should delete cred for owning member -// DELETE /credentials/:id should fail to delete cred for non-owning member -// DELETE /credentials/:id should fail if credential not found +test('DELETE /credentials/:id should delete cred for owner', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const savedCredential = await utils.saveCredential(CREATE_CRED_PAYLOAD, { + user: shell, + role: credentialOwnerRole, + }); + + const response = await authShellAgent.delete(`/credentials/${savedCredential.id}`); + expect(response.statusCode).toBe(200); + expect(response.body).toEqual({ data: true }); + + const deletedCredential = await Db.collections.Credentials!.findOne(savedCredential.id); + expect(deletedCredential).toBeUndefined(); // deleted +}); + +test('DELETE /credentials/:id should delete cred for owning member', async () => { + const member = await utils.createUser(); + const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); + + const savedCredential = await utils.saveCredential(CREATE_CRED_PAYLOAD, { + user: member, + role: credentialOwnerRole, + }); + + const response = await authMemberAgent.delete(`/credentials/${savedCredential.id}`); + expect(response.statusCode).toBe(200); + expect(response.body).toEqual({ data: true }); + + const deletedCredential = await Db.collections.Credentials!.findOne(savedCredential.id); + expect(deletedCredential).toBeUndefined(); // deleted +}); + +test('DELETE /credentials/:id should fail for non-owning member', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + + const member = await utils.createUser(); + const authMemberAgenet = await utils.createAgent(app, { auth: true, user: member }); + + const savedCredential = await utils.saveCredential(CREATE_CRED_PAYLOAD, { + user: shell, + role: credentialOwnerRole, + }); + + const response = await authMemberAgenet.delete(`/credentials/${savedCredential.id}`); + expect(response.statusCode).toBe(404); + + const shellCredential = await Db.collections.Credentials!.findOne(savedCredential.id); + expect(shellCredential).toBeDefined(); // not deleted +}); + +test('DELETE /credentials/:id should fail if no cred found', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const response = await authShellAgent.delete('/credentials/123'); + expect(response.statusCode).toBe(404); +}); // PATCH /credentials/:id should update cred for owner // PATCH /credentials/:id should update cred for owning member -// PATCH /credentials/:id should fail to update cred for non-owning member +// PATCH /credentials/:id should fail for non-owning member // PATCH /credentials/:id should fail with invalid inputs // PATCH /credentials/:id should fail if credential not found // PATCH /credentials/:id should fail with missing encryption key -// GET /credentials/:id should retrieve cred for owner -// GET /credentials/:id should retrieve cred for owning member +// GET /credentials/:id should retrieve all creds for owner + includeData +// GET /credentials/:id should retrieve all creds owning member + includeData // GET /credentials/:id should return empty for non-owning member // GET /credentials/:id should fail with missing encryption key -// GET /credentials/:id with includeData should retrieve cred and data -// GET /credentials should retrieve all creds for owner -// GET /credentials should retrieve owned creds for member +// GET /credentials should retrieve cred for owner + includeData +// GET /credentials should retrieve cred for member + includeData // GET /credentials should not return non-owned creds for member -// GET /credentials should fail with missing encryption key -// GET /credentials with includeData should retrieve cred and data +// GET /credentials should fail with missing encryption keya const CREATE_CRED_PAYLOAD = { name: randomName(), diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index b8396bc748512..36a2c9a8dc840 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -25,6 +25,7 @@ import { randomEmail, randomValidPassword, randomName } from './random'; import type { EndpointNamespace, NamespacesMap, SmtpTestAccount } from './types'; import { Role } from '../../../src/databases/entities/Role'; import { getLogger } from '../../../src/Logger'; +import { CredentialsEntity } from '../../../src/databases/entities/CredentialsEntity'; // ---------------------------------- // test server @@ -99,6 +100,21 @@ export async function truncate(entities: Array) { await getConnection().query('PRAGMA foreign_keys=ON'); } +/** + * Save a credential to the DB, sharing it with a user. + */ +export async function saveCredential(credData: object, { user, role }: { user: User; role: Role }) { + const savedCredential = (await Db.collections.Credentials!.save(credData)) as CredentialsEntity; + + await Db.collections.SharedCredentials!.save({ + user, + credentials: savedCredential, + role, + }); + + return savedCredential; +} + /** * Store a user in the DB, defaulting to a `member`. */ From 2624e11a5c7598327629976fb62407e18fe57e34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 16:51:07 +0100 Subject: [PATCH 12/37] :test_tube: Add patch tests --- .../src/endpoints/credentials.endpoints.ts | 2 + .../integration/credentials.endpoints.test.ts | 247 +++++++++++++----- packages/cli/test/integration/shared/utils.ts | 54 +++- 3 files changed, 238 insertions(+), 65 deletions(-) diff --git a/packages/cli/src/endpoints/credentials.endpoints.ts b/packages/cli/src/endpoints/credentials.endpoints.ts index a3d2e73221df6..608952cb102f9 100644 --- a/packages/cli/src/endpoints/credentials.endpoints.ts +++ b/packages/cli/src/endpoints/credentials.endpoints.ts @@ -178,6 +178,8 @@ export function credentialsEndpoints(this: N8nApp): void { const updateData = new CredentialsEntity(); Object.assign(updateData, req.body); + await validateEntity(updateData); + const shared = await Db.collections.SharedCredentials!.findOne({ relations: ['credentials'], where: whereClause({ diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 1fb095e7ae102..04bf547f155c1 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -1,11 +1,8 @@ import express = require('express'); import { getConnection } from 'typeorm'; -import { Credentials } from '../../../core/dist/src'; import { Db } from '../../src'; -import { CredentialsEntity } from '../../src/databases/entities/CredentialsEntity'; import { Role } from '../../src/databases/entities/Role'; -import { SharedCredentials } from '../../src/databases/entities/SharedCredentials'; import { randomName, randomString } from './shared/random'; import * as utils from './shared/utils'; import { getCredentialOwnerRole } from './shared/utils'; @@ -39,25 +36,25 @@ test('POST /credentials should create cred', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - const response = await authShellAgent.post('/credentials').send(CREATE_CRED_PAYLOAD); + const payload = credentialPayload(); + + const response = await authShellAgent.post('/credentials').send(payload); expect(response.statusCode).toBe(200); const { id, name, type, nodesAccess, data: hashedData } = response.body.data; - expect(id).toBe('1'); - expect(name).toBe(CREATE_CRED_PAYLOAD.name); - expect(type).toBe(CREATE_CRED_PAYLOAD.type); - expect(nodesAccess[0].nodeType).toBe(CREATE_CRED_PAYLOAD.nodesAccess[0].nodeType); - expect(hashedData).not.toBe(CREATE_CRED_PAYLOAD.data); + expect(name).toBe(payload.name); + expect(type).toBe(payload.type); + expect(nodesAccess[0].nodeType).toBe(payload.nodesAccess[0].nodeType); + expect(hashedData).not.toBe(payload.data); const credential = await Db.collections.Credentials!.findOneOrFail(id); - expect(credential.id).toBe(1); - expect(credential.name).toBe(CREATE_CRED_PAYLOAD.name); - expect(credential.type).toBe(CREATE_CRED_PAYLOAD.type); - expect(credential.nodesAccess[0].nodeType).toBe(CREATE_CRED_PAYLOAD.nodesAccess[0].nodeType); - expect(credential.data).not.toBe(CREATE_CRED_PAYLOAD.data); + expect(credential.name).toBe(payload.name); + expect(credential.type).toBe(payload.type); + expect(credential.nodesAccess[0].nodeType).toBe(payload.nodesAccess[0].nodeType); + expect(credential.data).not.toBe(payload.data); const sharedCredential = await Db.collections.SharedCredentials!.findOneOrFail({ relations: ['user', 'credentials'], @@ -65,40 +62,14 @@ test('POST /credentials should create cred', async () => { }); expect(sharedCredential.user.id).toBe(shell.id); - expect(sharedCredential.credentials.name).toBe(CREATE_CRED_PAYLOAD.name); + expect(sharedCredential.credentials.name).toBe(payload.name); }); test('POST /credentials should fail with invalid inputs', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - const invalidPayloads = [ - { - type: randomName(), - nodesAccess: [{ nodeType: randomName() }], - data: { accessToken: randomString(5, 15) }, - }, - { - name: randomName(), - nodesAccess: [{ nodeType: randomName() }], - data: { accessToken: randomString(5, 15) }, - }, - { - name: randomName(), - type: randomName(), - data: { accessToken: randomString(5, 15) }, - }, - { - name: randomName(), - type: randomName(), - nodesAccess: [{ nodeType: randomName() }], - }, - {}, - [], - undefined, - ]; - - for (const invalidPayload of invalidPayloads) { + for (const invalidPayload of INVALID_PAYLOADS) { const response = await authShellAgent.post('/credentials').send(invalidPayload); expect(response.statusCode).toBe(400); } @@ -112,7 +83,7 @@ test('POST /credentials should fail with missing encryption key', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - const response = await authShellAgent.post('/credentials').send(CREATE_CRED_PAYLOAD); + const response = await authShellAgent.post('/credentials').send(credentialPayload()); expect(response.statusCode).toBe(500); @@ -125,13 +96,13 @@ test('POST /credentials should ignore ID in payload', async () => { const firstResponse = await authShellAgent .post('/credentials') - .send({ id: '8', ...CREATE_CRED_PAYLOAD }); + .send({ id: '8', ...credentialPayload() }); expect(firstResponse.body.data.id).not.toBe('8'); const secondResponse = await authShellAgent .post('/credentials') - .send({ id: 8, ...CREATE_CRED_PAYLOAD }); + .send({ id: 8, ...credentialPayload() }); expect(secondResponse.body.data.id).not.toBe(8); }); @@ -140,7 +111,7 @@ test('DELETE /credentials/:id should delete cred for owner', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - const savedCredential = await utils.saveCredential(CREATE_CRED_PAYLOAD, { + const savedCredential = await utils.saveCredential(credentialPayload(), { user: shell, role: credentialOwnerRole, }); @@ -157,7 +128,7 @@ test('DELETE /credentials/:id should delete cred for owning member', async () => const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); - const savedCredential = await utils.saveCredential(CREATE_CRED_PAYLOAD, { + const savedCredential = await utils.saveCredential(credentialPayload(), { user: member, role: credentialOwnerRole, }); @@ -174,14 +145,14 @@ test('DELETE /credentials/:id should fail for non-owning member', async () => { const shell = await Db.collections.User!.findOneOrFail(); const member = await utils.createUser(); - const authMemberAgenet = await utils.createAgent(app, { auth: true, user: member }); + const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); - const savedCredential = await utils.saveCredential(CREATE_CRED_PAYLOAD, { + const savedCredential = await utils.saveCredential(credentialPayload(), { user: shell, role: credentialOwnerRole, }); - const response = await authMemberAgenet.delete(`/credentials/${savedCredential.id}`); + const response = await authMemberAgent.delete(`/credentials/${savedCredential.id}`); expect(response.statusCode).toBe(404); const shellCredential = await Db.collections.Credentials!.findOne(savedCredential.id); @@ -196,12 +167,144 @@ test('DELETE /credentials/:id should fail if no cred found', async () => { expect(response.statusCode).toBe(404); }); -// PATCH /credentials/:id should update cred for owner -// PATCH /credentials/:id should update cred for owning member -// PATCH /credentials/:id should fail for non-owning member -// PATCH /credentials/:id should fail with invalid inputs -// PATCH /credentials/:id should fail if credential not found -// PATCH /credentials/:id should fail with missing encryption key +test('PATCH /credentials/:id should update cred for owner', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const savedCredential = await utils.saveCredential(credentialPayload(), { + user: shell, + role: credentialOwnerRole, + }); + + const patchPayload = credentialPayload(); + + const response = await authShellAgent + .patch(`/credentials/${savedCredential.id}`) + .send(patchPayload); + + expect(response.statusCode).toBe(200); + + const { id, name, type, nodesAccess, data: encryptedData } = response.body.data; + + expect(name).toBe(patchPayload.name); + expect(type).toBe(patchPayload.type); + expect(nodesAccess[0].nodeType).toBe(patchPayload.nodesAccess[0].nodeType); + expect(encryptedData).not.toBe(patchPayload.data); + + const credential = await Db.collections.Credentials!.findOneOrFail(id); + + expect(credential.name).toBe(patchPayload.name); + expect(credential.type).toBe(patchPayload.type); + expect(credential.nodesAccess[0].nodeType).toBe(patchPayload.nodesAccess[0].nodeType); + expect(credential.data).not.toBe(patchPayload.data); + + const sharedCredential = await Db.collections.SharedCredentials!.findOneOrFail({ + relations: ['user', 'credentials'], + where: { credentials: credential }, + }); + + expect(sharedCredential.credentials.name).toBe(patchPayload.name); +}); + +test('PATCH /credentials/:id should update cred for owning member', async () => { + const member = await utils.createUser(); + const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); + + const savedCredential = await utils.saveCredential(credentialPayload(), { + user: member, + role: credentialOwnerRole, + }); + + const patchPayload = credentialPayload(); + + const response = await authMemberAgent + .patch(`/credentials/${savedCredential.id}`) + .send(patchPayload); + + expect(response.statusCode).toBe(200); + + const { id, name, type, nodesAccess, data: encryptedData } = response.body.data; + + expect(name).toBe(patchPayload.name); + expect(type).toBe(patchPayload.type); + expect(nodesAccess[0].nodeType).toBe(patchPayload.nodesAccess[0].nodeType); + expect(encryptedData).not.toBe(patchPayload.data); + + const credential = await Db.collections.Credentials!.findOneOrFail(id); + + expect(credential.name).toBe(patchPayload.name); + expect(credential.type).toBe(patchPayload.type); + expect(credential.nodesAccess[0].nodeType).toBe(patchPayload.nodesAccess[0].nodeType); + expect(credential.data).not.toBe(patchPayload.data); + + const sharedCredential = await Db.collections.SharedCredentials!.findOneOrFail({ + relations: ['user', 'credentials'], + where: { credentials: credential }, + }); + + expect(sharedCredential.credentials.name).toBe(patchPayload.name); +}); + +test('PATCH /credentials/:id should fail for non-owning member', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const member = await utils.createUser(); + const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); + + const savedCredential = await utils.saveCredential(credentialPayload(), { + user: shell, + role: credentialOwnerRole, + }); + + const patchPayload = credentialPayload(); + + const response = await authMemberAgent + .patch(`/credentials/${savedCredential.id}`) + .send(patchPayload); + expect(response.statusCode).toBe(404); + + const shellCredential = await Db.collections.Credentials!.findOneOrFail(savedCredential.id); + expect(shellCredential.name).not.toBe(patchPayload.name); // not updated +}); + +test('PATCH /credentials/:id should fail with invalid inputs', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const savedCredential = await utils.saveCredential(credentialPayload(), { + user: shell, + role: credentialOwnerRole, + }); + + for (const invalidPayload of INVALID_PAYLOADS) { + const response = await authShellAgent + .patch(`/credentials/${savedCredential.id}`) + .send(invalidPayload); + expect(response.statusCode).toBe(400); + } +}); + +test('PATCH /credentials/:id should fail if cred not found', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const response = await authShellAgent.patch('/credentials/123').send(credentialPayload()); + expect(response.statusCode).toBe(404); +}); + +test('PATCH /credentials/:id should fail with missing encryption key', async () => { + const { UserSettings } = require('n8n-core'); + const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); + mock.mockReturnValue(undefined); + + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const response = await authShellAgent.post('/credentials').send(credentialPayload()); + + expect(response.statusCode).toBe(500); + + mock.mockRestore(); +}); // GET /credentials/:id should retrieve all creds for owner + includeData // GET /credentials/:id should retrieve all creds owning member + includeData @@ -211,11 +314,37 @@ test('DELETE /credentials/:id should fail if no cred found', async () => { // GET /credentials should retrieve cred for owner + includeData // GET /credentials should retrieve cred for member + includeData // GET /credentials should not return non-owned creds for member -// GET /credentials should fail with missing encryption keya +// GET /credentials should fail with missing encryption key -const CREATE_CRED_PAYLOAD = { +const credentialPayload = () => ({ name: randomName(), type: randomName(), nodesAccess: [{ nodeType: randomName() }], data: { accessToken: randomString(5, 15) }, -}; +}); + +const INVALID_PAYLOADS = [ + { + type: randomName(), + nodesAccess: [{ nodeType: randomName() }], + data: { accessToken: randomString(5, 15) }, + }, + { + name: randomName(), + nodesAccess: [{ nodeType: randomName() }], + data: { accessToken: randomString(5, 15) }, + }, + { + name: randomName(), + type: randomName(), + data: { accessToken: randomString(5, 15) }, + }, + { + name: randomName(), + type: randomName(), + nodesAccess: [{ nodeType: randomName() }], + }, + {}, + [], + undefined, +]; diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index 36a2c9a8dc840..b8d4c4b3ee477 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -6,12 +6,13 @@ import bodyParser = require('body-parser'); import * as util from 'util'; import { createTestAccount } from 'nodemailer'; import { v4 as uuid } from 'uuid'; -import { LoggerProxy } from 'n8n-workflow'; +import { ICredentialDataDecryptedObject, ICredentialNodeAccess, LoggerProxy } from 'n8n-workflow'; +import { Credentials, UserSettings } from 'n8n-core'; import config = require('../../../config'); import { AUTHLESS_ENDPOINTS, REST_PATH_SEGMENT } from './constants'; import { addRoutes as authMiddleware } from '../../../src/UserManagement/routes'; -import { Db, ExternalHooks, IDatabaseCollections } from '../../../src'; +import { Db, ExternalHooks, ICredentialsDb, IDatabaseCollections } from '../../../src'; import { User } from '../../../src/databases/entities/User'; import { meNamespace as meEndpoints } from '../../../src/UserManagement/routes/me'; import { usersNamespace as usersEndpoints } from '../../../src/UserManagement/routes/users'; @@ -26,6 +27,9 @@ import type { EndpointNamespace, NamespacesMap, SmtpTestAccount } from './types' import { Role } from '../../../src/databases/entities/Role'; import { getLogger } from '../../../src/Logger'; import { CredentialsEntity } from '../../../src/databases/entities/CredentialsEntity'; +import { RESPONSE_ERROR_MESSAGES } from '../../../src/constants'; + +export const isTestRun = process.argv[1].split('/').includes('jest'); // TODO: Phase out // ---------------------------------- // test server @@ -103,8 +107,26 @@ export async function truncate(entities: Array) { /** * Save a credential to the DB, sharing it with a user. */ -export async function saveCredential(credData: object, { user, role }: { user: User; role: Role }) { - const savedCredential = (await Db.collections.Credentials!.save(credData)) as CredentialsEntity; +export async function saveCredential( + credData: { + name: string; + type: string; + nodesAccess: ICredentialNodeAccess[]; + data: ICredentialDataDecryptedObject; + }, + { user, role }: { user: User; role: Role }, +) { + const newCredential = new CredentialsEntity(); + + Object.assign(newCredential, credData); + + const encryptedData = await encryptCredentialData(newCredential); + + Object.assign(newCredential, encryptedData); + + const savedCredential = await Db.collections.Credentials!.save(newCredential); + + savedCredential.data = newCredential.data; await Db.collections.SharedCredentials!.save({ user, @@ -281,5 +303,25 @@ export async function getHasOwnerSetting() { */ export const getSmtpTestAccount = util.promisify(createTestAccount); -// TODO: Phase out -export const isTestRun = process.argv[1].split('/').includes('jest'); +// ---------------------------------- +// encryption +// ---------------------------------- + +async function encryptCredentialData(credential: CredentialsEntity) { + const encryptionKey = await UserSettings.getEncryptionKey(); + + if (!encryptionKey) { + throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); + } + + const coreCredential = new Credentials( + { id: null, name: credential.name }, + credential.type, + credential.nodesAccess, + ); + + // @ts-ignore + coreCredential.setData(credential.data, encryptionKey); + + return coreCredential.getDataToSave() as ICredentialsDb; +} From 937de2f2659f290e8af93ccc8f4826454c7d7c12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 18:01:34 +0100 Subject: [PATCH 13/37] :test_tube: Add get cred tests --- .../src/endpoints/credentials.endpoints.ts | 13 +- .../integration/credentials.endpoints.test.ts | 176 +++++++++++++++++- 2 files changed, 178 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/endpoints/credentials.endpoints.ts b/packages/cli/src/endpoints/credentials.endpoints.ts index 608952cb102f9..0de6c0f2222ca 100644 --- a/packages/cli/src/endpoints/credentials.endpoints.ts +++ b/packages/cli/src/endpoints/credentials.endpoints.ts @@ -209,7 +209,11 @@ export function credentialsEndpoints(this: N8nApp): void { const encryptionKey = await UserSettings.getEncryptionKey(); if (!encryptionKey) { - throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); + throw new ResponseHelper.ResponseError( + RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, + undefined, + 500, + ); } const coreCredential = new Credentials( @@ -305,7 +309,11 @@ export function credentialsEndpoints(this: N8nApp): void { const encryptionKey = await UserSettings.getEncryptionKey(); if (!encryptionKey) { - throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); + throw new ResponseHelper.ResponseError( + RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, + undefined, + 500, + ); } const coreCredential = new Credentials( @@ -360,6 +368,7 @@ export function credentialsEndpoints(this: N8nApp): void { }); } + // TODO: Useless segment coming from master - Remove? let encryptionKey; if (req.query.includeData === 'true') { diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 04bf547f155c1..bc65c3e1f82b6 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -5,10 +5,11 @@ import { Db } from '../../src'; import { Role } from '../../src/databases/entities/Role'; import { randomName, randomString } from './shared/random'; import * as utils from './shared/utils'; -import { getCredentialOwnerRole } from './shared/utils'; +import { getCredentialOwnerRole, getGlobalOwnerRole } from './shared/utils'; let app: express.Application; let credentialOwnerRole: Role; +let globalOwnerRole: Role; beforeAll(async () => { app = utils.initTestServer({ @@ -18,6 +19,7 @@ beforeAll(async () => { }); await utils.initTestDb(); credentialOwnerRole = await getCredentialOwnerRole(); + globalOwnerRole = await getGlobalOwnerRole(); }); beforeEach(async () => { @@ -306,15 +308,171 @@ test('PATCH /credentials/:id should fail with missing encryption key', async () mock.mockRestore(); }); -// GET /credentials/:id should retrieve all creds for owner + includeData -// GET /credentials/:id should retrieve all creds owning member + includeData -// GET /credentials/:id should return empty for non-owning member -// GET /credentials/:id should fail with missing encryption key +test('GET /credentials should retrieve all creds for owner', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + + for (let i = 0; i < 3; i++) { + await utils.saveCredential(credentialPayload(), { + user: shell, + role: credentialOwnerRole, + }); + } + + const response = await authShellAgent.get('/credentials'); + + expect(response.statusCode).toBe(200); + expect(response.body.data.length).toBe(3); + + for (const credential of response.body.data) { + const { name, type, nodesAccess, data: encryptedData } = credential; + expect(typeof name).toBe('string'); + expect(typeof type).toBe('string'); + expect(typeof nodesAccess[0].nodeType).toBe('string'); + expect(encryptedData).toBeUndefined(); + } +}); + +test('GET /credentials should retrieve owned creds for member', async () => { + const member = await utils.createUser(); + const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); + + for (let i = 0; i < 3; i++) { + await utils.saveCredential(credentialPayload(), { + user: member, + role: credentialOwnerRole, + }); + } + + const response = await authMemberAgent.get('/credentials'); + + expect(response.statusCode).toBe(200); + expect(response.body.data.length).toBe(3); + + for (const credential of response.body.data) { + const { name, type, nodesAccess, data: encryptedData } = credential; + expect(typeof name).toBe('string'); + expect(typeof type).toBe('string'); + expect(typeof nodesAccess[0].nodeType).toBe('string'); + expect(encryptedData).toBeUndefined(); + } +}); + +test('GET /credentials should not retrieve non-owned creds for member', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const member = await utils.createUser(); + const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); -// GET /credentials should retrieve cred for owner + includeData -// GET /credentials should retrieve cred for member + includeData -// GET /credentials should not return non-owned creds for member -// GET /credentials should fail with missing encryption key + for (let i = 0; i < 3; i++) { + await utils.saveCredential(credentialPayload(), { + user: shell, + role: credentialOwnerRole, + }); + } + + const response = await authMemberAgent.get('/credentials'); + + expect(response.statusCode).toBe(200); + expect(response.body.data.length).toBe(0); // shell's creds not returned +}); + +test('GET /credentials/:id should retrieve owned creds for owner', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const savedCredential = await utils.saveCredential(credentialPayload(), { + user: shell, + role: credentialOwnerRole, + }); + + const firstResponse = await authOwnerAgent.get(`/credentials/${savedCredential.id}`); + + expect(firstResponse.statusCode).toBe(200); + + expect(typeof firstResponse.body.data.name).toBe('string'); + expect(typeof firstResponse.body.data.type).toBe('string'); + expect(typeof firstResponse.body.data.nodesAccess[0].nodeType).toBe('string'); + expect(firstResponse.body.data.data).toBeUndefined(); + + const secondResponse = await authOwnerAgent + .get(`/credentials/${savedCredential.id}`) + .query({ includeData: true }); + + expect(secondResponse.statusCode).toBe(200); + + expect(typeof secondResponse.body.data.name).toBe('string'); + expect(typeof secondResponse.body.data.type).toBe('string'); + expect(typeof secondResponse.body.data.nodesAccess[0].nodeType).toBe('string'); + expect(secondResponse.body.data.data).toBeDefined(); +}); + +test('GET /credentials/:id should retrieve owned creds for owner', async () => { + const member = await utils.createUser(); + const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); + + const savedCredential = await utils.saveCredential(credentialPayload(), { + user: member, + role: credentialOwnerRole, + }); + + const firstResponse = await authMemberAgent.get(`/credentials/${savedCredential.id}`); + + expect(firstResponse.statusCode).toBe(200); + + expect(typeof firstResponse.body.data.name).toBe('string'); + expect(typeof firstResponse.body.data.type).toBe('string'); + expect(typeof firstResponse.body.data.nodesAccess[0].nodeType).toBe('string'); + expect(firstResponse.body.data.data).toBeUndefined(); + + const secondResponse = await authMemberAgent + .get(`/credentials/${savedCredential.id}`) + .query({ includeData: true }); + + expect(secondResponse.statusCode).toBe(200); + + expect(typeof secondResponse.body.data.name).toBe('string'); + expect(typeof secondResponse.body.data.type).toBe('string'); + expect(typeof secondResponse.body.data.nodesAccess[0].nodeType).toBe('string'); + expect(secondResponse.body.data.data).toBeDefined(); +}); + +test('GET /credentials/:id should not retrieve non-owned creds for member', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const member = await utils.createUser(); + const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); + + const savedCredential = await utils.saveCredential(credentialPayload(), { + user: shell, + role: credentialOwnerRole, + }); + + const response = await authMemberAgent.get(`/credentials/${savedCredential.id}`); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toEqual({}); // shell's cred not returned +}); + +test('GET /credentials/:id should fail with missing encryption key', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const savedCredential = await utils.saveCredential(credentialPayload(), { + user: shell, + role: credentialOwnerRole, + }); + + const { UserSettings } = require('n8n-core'); + const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); + mock.mockReturnValue(undefined); + + const response = await authShellAgent + .get(`/credentials/${savedCredential.id}`) + .query({ includeData: true }); + + expect(response.statusCode).toBe(500); + + mock.mockRestore(); +}); const credentialPayload = () => ({ name: randomName(), From 1f5af23783da9d44f5402eabb5f0da95a4cdaba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 18:04:42 +0100 Subject: [PATCH 14/37] :truck: Hoist import --- .../cli/test/integration/credentials.endpoints.test.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index bc65c3e1f82b6..8ee87ae506f4c 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -5,11 +5,12 @@ import { Db } from '../../src'; import { Role } from '../../src/databases/entities/Role'; import { randomName, randomString } from './shared/random'; import * as utils from './shared/utils'; -import { getCredentialOwnerRole, getGlobalOwnerRole } from './shared/utils'; +import { getCredentialOwnerRole } from './shared/utils'; + +const { UserSettings } = require('n8n-core'); let app: express.Application; let credentialOwnerRole: Role; -let globalOwnerRole: Role; beforeAll(async () => { app = utils.initTestServer({ @@ -19,7 +20,6 @@ beforeAll(async () => { }); await utils.initTestDb(); credentialOwnerRole = await getCredentialOwnerRole(); - globalOwnerRole = await getGlobalOwnerRole(); }); beforeEach(async () => { @@ -78,7 +78,6 @@ test('POST /credentials should fail with invalid inputs', async () => { }); test('POST /credentials should fail with missing encryption key', async () => { - const { UserSettings } = require('n8n-core'); const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); mock.mockReturnValue(undefined); @@ -294,7 +293,6 @@ test('PATCH /credentials/:id should fail if cred not found', async () => { }); test('PATCH /credentials/:id should fail with missing encryption key', async () => { - const { UserSettings } = require('n8n-core'); const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); mock.mockReturnValue(undefined); @@ -461,7 +459,6 @@ test('GET /credentials/:id should fail with missing encryption key', async () => role: credentialOwnerRole, }); - const { UserSettings } = require('n8n-core'); const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); mock.mockReturnValue(undefined); From 4786f6762842e2d2d02e528167a836a82ca1825f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 18:15:38 +0100 Subject: [PATCH 15/37] :pencil2: Make test descriptions consistent --- .../cli/test/integration/credentials.endpoints.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 8ee87ae506f4c..687ded15f4360 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -125,7 +125,7 @@ test('DELETE /credentials/:id should delete cred for owner', async () => { expect(deletedCredential).toBeUndefined(); // deleted }); -test('DELETE /credentials/:id should delete cred for owning member', async () => { +test('DELETE /credentials/:id should delete owned cred for member', async () => { const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); @@ -142,7 +142,7 @@ test('DELETE /credentials/:id should delete cred for owning member', async () => expect(deletedCredential).toBeUndefined(); // deleted }); -test('DELETE /credentials/:id should fail for non-owning member', async () => { +test('DELETE /credentials/:id should not delete non-owned cred for member', async () => { const shell = await Db.collections.User!.findOneOrFail(); const member = await utils.createUser(); @@ -207,7 +207,7 @@ test('PATCH /credentials/:id should update cred for owner', async () => { expect(sharedCredential.credentials.name).toBe(patchPayload.name); }); -test('PATCH /credentials/:id should update cred for owning member', async () => { +test('PATCH /credentials/:id should update owned cred for member', async () => { const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); @@ -246,7 +246,7 @@ test('PATCH /credentials/:id should update cred for owning member', async () => expect(sharedCredential.credentials.name).toBe(patchPayload.name); }); -test('PATCH /credentials/:id should fail for non-owning member', async () => { +test('PATCH /credentials/:id should not update non-owned cred for member', async () => { const shell = await Db.collections.User!.findOneOrFail(); const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); @@ -404,7 +404,7 @@ test('GET /credentials/:id should retrieve owned creds for owner', async () => { expect(secondResponse.body.data.data).toBeDefined(); }); -test('GET /credentials/:id should retrieve owned creds for owner', async () => { +test('GET /credentials/:id should retrieve owned creds for member', async () => { const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); From bac584f336a14d08738422ba5fc2cf034682725d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 18:18:15 +0100 Subject: [PATCH 16/37] :pencil2: Adjust description --- packages/cli/test/integration/credentials.endpoints.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 687ded15f4360..776795b854383 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -160,7 +160,7 @@ test('DELETE /credentials/:id should not delete non-owned cred for member', asyn expect(shellCredential).toBeDefined(); // not deleted }); -test('DELETE /credentials/:id should fail if no cred found', async () => { +test('DELETE /credentials/:id should fail if cred not found', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); From 4012671d2ca3facdfe47862c420ccc78683d54cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 16 Feb 2022 18:21:35 +0100 Subject: [PATCH 17/37] :test_tube: Add missing test --- .../cli/test/integration/credentials.endpoints.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 776795b854383..00fe9d94748e7 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -471,6 +471,16 @@ test('GET /credentials/:id should fail with missing encryption key', async () => mock.mockRestore(); }); +test('GET /credentials/:id should return empty if cred not found', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authMemberAgent = await utils.createAgent(app, { auth: true, user: shell }); + + const response = await authMemberAgent.get('/credentials/789'); + + expect(response.statusCode).toBe(200); + expect(response.body).toEqual({ data: {} }); +}); + const credentialPayload = () => ({ name: randomName(), type: randomName(), From 69c587a8141ee2b103bdce90c085bc18d1b0b504 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 17 Feb 2022 09:25:55 +0100 Subject: [PATCH 18/37] :pencil2: Make get descriptions consistent --- packages/cli/test/integration/credentials.endpoints.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 00fe9d94748e7..a2cbcb0dcafc6 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -374,7 +374,7 @@ test('GET /credentials should not retrieve non-owned creds for member', async () expect(response.body.data.length).toBe(0); // shell's creds not returned }); -test('GET /credentials/:id should retrieve owned creds for owner', async () => { +test('GET /credentials/:id should retrieve owned cred for owner', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authOwnerAgent = await utils.createAgent(app, { auth: true, user: shell }); @@ -404,7 +404,7 @@ test('GET /credentials/:id should retrieve owned creds for owner', async () => { expect(secondResponse.body.data.data).toBeDefined(); }); -test('GET /credentials/:id should retrieve owned creds for member', async () => { +test('GET /credentials/:id should retrieve owned cred for member', async () => { const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); @@ -434,7 +434,7 @@ test('GET /credentials/:id should retrieve owned creds for member', async () => expect(secondResponse.body.data.data).toBeDefined(); }); -test('GET /credentials/:id should not retrieve non-owned creds for member', async () => { +test('GET /credentials/:id should not retrieve non-owned cred for member', async () => { const shell = await Db.collections.User!.findOneOrFail(); const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); From 06aeb49b4d01ba7fd49bad5433cd6475a0a3e626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 17 Feb 2022 09:39:54 +0100 Subject: [PATCH 19/37] :rewind: Undo line break --- packages/cli/src/UserManagement/routes/users.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli/src/UserManagement/routes/users.ts b/packages/cli/src/UserManagement/routes/users.ts index c61d3737f2de5..cb73bfcef2689 100644 --- a/packages/cli/src/UserManagement/routes/users.ts +++ b/packages/cli/src/UserManagement/routes/users.ts @@ -19,6 +19,7 @@ import { User } from '../../databases/entities/User'; import { SharedWorkflow } from '../../databases/entities/SharedWorkflow'; import { SharedCredentials } from '../../databases/entities/SharedCredentials'; import { getInstance } from '../email/UserManagementMailer'; + import config = require('../../../config'); import { issueCookie } from '../auth/jwt'; From b002eed3fa1438fea9c6a2b6637587cec85c44cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 17 Feb 2022 10:22:35 +0100 Subject: [PATCH 20/37] :zap: Refactor to simplify `saveCredential` --- .../integration/credentials.endpoints.test.ts | 108 +++++------------- .../cli/test/integration/shared/types.d.ts | 19 ++- packages/cli/test/integration/shared/utils.ts | 43 ++++--- .../test/integration/users.endpoints.test.ts | 5 +- 4 files changed, 75 insertions(+), 100 deletions(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index a2cbcb0dcafc6..ee8dc1ca74bc1 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -2,15 +2,14 @@ import express = require('express'); import { getConnection } from 'typeorm'; import { Db } from '../../src'; -import { Role } from '../../src/databases/entities/Role'; import { randomName, randomString } from './shared/random'; import * as utils from './shared/utils'; -import { getCredentialOwnerRole } from './shared/utils'; +import type { SaveCredentialFunction } from './shared/types'; const { UserSettings } = require('n8n-core'); let app: express.Application; -let credentialOwnerRole: Role; +let saveCredential: SaveCredentialFunction; beforeAll(async () => { app = utils.initTestServer({ @@ -19,7 +18,8 @@ beforeAll(async () => { externalHooks: true, }); await utils.initTestDb(); - credentialOwnerRole = await getCredentialOwnerRole(); + const credentialOwnerRole = await utils.getCredentialOwnerRole(); + saveCredential = utils.affixRoleToSaveCredential(credentialOwnerRole); }); beforeEach(async () => { @@ -37,7 +37,6 @@ afterAll(() => { test('POST /credentials should create cred', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - const payload = credentialPayload(); const response = await authShellAgent.post('/credentials').send(payload); @@ -111,52 +110,45 @@ test('POST /credentials should ignore ID in payload', async () => { test('DELETE /credentials/:id should delete cred for owner', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - - const savedCredential = await utils.saveCredential(credentialPayload(), { - user: shell, - role: credentialOwnerRole, - }); + const savedCredential = await saveCredential(credentialPayload(), { user: shell }); const response = await authShellAgent.delete(`/credentials/${savedCredential.id}`); + expect(response.statusCode).toBe(200); expect(response.body).toEqual({ data: true }); const deletedCredential = await Db.collections.Credentials!.findOne(savedCredential.id); + expect(deletedCredential).toBeUndefined(); // deleted }); test('DELETE /credentials/:id should delete owned cred for member', async () => { const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); - - const savedCredential = await utils.saveCredential(credentialPayload(), { - user: member, - role: credentialOwnerRole, - }); + const savedCredential = await saveCredential(credentialPayload(), { user: member }); const response = await authMemberAgent.delete(`/credentials/${savedCredential.id}`); + expect(response.statusCode).toBe(200); expect(response.body).toEqual({ data: true }); const deletedCredential = await Db.collections.Credentials!.findOne(savedCredential.id); + expect(deletedCredential).toBeUndefined(); // deleted }); test('DELETE /credentials/:id should not delete non-owned cred for member', async () => { const shell = await Db.collections.User!.findOneOrFail(); - const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); - - const savedCredential = await utils.saveCredential(credentialPayload(), { - user: shell, - role: credentialOwnerRole, - }); + const savedCredential = await saveCredential(credentialPayload(), { user: shell }); const response = await authMemberAgent.delete(`/credentials/${savedCredential.id}`); + expect(response.statusCode).toBe(404); const shellCredential = await Db.collections.Credentials!.findOne(savedCredential.id); + expect(shellCredential).toBeDefined(); // not deleted }); @@ -165,18 +157,14 @@ test('DELETE /credentials/:id should fail if cred not found', async () => { const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); const response = await authShellAgent.delete('/credentials/123'); + expect(response.statusCode).toBe(404); }); test('PATCH /credentials/:id should update cred for owner', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - - const savedCredential = await utils.saveCredential(credentialPayload(), { - user: shell, - role: credentialOwnerRole, - }); - + const savedCredential = await saveCredential(credentialPayload(), { user: shell }); const patchPayload = credentialPayload(); const response = await authShellAgent @@ -210,12 +198,7 @@ test('PATCH /credentials/:id should update cred for owner', async () => { test('PATCH /credentials/:id should update owned cred for member', async () => { const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); - - const savedCredential = await utils.saveCredential(credentialPayload(), { - user: member, - role: credentialOwnerRole, - }); - + const savedCredential = await saveCredential(credentialPayload(), { user: member }); const patchPayload = credentialPayload(); const response = await authMemberAgent @@ -250,36 +233,30 @@ test('PATCH /credentials/:id should not update non-owned cred for member', async const shell = await Db.collections.User!.findOneOrFail(); const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); - - const savedCredential = await utils.saveCredential(credentialPayload(), { - user: shell, - role: credentialOwnerRole, - }); - + const savedCredential = await saveCredential(credentialPayload(), { user: shell }); const patchPayload = credentialPayload(); const response = await authMemberAgent .patch(`/credentials/${savedCredential.id}`) .send(patchPayload); + expect(response.statusCode).toBe(404); const shellCredential = await Db.collections.Credentials!.findOneOrFail(savedCredential.id); + expect(shellCredential.name).not.toBe(patchPayload.name); // not updated }); test('PATCH /credentials/:id should fail with invalid inputs', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - - const savedCredential = await utils.saveCredential(credentialPayload(), { - user: shell, - role: credentialOwnerRole, - }); + const savedCredential = await saveCredential(credentialPayload(), { user: shell }); for (const invalidPayload of INVALID_PAYLOADS) { const response = await authShellAgent .patch(`/credentials/${savedCredential.id}`) .send(invalidPayload); + expect(response.statusCode).toBe(400); } }); @@ -289,6 +266,7 @@ test('PATCH /credentials/:id should fail if cred not found', async () => { const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); const response = await authShellAgent.patch('/credentials/123').send(credentialPayload()); + expect(response.statusCode).toBe(404); }); @@ -311,10 +289,7 @@ test('GET /credentials should retrieve all creds for owner', async () => { const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); for (let i = 0; i < 3; i++) { - await utils.saveCredential(credentialPayload(), { - user: shell, - role: credentialOwnerRole, - }); + await saveCredential(credentialPayload(), { user: shell }); } const response = await authShellAgent.get('/credentials'); @@ -324,6 +299,7 @@ test('GET /credentials should retrieve all creds for owner', async () => { for (const credential of response.body.data) { const { name, type, nodesAccess, data: encryptedData } = credential; + expect(typeof name).toBe('string'); expect(typeof type).toBe('string'); expect(typeof nodesAccess[0].nodeType).toBe('string'); @@ -336,10 +312,7 @@ test('GET /credentials should retrieve owned creds for member', async () => { const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); for (let i = 0; i < 3; i++) { - await utils.saveCredential(credentialPayload(), { - user: member, - role: credentialOwnerRole, - }); + await saveCredential(credentialPayload(), { user: member }); } const response = await authMemberAgent.get('/credentials'); @@ -349,6 +322,7 @@ test('GET /credentials should retrieve owned creds for member', async () => { for (const credential of response.body.data) { const { name, type, nodesAccess, data: encryptedData } = credential; + expect(typeof name).toBe('string'); expect(typeof type).toBe('string'); expect(typeof nodesAccess[0].nodeType).toBe('string'); @@ -362,10 +336,7 @@ test('GET /credentials should not retrieve non-owned creds for member', async () const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); for (let i = 0; i < 3; i++) { - await utils.saveCredential(credentialPayload(), { - user: shell, - role: credentialOwnerRole, - }); + await saveCredential(credentialPayload(), { user: shell }); } const response = await authMemberAgent.get('/credentials'); @@ -377,11 +348,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 shell = await Db.collections.User!.findOneOrFail(); const authOwnerAgent = await utils.createAgent(app, { auth: true, user: shell }); - - const savedCredential = await utils.saveCredential(credentialPayload(), { - user: shell, - role: credentialOwnerRole, - }); + const savedCredential = await saveCredential(credentialPayload(), { user: shell }); const firstResponse = await authOwnerAgent.get(`/credentials/${savedCredential.id}`); @@ -397,7 +364,6 @@ test('GET /credentials/:id should retrieve owned cred for owner', async () => { .query({ includeData: true }); expect(secondResponse.statusCode).toBe(200); - expect(typeof secondResponse.body.data.name).toBe('string'); expect(typeof secondResponse.body.data.type).toBe('string'); expect(typeof secondResponse.body.data.nodesAccess[0].nodeType).toBe('string'); @@ -407,11 +373,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 utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); - - const savedCredential = await utils.saveCredential(credentialPayload(), { - user: member, - role: credentialOwnerRole, - }); + const savedCredential = await saveCredential(credentialPayload(), { user: member }); const firstResponse = await authMemberAgent.get(`/credentials/${savedCredential.id}`); @@ -438,11 +400,7 @@ test('GET /credentials/:id should not retrieve non-owned cred for member', async const shell = await Db.collections.User!.findOneOrFail(); const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); - - const savedCredential = await utils.saveCredential(credentialPayload(), { - user: shell, - role: credentialOwnerRole, - }); + const savedCredential = await saveCredential(credentialPayload(), { user: shell }); const response = await authMemberAgent.get(`/credentials/${savedCredential.id}`); @@ -453,11 +411,7 @@ 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 shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - - const savedCredential = await utils.saveCredential(credentialPayload(), { - user: shell, - role: credentialOwnerRole, - }); + const savedCredential = await saveCredential(credentialPayload(), { user: shell }); const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); mock.mockReturnValue(undefined); diff --git a/packages/cli/test/integration/shared/types.d.ts b/packages/cli/test/integration/shared/types.d.ts index 26c60f15e960f..cd88521434d56 100644 --- a/packages/cli/test/integration/shared/types.d.ts +++ b/packages/cli/test/integration/shared/types.d.ts @@ -1,4 +1,9 @@ -import type { N8nApp } from "../../../src/UserManagement/Interfaces"; +import type { ICredentialDataDecryptedObject, ICredentialNodeAccess } from 'n8n-workflow'; +import type { ICredentialsDb } from '../../../src'; +import type { CredentialsEntity } from '../../../src/databases/entities/CredentialsEntity'; +import type { User } from '../../../src/databases/entities/User'; + +import type { N8nApp } from '../../../src/UserManagement/Interfaces'; export type SmtpTestAccount = { user: string; @@ -13,3 +18,15 @@ export type SmtpTestAccount = { export type EndpointNamespace = 'me' | 'users' | 'auth' | 'owner' | 'passwordReset' | 'credentials'; export type NamespacesMap = Readonly void>>; + +export type CredentialPayload = { + name: string; + type: string; + nodesAccess: ICredentialNodeAccess[]; + data: ICredentialDataDecryptedObject; +}; + +export type SaveCredentialFunction = ( + credentialPayload: CredentialPayload, + { user }: { user: User }, +) => Promise; diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index b8d4c4b3ee477..933846ff4c0f9 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -6,28 +6,28 @@ import bodyParser = require('body-parser'); import * as util from 'util'; import { createTestAccount } from 'nodemailer'; import { v4 as uuid } from 'uuid'; -import { ICredentialDataDecryptedObject, ICredentialNodeAccess, LoggerProxy } from 'n8n-workflow'; +import { LoggerProxy } from 'n8n-workflow'; import { Credentials, UserSettings } from 'n8n-core'; +import { getConnection } from 'typeorm'; import config = require('../../../config'); import { AUTHLESS_ENDPOINTS, REST_PATH_SEGMENT } from './constants'; import { addRoutes as authMiddleware } from '../../../src/UserManagement/routes'; import { Db, ExternalHooks, ICredentialsDb, IDatabaseCollections } from '../../../src'; -import { User } from '../../../src/databases/entities/User'; import { meNamespace as meEndpoints } from '../../../src/UserManagement/routes/me'; import { usersNamespace as usersEndpoints } from '../../../src/UserManagement/routes/users'; import { authenticationMethods as authEndpoints } from '../../../src/UserManagement/routes/auth'; import { ownerNamespace as ownerEndpoints } from '../../../src/UserManagement/routes/owner'; import { passwordResetNamespace as passwordResetEndpoints } from '../../../src/UserManagement/routes/passwordReset'; import { credentialsEndpoints } from '../../../src/endpoints/credentials.endpoints'; -import { getConnection } from 'typeorm'; import { issueJWT } from '../../../src/UserManagement/auth/jwt'; import { randomEmail, randomValidPassword, randomName } from './random'; -import type { EndpointNamespace, NamespacesMap, SmtpTestAccount } from './types'; -import { Role } from '../../../src/databases/entities/Role'; import { getLogger } from '../../../src/Logger'; import { CredentialsEntity } from '../../../src/databases/entities/CredentialsEntity'; import { RESPONSE_ERROR_MESSAGES } from '../../../src/constants'; +import type { Role } from '../../../src/databases/entities/Role'; +import type { User } from '../../../src/databases/entities/User'; +import type { CredentialPayload, EndpointNamespace, NamespacesMap, SmtpTestAccount } from './types'; export const isTestRun = process.argv[1].split('/').includes('jest'); // TODO: Phase out @@ -35,11 +35,6 @@ export const isTestRun = process.argv[1].split('/').includes('jest'); // TODO: P // test server // ---------------------------------- -export const initLogger = () => { - config.set('logs.output', 'file'); // declutter console output during tests - LoggerProxy.init(getLogger()); -}; - /** * Initialize a test server to make requests to. * @@ -89,6 +84,18 @@ export function initTestServer({ return testServer.app; } +// ---------------------------------- +// test logger +// ---------------------------------- + +/** + * Initialize a silent logger for test runs. + */ +export const initTestLogger = () => { + config.set('logs.output', 'file'); + LoggerProxy.init(getLogger()); +}; + // ---------------------------------- // test DB // ---------------------------------- @@ -104,21 +111,21 @@ export async function truncate(entities: Array) { await getConnection().query('PRAGMA foreign_keys=ON'); } +export function affixRoleToSaveCredential(role: Role) { + return (credentialPayload: CredentialPayload, { user }: { user: User }) => + saveCredential(credentialPayload, { user, role }); +} + /** * Save a credential to the DB, sharing it with a user. */ -export async function saveCredential( - credData: { - name: string; - type: string; - nodesAccess: ICredentialNodeAccess[]; - data: ICredentialDataDecryptedObject; - }, +async function saveCredential( + credentialPayload: CredentialPayload, { user, role }: { user: User; role: Role }, ) { const newCredential = new CredentialsEntity(); - Object.assign(newCredential, credData); + Object.assign(newCredential, credentialPayload); const encryptedData = await encryptCredentialData(newCredential); diff --git a/packages/cli/test/integration/users.endpoints.test.ts b/packages/cli/test/integration/users.endpoints.test.ts index 5716aee460566..ccf18e5d445f7 100644 --- a/packages/cli/test/integration/users.endpoints.test.ts +++ b/packages/cli/test/integration/users.endpoints.test.ts @@ -7,8 +7,6 @@ import * as utils from './shared/utils'; import { Db } from '../../src'; import config = require('../../config'); import { SUCCESS_RESPONSE_BODY } from './shared/constants'; -import { getLogger } from '../../src/Logger'; -import { LoggerProxy } from 'n8n-workflow'; import { Role } from '../../src/databases/entities/Role'; import { randomEmail, @@ -43,8 +41,7 @@ beforeAll(async () => { workflowOwnerRole = fetchedWorkflowOwnerRole; credentialOwnerRole = fetchedCredentialOwnerRole; - config.set('logs.output', 'file'); // declutter console output - utils.initLogger(); + utils.initTestLogger(); }); beforeEach(async () => { From eac3b42bd6ff10d81dfe55a044e3dc7b2f38168b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 17 Feb 2022 10:28:31 +0100 Subject: [PATCH 21/37] :test_tube: Add non-owned tests for owner --- .../integration/credentials.endpoints.test.ts | 57 ++++++++++++++++++- 1 file changed, 54 insertions(+), 3 deletions(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index ee8dc1ca74bc1..1d7071c91dd3a 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -107,7 +107,7 @@ test('POST /credentials should ignore ID in payload', async () => { expect(secondResponse.body.data.id).not.toBe(8); }); -test('DELETE /credentials/:id should delete cred for owner', async () => { +test('DELETE /credentials/:id should delete owned cred for owner', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); const savedCredential = await saveCredential(credentialPayload(), { user: shell }); @@ -122,6 +122,22 @@ test('DELETE /credentials/:id should delete cred for owner', async () => { expect(deletedCredential).toBeUndefined(); // deleted }); +test('DELETE /credentials/:id should delete non-owned cred for owner', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + const member = await utils.createUser(); + const savedCredential = await saveCredential(credentialPayload(), { user: member }); + + const response = await authShellAgent.delete(`/credentials/${savedCredential.id}`); + + expect(response.statusCode).toBe(200); + expect(response.body).toEqual({ data: true }); + + const deletedCredential = await Db.collections.Credentials!.findOne(savedCredential.id); + + expect(deletedCredential).toBeUndefined(); // deleted +}); + test('DELETE /credentials/:id should delete owned cred for member', async () => { const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); @@ -161,7 +177,7 @@ test('DELETE /credentials/:id should fail if cred not found', async () => { expect(response.statusCode).toBe(404); }); -test('PATCH /credentials/:id should update cred for owner', async () => { +test('PATCH /credentials/:id should update owned cred for owner', async () => { const shell = await Db.collections.User!.findOneOrFail(); const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); const savedCredential = await saveCredential(credentialPayload(), { user: shell }); @@ -192,7 +208,42 @@ test('PATCH /credentials/:id should update cred for owner', async () => { where: { credentials: credential }, }); - expect(sharedCredential.credentials.name).toBe(patchPayload.name); + expect(sharedCredential.credentials.name).toBe(patchPayload.name); // updated +}); + +test('PATCH /credentials/:id should update non-owned cred for owner', async () => { + const shell = await Db.collections.User!.findOneOrFail(); + const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + const member = await utils.createUser(); + const savedCredential = await saveCredential(credentialPayload(), { user: member }); + const patchPayload = credentialPayload(); + + const response = await authShellAgent + .patch(`/credentials/${savedCredential.id}`) + .send(patchPayload); + + expect(response.statusCode).toBe(200); + + const { id, name, type, nodesAccess, data: encryptedData } = response.body.data; + + expect(name).toBe(patchPayload.name); + expect(type).toBe(patchPayload.type); + expect(nodesAccess[0].nodeType).toBe(patchPayload.nodesAccess[0].nodeType); + expect(encryptedData).not.toBe(patchPayload.data); + + const credential = await Db.collections.Credentials!.findOneOrFail(id); + + expect(credential.name).toBe(patchPayload.name); + expect(credential.type).toBe(patchPayload.type); + expect(credential.nodesAccess[0].nodeType).toBe(patchPayload.nodesAccess[0].nodeType); + expect(credential.data).not.toBe(patchPayload.data); + + const sharedCredential = await Db.collections.SharedCredentials!.findOneOrFail({ + relations: ['user', 'credentials'], + where: { credentials: credential }, + }); + + expect(sharedCredential.credentials.name).toBe(patchPayload.name); // updated }); test('PATCH /credentials/:id should update owned cred for member', async () => { From a0b43c8a8029d44665e1f1f97ecb0577e368b4ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 17 Feb 2022 10:31:41 +0100 Subject: [PATCH 22/37] :pencil2: Improve naming --- .../integration/credentials.endpoints.test.ts | 128 +++++++++--------- 1 file changed, 64 insertions(+), 64 deletions(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 1d7071c91dd3a..9d6645b70e64e 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -35,20 +35,20 @@ afterAll(() => { }); test('POST /credentials should create cred', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); const payload = credentialPayload(); - const response = await authShellAgent.post('/credentials').send(payload); + const response = await authOwnerAgent.post('/credentials').send(payload); expect(response.statusCode).toBe(200); - const { id, name, type, nodesAccess, data: hashedData } = response.body.data; + const { id, name, type, nodesAccess, data: encryptedData } = response.body.data; expect(name).toBe(payload.name); expect(type).toBe(payload.type); expect(nodesAccess[0].nodeType).toBe(payload.nodesAccess[0].nodeType); - expect(hashedData).not.toBe(payload.data); + expect(encryptedData).not.toBe(payload.data); const credential = await Db.collections.Credentials!.findOneOrFail(id); @@ -62,16 +62,16 @@ test('POST /credentials should create cred', async () => { where: { credentials: credential }, }); - expect(sharedCredential.user.id).toBe(shell.id); + expect(sharedCredential.user.id).toBe(owner.id); expect(sharedCredential.credentials.name).toBe(payload.name); }); test('POST /credentials should fail with invalid inputs', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); for (const invalidPayload of INVALID_PAYLOADS) { - const response = await authShellAgent.post('/credentials').send(invalidPayload); + const response = await authOwnerAgent.post('/credentials').send(invalidPayload); expect(response.statusCode).toBe(400); } }); @@ -80,10 +80,10 @@ test('POST /credentials should fail with missing encryption key', async () => { const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); mock.mockReturnValue(undefined); - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); - const response = await authShellAgent.post('/credentials').send(credentialPayload()); + const response = await authOwnerAgent.post('/credentials').send(credentialPayload()); expect(response.statusCode).toBe(500); @@ -91,16 +91,16 @@ test('POST /credentials should fail with missing encryption key', async () => { }); test('POST /credentials should ignore ID in payload', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); - const firstResponse = await authShellAgent + const firstResponse = await authOwnerAgent .post('/credentials') .send({ id: '8', ...credentialPayload() }); expect(firstResponse.body.data.id).not.toBe('8'); - const secondResponse = await authShellAgent + const secondResponse = await authOwnerAgent .post('/credentials') .send({ id: 8, ...credentialPayload() }); @@ -108,11 +108,11 @@ test('POST /credentials should ignore ID in payload', async () => { }); test('DELETE /credentials/:id should delete owned cred for owner', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - const savedCredential = await saveCredential(credentialPayload(), { user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); + const savedCredential = await saveCredential(credentialPayload(), { user: owner }); - const response = await authShellAgent.delete(`/credentials/${savedCredential.id}`); + const response = await authOwnerAgent.delete(`/credentials/${savedCredential.id}`); expect(response.statusCode).toBe(200); expect(response.body).toEqual({ data: true }); @@ -123,12 +123,12 @@ test('DELETE /credentials/:id should delete owned cred for owner', async () => { }); test('DELETE /credentials/:id should delete non-owned cred for owner', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); const member = await utils.createUser(); const savedCredential = await saveCredential(credentialPayload(), { user: member }); - const response = await authShellAgent.delete(`/credentials/${savedCredential.id}`); + const response = await authOwnerAgent.delete(`/credentials/${savedCredential.id}`); expect(response.statusCode).toBe(200); expect(response.body).toEqual({ data: true }); @@ -154,10 +154,10 @@ test('DELETE /credentials/:id should delete owned cred for member', async () => }); test('DELETE /credentials/:id should not delete non-owned cred for member', async () => { - const shell = await Db.collections.User!.findOneOrFail(); + const owner = await Db.collections.User!.findOneOrFail(); const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); - const savedCredential = await saveCredential(credentialPayload(), { user: shell }); + const savedCredential = await saveCredential(credentialPayload(), { user: owner }); const response = await authMemberAgent.delete(`/credentials/${savedCredential.id}`); @@ -169,21 +169,21 @@ test('DELETE /credentials/:id should not delete non-owned cred for member', asyn }); test('DELETE /credentials/:id should fail if cred not found', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); - const response = await authShellAgent.delete('/credentials/123'); + const response = await authOwnerAgent.delete('/credentials/123'); expect(response.statusCode).toBe(404); }); test('PATCH /credentials/:id should update owned cred for owner', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - const savedCredential = await saveCredential(credentialPayload(), { user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); + const savedCredential = await saveCredential(credentialPayload(), { user: owner }); const patchPayload = credentialPayload(); - const response = await authShellAgent + const response = await authOwnerAgent .patch(`/credentials/${savedCredential.id}`) .send(patchPayload); @@ -212,13 +212,13 @@ test('PATCH /credentials/:id should update owned cred for owner', async () => { }); test('PATCH /credentials/:id should update non-owned cred for owner', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); const member = await utils.createUser(); const savedCredential = await saveCredential(credentialPayload(), { user: member }); const patchPayload = credentialPayload(); - const response = await authShellAgent + const response = await authOwnerAgent .patch(`/credentials/${savedCredential.id}`) .send(patchPayload); @@ -281,10 +281,10 @@ test('PATCH /credentials/:id should update owned cred for member', async () => { }); test('PATCH /credentials/:id should not update non-owned cred for member', async () => { - const shell = await Db.collections.User!.findOneOrFail(); + const owner = await Db.collections.User!.findOneOrFail(); const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); - const savedCredential = await saveCredential(credentialPayload(), { user: shell }); + const savedCredential = await saveCredential(credentialPayload(), { user: owner }); const patchPayload = credentialPayload(); const response = await authMemberAgent @@ -299,12 +299,12 @@ test('PATCH /credentials/:id should not update non-owned cred for member', async }); test('PATCH /credentials/:id should fail with invalid inputs', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - const savedCredential = await saveCredential(credentialPayload(), { user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); + const savedCredential = await saveCredential(credentialPayload(), { user: owner }); for (const invalidPayload of INVALID_PAYLOADS) { - const response = await authShellAgent + const response = await authOwnerAgent .patch(`/credentials/${savedCredential.id}`) .send(invalidPayload); @@ -313,10 +313,10 @@ test('PATCH /credentials/:id should fail with invalid inputs', async () => { }); test('PATCH /credentials/:id should fail if cred not found', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); - const response = await authShellAgent.patch('/credentials/123').send(credentialPayload()); + const response = await authOwnerAgent.patch('/credentials/123').send(credentialPayload()); expect(response.statusCode).toBe(404); }); @@ -325,10 +325,10 @@ test('PATCH /credentials/:id should fail with missing encryption key', async () const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); mock.mockReturnValue(undefined); - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); - const response = await authShellAgent.post('/credentials').send(credentialPayload()); + const response = await authOwnerAgent.post('/credentials').send(credentialPayload()); expect(response.statusCode).toBe(500); @@ -336,14 +336,14 @@ test('PATCH /credentials/:id should fail with missing encryption key', async () }); test('GET /credentials should retrieve all creds for owner', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); for (let i = 0; i < 3; i++) { - await saveCredential(credentialPayload(), { user: shell }); + await saveCredential(credentialPayload(), { user: owner }); } - const response = await authShellAgent.get('/credentials'); + const response = await authOwnerAgent.get('/credentials'); expect(response.statusCode).toBe(200); expect(response.body.data.length).toBe(3); @@ -382,12 +382,12 @@ test('GET /credentials should retrieve owned creds for member', async () => { }); test('GET /credentials should not retrieve non-owned creds for member', async () => { - const shell = await Db.collections.User!.findOneOrFail(); + const owner = await Db.collections.User!.findOneOrFail(); const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); for (let i = 0; i < 3; i++) { - await saveCredential(credentialPayload(), { user: shell }); + await saveCredential(credentialPayload(), { user: owner }); } const response = await authMemberAgent.get('/credentials'); @@ -397,9 +397,9 @@ test('GET /credentials should not retrieve non-owned creds for member', async () }); test('GET /credentials/:id should retrieve owned cred for owner', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authOwnerAgent = await utils.createAgent(app, { auth: true, user: shell }); - const savedCredential = await saveCredential(credentialPayload(), { user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); + const savedCredential = await saveCredential(credentialPayload(), { user: owner }); const firstResponse = await authOwnerAgent.get(`/credentials/${savedCredential.id}`); @@ -448,10 +448,10 @@ test('GET /credentials/:id should retrieve owned cred for member', async () => { }); test('GET /credentials/:id should not retrieve non-owned cred for member', async () => { - const shell = await Db.collections.User!.findOneOrFail(); + const owner = await Db.collections.User!.findOneOrFail(); const member = await utils.createUser(); const authMemberAgent = await utils.createAgent(app, { auth: true, user: member }); - const savedCredential = await saveCredential(credentialPayload(), { user: shell }); + const savedCredential = await saveCredential(credentialPayload(), { user: owner }); const response = await authMemberAgent.get(`/credentials/${savedCredential.id}`); @@ -460,14 +460,14 @@ 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 shell = await Db.collections.User!.findOneOrFail(); - const authShellAgent = await utils.createAgent(app, { auth: true, user: shell }); - const savedCredential = await saveCredential(credentialPayload(), { user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); + const savedCredential = await saveCredential(credentialPayload(), { user: owner }); const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); mock.mockReturnValue(undefined); - const response = await authShellAgent + const response = await authOwnerAgent .get(`/credentials/${savedCredential.id}`) .query({ includeData: true }); @@ -477,8 +477,8 @@ test('GET /credentials/:id should fail with missing encryption key', async () => }); test('GET /credentials/:id should return empty if cred not found', async () => { - const shell = await Db.collections.User!.findOneOrFail(); - const authMemberAgent = await utils.createAgent(app, { auth: true, user: shell }); + const owner = await Db.collections.User!.findOneOrFail(); + const authMemberAgent = await utils.createAgent(app, { auth: true, user: owner }); const response = await authMemberAgent.get('/credentials/789'); From 7dc103e7db6aa04f4b2e06a25649a441df5dbe5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 17 Feb 2022 10:33:30 +0100 Subject: [PATCH 23/37] :pencil2: Add clarifying comments --- packages/cli/test/integration/credentials.endpoints.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 9d6645b70e64e..dc7c53f225950 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -277,7 +277,7 @@ test('PATCH /credentials/:id should update owned cred for member', async () => { where: { credentials: credential }, }); - expect(sharedCredential.credentials.name).toBe(patchPayload.name); + expect(sharedCredential.credentials.name).toBe(patchPayload.name); // updated }); test('PATCH /credentials/:id should not update non-owned cred for member', async () => { From 80162a32f34caaa52bc5e24f13e11c36dbaadc53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 17 Feb 2022 11:58:58 +0100 Subject: [PATCH 24/37] :truck: Improve imports --- .../cli/test/integration/credentials.endpoints.test.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index dc7c53f225950..715a3050f86a9 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -1,13 +1,12 @@ import express = require('express'); import { getConnection } from 'typeorm'; +import { UserSettings } from 'n8n-core'; import { Db } from '../../src'; import { randomName, randomString } from './shared/random'; import * as utils from './shared/utils'; import type { SaveCredentialFunction } from './shared/types'; -const { UserSettings } = require('n8n-core'); - let app: express.Application; let saveCredential: SaveCredentialFunction; @@ -78,7 +77,7 @@ test('POST /credentials should fail with invalid inputs', async () => { test('POST /credentials should fail with missing encryption key', async () => { const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); - mock.mockReturnValue(undefined); + mock.mockReturnValue(Promise.resolve(undefined)); const owner = await Db.collections.User!.findOneOrFail(); const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); @@ -323,7 +322,7 @@ test('PATCH /credentials/:id should fail if cred not found', async () => { test('PATCH /credentials/:id should fail with missing encryption key', async () => { const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); - mock.mockReturnValue(undefined); + mock.mockReturnValue(Promise.resolve(undefined)); const owner = await Db.collections.User!.findOneOrFail(); const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); @@ -465,7 +464,7 @@ test('GET /credentials/:id should fail with missing encryption key', async () => const savedCredential = await saveCredential(credentialPayload(), { user: owner }); const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); - mock.mockReturnValue(undefined); + mock.mockReturnValue(Promise.resolve(undefined)); const response = await authOwnerAgent .get(`/credentials/${savedCredential.id}`) From 390cdb87466289e899d0e3ee5d4f4543f2d8c987 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 17 Feb 2022 14:32:18 +0100 Subject: [PATCH 25/37] :zap: Initialize config file --- .../integration/credentials.endpoints.test.ts | 3 +++ packages/cli/test/integration/shared/utils.ts | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 715a3050f86a9..7281fa8161a6a 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -1,4 +1,5 @@ import express = require('express'); +import { existsSync } from 'fs'; import { getConnection } from 'typeorm'; import { UserSettings } from 'n8n-core'; @@ -17,6 +18,8 @@ beforeAll(async () => { externalHooks: true, }); await utils.initTestDb(); + utils.initConfigFile(); + const credentialOwnerRole = await utils.getCredentialOwnerRole(); saveCredential = utils.affixRoleToSaveCredential(credentialOwnerRole); }); diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index 933846ff4c0f9..2e05e2102c303 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -1,3 +1,5 @@ +import { randomBytes } from 'crypto'; +import { existsSync } from 'fs'; import express = require('express'); import * as superagent from 'superagent'; import * as request from 'supertest'; @@ -91,11 +93,23 @@ export function initTestServer({ /** * Initialize a silent logger for test runs. */ -export const initTestLogger = () => { +export function initTestLogger() { config.set('logs.output', 'file'); LoggerProxy.init(getLogger()); }; +/** + * Initialize a config file if non-existent. + */ +export function initConfigFile() { + const settingsPath = UserSettings.getUserSettingsPath(); + + if (!existsSync(settingsPath)) { + const userSettings = { encryptionKey: randomBytes(24).toString('base64') }; + UserSettings.writeUserSettings(userSettings, settingsPath); + } +} + // ---------------------------------- // test DB // ---------------------------------- From 7682701a5aba69c18ce100c9b4e16f4fb35b23d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 17 Feb 2022 14:33:17 +0100 Subject: [PATCH 26/37] :fire: Remove unneeded import --- packages/cli/test/integration/credentials.endpoints.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 7281fa8161a6a..a2a0a446388c6 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -1,5 +1,4 @@ import express = require('express'); -import { existsSync } from 'fs'; import { getConnection } from 'typeorm'; import { UserSettings } from 'n8n-core'; From 24e2aee5aa0c492aca90a812663a42b8343642ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Sat, 19 Feb 2022 13:57:23 +0100 Subject: [PATCH 27/37] :truck: Rename dir --- packages/cli/src/Server.ts | 2 +- .../namespaces/credentials.ts} | 14 +++++++------- packages/cli/test/integration/shared/utils.ts | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) rename packages/cli/src/{endpoints/credentials.endpoints.ts => api/namespaces/credentials.ts} (95%) diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index cf8b86c72ee57..03315ebc2c75b 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -164,7 +164,7 @@ import { DEFAULT_EXECUTIONS_GET_ALL_LIMIT, validateEntity } from './GenericHelpe import { ExecutionEntity } from './databases/entities/ExecutionEntity'; import { SharedWorkflow } from './databases/entities/SharedWorkflow'; import { RESPONSE_ERROR_MESSAGES } from './constants'; -import { credentialsEndpoints } from './endpoints/credentials.endpoints'; +import { credentialsEndpoints } from './api/namespaces/credentials'; require('body-parser-xml')(bodyParser); diff --git a/packages/cli/src/endpoints/credentials.endpoints.ts b/packages/cli/src/api/namespaces/credentials.ts similarity index 95% rename from packages/cli/src/endpoints/credentials.endpoints.ts rename to packages/cli/src/api/namespaces/credentials.ts index 0de6c0f2222ca..d7c9b0f7b74c7 100644 --- a/packages/cli/src/endpoints/credentials.endpoints.ts +++ b/packages/cli/src/api/namespaces/credentials.ts @@ -15,13 +15,13 @@ import { ICredentialsResponse, ResponseHelper, whereClause, -} from '..'; -import { RESPONSE_ERROR_MESSAGES } from '../constants'; -import { CredentialsEntity } from '../databases/entities/CredentialsEntity'; -import { SharedCredentials } from '../databases/entities/SharedCredentials'; -import { validateEntity } from '../GenericHelpers'; -import type { CredentialRequest } from '../requests'; -import type { N8nApp } from '../UserManagement/Interfaces'; +} from '../..'; +import { RESPONSE_ERROR_MESSAGES } from '../../constants'; +import { CredentialsEntity } from '../../databases/entities/CredentialsEntity'; +import { SharedCredentials } from '../../databases/entities/SharedCredentials'; +import { validateEntity } from '../../GenericHelpers'; +import type { CredentialRequest } from '../../requests'; +import type { N8nApp } from '../../UserManagement/Interfaces'; export function credentialsEndpoints(this: N8nApp): void { /** diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index 2e05e2102c303..f070628596c47 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -21,7 +21,7 @@ import { usersNamespace as usersEndpoints } from '../../../src/UserManagement/ro import { authenticationMethods as authEndpoints } from '../../../src/UserManagement/routes/auth'; import { ownerNamespace as ownerEndpoints } from '../../../src/UserManagement/routes/owner'; import { passwordResetNamespace as passwordResetEndpoints } from '../../../src/UserManagement/routes/passwordReset'; -import { credentialsEndpoints } from '../../../src/endpoints/credentials.endpoints'; +import { credentialsEndpoints } from '../../../src/api/namespaces/credentials'; import { issueJWT } from '../../../src/UserManagement/auth/jwt'; import { randomEmail, randomValidPassword, randomName } from './random'; import { getLogger } from '../../../src/Logger'; From 7aa60c9efdaafcdcd0a4102708cf2dcf2c0b0b3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Sat, 19 Feb 2022 14:31:51 +0100 Subject: [PATCH 28/37] :zap: Adjust deletion call --- packages/cli/src/api/namespaces/credentials.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/api/namespaces/credentials.ts b/packages/cli/src/api/namespaces/credentials.ts index d7c9b0f7b74c7..14edca5429718 100644 --- a/packages/cli/src/api/namespaces/credentials.ts +++ b/packages/cli/src/api/namespaces/credentials.ts @@ -161,7 +161,7 @@ export function credentialsEndpoints(this: N8nApp): void { await this.externalHooks.run('credentials.delete', [credentialId]); - await Db.collections.Credentials!.delete(credentialId); + await Db.collections.Credentials!.remove(shared.credentials); return true; }), From 282126508516bd6d5d8b0227bd3bb82e8b894658 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Sat, 19 Feb 2022 14:32:26 +0100 Subject: [PATCH 29/37] :zap: Adjust error code --- packages/cli/src/api/namespaces/credentials.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/api/namespaces/credentials.ts b/packages/cli/src/api/namespaces/credentials.ts index 14edca5429718..7b4498083153e 100644 --- a/packages/cli/src/api/namespaces/credentials.ts +++ b/packages/cli/src/api/namespaces/credentials.ts @@ -260,7 +260,7 @@ export function credentialsEndpoints(this: N8nApp): void { throw new ResponseHelper.ResponseError( `Credential ID "${credentialId}" could not be found to be updated.`, undefined, - 400, + 404, ); } From f096c1952818463c2ca5f7dffa1b32dfd24cf35c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Sat, 19 Feb 2022 14:32:44 +0100 Subject: [PATCH 30/37] :pencil2: Touch up comment --- packages/cli/src/api/namespaces/credentials.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/api/namespaces/credentials.ts b/packages/cli/src/api/namespaces/credentials.ts index 7b4498083153e..e3175580f9103 100644 --- a/packages/cli/src/api/namespaces/credentials.ts +++ b/packages/cli/src/api/namespaces/credentials.ts @@ -253,7 +253,7 @@ export function credentialsEndpoints(this: N8nApp): void { await Db.collections.Credentials!.update(credentialId, newCredentialData); // We sadly get nothing back from "update". Neither if it updated a record - // nor the new value. So query now the hopefully updated entry. + // nor the new value. So query now the updated entry. const responseData = await Db.collections.Credentials!.findOne(credentialId); if (responseData === undefined) { From 5e3e0238df48a54084af5537643ff25da803b920 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Sat, 19 Feb 2022 14:43:13 +0100 Subject: [PATCH 31/37] :zap: Optimize fetching with `@RelationId` --- packages/cli/src/api/namespaces/credentials.ts | 3 +-- .../cli/src/databases/entities/SharedCredentials.ts | 12 +++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/api/namespaces/credentials.ts b/packages/cli/src/api/namespaces/credentials.ts index e3175580f9103..9c51209122b5f 100644 --- a/packages/cli/src/api/namespaces/credentials.ts +++ b/packages/cli/src/api/namespaces/credentials.ts @@ -350,7 +350,6 @@ export function credentialsEndpoints(this: N8nApp): void { }); } else { const shared = await Db.collections.SharedCredentials!.find({ - relations: ['credentials'], where: whereClause({ user: req.user, entityType: 'credentials', @@ -362,7 +361,7 @@ export function credentialsEndpoints(this: N8nApp): void { credentials = await Db.collections.Credentials!.find({ select: ['id', 'name', 'type', 'nodesAccess', 'createdAt', 'updatedAt'], where: { - id: In(shared.map(({ credentials }) => credentials.id)), + id: In(shared.map(({ credentialId }) => credentialId)), ...filter, }, }); diff --git a/packages/cli/src/databases/entities/SharedCredentials.ts b/packages/cli/src/databases/entities/SharedCredentials.ts index fac6e2d8eaa0a..a92f147b9e06e 100644 --- a/packages/cli/src/databases/entities/SharedCredentials.ts +++ b/packages/cli/src/databases/entities/SharedCredentials.ts @@ -1,5 +1,12 @@ /* eslint-disable import/no-cycle */ -import { BeforeUpdate, CreateDateColumn, Entity, ManyToOne, UpdateDateColumn } from 'typeorm'; +import { + BeforeUpdate, + CreateDateColumn, + Entity, + ManyToOne, + RelationId, + UpdateDateColumn, +} from 'typeorm'; import { IsDate, IsOptional } from 'class-validator'; import config = require('../../../config'); @@ -36,6 +43,9 @@ export class SharedCredentials { }) credentials: CredentialsEntity; + @RelationId((sharedCredential: SharedCredentials) => sharedCredential.credentials) + credentialId: number; + @CreateDateColumn({ precision: 3, default: () => getTimestampSyntax() }) @IsOptional() // ignored by validation because set at DB level @IsDate() From e99c60f02bd53ec56448a70d81622ab5763a0ca6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Sat, 19 Feb 2022 15:05:50 +0100 Subject: [PATCH 32/37] :test_tube: Add expectations --- .../integration/credentials.endpoints.test.ts | 34 +++++++++++++++---- .../test/integration/users.endpoints.test.ts | 18 ++++------ 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index a2a0a446388c6..297c930d2b00f 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -28,7 +28,7 @@ beforeEach(async () => { }); afterEach(async () => { - await utils.truncate(['User', 'Credentials']); + await utils.truncate(['User', 'Credentials', 'SharedCredentials']); }); afterAll(() => { @@ -121,6 +121,10 @@ test('DELETE /credentials/:id should delete owned cred for owner', async () => { const deletedCredential = await Db.collections.Credentials!.findOne(savedCredential.id); expect(deletedCredential).toBeUndefined(); // deleted + + const deletedSharedCredential = await Db.collections.SharedCredentials!.findOne(); + + expect(deletedSharedCredential).toBeUndefined(); // deleted }); test('DELETE /credentials/:id should delete non-owned cred for owner', async () => { @@ -137,6 +141,10 @@ test('DELETE /credentials/:id should delete non-owned cred for owner', async () const deletedCredential = await Db.collections.Credentials!.findOne(savedCredential.id); expect(deletedCredential).toBeUndefined(); // deleted + + const deletedSharedCredential = await Db.collections.SharedCredentials!.findOne(); + + expect(deletedSharedCredential).toBeUndefined(); // deleted }); test('DELETE /credentials/:id should delete owned cred for member', async () => { @@ -152,6 +160,10 @@ test('DELETE /credentials/:id should delete owned cred for member', async () => const deletedCredential = await Db.collections.Credentials!.findOne(savedCredential.id); expect(deletedCredential).toBeUndefined(); // deleted + + const deletedSharedCredential = await Db.collections.SharedCredentials!.findOne(); + + expect(deletedSharedCredential).toBeUndefined(); // deleted }); test('DELETE /credentials/:id should not delete non-owned cred for member', async () => { @@ -167,6 +179,10 @@ test('DELETE /credentials/:id should not delete non-owned cred for member', asyn const shellCredential = await Db.collections.Credentials!.findOne(savedCredential.id); expect(shellCredential).toBeDefined(); // not deleted + + const deletedSharedCredential = await Db.collections.SharedCredentials!.findOne(); + + expect(deletedSharedCredential).toBeDefined(); // not deleted }); test('DELETE /credentials/:id should fail if cred not found', async () => { @@ -205,7 +221,7 @@ test('PATCH /credentials/:id should update owned cred for owner', async () => { expect(credential.data).not.toBe(patchPayload.data); const sharedCredential = await Db.collections.SharedCredentials!.findOneOrFail({ - relations: ['user', 'credentials'], + relations: ['credentials'], where: { credentials: credential }, }); @@ -240,7 +256,7 @@ test('PATCH /credentials/:id should update non-owned cred for owner', async () = expect(credential.data).not.toBe(patchPayload.data); const sharedCredential = await Db.collections.SharedCredentials!.findOneOrFail({ - relations: ['user', 'credentials'], + relations: ['credentials'], where: { credentials: credential }, }); @@ -274,7 +290,7 @@ test('PATCH /credentials/:id should update owned cred for member', async () => { expect(credential.data).not.toBe(patchPayload.data); const sharedCredential = await Db.collections.SharedCredentials!.findOneOrFail({ - relations: ['user', 'credentials'], + relations: ['credentials'], where: { credentials: credential }, }); @@ -344,10 +360,14 @@ test('GET /credentials should retrieve all creds for owner', async () => { await saveCredential(credentialPayload(), { user: owner }); } + const member = await utils.createUser(); + + await saveCredential(credentialPayload(), { user: member }); + const response = await authOwnerAgent.get('/credentials'); expect(response.statusCode).toBe(200); - expect(response.body.data.length).toBe(3); + expect(response.body.data.length).toBe(4); // 3 owner + 1 member for (const credential of response.body.data) { const { name, type, nodesAccess, data: encryptedData } = credential; @@ -394,7 +414,7 @@ test('GET /credentials should not retrieve non-owned creds for member', async () const response = await authMemberAgent.get('/credentials'); expect(response.statusCode).toBe(200); - expect(response.body.data.length).toBe(0); // shell's creds not returned + expect(response.body.data.length).toBe(0); // owner's creds not returned }); test('GET /credentials/:id should retrieve owned cred for owner', async () => { @@ -457,7 +477,7 @@ test('GET /credentials/:id should not retrieve non-owned cred for member', async const response = await authMemberAgent.get(`/credentials/${savedCredential.id}`); expect(response.statusCode).toBe(200); - expect(response.body.data).toEqual({}); // shell's cred not returned + expect(response.body.data).toEqual({}); // owner's cred not returned }); test('GET /credentials/:id should fail with missing encryption key', async () => { diff --git a/packages/cli/test/integration/users.endpoints.test.ts b/packages/cli/test/integration/users.endpoints.test.ts index ccf18e5d445f7..427524a8e3d0f 100644 --- a/packages/cli/test/integration/users.endpoints.test.ts +++ b/packages/cli/test/integration/users.endpoints.test.ts @@ -45,7 +45,7 @@ beforeAll(async () => { }); beforeEach(async () => { - await utils.truncate(['User']); + await utils.truncate(['User', 'Workflow', 'Credentials', 'SharedCredentials', 'SharedWorkflow']); jest.isolateModules(() => { jest.mock('../../config'); @@ -64,10 +64,6 @@ beforeEach(async () => { config.set('userManagement.emails.mode', ''); }); -afterEach(async () => { - await utils.truncate(['User']); -}); - afterAll(() => { return getConnection().close(); }); @@ -151,25 +147,25 @@ test('DELETE /users/:id should delete the user', async () => { expect(response.body).toEqual(SUCCESS_RESPONSE_BODY); const user = await Db.collections.User!.findOne(userToDelete.id); - expect(user).toBeUndefined(); + expect(user).toBeUndefined(); // deleted const sharedWorkflow = await Db.collections.SharedWorkflow!.findOne({ relations: ['user'], where: { user: userToDelete }, }); - expect(sharedWorkflow).toBeUndefined(); + expect(sharedWorkflow).toBeUndefined(); // deleted const sharedCredential = await Db.collections.SharedCredentials!.findOne({ relations: ['user'], where: { user: userToDelete }, }); - expect(sharedCredential).toBeUndefined(); + expect(sharedCredential).toBeUndefined(); // deleted const workflow = await Db.collections.Workflow!.findOne(savedWorkflow.id); - expect(workflow).toBeUndefined(); + expect(workflow).toBeUndefined(); // deleted const credential = await Db.collections.Credentials!.findOne(savedCredential.id); - expect(credential).toBeUndefined(); + expect(credential).toBeUndefined(); // deleted }); test('DELETE /users/:id should fail to delete self', async () => { @@ -269,8 +265,6 @@ test('DELETE /users/:id with transferId should perform transfer', async () => { expect(sharedWorkflow.user.id).toBe(owner.id); expect(sharedCredential.user.id).toBe(owner.id); expect(deletedUser).toBeUndefined(); - - await utils.truncate(['Credentials', 'Workflow']); }); test('GET /resolve-signup-token should validate invite token', async () => { From 13c39f8da68a679d56b3be654e6cde23436e901b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Sat, 19 Feb 2022 15:21:36 +0100 Subject: [PATCH 33/37] :zap: Simplify mock calls --- packages/cli/test/integration/credentials.endpoints.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli/test/integration/credentials.endpoints.test.ts b/packages/cli/test/integration/credentials.endpoints.test.ts index 297c930d2b00f..157e960123ce1 100644 --- a/packages/cli/test/integration/credentials.endpoints.test.ts +++ b/packages/cli/test/integration/credentials.endpoints.test.ts @@ -79,7 +79,7 @@ test('POST /credentials should fail with invalid inputs', async () => { test('POST /credentials should fail with missing encryption key', async () => { const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); - mock.mockReturnValue(Promise.resolve(undefined)); + mock.mockResolvedValue(undefined); const owner = await Db.collections.User!.findOneOrFail(); const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); @@ -340,7 +340,7 @@ test('PATCH /credentials/:id should fail if cred not found', async () => { test('PATCH /credentials/:id should fail with missing encryption key', async () => { const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); - mock.mockReturnValue(Promise.resolve(undefined)); + mock.mockResolvedValue(undefined); const owner = await Db.collections.User!.findOneOrFail(); const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner }); @@ -486,7 +486,7 @@ test('GET /credentials/:id should fail with missing encryption key', async () => const savedCredential = await saveCredential(credentialPayload(), { user: owner }); const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); - mock.mockReturnValue(Promise.resolve(undefined)); + mock.mockResolvedValue(undefined); const response = await authOwnerAgent .get(`/credentials/${savedCredential.id}`) From 8a8fe695af4ca912a4df60559a2da84985033756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 21 Feb 2022 09:39:38 +0100 Subject: [PATCH 34/37] :blue_book: Set deep readonly to object constants --- packages/cli/test/integration/shared/constants.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/cli/test/integration/shared/constants.ts b/packages/cli/test/integration/shared/constants.ts index 76ee8b402f5cf..413f01f8f72ee 100644 --- a/packages/cli/test/integration/shared/constants.ts +++ b/packages/cli/test/integration/shared/constants.ts @@ -24,14 +24,14 @@ export const TEST_CONNECTION_OPTIONS: Readonly = { logging: false, }; -export const SUCCESS_RESPONSE_BODY: Readonly = { +export const SUCCESS_RESPONSE_BODY = { data: { success: true, }, -}; +} as const; -export const LOGGED_OUT_RESPONSE_BODY: Readonly = { +export const LOGGED_OUT_RESPONSE_BODY = { data: { loggedOut: true, }, -}; +} as const; From b405c33f4acc1d88ea7c5db1a0251ebf65f87248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 22 Feb 2022 12:37:09 +0100 Subject: [PATCH 35/37] :fire: Remove unused param and encryption key --- packages/cli/src/api/namespaces/credentials.ts | 11 ----------- packages/cli/src/requests.d.ts | 2 +- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/packages/cli/src/api/namespaces/credentials.ts b/packages/cli/src/api/namespaces/credentials.ts index 9c51209122b5f..7435c08e3269a 100644 --- a/packages/cli/src/api/namespaces/credentials.ts +++ b/packages/cli/src/api/namespaces/credentials.ts @@ -367,17 +367,6 @@ export function credentialsEndpoints(this: N8nApp): void { }); } - // TODO: Useless segment coming from master - Remove? - let encryptionKey; - - if (req.query.includeData === 'true') { - encryptionKey = await UserSettings.getEncryptionKey(); - - if (!encryptionKey) { - throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY); - } - } - return credentials.map(({ id, ...rest }) => ({ id: id.toString(), ...rest })); }), ); diff --git a/packages/cli/src/requests.d.ts b/packages/cli/src/requests.d.ts index 4d076701df8ff..8480dc7e9e2cd 100644 --- a/packages/cli/src/requests.d.ts +++ b/packages/cli/src/requests.d.ts @@ -71,7 +71,7 @@ export declare namespace CredentialRequest { type Delete = Get; - type GetAll = AuthenticatedRequest<{}, {}, {}, { filter: string; includeData: string }>; + type GetAll = AuthenticatedRequest<{}, {}, {}, { filter: string }>; type Update = AuthenticatedRequest<{ id: string }, {}, RequestBody>; From 5f1f00bc72dd3936fc0548637217c3c91b6f6e23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 22 Feb 2022 12:49:05 +0100 Subject: [PATCH 36/37] :zap: Add more `@RelationId` calls in models --- .../src/databases/entities/SharedCredentials.ts | 3 +++ .../cli/src/databases/entities/SharedWorkflow.ts | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/databases/entities/SharedCredentials.ts b/packages/cli/src/databases/entities/SharedCredentials.ts index a92f147b9e06e..43bbc4e9240cb 100644 --- a/packages/cli/src/databases/entities/SharedCredentials.ts +++ b/packages/cli/src/databases/entities/SharedCredentials.ts @@ -37,6 +37,9 @@ export class SharedCredentials { @ManyToOne(() => User, (user) => user.sharedCredentials, { primary: true }) user: User; + @RelationId((sharedCredential: SharedCredentials) => sharedCredential.user) + userId: string; + @ManyToOne(() => CredentialsEntity, (credentials) => credentials.shared, { primary: true, onDelete: 'CASCADE', diff --git a/packages/cli/src/databases/entities/SharedWorkflow.ts b/packages/cli/src/databases/entities/SharedWorkflow.ts index 552649728c39c..5e20477e360b6 100644 --- a/packages/cli/src/databases/entities/SharedWorkflow.ts +++ b/packages/cli/src/databases/entities/SharedWorkflow.ts @@ -1,5 +1,12 @@ /* eslint-disable import/no-cycle */ -import { BeforeUpdate, CreateDateColumn, Entity, ManyToOne, UpdateDateColumn } from 'typeorm'; +import { + BeforeUpdate, + CreateDateColumn, + Entity, + ManyToOne, + RelationId, + UpdateDateColumn, +} from 'typeorm'; import { IsDate, IsOptional } from 'class-validator'; import config = require('../../../config'); @@ -30,12 +37,18 @@ export class SharedWorkflow { @ManyToOne(() => User, (user) => user.sharedWorkflows, { primary: true }) user: User; + @RelationId((sharedWorkflow: SharedWorkflow) => sharedWorkflow.user) + userId: string; + @ManyToOne(() => WorkflowEntity, (workflow) => workflow.shared, { primary: true, onDelete: 'CASCADE', }) workflow: WorkflowEntity; + @RelationId((sharedWorkflow: SharedWorkflow) => sharedWorkflow.workflow) + workflowId: number; + @CreateDateColumn({ precision: 3, default: () => getTimestampSyntax() }) @IsOptional() // ignored by validation because set at DB level @IsDate() From 3104fc5a952defb43996d116368db40de1300f08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 23 Feb 2022 09:20:48 +0100 Subject: [PATCH 37/37] :rewind: Restore --- packages/cli/test/integration/shared/utils.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index ab84f3bb9f5e5..14fe18669e5bd 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -37,6 +37,11 @@ export const isTestRun = process.argv[1].split('/').includes('jest'); // TODO: P // test server // ---------------------------------- +export const initLogger = () => { + config.set('logs.output', 'file'); // declutter console output during tests + LoggerProxy.init(getLogger()); +}; + /** * Initialize a test server to make requests to. *