fix(core): Handle credential decryption failures gracefully on the API (#13166)

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™ 2025-02-10 13:40:31 +01:00 committed by GitHub
parent 5d05f7f436
commit a4c5334853
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 67 additions and 35 deletions

View file

@ -1,6 +1,6 @@
import { mock } from 'jest-mock-extended';
import { nanoId, date } from 'minifaker';
import { Credentials } from 'n8n-core';
import { CREDENTIAL_ERRORS, CredentialDataError, Credentials, type ErrorReporter } from 'n8n-core';
import { CREDENTIAL_EMPTY_VALUE, type ICredentialType } from 'n8n-workflow';
import { CREDENTIAL_BLANKING_VALUE } from '@/constants';
@ -32,12 +32,14 @@ describe('CredentialsService', () => {
],
});
const errorReporter = mock<ErrorReporter>();
const credentialTypes = mock<CredentialTypes>();
const service = new CredentialsService(
mock(),
mock(),
mock(),
mock(),
errorReporter,
mock(),
mock(),
credentialTypes,
@ -47,6 +49,8 @@ describe('CredentialsService', () => {
mock(),
);
beforeEach(() => jest.resetAllMocks());
describe('redact', () => {
it('should redact sensitive values', () => {
const credential = mock<CredentialsEntity>({
@ -141,29 +145,34 @@ describe('CredentialsService', () => {
});
describe('decrypt', () => {
const data = {
clientId: 'abc123',
clientSecret: 'sensitiveSecret',
accessToken: '',
oauthTokenData: 'super-secret',
csrfSecret: 'super-secret',
};
const credentialEntity = mock<CredentialsEntity>({
id: '123',
name: 'Test Credential',
type: 'oauth2',
});
const credentials = mock<Credentials>({ id: credentialEntity.id });
beforeEach(() => {
credentialTypes.getByName.calledWith(credentialEntity.type).mockReturnValueOnce(credType);
});
it('should redact sensitive values by default', () => {
// ARRANGE
const data = {
clientId: 'abc123',
clientSecret: 'sensitiveSecret',
accessToken: '',
oauthTokenData: 'super-secret',
csrfSecret: 'super-secret',
};
const credential = mock<CredentialsEntity>({
id: '123',
name: 'Test Credential',
type: 'oauth2',
});
jest.spyOn(Credentials.prototype, 'getData').mockReturnValueOnce(data);
credentialTypes.getByName.calledWith(credential.type).mockReturnValueOnce(credType);
// ACT
const redactedData = service.decrypt(credential);
const redactedData = service.decrypt(credentialEntity);
// ASSERT
expect(redactedData).toEqual({
clientId: 'abc123',
...data,
clientSecret: CREDENTIAL_BLANKING_VALUE,
accessToken: CREDENTIAL_EMPTY_VALUE,
oauthTokenData: CREDENTIAL_BLANKING_VALUE,
@ -173,26 +182,36 @@ describe('CredentialsService', () => {
it('should return sensitive values if `includeRawData` is true', () => {
// ARRANGE
const data = {
clientId: 'abc123',
clientSecret: 'sensitiveSecret',
accessToken: '',
oauthTokenData: 'super-secret',
csrfSecret: 'super-secret',
};
const credential = mock<CredentialsEntity>({
id: '123',
name: 'Test Credential',
type: 'oauth2',
});
jest.spyOn(Credentials.prototype, 'getData').mockReturnValueOnce(data);
credentialTypes.getByName.calledWith(credential.type).mockReturnValueOnce(credType);
// ACT
const redactedData = service.decrypt(credential, true);
const redactedData = service.decrypt(credentialEntity, true);
// ASSERT
expect(redactedData).toEqual(data);
});
it('should return return an empty object if decryption fails', () => {
// ARRANGE
const decryptionError = new CredentialDataError(
credentials,
CREDENTIAL_ERRORS.DECRYPTION_FAILED,
);
jest.spyOn(Credentials.prototype, 'getData').mockImplementation(() => {
throw decryptionError;
});
// ACT
const redactedData = service.decrypt(credentialEntity, true);
// ASSERT
expect(redactedData).toEqual({});
expect(credentialTypes.getByName).not.toHaveBeenCalled();
expect(errorReporter.error).toHaveBeenCalledWith(decryptionError, {
extra: { credentialId: credentialEntity.id },
level: 'error',
tags: { credentialType: credentialEntity.type },
});
});
});
});

View file

@ -7,7 +7,7 @@ import {
type FindOptionsRelations,
type FindOptionsWhere,
} from '@n8n/typeorm';
import { Credentials, Logger } from 'n8n-core';
import { CredentialDataError, Credentials, ErrorReporter, Logger } from 'n8n-core';
import type {
ICredentialDataDecryptedObject,
ICredentialsDecrypted,
@ -51,6 +51,7 @@ export class CredentialsService {
private readonly sharedCredentialsRepository: SharedCredentialsRepository,
private readonly ownershipService: OwnershipService,
private readonly logger: Logger,
private readonly errorReporter: ErrorReporter,
private readonly credentialsTester: CredentialsTester,
private readonly externalHooks: ExternalHooks,
private readonly credentialTypes: CredentialTypes,
@ -330,11 +331,23 @@ export class CredentialsService {
*/
decrypt(credential: CredentialsEntity, includeRawData = false) {
const coreCredential = createCredentialsFromCredentialsEntity(credential);
const data = coreCredential.getData();
if (includeRawData) {
return data;
try {
const data = coreCredential.getData();
if (includeRawData) {
return data;
}
return this.redact(data, credential);
} catch (error) {
if (error instanceof CredentialDataError) {
this.errorReporter.error(error, {
level: 'error',
extra: { credentialId: credential.id },
tags: { credentialType: credential.type },
});
return {};
}
throw error;
}
return this.redact(data, credential);
}
async update(credentialId: string, newCredentialData: ICredentialsDb) {