fix(core): Return unredacted credentials from GET credentials/:id (#12447)
Some checks are pending
Test Master / install-and-build (push) Waiting to run
Test Master / Unit tests (18.x) (push) Blocked by required conditions
Test Master / Unit tests (20.x) (push) Blocked by required conditions
Test Master / Unit tests (22.4) (push) Blocked by required conditions
Test Master / Lint (push) Blocked by required conditions
Test Master / Notify Slack on failure (push) Blocked by required conditions

This commit is contained in:
Danny Martini 2025-01-07 11:17:26 +01:00 committed by GitHub
parent 7df5eb1e4d
commit ecabe34705
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 37 additions and 2 deletions

View file

@ -87,7 +87,7 @@ export class EnterpriseCredentialsService {
if (credential) { if (credential) {
// Decrypt the data if we found the credential with the `credential:update` // Decrypt the data if we found the credential with the `credential:update`
// scope. // scope.
decryptedData = this.credentialsService.decrypt(credential); decryptedData = this.credentialsService.decrypt(credential, true);
} else { } else {
// Otherwise try to find them with only the `credential:read` scope. In // Otherwise try to find them with only the `credential:read` scope. In
// that case we return them without the decrypted data. // that case we return them without the decrypted data.

View file

@ -542,7 +542,7 @@ export class CredentialsService {
if (sharing) { if (sharing) {
// Decrypt the data if we found the credential with the `credential:update` // Decrypt the data if we found the credential with the `credential:update`
// scope. // scope.
decryptedData = this.decrypt(sharing.credentials); decryptedData = this.decrypt(sharing.credentials, true);
} else { } else {
// Otherwise try to find them with only the `credential:read` scope. In // Otherwise try to find them with only the `credential:read` scope. In
// that case we return them without the decrypted data. // that case we return them without the decrypted data.

View file

@ -2,6 +2,7 @@ import { Container } from '@n8n/di';
import { In } from '@n8n/typeorm'; import { In } from '@n8n/typeorm';
import config from '@/config'; import config from '@/config';
import { CredentialsService } from '@/credentials/credentials.service';
import type { Project } from '@/databases/entities/project'; import type { Project } from '@/databases/entities/project';
import type { ProjectRole } from '@/databases/entities/project-relation'; import type { ProjectRole } from '@/databases/entities/project-relation';
import type { User } from '@/databases/entities/user'; import type { User } from '@/databases/entities/user';
@ -555,6 +556,22 @@ describe('GET /credentials/:id', () => {
expect(secondCredential.data).toBeDefined(); 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 () => { test('should retrieve non-owned cred for owner', async () => {
const [member1, member2] = await createManyUsers(2, { const [member1, member2] = await createManyUsers(2, {
role: 'global:member', role: 'global:member',

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 { 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';
import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository';
@ -1272,6 +1273,23 @@ describe('GET /credentials/:id', () => {
expect(secondResponse.body.data.data).toBeDefined(); 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 () => { test('should retrieve owned cred for member', async () => {
const savedCredential = await saveCredential(randomCredentialPayload(), { const savedCredential = await saveCredential(randomCredentialPayload(), {
user: member, user: member,