diff --git a/packages/cli/src/credentials/credentials.controller.ee.ts b/packages/cli/src/credentials/credentials.controller.ee.ts index 553dbdce4aecb..9c6711a2eacf8 100644 --- a/packages/cli/src/credentials/credentials.controller.ee.ts +++ b/packages/cli/src/credentials/credentials.controller.ee.ts @@ -28,7 +28,7 @@ EECredentialsController.post('/:id/share', async (req: CredentialRequest.Share, const { id } = req.params; const { shareeId } = req.body; - const isOwned = EECredentials.isOwned(req.user.id, id); + const isOwned = EECredentials.isOwned(req.user, id); const getSharee = UserService.get({ id: shareeId }); // parallelize DB requests and destructure results @@ -57,7 +57,7 @@ EECredentialsController.delete('/:id/share', async (req: CredentialRequest.Share const { id } = req.params; const { shareeId } = req.body; - const { ownsCredential } = await EECredentials.isOwned(req.user.id, id); + const { ownsCredential } = await EECredentials.isOwned(req.user, id); if (!ownsCredential) { return res.status(403).send(); diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 2e724954f5964..7d0e8b4eba391 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'; @@ -52,38 +36,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.getFiltered(req.user, filter); return credentials.map((credential) => { // eslint-disable-next-line no-param-reassign @@ -122,20 +77,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.test(req.user, encryptionKey, credentials, nodeToTestWith); }), ); @@ -145,68 +88,16 @@ credentialsController.post( credentialsController.post( '/', 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 = 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, - }); + const newCredential = await CredentialsService.prepareCreateData(req.body); - await transactionManager.save(newSharedCredential); + const encryptionKey = await CredentialsService.getEncryptionKey(); + const encryptedData = CredentialsService.createEncryptedData( + encryptionKey, + null, + newCredential, + ); + const { id, ...rest } = await CredentialsService.save(newCredential, encryptedData, req.user); - return savedCredential; - }); - LoggerProxy.verbose('New credential created', { - credentialId: newCredential.id, - ownerId: req.user.id, - }); return { id: id.toString(), ...rest }; }), ); @@ -219,14 +110,7 @@ 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.getShared(req.user, credentialId); if (!shared) { LoggerProxy.info('Attempt to delete credential blocked due to lack of permissions', { @@ -240,9 +124,7 @@ credentialsController.delete( ); } - await externalHooks.run('credentials.delete', [credentialId]); - - await Db.collections.Credentials.remove(shared.credentials); + await CredentialsService.delete(shared.credentials); return true; }), @@ -256,19 +138,7 @@ 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.getShared(req.user, credentialId); if (!shared) { LoggerProxy.info('Attempt to update credential blocked due to lack of permissions', { @@ -284,58 +154,19 @@ 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.decrypt(encryptionKey, credential); + const preparedCredentialData = await CredentialsService.prepareUpdateData( + req.body, + decryptedData, + ); + const newCredentialData = CredentialsService.createEncryptedData( + 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.update(credentialId, newCredentialData); if (responseData === undefined) { throw new ResponseHelper.ResponseError( @@ -365,7 +196,7 @@ credentialsController.get( ResponseHelper.send(async (req: CredentialRequest.Get) => { const { id: credentialId } = req.params; - const shared = await CredentialsService.getSharing(req.user.id, credentialId, ['credentials']); + const shared = await CredentialsService.getShared(req.user, credentialId, ['credentials']); if (!shared) { throw new ResponseHelper.ResponseError( @@ -388,22 +219,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.decrypt(encryptionKey, credential); return { id: id.toString(), - data: coreCredential.getData(encryptionKey), + data: decryptedData, ...rest, }; }), diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index cf9dca1835629..be78fea78e993 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -8,10 +8,12 @@ import { RoleService } from '../role/role.service'; export class EECredentialsService extends CredentialsService { static async isOwned( - userId: string, + user: User, credentialId: string, ): Promise<{ ownsCredential: boolean; credential?: CredentialsEntity }> { - const sharing = await this.getSharing(userId, credentialId, ['credentials']); + 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 215c8e0b4b7f3..c22d9ae8f0ece 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -1,31 +1,98 @@ +/* eslint-disable no-restricted-syntax */ /* eslint-disable import/no-cycle */ -import { Credentials } from 'n8n-core'; -import { FindOneOptions } from 'typeorm'; +import { Credentials, UserSettings } from 'n8n-core'; +import { + ICredentialDataDecryptedObject, + ICredentialsDecrypted, + INodeCredentialTestResult, + LoggerProxy, +} from 'n8n-workflow'; +import { FindOneOptions, In } from 'typeorm'; -import { Db } 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'; +import { User } from '../databases/entities/User'; +import { validateEntity } from '../GenericHelpers'; +import { CredentialRequest } from '../requests'; +import { externalHooks } from '../Server'; export class CredentialsService { - static async getSharing( - userId: string, + static async getShared( + user: User, credentialId: number | string, - relations?: string[], + relations: string[] | undefined = ['credentials'], + { allowGlobalOwner }: { allowGlobalOwner: boolean } = { allowGlobalOwner: true }, ): Promise { const options: FindOneOptions = { where: { - user: { id: userId }, credentials: { id: credentialId }, }, }; - if (relations) { + // 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?.length) { options.relations = relations; } return Db.collections.SharedCredentials.findOne(options); } + 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: selectFields, + 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: selectFields, + 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, @@ -36,4 +103,166 @@ export class CredentialsService { } return new Credentials({ id: id.toString(), name }, type, nodesAccess, data); } + + static async prepareCreateData( + data: CredentialRequest.CredentialProperties, + ): Promise { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { id, ...rest } = data; + + // 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); + + // Add the date for newly added node access permissions + for (const nodeAccess of newCredentials.nodesAccess) { + nodeAccess.date = new Date(); + } + + return newCredentials; + } + + static async prepareUpdateData( + data: CredentialRequest.CredentialProperties, + decryptedData: ICredentialDataDecryptedObject, + ): Promise { + // 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); + + // 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 createEncryptedData( + encryptionKey: string, + credentialsId: string | null, + data: CredentialsEntity, + ): ICredentialsDb { + const credentials = new Credentials( + { id: credentialsId, name: data.name }, + data.type, + data.nodesAccess, + ); + + credentials.setData(data.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 { + try { + return await UserSettings.getEncryptionKey(); + } catch (error) { + throw new ResponseHelper.ResponseError( + RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, + undefined, + 500, + ); + } + } + + static async decrypt( + encryptionKey: string, + credential: CredentialsEntity, + ): Promise { + const coreCredential = createCredentialsFromCredentialsEntity(credential); + return coreCredential.getData(encryptionKey); + } + + static async update( + credentialId: string, + newCredentialData: ICredentialsDb, + ): Promise { + 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. + return Db.collections.Credentials.findOne(credentialId); + } + + static async save( + credential: CredentialsEntity, + encryptedData: ICredentialsDb, + user: User, + ): Promise { + // To avoid side effects + const newCredential = new CredentialsEntity(); + Object.assign(newCredential, credential, 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; + } + + static async delete(credentials: CredentialsEntity): Promise { + await externalHooks.run('credentials.delete', [credentials.id]); + + await Db.collections.Credentials.remove(credentials); + } + + static async test( + user: User, + encryptionKey: string, + credentials: ICredentialsDecrypted, + nodeToTestWith: string | undefined, + ): Promise { + const helper = new CredentialsHelper(encryptionKey); + + return helper.testCredentials(user, credentials.type, credentials, nodeToTestWith); + } } diff --git a/packages/cli/test/integration/credentials.test.ts b/packages/cli/test/integration/credentials.test.ts index 28c9302d22b9e..6751e45c40d81 100644 --- a/packages/cli/test/integration/credentials.test.ts +++ b/packages/cli/test/integration/credentials.test.ts @@ -61,7 +61,10 @@ 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); @@ -218,7 +221,11 @@ 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); @@ -252,7 +259,12 @@ 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); @@ -285,7 +297,12 @@ 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);