don't save blanks in the db when updating a credential with oauthTokenData

This commit is contained in:
Danny Martini 2025-01-10 23:57:07 +01:00
parent be62cab526
commit e1822302f3
No known key found for this signature in database
2 changed files with 69 additions and 1 deletions

View file

@ -198,7 +198,7 @@ export class CredentialsController {
throw new BadRequestError('Managed credentials cannot be updated'); 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( const preparedCredentialData = await this.credentialsService.prepareUpdateData(
req.body, req.body,
decryptedData, decryptedData,

View file

@ -4,6 +4,7 @@ import type { Scope } from '@sentry/node';
import { Credentials } from 'n8n-core'; import { Credentials } from 'n8n-core';
import { randomString } from 'n8n-workflow'; import { randomString } from 'n8n-workflow';
import { CREDENTIAL_BLANKING_VALUE } from '@/constants';
import { CredentialsService } from '@/credentials/credentials.service'; import { CredentialsService } from '@/credentials/credentials.service';
import type { Project } from '@/databases/entities/project'; import type { Project } from '@/databases/entities/project';
import type { User } from '@/databases/entities/user'; import type { User } from '@/databases/entities/user';
@ -1164,6 +1165,73 @@ describe('PATCH /credentials/:id', () => {
expect(shellCredential.name).toBe(patchPayload.name); // updated 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 () => { test('should fail with invalid inputs', async () => {
const savedCredential = await saveCredential(randomCredentialPayload(), { const savedCredential = await saveCredential(randomCredentialPayload(), {
user: owner, user: owner,