From 23e018c4e09b6a251ce77c718e970cc8b11f9fbe Mon Sep 17 00:00:00 2001 From: Ben Hesseldieck Date: Thu, 16 Jun 2022 22:24:11 +0200 Subject: [PATCH 01/13] =?UTF-8?q?=F0=9F=8E=89=20initial=20design?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/cli/config/schema.ts | 6 +++ .../src/credentials/credentials.controller.ts | 34 ++++++++++++ .../ee/src/credentials/credentials.service.ts | 28 ++++++++++ packages/cli/src/Server.ts | 2 +- .../credentials.controller.ts} | 42 +++++++++++---- .../src/credentials/credentials.service.ts | 39 ++++++++++++++ .../test/integration/credentials.api.test.ts | 54 ++++++++++++++++++- packages/cli/test/integration/shared/utils.ts | 4 +- 8 files changed, 195 insertions(+), 14 deletions(-) create mode 100644 packages/cli/ee/src/credentials/credentials.controller.ts create mode 100644 packages/cli/ee/src/credentials/credentials.service.ts rename packages/cli/src/{api/credentials.api.ts => credentials/credentials.controller.ts} (90%) create mode 100644 packages/cli/src/credentials/credentials.service.ts diff --git a/packages/cli/config/schema.ts b/packages/cli/config/schema.ts index d422776f5594a..1eaab9528745d 100644 --- a/packages/cli/config/schema.ts +++ b/packages/cli/config/schema.ts @@ -852,6 +852,12 @@ export const schema = { default: 'default', env: 'N8N_DEPLOYMENT_TYPE', }, + paid: { + doc: 'Whether paid features are enabled.', + format: Boolean, + default: true, + env: 'N8N_PAID', + }, }, hiringBanner: { diff --git a/packages/cli/ee/src/credentials/credentials.controller.ts b/packages/cli/ee/src/credentials/credentials.controller.ts new file mode 100644 index 0000000000000..67a3864c7cfef --- /dev/null +++ b/packages/cli/ee/src/credentials/credentials.controller.ts @@ -0,0 +1,34 @@ +import express from 'express'; + +import * as config from '../../../config'; +import { credentialsService } from '../../../src/credentials/credentials.controller'; +import { CredentialRequest } from '../../../src/requests'; + +export const eeCredentialsController = express.Router(); + +// @ts-ignore +eeCredentialsController.use((req, res, next) => { + if (config.getEnv('deployment.paid')) { + // use ee router + next(); + return; + } + // skip ee router and use free one + next('router'); +}); + +eeCredentialsController.get('/shared', (req, res) => { + console.log('ENTERPRISE SHARING'); + return res.json({ enterprise: true }); +}); + +eeCredentialsController.get('/:id', async (req: CredentialRequest.Get, res) => { + const { id: credentialId } = req.params; + + console.log('ENTERPRISE ID FETCH'); + const shared = await credentialsService.getSharedCredentials(req.user.id, credentialId, [ + 'credentials', + ]); + console.log(shared); + return res.json({ enterprise: true }); +}); diff --git a/packages/cli/ee/src/credentials/credentials.service.ts b/packages/cli/ee/src/credentials/credentials.service.ts new file mode 100644 index 0000000000000..9cb1afa83db73 --- /dev/null +++ b/packages/cli/ee/src/credentials/credentials.service.ts @@ -0,0 +1,28 @@ +import { FindOneOptions } from 'typeorm'; + +import { Db } from '../../../src'; +import { CredentialsService } from '../../../src/credentials/credentials.service'; +import { SharedCredentials } from '../../../src/databases/entities/SharedCredentials'; + +export class EECreditentialsService extends CredentialsService { + async getSharedCredentials( + userId: string, + credentialId: number | string, + relations?: string[], + ): Promise { + console.log('ENTERPRISE SERVICE'); + + const options: FindOneOptions = { + where: { + user: { id: userId }, + credentials: { id: credentialId }, + }, + }; + + if (relations) { + options.relations = relations; + } + + return Db.collections.SharedCredentials.findOne(options); + } +} diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index ef0f3d80050c5..6685944aa8f7d 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -158,7 +158,7 @@ import { DEFAULT_EXECUTIONS_GET_ALL_LIMIT, validateEntity } from './GenericHelpe import { ExecutionEntity } from './databases/entities/ExecutionEntity'; import { SharedWorkflow } from './databases/entities/SharedWorkflow'; import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES } from './constants'; -import { credentialsController } from './api/credentials.api'; +import { credentialsController } from './credentials/credentials.controller'; import { getInstanceBaseUrl, isEmailSetUp, diff --git a/packages/cli/src/api/credentials.api.ts b/packages/cli/src/credentials/credentials.controller.ts similarity index 90% rename from packages/cli/src/api/credentials.api.ts rename to packages/cli/src/credentials/credentials.controller.ts index 67d235957c1f5..e75c89b478dcf 100644 --- a/packages/cli/src/api/credentials.api.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -27,8 +27,26 @@ import { createCredentiasFromCredentialsEntity } from '../CredentialsHelper'; import type { CredentialRequest } from '../requests'; import * as config from '../../config'; import { externalHooks } from '../Server'; +import { CredentialsService } from './credentials.service'; +import { EECreditentialsService } from '../../ee/src/credentials/credentials.service'; +import { eeCredentialsController } from '../../ee/src/credentials/credentials.controller'; export const credentialsController = express.Router(); +// eslint-disable-next-line import/no-mutable-exports +export let credentialsService: CredentialsService | EECreditentialsService; + +export function setServices(): void { + // EE enabled + if (config.getEnv('deployment.paid')) { + credentialsService = new EECreditentialsService(); + return; + } + + // FREE + credentialsService = new CredentialsService(); +} + +setServices(); /** * Initialize Logger if needed @@ -42,6 +60,8 @@ credentialsController.use((req, res, next) => { next(); }); +credentialsController.use('/', eeCredentialsController); + /** * GET /credentials */ @@ -361,14 +381,18 @@ credentialsController.get( 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, - }), - }); + // const shared = await Db.collections.SharedCredentials.findOne({ + // relations: ['credentials'], + // where: whereClause({ + // user: req.user, + // entityType: 'credentials', + // entityId: credentialId, + // }), + // }); + + const shared = await credentialsService.getSharedCredentials(req.user.id, credentialId, [ + 'credentials', + ]); if (!shared) { throw new ResponseHelper.ResponseError( @@ -402,7 +426,7 @@ credentialsController.get( ); } - const coreCredential = createCredentiasFromCredentialsEntity(credential); + const coreCredential = credentialsService.createCredentiasFromCredentialsEntity(credential); return { id: id.toString(), diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts new file mode 100644 index 0000000000000..963588de8e972 --- /dev/null +++ b/packages/cli/src/credentials/credentials.service.ts @@ -0,0 +1,39 @@ +/* eslint-disable import/no-cycle */ +import { Credentials } from 'n8n-core'; +import { FindOneOptions } from 'typeorm'; + +import { Db } from '..'; +import { CredentialsEntity } from '../databases/entities/CredentialsEntity'; +import { SharedCredentials } from '../databases/entities/SharedCredentials'; + +export class CredentialsService { + async getSharedCredentials( + userId: string, + credentialId: number | string, + relations?: string[], + ): Promise { + const options: FindOneOptions = { + where: { + user: { id: userId }, + credentials: { id: credentialId }, + }, + }; + + if (relations) { + options.relations = relations; + } + + return Db.collections.SharedCredentials.findOne(options); + } + + createCredentiasFromCredentialsEntity( + credential: CredentialsEntity, + encrypt = false, + ): Credentials { + const { id, name, type, nodesAccess, data } = credential; + if (encrypt) { + return new Credentials({ id: null, name }, type, nodesAccess); + } + return new Credentials({ id: id.toString(), name }, type, nodesAccess, data); + } +} diff --git a/packages/cli/test/integration/credentials.api.test.ts b/packages/cli/test/integration/credentials.api.test.ts index cc6e8275f0468..de1ee60a0294a 100644 --- a/packages/cli/test/integration/credentials.api.test.ts +++ b/packages/cli/test/integration/credentials.api.test.ts @@ -2,6 +2,7 @@ import express from 'express'; import { UserSettings } from 'n8n-core'; import { Db } from '../../src'; import { randomName, randomString } from './shared/random'; +import config from '../../config'; import * as utils from './shared/utils'; import type { CredentialPayload, SaveCredentialFunction } from './shared/types'; import type { Role } from '../../src/databases/entities/Role'; @@ -357,7 +358,6 @@ test('PATCH /credentials/:id should fail with missing encryption key', async () const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); mock.mockRejectedValue(new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY)); - const ownerShell = await testDb.createUserShell(globalOwnerRole); const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell }); @@ -508,7 +508,6 @@ test('GET /credentials/:id should fail with missing encryption key', async () => const mock = jest.spyOn(UserSettings, 'getEncryptionKey'); mock.mockRejectedValue(new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY)); - const response = await authOwnerAgent .get(`/credentials/${savedCredential.id}`) .query({ includeData: true }); @@ -527,6 +526,57 @@ test('GET /credentials/:id should return 404 if cred not found', async () => { expect(response.statusCode).toBe(404); }); +test.only('GET /credentials/:id should hit different Routers', async () => { + const ownerShell = await testDb.createUserShell(globalOwnerRole); + const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell }); + const savedCredential = await saveCredential(credentialPayload(), { user: ownerShell }); + + config.set('deployment.paid', false); + + 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(); + + config.set('deployment.paid', true); + + const third = await authOwnerAgent.get(`/credentials/${savedCredential.id}`); + + expect(third.statusCode).toBe(200); + + expect(third.body).toEqual({ enterprise: true }); + + const fourth = await authOwnerAgent.get(`/credentials/${savedCredential.id}`); + + expect(fourth.statusCode).toBe(200); + expect(fourth.body).toEqual({ enterprise: true }); + + config.set('deployment.paid', false); + + const fifth = await authOwnerAgent.get(`/credentials/${savedCredential.id}`); + + expect(fifth.statusCode).toBe(200); + + expect(typeof fifth.body.data.name).toBe('string'); + expect(typeof fifth.body.data.type).toBe('string'); + expect(typeof fifth.body.data.nodesAccess[0].nodeType).toBe('string'); + expect(fifth.body.data.data).toBeUndefined(); +}); + const credentialPayload = () => ({ name: randomName(), type: randomName(), diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index f3d205476f569..b60ae6803c54f 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -41,7 +41,7 @@ import { ownerNamespace as ownerEndpoints } from '../../../src/UserManagement/ro import { passwordResetNamespace as passwordResetEndpoints } from '../../../src/UserManagement/routes/passwordReset'; import { issueJWT } from '../../../src/UserManagement/auth/jwt'; import { getLogger } from '../../../src/Logger'; -import { credentialsController } from '../../../src/api/credentials.api'; +import { credentialsController } from '../../../src/credentials/credentials.controller'; import { loadPublicApiVersions } from '../../../src/PublicApi/'; import type { User } from '../../../src/databases/entities/User'; import type { ApiPath, EndpointGroup, PostgresSchemaSection, SmtpTestAccount } from './types'; @@ -96,7 +96,7 @@ export async function initTestServer({ const { apiRouters } = await loadPublicApiVersions(testServer.publicApiEndpoint); const map: Record = { credentials: credentialsController, - publicApi: apiRouters + publicApi: apiRouters, }; for (const group of routerEndpoints) { From a16e282132ae725e82a29d43c3c63c94f86d8dda Mon Sep 17 00:00:00 2001 From: Ben Hesseldieck Date: Fri, 24 Jun 2022 17:10:01 +0200 Subject: [PATCH 02/13] =?UTF-8?q?=E2=9C=A8=20sharing/unsharing=20of=20cred?= =?UTF-8?q?entials?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/credentials/credentials.controller.ts | 34 ---------- .../ee/src/credentials/credentials.service.ts | 28 --------- packages/cli/src/CredentialsHelper.ts | 2 +- .../credentials/credentials.controller.ee.ts | 62 +++++++++++++++++++ .../src/credentials/credentials.controller.ts | 30 +++------ .../src/credentials/credentials.service.ee.ts | 40 ++++++++++++ .../src/credentials/credentials.service.ts | 4 +- packages/cli/src/credentials/helpers.ee.ts | 0 packages/cli/src/credentials/helpers.ts | 0 packages/cli/src/role/role.service.ts | 8 +++ packages/cli/src/user/user.service.ts | 8 +++ .../cli/test/integration/shared/testDb.ts | 4 +- 12 files changed, 130 insertions(+), 90 deletions(-) delete mode 100644 packages/cli/ee/src/credentials/credentials.controller.ts delete mode 100644 packages/cli/ee/src/credentials/credentials.service.ts create mode 100644 packages/cli/src/credentials/credentials.controller.ee.ts create mode 100644 packages/cli/src/credentials/credentials.service.ee.ts create mode 100644 packages/cli/src/credentials/helpers.ee.ts create mode 100644 packages/cli/src/credentials/helpers.ts create mode 100644 packages/cli/src/role/role.service.ts create mode 100644 packages/cli/src/user/user.service.ts diff --git a/packages/cli/ee/src/credentials/credentials.controller.ts b/packages/cli/ee/src/credentials/credentials.controller.ts deleted file mode 100644 index 67a3864c7cfef..0000000000000 --- a/packages/cli/ee/src/credentials/credentials.controller.ts +++ /dev/null @@ -1,34 +0,0 @@ -import express from 'express'; - -import * as config from '../../../config'; -import { credentialsService } from '../../../src/credentials/credentials.controller'; -import { CredentialRequest } from '../../../src/requests'; - -export const eeCredentialsController = express.Router(); - -// @ts-ignore -eeCredentialsController.use((req, res, next) => { - if (config.getEnv('deployment.paid')) { - // use ee router - next(); - return; - } - // skip ee router and use free one - next('router'); -}); - -eeCredentialsController.get('/shared', (req, res) => { - console.log('ENTERPRISE SHARING'); - return res.json({ enterprise: true }); -}); - -eeCredentialsController.get('/:id', async (req: CredentialRequest.Get, res) => { - const { id: credentialId } = req.params; - - console.log('ENTERPRISE ID FETCH'); - const shared = await credentialsService.getSharedCredentials(req.user.id, credentialId, [ - 'credentials', - ]); - console.log(shared); - return res.json({ enterprise: true }); -}); diff --git a/packages/cli/ee/src/credentials/credentials.service.ts b/packages/cli/ee/src/credentials/credentials.service.ts deleted file mode 100644 index 9cb1afa83db73..0000000000000 --- a/packages/cli/ee/src/credentials/credentials.service.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { FindOneOptions } from 'typeorm'; - -import { Db } from '../../../src'; -import { CredentialsService } from '../../../src/credentials/credentials.service'; -import { SharedCredentials } from '../../../src/databases/entities/SharedCredentials'; - -export class EECreditentialsService extends CredentialsService { - async getSharedCredentials( - userId: string, - credentialId: number | string, - relations?: string[], - ): Promise { - console.log('ENTERPRISE SERVICE'); - - const options: FindOneOptions = { - where: { - user: { id: userId }, - credentials: { id: credentialId }, - }, - }; - - if (relations) { - options.relations = relations; - } - - return Db.collections.SharedCredentials.findOne(options); - } -} diff --git a/packages/cli/src/CredentialsHelper.ts b/packages/cli/src/CredentialsHelper.ts index b551ec73a3904..4bac19d2d112b 100644 --- a/packages/cli/src/CredentialsHelper.ts +++ b/packages/cli/src/CredentialsHelper.ts @@ -771,7 +771,7 @@ export async function getCredentialWithoutUser( return credential; } -export function createCredentiasFromCredentialsEntity( +export function createCredentialsFromCredentialsEntity( credential: CredentialsEntity, encrypt = false, ): Credentials { diff --git a/packages/cli/src/credentials/credentials.controller.ee.ts b/packages/cli/src/credentials/credentials.controller.ee.ts new file mode 100644 index 0000000000000..9f72f6faf21b0 --- /dev/null +++ b/packages/cli/src/credentials/credentials.controller.ee.ts @@ -0,0 +1,62 @@ +/* eslint-disable import/no-cycle */ +/* eslint-disable @typescript-eslint/no-unsafe-assignment */ +/* eslint-disable @typescript-eslint/no-unsafe-call */ +import express from 'express'; + +import * as config from '../../config'; +import { UserService } from '../user/user.service'; +import { EECreditentialsService as EECredentials } from './credentials.service.ee'; + +// eslint-disable-next-line @typescript-eslint/naming-convention +export const EECredentialsController = express.Router(); + +// @ts-ignore +EECredentialsController.use((req, res, next) => { + if (config.getEnv('deployment.paid')) { + // use ee router + next(); + return; + } + // skip ee router and use free one + next('router'); +}); + +// sharing a credential +EECredentialsController.post('/:id/share', async (req, res) => { + const { id } = req.params; + const { shareeId } = req.body; + + const isOwned = EECredentials.isOwned(req.user, id); + const getSharee = UserService.get({ id: shareeId }); + + // parallelize DB requests and destructure results + const [[ownsCredential, credentials], sharee] = await Promise.all([isOwned, getSharee]); + + if (!ownsCredential) { + return res.status(403).send('Forbidden'); + } + + if (!sharee || sharee.isPending) { + return res.status(400).send('Bad Request'); + } + + await EECredentials.share(credentials!, sharee); + + return res.status(200).send(); +}); + +// unshare a credential +EECredentialsController.delete('/:id/share', async (req, res) => { + const { id } = req.params; + const { shareeId } = req.body; + + const [ownsCredential] = await EECredentials.isOwned(req.user, id); + + if (!ownsCredential) { + return res.status(403).send('Forbidden'); + } + + await EECredentials.unshare(id, shareeId); + + return res.status(200).send(); +}); diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index e75c89b478dcf..9ab5c6883853d 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -23,30 +23,14 @@ import { RESPONSE_ERROR_MESSAGES } from '../constants'; import { CredentialsEntity } from '../databases/entities/CredentialsEntity'; import { SharedCredentials } from '../databases/entities/SharedCredentials'; import { validateEntity } from '../GenericHelpers'; -import { createCredentiasFromCredentialsEntity } from '../CredentialsHelper'; +import { createCredentialsFromCredentialsEntity } from '../CredentialsHelper'; import type { CredentialRequest } from '../requests'; import * as config from '../../config'; import { externalHooks } from '../Server'; import { CredentialsService } from './credentials.service'; -import { EECreditentialsService } from '../../ee/src/credentials/credentials.service'; -import { eeCredentialsController } from '../../ee/src/credentials/credentials.controller'; +import { EECredentialsController } from './credentials.controller.ee'; export const credentialsController = express.Router(); -// eslint-disable-next-line import/no-mutable-exports -export let credentialsService: CredentialsService | EECreditentialsService; - -export function setServices(): void { - // EE enabled - if (config.getEnv('deployment.paid')) { - credentialsService = new EECreditentialsService(); - return; - } - - // FREE - credentialsService = new CredentialsService(); -} - -setServices(); /** * Initialize Logger if needed @@ -60,7 +44,7 @@ credentialsController.use((req, res, next) => { next(); }); -credentialsController.use('/', eeCredentialsController); +credentialsController.use('/', EECredentialsController); /** * GET /credentials @@ -186,7 +170,7 @@ credentialsController.post( } // Encrypt the data - const coreCredential = createCredentiasFromCredentialsEntity(newCredential, true); + const coreCredential = createCredentialsFromCredentialsEntity(newCredential, true); // @ts-ignore coreCredential.setData(newCredential.data, encryptionKey); @@ -318,7 +302,7 @@ credentialsController.patch( ); } - const coreCredential = createCredentiasFromCredentialsEntity(credential); + const coreCredential = createCredentialsFromCredentialsEntity(credential); const decryptedData = coreCredential.getData(encryptionKey); @@ -390,7 +374,7 @@ credentialsController.get( // }), // }); - const shared = await credentialsService.getSharedCredentials(req.user.id, credentialId, [ + const shared = await CredentialsService.getSharedCredentials(req.user.id, credentialId, [ 'credentials', ]); @@ -426,7 +410,7 @@ credentialsController.get( ); } - const coreCredential = credentialsService.createCredentiasFromCredentialsEntity(credential); + const coreCredential = CredentialsService.createCredentialsFromCredentialsEntity(credential); return { id: id.toString(), diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts new file mode 100644 index 0000000000000..56aec53be6ed3 --- /dev/null +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -0,0 +1,40 @@ +/* eslint-disable import/no-cycle */ +import { Db } from '..'; +import { CredentialsService } from './credentials.service'; +import { CredentialsEntity } from '../databases/entities/CredentialsEntity'; +import { SharedCredentials } from '../databases/entities/SharedCredentials'; +import { User } from '../databases/entities/User'; +import { RoleService } from '../role/role.service'; + +export class EECreditentialsService extends CredentialsService { + static async isOwned( + user: User, + credentialId: string, + ): Promise<[boolean, CredentialsEntity | null]> { + const sharedCredentials = await this.getSharedCredentials(user.id, credentialId, [ + 'credentials', + ]); + + return sharedCredentials ? [true, sharedCredentials.credentials] : [false, null]; + } + + static async share(credentials: CredentialsEntity, sharee: User): Promise { + const role = await RoleService.get({ scope: 'credential', name: 'editor' }); + + const newSharedCredential = new SharedCredentials(); + Object.assign(newSharedCredential, { + credentials, + user: sharee, + role, + }); + + return Db.collections.SharedCredentials.save(newSharedCredential); + } + + static async unshare(credentialsId: string, shareeId: string): Promise { + return Db.collections.SharedCredentials.delete({ + credentials: { id: credentialsId }, + user: { id: shareeId }, + }); + } +} diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 963588de8e972..73bc43a29309e 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -7,7 +7,7 @@ import { CredentialsEntity } from '../databases/entities/CredentialsEntity'; import { SharedCredentials } from '../databases/entities/SharedCredentials'; export class CredentialsService { - async getSharedCredentials( + static async getSharedCredentials( userId: string, credentialId: number | string, relations?: string[], @@ -26,7 +26,7 @@ export class CredentialsService { return Db.collections.SharedCredentials.findOne(options); } - createCredentiasFromCredentialsEntity( + static createCredentialsFromCredentialsEntity( credential: CredentialsEntity, encrypt = false, ): Credentials { diff --git a/packages/cli/src/credentials/helpers.ee.ts b/packages/cli/src/credentials/helpers.ee.ts new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/packages/cli/src/credentials/helpers.ts b/packages/cli/src/credentials/helpers.ts new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/packages/cli/src/role/role.service.ts b/packages/cli/src/role/role.service.ts new file mode 100644 index 0000000000000..bc3b86786e6f5 --- /dev/null +++ b/packages/cli/src/role/role.service.ts @@ -0,0 +1,8 @@ +import { Db } from '..'; +import { Role } from '../databases/entities/Role'; + +export class RoleService { + static async get(role: Partial): Promise { + return Db.collections.Role.findOne(role); + } +} diff --git a/packages/cli/src/user/user.service.ts b/packages/cli/src/user/user.service.ts new file mode 100644 index 0000000000000..43acbcf322ea4 --- /dev/null +++ b/packages/cli/src/user/user.service.ts @@ -0,0 +1,8 @@ +import { Db } from '..'; +import { User } from '../databases/entities/User'; + +export class UserService { + static async get(user: Partial): Promise { + return Db.collections.User.findOne(user); + } +} diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index d250fa99ecf91..c7131559a6947 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -15,7 +15,7 @@ import { mysqlMigrations } from '../../../src/databases/mysqldb/migrations'; import { postgresMigrations } from '../../../src/databases/postgresdb/migrations'; import { sqliteMigrations } from '../../../src/databases/sqlite/migrations'; import { categorize, getPostgresSchemaSection } from './utils'; -import { createCredentiasFromCredentialsEntity } from '../../../src/CredentialsHelper'; +import { createCredentialsFromCredentialsEntity } from '../../../src/CredentialsHelper'; import type { Role } from '../../../src/databases/entities/Role'; import { User } from '../../../src/databases/entities/User'; @@ -613,7 +613,7 @@ export const getMySqlOptions = ({ name }: { name: string }): ConnectionOptions = async function encryptCredentialData(credential: CredentialsEntity) { const encryptionKey = await UserSettings.getEncryptionKey(); - const coreCredential = createCredentiasFromCredentialsEntity(credential, true); + const coreCredential = createCredentialsFromCredentialsEntity(credential, true); // @ts-ignore coreCredential.setData(credential.data, encryptionKey); From e76b945f02521f3552c42812b04d428950ccc170 Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Thu, 7 Jul 2022 12:21:21 +0100 Subject: [PATCH 03/13] :recycle: split credential update route into controller and service --- .../src/credentials/credentials.controller.ts | 82 ++++---------- .../src/credentials/credentials.service.ts | 101 +++++++++++++++++- 2 files changed, 116 insertions(+), 67 deletions(-) diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 9ab5c6883853d..8538a56bb8d5f 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -256,21 +256,12 @@ credentialsController.patch( ResponseHelper.send(async (req: CredentialRequest.Update): Promise => { const { id: credentialId } = req.params; - const updateData = new CredentialsEntity(); - Object.assign(updateData, req.body); - - await validateEntity(updateData); - - const shared = await Db.collections.SharedCredentials.findOne({ - relations: ['credentials'], - where: whereClause({ - user: req.user, - entityType: 'credentials', - entityId: credentialId, - }), - }); + const shared = await CredentialsService.getSharedCredentials(req.user, credentialId, [ + 'credentials', + ]); if (!shared) { + console.error('cannae update'); LoggerProxy.info('Attempt to update credential blocked due to lack of permissions', { credentialId, userId: req.user.id, @@ -284,58 +275,23 @@ credentialsController.patch( const { credentials: credential } = shared; - // Add the date for newly added node access permissions - for (const nodeAccess of updateData.nodesAccess) { - if (!nodeAccess.date) { - nodeAccess.date = new Date(); - } - } - - let encryptionKey: string; - try { - encryptionKey = await UserSettings.getEncryptionKey(); - } catch (error) { - throw new ResponseHelper.ResponseError( - RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, - undefined, - 500, - ); - } - - const coreCredential = createCredentialsFromCredentialsEntity(credential); - - 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, + const encryptionKey = await CredentialsService.getEncryptionKey(); + const decryptedData = await CredentialsService.decryptCredential(encryptionKey, credential); + const preparedCredentialData = await CredentialsService.prepareCredentialsUpdateData( + req.body, + decryptedData, + ); + const newCredentialData = CredentialsService.createEncryptedCredentials( + encryptionKey, + credentialId, + preparedCredentialData, ); - - // @ts-ignore - credentials.setData(updateData.data, encryptionKey); - - const newCredentialData = credentials.getDataToSave() as ICredentialsDb; - - // Add special database related data - newCredentialData.updatedAt = new Date(); - await 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 updated entry. - const responseData = await Db.collections.Credentials.findOne(credentialId); + const responseData = await CredentialsService.updateCredentials( + credentialId, + newCredentialData, + ); if (responseData === undefined) { throw new ResponseHelper.ResponseError( @@ -374,7 +330,7 @@ credentialsController.get( // }), // }); - const shared = await CredentialsService.getSharedCredentials(req.user.id, credentialId, [ + const shared = await CredentialsService.getSharedCredentials(req.user, credentialId, [ 'credentials', ]); diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 73bc43a29309e..78f574a91c5ac 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -1,24 +1,34 @@ +/* eslint-disable no-restricted-syntax */ /* eslint-disable import/no-cycle */ -import { Credentials } from 'n8n-core'; +import { Credentials, UserSettings } from 'n8n-core'; +import { ICredentialDataDecryptedObject } from 'n8n-workflow'; import { FindOneOptions } from 'typeorm'; -import { Db } from '..'; +import { createCredentialsFromCredentialsEntity, Db, ICredentialsDb, ResponseHelper } from '..'; +import { RESPONSE_ERROR_MESSAGES } from '../constants'; import { CredentialsEntity } from '../databases/entities/CredentialsEntity'; import { SharedCredentials } from '../databases/entities/SharedCredentials'; +import { User } from '../databases/entities/User'; +import { validateEntity } from '../GenericHelpers'; +import { CredentialRequest } from '../requests'; export class CredentialsService { static async getSharedCredentials( - userId: string, + user: User, credentialId: number | string, relations?: string[], ): Promise { const options: FindOneOptions = { where: { - user: { id: userId }, credentials: { id: credentialId }, }, }; + if (user.globalRole.name !== 'owner') { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + options.where.user = { id: user.id }; + } + if (relations) { options.relations = relations; } @@ -36,4 +46,87 @@ export class CredentialsService { } return new Credentials({ id: id.toString(), name }, type, nodesAccess, data); } + + static async prepareCredentialsUpdateData( + data: CredentialRequest.RequestBody, + decryptedData: ICredentialDataDecryptedObject, + ): Promise { + const updateData = new CredentialsEntity(); + Object.assign(updateData, data); + + await validateEntity(updateData); + + // Add the date for newly added node access permissions + for (const nodeAccess of updateData.nodesAccess) { + if (!nodeAccess.date) { + nodeAccess.date = new Date(); + } + } + + // 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; + } + return updateData; + } + + static createEncryptedCredentials( + encryptionKey: string, + credentialsId: string, + updateData: CredentialsEntity, + ): ICredentialsDb { + const credentials = new Credentials( + { id: credentialsId, name: updateData.name }, + updateData.type, + updateData.nodesAccess, + ); + + credentials.setData( + updateData.data as unknown as ICredentialDataDecryptedObject, + encryptionKey, + ); + + const newCredentialData = credentials.getDataToSave() as ICredentialsDb; + + // Add special database related data + newCredentialData.updatedAt = new Date(); + + return newCredentialData; + } + + static async getEncryptionKey(): Promise { + let encryptionKey: string; + try { + encryptionKey = await UserSettings.getEncryptionKey(); + } catch (error) { + throw new ResponseHelper.ResponseError( + RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, + undefined, + 500, + ); + } + return encryptionKey; + } + + static async decryptCredential( + encryptionKey: string, + credential: CredentialsEntity, + ): Promise { + const coreCredential = createCredentialsFromCredentialsEntity(credential); + return coreCredential.getData(encryptionKey); + } + + static async updateCredentials( + credentialId: string, + newCredentialData: ICredentialsDb, + ): Promise { + // 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 updated entry. + return Db.collections.Credentials.findOne(credentialId); + } } From d607446195a8f835a5a2d1da2456d6ba34758f05 Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Thu, 7 Jul 2022 12:21:28 +0100 Subject: [PATCH 04/13] :fire: remove credentials test that is no longer applicable --- .../test/integration/credentials.api.test.ts | 51 ------------------- 1 file changed, 51 deletions(-) diff --git a/packages/cli/test/integration/credentials.api.test.ts b/packages/cli/test/integration/credentials.api.test.ts index de1ee60a0294a..49fd4cf09931a 100644 --- a/packages/cli/test/integration/credentials.api.test.ts +++ b/packages/cli/test/integration/credentials.api.test.ts @@ -526,57 +526,6 @@ test('GET /credentials/:id should return 404 if cred not found', async () => { expect(response.statusCode).toBe(404); }); -test.only('GET /credentials/:id should hit different Routers', async () => { - const ownerShell = await testDb.createUserShell(globalOwnerRole); - const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell }); - const savedCredential = await saveCredential(credentialPayload(), { user: ownerShell }); - - config.set('deployment.paid', false); - - 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(); - - config.set('deployment.paid', true); - - const third = await authOwnerAgent.get(`/credentials/${savedCredential.id}`); - - expect(third.statusCode).toBe(200); - - expect(third.body).toEqual({ enterprise: true }); - - const fourth = await authOwnerAgent.get(`/credentials/${savedCredential.id}`); - - expect(fourth.statusCode).toBe(200); - expect(fourth.body).toEqual({ enterprise: true }); - - config.set('deployment.paid', false); - - const fifth = await authOwnerAgent.get(`/credentials/${savedCredential.id}`); - - expect(fifth.statusCode).toBe(200); - - expect(typeof fifth.body.data.name).toBe('string'); - expect(typeof fifth.body.data.type).toBe('string'); - expect(typeof fifth.body.data.nodesAccess[0].nodeType).toBe('string'); - expect(fifth.body.data.data).toBeUndefined(); -}); - const credentialPayload = () => ({ name: randomName(), type: randomName(), From fc3b72d8970760f6578ddcb9aaf3180ca5add070 Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Mon, 11 Jul 2022 10:16:34 +0100 Subject: [PATCH 05/13] :recycle: split credential creation route into controller and service --- .../src/credentials/credentials.controller.ts | 84 +++--------------- .../src/credentials/credentials.service.ts | 85 ++++++++++++++++--- 2 files changed, 87 insertions(+), 82 deletions(-) diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 8538a56bb8d5f..557afca4ab32a 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -145,68 +145,20 @@ credentialsController.post( credentialsController.post( '/', ResponseHelper.send(async (req: CredentialRequest.Create) => { - delete req.body.id; // delete if sent + const newCredential = await CredentialsService.prepareCredentialsCreateData(req.body); - 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 = new Date(); - } - - let encryptionKey: string; - try { - encryptionKey = await UserSettings.getEncryptionKey(); - } catch (error) { - throw new ResponseHelper.ResponseError( - RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, - undefined, - 500, - ); - } - - // Encrypt the data - const coreCredential = createCredentialsFromCredentialsEntity(newCredential, true); - - // @ts-ignore - coreCredential.setData(newCredential.data, encryptionKey); - - const encryptedData = coreCredential.getDataToSave() as ICredentialsDb; - - Object.assign(newCredential, encryptedData); - - await externalHooks.run('credentials.create', [encryptedData]); - - const role = await Db.collections.Role.findOneOrFail({ - name: 'owner', - scope: 'credential', - }); - - const { id, ...rest } = await Db.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); + const encryptionKey = await CredentialsService.getEncryptionKey(); + const encryptedData = CredentialsService.createEncryptedCredentialsData( + encryptionKey, + null, + newCredential, + ); + const { id, ...rest } = await CredentialsService.saveCredentials( + newCredential, + encryptedData, + req.user, + ); - return savedCredential; - }); - LoggerProxy.verbose('New credential created', { - credentialId: newCredential.id, - ownerId: req.user.id, - }); return { id: id.toString(), ...rest }; }), ); @@ -281,12 +233,11 @@ credentialsController.patch( req.body, decryptedData, ); - const newCredentialData = CredentialsService.createEncryptedCredentials( + const newCredentialData = CredentialsService.createEncryptedCredentialsData( encryptionKey, credentialId, preparedCredentialData, ); - await externalHooks.run('credentials.update', [newCredentialData]); const responseData = await CredentialsService.updateCredentials( credentialId, @@ -321,15 +272,6 @@ credentialsController.get( 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, - // }), - // }); - const shared = await CredentialsService.getSharedCredentials(req.user, credentialId, [ 'credentials', ]); diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 78f574a91c5ac..05d3e35faded7 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -1,8 +1,9 @@ /* eslint-disable no-restricted-syntax */ /* eslint-disable import/no-cycle */ import { Credentials, UserSettings } from 'n8n-core'; -import { ICredentialDataDecryptedObject } from 'n8n-workflow'; +import { ICredentialDataDecryptedObject, LoggerProxy } from 'n8n-workflow'; import { FindOneOptions } from 'typeorm'; +import { clone } from 'lodash'; import { createCredentialsFromCredentialsEntity, Db, ICredentialsDb, ResponseHelper } from '..'; import { RESPONSE_ERROR_MESSAGES } from '../constants'; @@ -11,6 +12,7 @@ import { SharedCredentials } from '../databases/entities/SharedCredentials'; import { User } from '../databases/entities/User'; import { validateEntity } from '../GenericHelpers'; import { CredentialRequest } from '../requests'; +import { externalHooks } from '../Server'; export class CredentialsService { static async getSharedCredentials( @@ -47,6 +49,26 @@ export class CredentialsService { return new Credentials({ id: id.toString(), name }, type, nodesAccess, data); } + static async prepareCredentialsCreateData( + data: CredentialRequest.RequestBody, + ): Promise { + // Make a copy so we can delete the provided ID + const dataCopy = clone(data); + delete dataCopy.id; + + const newCredentials = new CredentialsEntity(); + Object.assign(newCredentials, dataCopy); + + await validateEntity(newCredentials); + + // Add the date for newly added node access permissions + for (const nodeAccess of newCredentials.nodesAccess) { + nodeAccess.date = new Date(); + } + + return newCredentials; + } + static async prepareCredentialsUpdateData( data: CredentialRequest.RequestBody, decryptedData: ICredentialDataDecryptedObject, @@ -72,21 +94,18 @@ export class CredentialsService { return updateData; } - static createEncryptedCredentials( + static createEncryptedCredentialsData( encryptionKey: string, - credentialsId: string, - updateData: CredentialsEntity, + credentialsId: string | null, + data: CredentialsEntity, ): ICredentialsDb { const credentials = new Credentials( - { id: credentialsId, name: updateData.name }, - updateData.type, - updateData.nodesAccess, + { id: credentialsId, name: data.name }, + data.type, + data.nodesAccess, ); - credentials.setData( - updateData.data as unknown as ICredentialDataDecryptedObject, - encryptionKey, - ); + credentials.setData(data.data as unknown as ICredentialDataDecryptedObject, encryptionKey); const newCredentialData = credentials.getDataToSave() as ICredentialsDb; @@ -122,6 +141,8 @@ export class CredentialsService { credentialId: string, newCredentialData: ICredentialsDb, ): Promise { + await externalHooks.run('credentials.update', [newCredentialData]); + // Update the credentials in DB await Db.collections.Credentials.update(credentialId, newCredentialData); @@ -129,4 +150,46 @@ export class CredentialsService { // nor the new value. So query now the updated entry. return Db.collections.Credentials.findOne(credentialId); } + + static async saveCredentials( + credential: CredentialsEntity, + encryptedData: ICredentialsDb, + user: User, + ): Promise { + // To avoid side effects + const newCredential = new CredentialsEntity(); + Object.assign(newCredential, credential); + + Object.assign(newCredential, encryptedData); + + await externalHooks.run('credentials.create', [encryptedData]); + + const role = await Db.collections.Role.findOneOrFail({ + name: 'owner', + scope: 'credential', + }); + + const result = await Db.transaction(async (transactionManager) => { + const savedCredential = await transactionManager.save(newCredential); + + savedCredential.data = newCredential.data; + + const newSharedCredential = new SharedCredentials(); + + Object.assign(newSharedCredential, { + role, + user, + credentials: savedCredential, + }); + + await transactionManager.save(newSharedCredential); + + return savedCredential; + }); + LoggerProxy.verbose('New credential created', { + credentialId: newCredential.id, + ownerId: user.id, + }); + return result; + } } From 46e57fbf43acb6860363f71aaf40cd759092d614 Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Mon, 11 Jul 2022 10:20:36 +0100 Subject: [PATCH 06/13] :recycle: split single credential get --- .../src/credentials/credentials.controller.ts | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 557afca4ab32a..aa85e11075cca 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -297,22 +297,12 @@ credentialsController.get( const { data, id, ...rest } = credential; - let encryptionKey: string; - try { - encryptionKey = await UserSettings.getEncryptionKey(); - } catch (error) { - throw new ResponseHelper.ResponseError( - RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, - undefined, - 500, - ); - } - - const coreCredential = CredentialsService.createCredentialsFromCredentialsEntity(credential); + const encryptionKey = await CredentialsService.getEncryptionKey(); + const decryptedData = await CredentialsService.decryptCredential(encryptionKey, credential); return { id: id.toString(), - data: coreCredential.getData(encryptionKey), + data: decryptedData, ...rest, }; }), From ce76c722f70d7d98ba27f8707dc39c96065e37ca Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Mon, 11 Jul 2022 10:46:02 +0100 Subject: [PATCH 07/13] :recycle: split delete credentials route --- .../cli/src/credentials/credentials.controller.ts | 15 ++++----------- .../cli/src/credentials/credentials.service.ts | 6 ++++++ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index aa85e11075cca..c07fb8fdf0848 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -171,14 +171,9 @@ credentialsController.delete( 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, - }), - }); + const shared = await CredentialsService.getSharedCredentials(req.user, credentialId, [ + 'credentials', + ]); if (!shared) { LoggerProxy.info('Attempt to delete credential blocked due to lack of permissions', { @@ -192,9 +187,7 @@ credentialsController.delete( ); } - await externalHooks.run('credentials.delete', [credentialId]); - - await Db.collections.Credentials.remove(shared.credentials); + await CredentialsService.deleteCredentials(shared.credentials); return true; }), diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 05d3e35faded7..aefc97ac70d99 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -192,4 +192,10 @@ export class CredentialsService { }); return result; } + + static async deleteCredentials(credentials: CredentialsEntity): Promise { + await externalHooks.run('credentials.delete', [credentials.id]); + + await Db.collections.Credentials.remove(credentials); + } } From 5783652bcbc81684c2c738d8adf8e04c629ebde8 Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Mon, 11 Jul 2022 11:26:18 +0100 Subject: [PATCH 08/13] :recycle: split get all credentials route --- .../src/credentials/credentials.controller.ts | 47 +------------- .../src/credentials/credentials.service.ts | 64 ++++++++++++++++++- 2 files changed, 64 insertions(+), 47 deletions(-) diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index c07fb8fdf0848..81b43a99444a1 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -52,38 +52,9 @@ credentialsController.use('/', EECredentialsController); credentialsController.get( '/', ResponseHelper.send(async (req: CredentialRequest.GetAll): Promise => { - let credentials: ICredentialsDb[] = []; - const filter = req.query.filter ? (JSON.parse(req.query.filter) as Record) : {}; - try { - 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({ - 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(({ credentialId }) => credentialId)), - ...filter, - }, - }); - } - } catch (error) { - LoggerProxy.error('Request to list credentials failed', error); - throw error; - } + const credentials = await CredentialsService.getFilteredCredentials(req.user, filter); return credentials.map((credential) => { // eslint-disable-next-line no-param-reassign @@ -122,20 +93,8 @@ credentialsController.post( ResponseHelper.send(async (req: CredentialRequest.Test): Promise => { const { credentials, nodeToTestWith } = req.body; - let encryptionKey: string; - try { - encryptionKey = await UserSettings.getEncryptionKey(); - } catch (error) { - throw new ResponseHelper.ResponseError( - RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, - undefined, - 500, - ); - } - - const helper = new CredentialsHelper(encryptionKey); - - return helper.testCredentials(req.user, credentials.type, credentials, nodeToTestWith); + const encryptionKey = await CredentialsService.getEncryptionKey(); + return CredentialsService.testCredentials(req.user, encryptionKey, credentials, nodeToTestWith); }), ); diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index aefc97ac70d99..45418d91441ca 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -1,11 +1,23 @@ /* eslint-disable no-restricted-syntax */ /* eslint-disable import/no-cycle */ import { Credentials, UserSettings } from 'n8n-core'; -import { ICredentialDataDecryptedObject, LoggerProxy } from 'n8n-workflow'; -import { FindOneOptions } from 'typeorm'; +import { + ICredentialDataDecryptedObject, + ICredentialsDecrypted, + INodeCredentialTestResult, + LoggerProxy, +} from 'n8n-workflow'; +import { FindOneOptions, In } from 'typeorm'; import { clone } from 'lodash'; -import { createCredentialsFromCredentialsEntity, Db, ICredentialsDb, ResponseHelper } from '..'; +import { + createCredentialsFromCredentialsEntity, + CredentialsHelper, + Db, + ICredentialsDb, + ResponseHelper, + whereClause, +} from '..'; import { RESPONSE_ERROR_MESSAGES } from '../constants'; import { CredentialsEntity } from '../databases/entities/CredentialsEntity'; import { SharedCredentials } from '../databases/entities/SharedCredentials'; @@ -38,6 +50,41 @@ export class CredentialsService { return Db.collections.SharedCredentials.findOne(options); } + static async getFilteredCredentials( + user: User, + filter: Record, + ): Promise { + try { + if (user.globalRole.name === 'owner') { + return await Db.collections.Credentials.find({ + select: ['id', 'name', 'type', 'nodesAccess', 'createdAt', 'updatedAt'], + where: filter, + }); + } + const shared = await Db.collections.SharedCredentials.find({ + where: whereClause({ + user, + entityType: 'credentials', + }), + }); + + if (!shared.length) return []; + + return await Db.collections.Credentials.find({ + select: ['id', 'name', 'type', 'nodesAccess', 'createdAt', 'updatedAt'], + where: { + // The ordering is important here. If id is before the object spread then + // a user can control the id field + ...filter, + id: In(shared.map(({ credentialId }) => credentialId)), + }, + }); + } catch (error) { + LoggerProxy.error('Request to list credentials failed', error); + throw error; + } + } + static createCredentialsFromCredentialsEntity( credential: CredentialsEntity, encrypt = false, @@ -198,4 +245,15 @@ export class CredentialsService { await Db.collections.Credentials.remove(credentials); } + + static async testCredentials( + user: User, + encryptionKey: string, + credentials: ICredentialsDecrypted, + nodeToTestWith: string | undefined, + ): Promise { + const helper = new CredentialsHelper(encryptionKey); + + return helper.testCredentials(user, credentials.type, credentials, nodeToTestWith); + } } From 2fc21cb31518e323be1aa17e225e344cbc26c6eb Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Mon, 11 Jul 2022 11:36:33 +0100 Subject: [PATCH 09/13] :fire: remove unused imports in credentials contoller --- .../src/credentials/credentials.controller.ts | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 81b43a99444a1..161a4a85fecc3 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -4,29 +4,13 @@ /* eslint-disable @typescript-eslint/no-non-null-assertion */ /* eslint-disable import/no-cycle */ import express from 'express'; -import { In } from 'typeorm'; -import { UserSettings, Credentials } from 'n8n-core'; import { INodeCredentialTestResult, LoggerProxy } from 'n8n-workflow'; import { getLogger } from '../Logger'; -import { - CredentialsHelper, - Db, - GenericHelpers, - ICredentialsDb, - ICredentialsResponse, - whereClause, - ResponseHelper, -} from '..'; - -import { RESPONSE_ERROR_MESSAGES } from '../constants'; -import { CredentialsEntity } from '../databases/entities/CredentialsEntity'; -import { SharedCredentials } from '../databases/entities/SharedCredentials'; -import { validateEntity } from '../GenericHelpers'; -import { createCredentialsFromCredentialsEntity } from '../CredentialsHelper'; +import { GenericHelpers, ICredentialsResponse, ResponseHelper } from '..'; + import type { CredentialRequest } from '../requests'; import * as config from '../../config'; -import { externalHooks } from '../Server'; import { CredentialsService } from './credentials.service'; import { EECredentialsController } from './credentials.controller.ee'; From 1d45f3a705e08d6b890fdedbb9c0ca53ea4fcf19 Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Tue, 12 Jul 2022 16:51:22 +0100 Subject: [PATCH 10/13] :fire: remove console.log --- packages/cli/src/credentials/credentials.controller.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 161a4a85fecc3..e1274bcc348fd 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -149,7 +149,6 @@ credentialsController.patch( ]); if (!shared) { - console.error('cannae update'); LoggerProxy.info('Attempt to update credential blocked due to lack of permissions', { credentialId, userId: req.user.id, From 5cc03cb7ceef06feb42706c5da421c276b225bf0 Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Tue, 19 Jul 2022 12:47:27 +0100 Subject: [PATCH 11/13] :refactor: changes to credentials controller and service from review - removed credentials from service function names - made relations list optional - put allowGlobalOwner in options objects - check length of relations array so join doesn't happen if empty - update some comments to further explain rationale - remove unneeded `Object.assign` - remove non-null assertion from test --- .../src/credentials/credentials.controller.ts | 41 ++++++------------ .../src/credentials/credentials.service.ee.ts | 4 +- .../src/credentials/credentials.service.ts | 43 +++++++++---------- .../cli/test/integration/credentials.test.ts | 28 ++++++++---- 4 files changed, 57 insertions(+), 59 deletions(-) diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index e1274bcc348fd..7d0e8b4eba391 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -38,7 +38,7 @@ credentialsController.get( ResponseHelper.send(async (req: CredentialRequest.GetAll): Promise => { const filter = req.query.filter ? (JSON.parse(req.query.filter) as Record) : {}; - const credentials = await CredentialsService.getFilteredCredentials(req.user, filter); + const credentials = await CredentialsService.getFiltered(req.user, filter); return credentials.map((credential) => { // eslint-disable-next-line no-param-reassign @@ -78,7 +78,7 @@ credentialsController.post( const { credentials, nodeToTestWith } = req.body; const encryptionKey = await CredentialsService.getEncryptionKey(); - return CredentialsService.testCredentials(req.user, encryptionKey, credentials, nodeToTestWith); + return CredentialsService.test(req.user, encryptionKey, credentials, nodeToTestWith); }), ); @@ -88,19 +88,15 @@ credentialsController.post( credentialsController.post( '/', ResponseHelper.send(async (req: CredentialRequest.Create) => { - const newCredential = await CredentialsService.prepareCredentialsCreateData(req.body); + const newCredential = await CredentialsService.prepareCreateData(req.body); const encryptionKey = await CredentialsService.getEncryptionKey(); - const encryptedData = CredentialsService.createEncryptedCredentialsData( + const encryptedData = CredentialsService.createEncryptedData( encryptionKey, null, newCredential, ); - const { id, ...rest } = await CredentialsService.saveCredentials( - newCredential, - encryptedData, - req.user, - ); + const { id, ...rest } = await CredentialsService.save(newCredential, encryptedData, req.user); return { id: id.toString(), ...rest }; }), @@ -114,9 +110,7 @@ credentialsController.delete( ResponseHelper.send(async (req: CredentialRequest.Delete) => { const { id: credentialId } = req.params; - const shared = await CredentialsService.getSharedCredentials(req.user, credentialId, [ - 'credentials', - ]); + const shared = await CredentialsService.getShared(req.user, credentialId); if (!shared) { LoggerProxy.info('Attempt to delete credential blocked due to lack of permissions', { @@ -130,7 +124,7 @@ credentialsController.delete( ); } - await CredentialsService.deleteCredentials(shared.credentials); + await CredentialsService.delete(shared.credentials); return true; }), @@ -144,9 +138,7 @@ credentialsController.patch( ResponseHelper.send(async (req: CredentialRequest.Update): Promise => { const { id: credentialId } = req.params; - const shared = await CredentialsService.getSharedCredentials(req.user, credentialId, [ - 'credentials', - ]); + const shared = await CredentialsService.getShared(req.user, credentialId); if (!shared) { LoggerProxy.info('Attempt to update credential blocked due to lack of permissions', { @@ -163,21 +155,18 @@ credentialsController.patch( const { credentials: credential } = shared; const encryptionKey = await CredentialsService.getEncryptionKey(); - const decryptedData = await CredentialsService.decryptCredential(encryptionKey, credential); - const preparedCredentialData = await CredentialsService.prepareCredentialsUpdateData( + const decryptedData = await CredentialsService.decrypt(encryptionKey, credential); + const preparedCredentialData = await CredentialsService.prepareUpdateData( req.body, decryptedData, ); - const newCredentialData = CredentialsService.createEncryptedCredentialsData( + const newCredentialData = CredentialsService.createEncryptedData( encryptionKey, credentialId, preparedCredentialData, ); - const responseData = await CredentialsService.updateCredentials( - credentialId, - newCredentialData, - ); + const responseData = await CredentialsService.update(credentialId, newCredentialData); if (responseData === undefined) { throw new ResponseHelper.ResponseError( @@ -207,9 +196,7 @@ credentialsController.get( ResponseHelper.send(async (req: CredentialRequest.Get) => { const { id: credentialId } = req.params; - const shared = await CredentialsService.getSharedCredentials(req.user, credentialId, [ - 'credentials', - ]); + const shared = await CredentialsService.getShared(req.user, credentialId, ['credentials']); if (!shared) { throw new ResponseHelper.ResponseError( @@ -233,7 +220,7 @@ credentialsController.get( const { data, id, ...rest } = credential; const encryptionKey = await CredentialsService.getEncryptionKey(); - const decryptedData = await CredentialsService.decryptCredential(encryptionKey, credential); + const decryptedData = await CredentialsService.decrypt(encryptionKey, credential); return { id: id.toString(), diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index cdc875991e64a..be78fea78e993 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -11,7 +11,9 @@ export class EECredentialsService extends CredentialsService { user: User, credentialId: string, ): Promise<{ ownsCredential: boolean; credential?: CredentialsEntity }> { - const sharing = await this.getSharedCredentials(user, credentialId, ['credentials'], false); + const sharing = await this.getShared(user, credentialId, ['credentials'], { + allowGlobalOwner: false, + }); return sharing ? { ownsCredential: true, credential: sharing.credentials } diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index da1e8287b78ad..5a75313e5b345 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -27,11 +27,11 @@ import { CredentialRequest } from '../requests'; import { externalHooks } from '../Server'; export class CredentialsService { - static async getSharedCredentials( + static async getShared( user: User, credentialId: number | string, - relations?: string[], - allowGlobalOwner = true, + relations: string[] | undefined = ['credentials'], + { allowGlobalOwner }: { allowGlobalOwner: boolean } = { allowGlobalOwner: true }, ): Promise { const options: FindOneOptions = { where: { @@ -39,22 +39,22 @@ export class CredentialsService { }, }; + // Omit user from where if the requesting user is the global + // owner. This allows the global owner to view and delete + // credentials they don't own. if (!allowGlobalOwner || user.globalRole.name !== 'owner') { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access options.where.user = { id: user.id }; } - if (relations) { + if (relations?.length) { options.relations = relations; } return Db.collections.SharedCredentials.findOne(options); } - static async getFilteredCredentials( - user: User, - filter: Record, - ): Promise { + static async getFiltered(user: User, filter: Record): Promise { try { if (user.globalRole.name === 'owner') { return await Db.collections.Credentials.find({ @@ -97,10 +97,11 @@ export class CredentialsService { return new Credentials({ id: id.toString(), name }, type, nodesAccess, data); } - static async prepareCredentialsCreateData( + static async prepareCreateData( data: CredentialRequest.CredentialProperties, ): Promise { - // Make a copy so we can delete the provided ID + // Make a copy so we can delete the provided ID and + // so we can modify nodesAccess const dataCopy = clone(data); delete dataCopy.id; @@ -117,7 +118,7 @@ export class CredentialsService { return newCredentials; } - static async prepareCredentialsUpdateData( + static async prepareUpdateData( data: CredentialRequest.CredentialProperties, decryptedData: ICredentialDataDecryptedObject, ): Promise { @@ -142,7 +143,7 @@ export class CredentialsService { return updateData; } - static createEncryptedCredentialsData( + static createEncryptedData( encryptionKey: string, credentialsId: string | null, data: CredentialsEntity, @@ -164,9 +165,8 @@ export class CredentialsService { } static async getEncryptionKey(): Promise { - let encryptionKey: string; try { - encryptionKey = await UserSettings.getEncryptionKey(); + return await UserSettings.getEncryptionKey(); } catch (error) { throw new ResponseHelper.ResponseError( RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, @@ -174,10 +174,9 @@ export class CredentialsService { 500, ); } - return encryptionKey; } - static async decryptCredential( + static async decrypt( encryptionKey: string, credential: CredentialsEntity, ): Promise { @@ -185,7 +184,7 @@ export class CredentialsService { return coreCredential.getData(encryptionKey); } - static async updateCredentials( + static async update( credentialId: string, newCredentialData: ICredentialsDb, ): Promise { @@ -199,16 +198,14 @@ export class CredentialsService { return Db.collections.Credentials.findOne(credentialId); } - static async saveCredentials( + static async save( credential: CredentialsEntity, encryptedData: ICredentialsDb, user: User, ): Promise { // To avoid side effects const newCredential = new CredentialsEntity(); - Object.assign(newCredential, credential); - - Object.assign(newCredential, encryptedData); + Object.assign(newCredential, credential, encryptedData); await externalHooks.run('credentials.create', [encryptedData]); @@ -241,13 +238,13 @@ export class CredentialsService { return result; } - static async deleteCredentials(credentials: CredentialsEntity): Promise { + static async delete(credentials: CredentialsEntity): Promise { await externalHooks.run('credentials.delete', [credentials.id]); await Db.collections.Credentials.remove(credentials); } - static async testCredentials( + static async test( user: User, encryptionKey: string, credentials: ICredentialsDecrypted, diff --git a/packages/cli/test/integration/credentials.test.ts b/packages/cli/test/integration/credentials.test.ts index c3929db20a36c..875eac29b67eb 100644 --- a/packages/cli/test/integration/credentials.test.ts +++ b/packages/cli/test/integration/credentials.test.ts @@ -59,14 +59,17 @@ test('POST /credentials should create cred', async () => { expect(name).toBe(payload.name); expect(type).toBe(payload.type); - expect(nodesAccess[0].nodeType).toBe(payload.nodesAccess![0].nodeType); + if (!payload.nodesAccess) { + fail('Payload did not contain a nodesAccess array'); + } + expect(nodesAccess[0].nodeType).toBe(payload.nodesAccess[0].nodeType); expect(encryptedData).not.toBe(payload.data); const credential = await Db.collections.Credentials.findOneOrFail(id); expect(credential.name).toBe(payload.name); expect(credential.type).toBe(payload.type); - expect(credential.nodesAccess[0].nodeType).toBe(payload.nodesAccess![0].nodeType); + expect(credential.nodesAccess[0].nodeType).toBe(payload.nodesAccess[0].nodeType); expect(credential.data).not.toBe(payload.data); const sharedCredential = await Db.collections.SharedCredentials.findOneOrFail({ @@ -223,14 +226,17 @@ test('PATCH /credentials/:id should update owned cred for owner', async () => { expect(name).toBe(patchPayload.name); expect(type).toBe(patchPayload.type); - expect(nodesAccess[0].nodeType).toBe(patchPayload.nodesAccess![0].nodeType); + if (!patchPayload.nodesAccess) { + fail('Payload did not contain a nodesAccess array'); + } + 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.nodesAccess[0].nodeType).toBe(patchPayload.nodesAccess[0].nodeType); expect(credential.data).not.toBe(patchPayload.data); const sharedCredential = await Db.collections.SharedCredentials.findOneOrFail({ @@ -258,14 +264,17 @@ test('PATCH /credentials/:id should update non-owned cred for owner', async () = expect(name).toBe(patchPayload.name); expect(type).toBe(patchPayload.type); - expect(nodesAccess[0].nodeType).toBe(patchPayload.nodesAccess![0].nodeType); + if (!patchPayload.nodesAccess) { + fail('Payload did not contain a nodesAccess array'); + } + 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.nodesAccess[0].nodeType).toBe(patchPayload.nodesAccess[0].nodeType); expect(credential.data).not.toBe(patchPayload.data); const sharedCredential = await Db.collections.SharedCredentials.findOneOrFail({ @@ -292,14 +301,17 @@ test('PATCH /credentials/:id should update owned cred for member', async () => { expect(name).toBe(patchPayload.name); expect(type).toBe(patchPayload.type); - expect(nodesAccess[0].nodeType).toBe(patchPayload.nodesAccess![0].nodeType); + if (!patchPayload.nodesAccess) { + fail('Payload did not contain a nodesAccess array'); + } + 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.nodesAccess[0].nodeType).toBe(patchPayload.nodesAccess[0].nodeType); expect(credential.data).not.toBe(patchPayload.data); const sharedCredential = await Db.collections.SharedCredentials.findOneOrFail({ From 42a5748476b6f2b5d82c41625d8d1fdcc693ac79 Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Wed, 20 Jul 2022 14:30:41 +0100 Subject: [PATCH 12/13] :recycle: move filtered credentials selected fields to variable --- packages/cli/src/credentials/credentials.service.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 5a75313e5b345..271ffcc233677 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -55,10 +55,18 @@ export class CredentialsService { } static async getFiltered(user: User, filter: Record): Promise { + const selectFields: Array = [ + 'id', + 'name', + 'type', + 'nodesAccess', + 'createdAt', + 'updatedAt', + ]; try { if (user.globalRole.name === 'owner') { return await Db.collections.Credentials.find({ - select: ['id', 'name', 'type', 'nodesAccess', 'createdAt', 'updatedAt'], + select: selectFields, where: filter, }); } @@ -72,7 +80,7 @@ export class CredentialsService { if (!shared.length) return []; return await Db.collections.Credentials.find({ - select: ['id', 'name', 'type', 'nodesAccess', 'createdAt', 'updatedAt'], + select: selectFields, where: { // The ordering is important here. If id is before the object spread then // a user can control the id field From 0b1b9fed091ff68c13800bce625b7393f39213e4 Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Wed, 20 Jul 2022 14:58:44 +0100 Subject: [PATCH 13/13] :recycle: remove unneeded merges in credentials service --- .../src/credentials/credentials.service.ts | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 271ffcc233677..c22d9ae8f0ece 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -8,7 +8,6 @@ import { LoggerProxy, } from 'n8n-workflow'; import { FindOneOptions, In } from 'typeorm'; -import { clone } from 'lodash'; import { createCredentialsFromCredentialsEntity, @@ -108,13 +107,14 @@ export class CredentialsService { static async prepareCreateData( data: CredentialRequest.CredentialProperties, ): Promise { - // Make a copy so we can delete the provided ID and - // so we can modify nodesAccess - const dataCopy = clone(data); - delete dataCopy.id; + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { id, ...rest } = data; - const newCredentials = new CredentialsEntity(); - Object.assign(newCredentials, dataCopy); + // This saves us a merge but requires some type casting. These + // types are compatiable for this case. + const newCredentials = Db.collections.Credentials.create( + rest as ICredentialsDb, + ) as CredentialsEntity; await validateEntity(newCredentials); @@ -130,8 +130,11 @@ export class CredentialsService { data: CredentialRequest.CredentialProperties, decryptedData: ICredentialDataDecryptedObject, ): Promise { - const updateData = new CredentialsEntity(); - Object.assign(updateData, data); + // This saves us a merge but requires some type casting. These + // types are compatiable for this case. + const updateData = Db.collections.Credentials.create( + data as ICredentialsDb, + ) as CredentialsEntity; await validateEntity(updateData);