From ecabe34705bbbba07613ba14760449ef38e1b31f Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Tue, 7 Jan 2025 11:17:26 +0100 Subject: [PATCH] fix(core): Return unredacted credentials from `GET credentials/:id` (#12447) --- .../src/credentials/credentials.service.ee.ts | 2 +- .../cli/src/credentials/credentials.service.ts | 2 +- .../credentials/credentials.api.ee.test.ts | 17 +++++++++++++++++ .../credentials/credentials.api.test.ts | 18 ++++++++++++++++++ 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index c53110666b..b183e20431 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -87,7 +87,7 @@ export class EnterpriseCredentialsService { if (credential) { // Decrypt the data if we found the credential with the `credential:update` // scope. - decryptedData = this.credentialsService.decrypt(credential); + decryptedData = this.credentialsService.decrypt(credential, true); } else { // Otherwise try to find them with only the `credential:read` scope. In // that case we return them without the decrypted data. diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 0761a4ad48..e0939beccb 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -542,7 +542,7 @@ export class CredentialsService { if (sharing) { // Decrypt the data if we found the credential with the `credential:update` // scope. - decryptedData = this.decrypt(sharing.credentials); + decryptedData = this.decrypt(sharing.credentials, true); } else { // Otherwise try to find them with only the `credential:read` scope. In // that case we return them without the decrypted data. diff --git a/packages/cli/test/integration/credentials/credentials.api.ee.test.ts b/packages/cli/test/integration/credentials/credentials.api.ee.test.ts index 039791eaf2..090de90603 100644 --- a/packages/cli/test/integration/credentials/credentials.api.ee.test.ts +++ b/packages/cli/test/integration/credentials/credentials.api.ee.test.ts @@ -2,6 +2,7 @@ import { Container } from '@n8n/di'; import { In } from '@n8n/typeorm'; import config from '@/config'; +import { CredentialsService } from '@/credentials/credentials.service'; import type { Project } from '@/databases/entities/project'; import type { ProjectRole } from '@/databases/entities/project-relation'; import type { User } from '@/databases/entities/user'; @@ -555,6 +556,22 @@ describe('GET /credentials/:id', () => { expect(secondCredential.data).toBeDefined(); }); + test('should not redact the data when `includeData:true` is passed', async () => { + const credentialService = Container.get(CredentialsService); + const redactSpy = jest.spyOn(credentialService, 'redact'); + const savedCredential = await saveCredential(randomCredentialPayload(), { + user: owner, + }); + + const response = await authOwnerAgent + .get(`/credentials/${savedCredential.id}`) + .query({ includeData: true }); + + validateMainCredentialData(response.body.data); + expect(response.body.data.data).toBeDefined(); + expect(redactSpy).not.toHaveBeenCalled(); + }); + test('should retrieve non-owned cred for owner', async () => { const [member1, member2] = await createManyUsers(2, { role: 'global:member', diff --git a/packages/cli/test/integration/credentials/credentials.api.test.ts b/packages/cli/test/integration/credentials/credentials.api.test.ts index 9cfe3260ba..7bb0a8918a 100644 --- a/packages/cli/test/integration/credentials/credentials.api.test.ts +++ b/packages/cli/test/integration/credentials/credentials.api.test.ts @@ -4,6 +4,7 @@ import type { Scope } from '@sentry/node'; import { Credentials } from 'n8n-core'; import { randomString } from 'n8n-workflow'; +import { CredentialsService } from '@/credentials/credentials.service'; import type { Project } from '@/databases/entities/project'; import type { User } from '@/databases/entities/user'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; @@ -1272,6 +1273,23 @@ describe('GET /credentials/:id', () => { expect(secondResponse.body.data.data).toBeDefined(); }); + test('should not redact the data when `includeData:true` is passed', async () => { + const credentialService = Container.get(CredentialsService); + const redactSpy = jest.spyOn(credentialService, 'redact'); + const savedCredential = await saveCredential(randomCredentialPayload(), { + user: owner, + role: 'credential:owner', + }); + + const response = await authOwnerAgent + .get(`/credentials/${savedCredential.id}`) + .query({ includeData: true }); + + validateMainCredentialData(response.body.data); + expect(response.body.data.data).toBeDefined(); + expect(redactSpy).not.toHaveBeenCalled(); + }); + test('should retrieve owned cred for member', async () => { const savedCredential = await saveCredential(randomCredentialPayload(), { user: member,