From b350568505d48ec880fe98d2b62ef090d5399c5f Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Tue, 31 Oct 2023 13:15:09 +0100 Subject: [PATCH] fix(core): Fix data decryption on credentials import (#7560) Due to a change, during the credentials import command, the core's Credential object is being called through its prototype. This caused the Credential's cipher variable to not be set, thus no cipher service being available during import. This fix catches this edge case and provides a fix. --- .../cli/src/commands/import/credentials.ts | 14 +++--- .../commands/credentials.cmd.test.ts | 47 +++++++++++++++++++ .../importCredentials/credentials.json | 15 ++++++ .../cli/test/integration/shared/testDb.ts | 4 ++ 4 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 packages/cli/test/integration/commands/credentials.cmd.test.ts create mode 100644 packages/cli/test/integration/commands/importCredentials/credentials.json diff --git a/packages/cli/src/commands/import/credentials.ts b/packages/cli/src/commands/import/credentials.ts index 81f724dda4..5e97e000df 100644 --- a/packages/cli/src/commands/import/credentials.ts +++ b/packages/cli/src/commands/import/credentials.ts @@ -1,5 +1,5 @@ import { flags } from '@oclif/command'; -import { Credentials } from 'n8n-core'; +import { Cipher } from 'n8n-core'; import fs from 'fs'; import glob from 'fast-glob'; import { Container } from 'typedi'; @@ -69,6 +69,7 @@ export class ImportCredentialsCommand extends BaseCommand { let totalImported = 0; + const cipher = Container.get(Cipher); await this.initOwnerCredentialRole(); const user = flags.userId ? await this.getAssignee(flags.userId) : await this.getOwner(); @@ -92,12 +93,10 @@ export class ImportCredentialsCommand extends BaseCommand { const credential = jsonParse( fs.readFileSync(file, { encoding: 'utf8' }), ); - if (typeof credential.data === 'object') { // plain data / decrypted input. Should be encrypted first. - Credentials.prototype.setData.call(credential, credential.data); + credential.data = cipher.encrypt(credential.data); } - await this.storeCredential(credential, user); } }); @@ -123,7 +122,7 @@ export class ImportCredentialsCommand extends BaseCommand { for (const credential of credentials) { if (typeof credential.data === 'object') { // plain data / decrypted input. Should be encrypted first. - Credentials.prototype.setData.call(credential, credential.data); + credential.data = cipher.encrypt(credential.data); } await this.storeCredential(credential, user); } @@ -155,7 +154,10 @@ export class ImportCredentialsCommand extends BaseCommand { this.ownerCredentialRole = ownerCredentialRole; } - private async storeCredential(credential: object, user: User) { + private async storeCredential(credential: Partial, user: User) { + if (!credential.nodesAccess) { + credential.nodesAccess = []; + } const result = await this.transactionManager.upsert(CredentialsEntity, credential, ['id']); await this.transactionManager.upsert( SharedCredentials, diff --git a/packages/cli/test/integration/commands/credentials.cmd.test.ts b/packages/cli/test/integration/commands/credentials.cmd.test.ts new file mode 100644 index 0000000000..d7ed10c60e --- /dev/null +++ b/packages/cli/test/integration/commands/credentials.cmd.test.ts @@ -0,0 +1,47 @@ +import * as Config from '@oclif/config'; + +import { InternalHooks } from '@/InternalHooks'; +import { ImportCredentialsCommand } from '@/commands/import/credentials'; +import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; +import * as testDb from '../shared/testDb'; +import { mockInstance } from '../shared/utils'; + +beforeAll(async () => { + mockInstance(InternalHooks); + mockInstance(LoadNodesAndCredentials); + await testDb.init(); +}); + +beforeEach(async () => { + await testDb.truncate(['Credentials']); +}); + +afterAll(async () => { + await testDb.terminate(); +}); + +test('import:credentials should import a credential', async () => { + const config: Config.IConfig = new Config.Config({ root: __dirname }); + const before = await testDb.getAllCredentials(); + expect(before.length).toBe(0); + const importer = new ImportCredentialsCommand( + ['--input=./test/integration/commands/importCredentials/credentials.json'], + config, + ); + const mockExit = jest.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit'); + }); + + await importer.init(); + try { + await importer.run(); + } catch (error) { + expect(error.message).toBe('process.exit'); + } + const after = await testDb.getAllCredentials(); + expect(after.length).toBe(1); + expect(after[0].name).toBe('cred-aws-test'); + expect(after[0].id).toBe('123'); + expect(after[0].nodesAccess).toStrictEqual([]); + mockExit.mockRestore(); +}); diff --git a/packages/cli/test/integration/commands/importCredentials/credentials.json b/packages/cli/test/integration/commands/importCredentials/credentials.json new file mode 100644 index 0000000000..136a2205b6 --- /dev/null +++ b/packages/cli/test/integration/commands/importCredentials/credentials.json @@ -0,0 +1,15 @@ +[ + { + "createdAt": "2023-07-10T14:50:49.193Z", + "updatedAt": "2023-10-27T13:34:42.917Z", + "id": "123", + "name": "cred-aws-test", + "data": { + "region": "eu-west-1", + "accessKeyId": "999999999999", + "secretAccessKey": "aaaaaaaaaaaaa" + }, + "type": "aws", + "nodesAccess": "" + } +] diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index 7187a67fb1..9ef0e00c0f 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -181,6 +181,10 @@ export function affixRoleToSaveCredential(role: Role) { saveCredential(credentialPayload, { user, role }); } +export async function getAllCredentials() { + return Db.collections.Credentials.find(); +} + // ---------------------------------- // user creation // ----------------------------------