From e1822302f348e4d28be0f41b2f2114caab1b97cf Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Fri, 10 Jan 2025 23:57:07 +0100 Subject: [PATCH] don't save blanks in the db when updating a credential with oauthTokenData --- .../src/credentials/credentials.controller.ts | 2 +- .../credentials/credentials.api.test.ts | 68 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 4cc0b500f2..73888e1977 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -198,7 +198,7 @@ export class CredentialsController { throw new BadRequestError('Managed credentials cannot be updated'); } - const decryptedData = this.credentialsService.decrypt(credential); + const decryptedData = this.credentialsService.decrypt(credential, true); const preparedCredentialData = await this.credentialsService.prepareUpdateData( req.body, decryptedData, diff --git a/packages/cli/test/integration/credentials/credentials.api.test.ts b/packages/cli/test/integration/credentials/credentials.api.test.ts index a988569890..ece8340e83 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 { CREDENTIAL_BLANKING_VALUE } from '@/constants'; import { CredentialsService } from '@/credentials/credentials.service'; import type { Project } from '@/databases/entities/project'; import type { User } from '@/databases/entities/user'; @@ -1164,6 +1165,73 @@ describe('PATCH /credentials/:id', () => { expect(shellCredential.name).toBe(patchPayload.name); // updated }); + test('should not store redacted value in the db for oauthTokenData', async () => { + // ARRANGE + const credentialService = Container.get(CredentialsService); + const redactSpy = jest.spyOn(credentialService, 'redact').mockReturnValueOnce({ + accessToken: CREDENTIAL_BLANKING_VALUE, + oauthTokenData: CREDENTIAL_BLANKING_VALUE, + }); + + const payload = randomCredentialPayload(); + payload.data.oauthTokenData = { tokenData: true }; + const savedCredential = await saveCredential(payload, { + user: owner, + role: 'credential:owner', + }); + + // ACT + const patchPayload = { ...payload, data: { foo: 'bar' } }; + await authOwnerAgent.patch(`/credentials/${savedCredential.id}`).send(patchPayload).expect(200); + + // ASSERT + const response = await authOwnerAgent + .get(`/credentials/${savedCredential.id}`) + .query({ includeData: true }) + .expect(200); + + const { id, data } = response.body.data; + + expect(id).toBe(savedCredential.id); + expect(data).toEqual({ + ...patchPayload.data, + // should be the original + oauthTokenData: payload.data.oauthTokenData, + }); + expect(redactSpy).not.toHaveBeenCalled(); + }); + + test('should not allow to overwrite oauthTokenData', async () => { + // ARRANGE + const payload = randomCredentialPayload(); + payload.data.oauthTokenData = { tokenData: true }; + const savedCredential = await saveCredential(payload, { + user: owner, + role: 'credential:owner', + }); + + // ACT + const patchPayload = { + ...payload, + data: { accessToken: 'new', oauthTokenData: { tokenData: false } }, + }; + await authOwnerAgent.patch(`/credentials/${savedCredential.id}`).send(patchPayload).expect(200); + + // ASSERT + const response = await authOwnerAgent + .get(`/credentials/${savedCredential.id}`) + .query({ includeData: true }) + .expect(200); + + const { id, data } = response.body.data; + + expect(id).toBe(savedCredential.id); + // was overwritten + expect(data.accessToken).toBe(patchPayload.data.accessToken); + // was not overwritten + expect(data.oauthTokenData).toEqual(payload.data.oauthTokenData); + }); + test('should fail with invalid inputs', async () => { const savedCredential = await saveCredential(randomCredentialPayload(), { user: owner,