feat(core): Introduce CredentialDependency entity to track credential dependencies (#27151)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Irénée <irenee.ajeneza@n8n.io>
This commit is contained in:
Ali Elkhateeb
2026-03-23 14:37:09 +00:00
committed by GitHub
co-authored by Claude Sonnet 4.6 Irénée
parent d2da928429
commit 835094c34e
30 changed files with 1990 additions and 410 deletions
@@ -0,0 +1,37 @@
import {
Column,
Entity,
Index,
JoinColumn,
ManyToOne,
PrimaryGeneratedColumn,
Relation,
Unique,
} from '@n8n/typeorm';
import { WithCreatedAt } from './abstract-entity';
import type { CredentialsEntity } from './credentials-entity';
export type CredentialDependencyType = 'externalSecretProvider';
@Entity({ name: 'credential_dependency' })
@Index(['dependencyType', 'dependencyId'])
@Unique(['credentialId', 'dependencyType', 'dependencyId'])
export class CredentialDependency extends WithCreatedAt {
@PrimaryGeneratedColumn()
id: number;
@Column({ length: 36 })
@Index()
credentialId: string;
@Column({ length: 64 })
dependencyType: CredentialDependencyType;
@Column({ length: 255 })
dependencyId: string;
@ManyToOne('CredentialsEntity', { onDelete: 'CASCADE' })
@JoinColumn({ name: 'credentialId' })
credential: Relation<CredentialsEntity>;
}
+7
View File
@@ -4,6 +4,10 @@ import { ApiKey } from './api-key';
import { AuthIdentity } from './auth-identity';
import { AuthProviderSyncHistory } from './auth-provider-sync-history';
import { BinaryDataFile, SourceTypeSchema, type SourceType } from './binary-data-file';
import {
CredentialDependency,
type CredentialDependencyType,
} from './credential-dependency-entity';
import { CredentialsEntity } from './credentials-entity';
import { ExecutionAnnotation } from './execution-annotation.ee';
import { ExecutionData } from './execution-data';
@@ -51,6 +55,8 @@ export {
WebhookEntity,
AuthIdentity,
CredentialsEntity,
CredentialDependency,
type CredentialDependencyType,
Folder,
Project,
ProjectRelation,
@@ -92,6 +98,7 @@ export const entities = {
WebhookEntity,
AuthIdentity,
CredentialsEntity,
CredentialDependency,
Folder,
Project,
ProjectRelation,
@@ -0,0 +1,203 @@
import { Container } from '@n8n/di';
import { Cipher } from 'n8n-core';
import type { MigrationContext, ReversibleMigration } from '../migration-types';
const credentialDependencyTable = 'credential_dependency';
const externalSecretProviderDependencyType = 'externalSecretProvider';
const providerKeyPattern = '[A-Za-z0-9]+(?:-[A-Za-z0-9]+)*';
type CredentialRow = {
id: string;
data: string;
};
export class CreateCredentialDependencyTable1773000000000 implements ReversibleMigration {
private readonly cipher = Container.get(Cipher);
async up({
schemaBuilder: { createTable, column, createIndex },
escape,
runQuery,
runInBatches,
logger,
migrationName,
}: MigrationContext) {
await createTable(credentialDependencyTable)
.withColumns(
column('id').int.primary.autoGenerate2,
column('credentialId').varchar(36).notNull,
column('dependencyType').varchar(64).notNull,
column('dependencyId').varchar(255).notNull,
)
.withForeignKey('credentialId', {
tableName: 'credentials_entity',
columnName: 'id',
onDelete: 'CASCADE',
})
.withIndexOn(['credentialId'])
.withIndexOn(['dependencyType', 'dependencyId']).withCreatedAt;
await createIndex(
credentialDependencyTable,
['credentialId', 'dependencyType', 'dependencyId'],
true,
);
const credentialsTable = escape.tableName('credentials_entity');
const query = `SELECT c.id AS id, c.data AS data FROM ${credentialsTable} c ORDER BY c.id`;
const providerIdByKey = await this.loadProviderIdByKey(runQuery, escape);
let processedCount = 0;
let insertedCount = 0;
await runInBatches<CredentialRow>(query, async (rows) => {
if (rows.length === 0) return;
const batchDependencies = rows.flatMap((row) => {
const providerKeys = this.extractProviderKeysFromCredentialData(row.data);
const providerIds = providerKeys
.map((providerKey) => providerIdByKey.get(providerKey))
.filter((providerId): providerId is string => providerId !== undefined);
return providerIds.map((providerId) => ({
credentialId: row.id,
dependencyType: externalSecretProviderDependencyType,
dependencyId: providerId,
}));
});
processedCount += rows.length;
if (batchDependencies.length === 0) {
return;
}
await this.insertDependencies(batchDependencies, escape, runQuery);
insertedCount += batchDependencies.length;
});
logger.info(
`[${migrationName}] Backfilled credential dependencies for ${processedCount} credentials. Inserted ${insertedCount} dependencies.`,
);
}
async down({ schemaBuilder: { dropTable } }: MigrationContext) {
await dropTable(credentialDependencyTable);
}
private extractProviderKeysFromCredentialData(encryptedCredentialData: string): string[] {
const decrypted = this.tryDecryptCredentialData(encryptedCredentialData);
if (decrypted === null) return [];
return this.extractProviderKeysFromDecryptedData(decrypted);
}
private extractProviderKeysFromDecryptedData(decryptedCredentialData: unknown): string[] {
const uniqueKeys = new Set<string>();
const valuesToScan: unknown[] = [decryptedCredentialData];
while (valuesToScan.length > 0) {
const currentValue = valuesToScan.pop();
if (typeof currentValue === 'string') {
if (!currentValue.includes('$secrets')) continue;
for (const dependencyKey of this.extractProviderKeys(currentValue)) {
uniqueKeys.add(dependencyKey);
}
continue;
}
if (Array.isArray(currentValue)) {
for (const value of currentValue as unknown[]) {
valuesToScan.push(value);
}
continue;
}
if (typeof currentValue === 'object' && currentValue !== null) {
for (const value of Object.values(currentValue as Record<string, unknown>)) {
valuesToScan.push(value);
}
}
}
return [...uniqueKeys];
}
private tryDecryptCredentialData(encryptedCredentialData: string): unknown {
try {
const decrypted = this.cipher.decrypt(encryptedCredentialData);
return JSON.parse(decrypted) as unknown;
} catch {
return null;
}
}
private extractProviderKeys(expression: string): string[] {
const providerKeys = new Set<string>();
const expressionBlocks = expression.matchAll(/\{\{(.*?)\}\}/gs);
for (const block of expressionBlocks) {
const expressionContent = block[1];
const dotMatches = expressionContent.matchAll(
new RegExp(`\\$secrets\\.(${providerKeyPattern})`, 'g'),
);
for (const match of dotMatches) {
providerKeys.add(match[1]);
}
const bracketMatches = expressionContent.matchAll(
new RegExp(`\\$secrets\\[['"](${providerKeyPattern})['"]\\]`, 'g'),
);
for (const match of bracketMatches) {
providerKeys.add(match[1]);
}
}
return [...providerKeys];
}
private async insertDependencies(
dependencies: Array<{
credentialId: string;
dependencyType: string;
dependencyId: string;
}>,
escape: MigrationContext['escape'],
runQuery: MigrationContext['runQuery'],
) {
const dependencyTable = escape.tableName(credentialDependencyTable);
const credentialIdColumn = escape.columnName('credentialId');
const dependencyTypeColumn = escape.columnName('dependencyType');
const dependencyIdColumn = escape.columnName('dependencyId');
const namedParameters: Record<string, string> = {};
const valuesSql = dependencies
.map((dependency, index) => {
namedParameters[`credentialId${index}`] = dependency.credentialId;
namedParameters[`dependencyType${index}`] = dependency.dependencyType;
namedParameters[`dependencyId${index}`] = dependency.dependencyId;
return `(:credentialId${index}, :dependencyType${index}, :dependencyId${index})`;
})
.join(', ');
await runQuery(
`INSERT INTO ${dependencyTable} (${credentialIdColumn}, ${dependencyTypeColumn}, ${dependencyIdColumn}) VALUES ${valuesSql} ON CONFLICT (${credentialIdColumn}, ${dependencyTypeColumn}, ${dependencyIdColumn}) DO NOTHING;`,
namedParameters,
);
}
private async loadProviderIdByKey(
runQuery: MigrationContext['runQuery'],
escape: MigrationContext['escape'],
): Promise<Map<string, string>> {
const providerTable = escape.tableName('secrets_provider_connection');
const idColumn = escape.columnName('id');
const providerKeyColumn = escape.columnName('providerKey');
const rows = await runQuery<Array<{ id: number; providerKey: string }>>(
`SELECT ${idColumn} AS ${idColumn}, ${providerKeyColumn} AS ${providerKeyColumn} FROM ${providerTable}`,
);
return new Map(rows.map(({ id, providerKey }) => [providerKey, id.toString()]));
}
}
@@ -152,6 +152,7 @@ import { AddSuggestedPromptsToAgentTable1772000000000 } from '../common/17720000
import { AddRoleColumnToProjectSecretsProviderAccess1772619247761 } from '../common/1772619247761-AddRoleColumnToProjectSecretsProviderAccess';
import { ChangeWorkflowPublishedVersionFKsToRestrict1772619247762 } from '../common/1772619247762-ChangeWorkflowPublishedVersionFKsToRestrict';
import { AddTypeToChatHubSessions1772700000000 } from '../common/1772700000000-AddTypeToChatHubSessions';
import { CreateCredentialDependencyTable1773000000000 } from '../common/1773000000000-CreateCredentialDependencyTable';
import type { Migration } from '../migration-types';
export const postgresMigrations: Migration[] = [
@@ -309,4 +310,5 @@ export const postgresMigrations: Migration[] = [
AddRoleColumnToProjectSecretsProviderAccess1772619247761,
ChangeWorkflowPublishedVersionFKsToRestrict1772619247762,
AddTypeToChatHubSessions1772700000000,
CreateCredentialDependencyTable1773000000000,
];
@@ -146,6 +146,7 @@ import { AddSuggestedPromptsToAgentTable1772000000000 } from '../common/17720000
import { AddRoleColumnToProjectSecretsProviderAccess1772619247761 } from '../common/1772619247761-AddRoleColumnToProjectSecretsProviderAccess';
import { ChangeWorkflowPublishedVersionFKsToRestrict1772619247762 } from '../common/1772619247762-ChangeWorkflowPublishedVersionFKsToRestrict';
import { AddTypeToChatHubSessions1772700000000 } from '../common/1772700000000-AddTypeToChatHubSessions';
import { CreateCredentialDependencyTable1773000000000 } from '../common/1773000000000-CreateCredentialDependencyTable';
import type { Migration } from '../migration-types';
const sqliteMigrations: Migration[] = [
@@ -297,6 +298,7 @@ const sqliteMigrations: Migration[] = [
AddRoleColumnToProjectSecretsProviderAccess1772619247761,
ChangeWorkflowPublishedVersionFKsToRestrict1772619247762,
AddTypeToChatHubSessions1772700000000,
CreateCredentialDependencyTable1773000000000,
];
export { sqliteMigrations };
@@ -0,0 +1,176 @@
import { Container } from '@n8n/di';
import { In } from '@n8n/typeorm';
import { CredentialDependency } from '../../entities';
import { mockEntityManager } from '../../utils/test-utils/mock-entity-manager';
import {
addCredentialDependencyExistsFilter,
CredentialDependencyRepository,
} from '../credential-dependency.repository';
describe('CredentialDependencyRepository', () => {
const entityManager = mockEntityManager(CredentialDependency);
const repository = Container.get(CredentialDependencyRepository);
beforeEach(() => {
jest.resetAllMocks();
});
describe('findCredentialIdsByDependencyId', () => {
it('returns matching credential ids', async () => {
entityManager.find.mockResolvedValueOnce([
{ credentialId: 'cred-1' } as CredentialDependency,
{ credentialId: 'cred-2' } as CredentialDependency,
]);
const result = await repository.findCredentialIdsByDependencyId(
'externalSecretProvider',
'provider-1',
);
expect(entityManager.find).toHaveBeenCalledWith(CredentialDependency, {
select: ['credentialId'],
where: { dependencyType: 'externalSecretProvider', dependencyId: 'provider-1' },
});
expect(result).toEqual(['cred-1', 'cred-2']);
});
});
describe('upsertDependenciesForCredential', () => {
it('deduplicates ids and inserts once with orIgnore', async () => {
const qb = {
insert: jest.fn().mockReturnThis(),
into: jest.fn().mockReturnThis(),
values: jest.fn().mockReturnThis(),
orIgnore: jest.fn().mockReturnThis(),
execute: jest.fn().mockResolvedValue(undefined),
};
entityManager.createQueryBuilder.mockReturnValue(qb as never);
await repository.upsertDependenciesForCredential({
credentialId: 'cred-1',
dependencyType: 'externalSecretProvider',
dependencyIds: ['provider-1', 'provider-1', 'provider-2'],
entityManager,
});
expect(qb.values).toHaveBeenCalledWith([
{
credentialId: 'cred-1',
dependencyType: 'externalSecretProvider',
dependencyId: 'provider-1',
},
{
credentialId: 'cred-1',
dependencyType: 'externalSecretProvider',
dependencyId: 'provider-2',
},
]);
expect(qb.orIgnore).toHaveBeenCalled();
expect(qb.execute).toHaveBeenCalled();
});
it('returns early when there is nothing to insert', async () => {
await repository.upsertDependenciesForCredential({
credentialId: 'cred-1',
dependencyType: 'externalSecretProvider',
dependencyIds: [],
entityManager,
});
expect(entityManager.createQueryBuilder).not.toHaveBeenCalled();
});
});
describe('syncDependenciesForCredential', () => {
it('deletes removed ids and inserts new ids', async () => {
entityManager.findBy.mockResolvedValueOnce([
{ dependencyId: 'provider-old' } as CredentialDependency,
{ dependencyId: 'provider-keep' } as CredentialDependency,
]);
const upsertSpy = jest
.spyOn(repository, 'upsertDependenciesForCredential')
.mockResolvedValue(undefined);
await repository.syncDependenciesForCredential({
credentialId: 'cred-1',
dependencyType: 'externalSecretProvider',
dependencyIds: ['provider-keep', 'provider-new'],
entityManager,
});
expect(entityManager.delete).toHaveBeenCalledWith(CredentialDependency, {
credentialId: 'cred-1',
dependencyType: 'externalSecretProvider',
dependencyId: In(['provider-old']),
});
expect(upsertSpy).toHaveBeenCalledWith({
credentialId: 'cred-1',
dependencyType: 'externalSecretProvider',
dependencyIds: ['provider-new'],
entityManager,
});
});
});
describe('deleteByDependency', () => {
it('deletes all dependency rows by type and id', async () => {
await repository.deleteByDependency({
dependencyType: 'externalSecretProvider',
dependencyId: 'provider-1',
entityManager,
});
expect(entityManager.delete).toHaveBeenCalledWith(CredentialDependency, {
dependencyType: 'externalSecretProvider',
dependencyId: 'provider-1',
});
});
});
describe('deleteByDependencies', () => {
it('deletes all dependency rows by type and ids', async () => {
await repository.deleteByDependencies({
dependencyType: 'externalSecretProvider',
dependencyIds: ['provider-1', 'provider-2'],
entityManager,
});
expect(entityManager.delete).toHaveBeenCalledWith(CredentialDependency, {
dependencyType: 'externalSecretProvider',
dependencyId: In(['provider-1', 'provider-2']),
});
});
it('returns early when there is nothing to delete', async () => {
await repository.deleteByDependencies({
dependencyType: 'externalSecretProvider',
dependencyIds: [],
entityManager,
});
expect(entityManager.delete).not.toHaveBeenCalled();
});
});
});
describe('addCredentialDependencyExistsFilter', () => {
it('applies the EXISTS dependency filter using andWhere', () => {
const qb = {
andWhere: jest.fn().mockReturnThis(),
};
const filter = {
dependencyType: 'externalSecretProvider',
dependencyId: 'provider-1',
} as const;
const result = addCredentialDependencyExistsFilter(qb as never, filter);
expect(qb.andWhere).toHaveBeenCalledWith(
expect.stringContaining('FROM credential_dependency cd'),
filter,
);
expect(result).toBe(qb);
});
});
@@ -1,5 +1,7 @@
import { Container } from '@n8n/di';
import type { SelectQueryBuilder } from '@n8n/typeorm';
import { In } from '@n8n/typeorm';
import { mock } from 'jest-mock-extended';
import { CredentialsEntity } from '../../entities';
import { mockEntityManager } from '../../utils/test-utils/mock-entity-manager';
@@ -53,4 +55,35 @@ describe('CredentialsRepository', () => {
expect(callArg!.where).toEqual(expect.objectContaining({ id: In(['id1', 'id2']) }));
});
});
describe('findAllGlobalCredentials', () => {
it('applies dependency filter through query builder when provided', async () => {
const andWhereSpy = jest.fn().mockReturnThis();
const getManySpy = jest.fn().mockResolvedValue([]);
const qb = mock<SelectQueryBuilder<CredentialsEntity>>({
andWhere: andWhereSpy,
getMany: getManySpy,
});
jest.spyOn(credentialsRepository, 'createQueryBuilder').mockReturnValue(qb);
await credentialsRepository.findAllGlobalCredentials({
filters: {
dependency: {
dependencyType: 'externalSecretProvider',
dependencyId: 'provider-1',
},
},
});
expect(andWhereSpy).toHaveBeenCalledWith(
expect.stringContaining('FROM credential_dependency cd'),
{
dependencyType: 'externalSecretProvider',
dependencyId: 'provider-1',
},
);
expect(getManySpy).toHaveBeenCalledTimes(1);
expect(entityManager.find).not.toHaveBeenCalled();
});
});
});
@@ -9,26 +9,25 @@ import { SecretsProviderConnectionRepository } from '../secrets-provider-connect
describe('SecretsProviderConnectionRepository', () => {
const entityManager = mockEntityManager(SecretsProviderConnection);
const repository = Container.get(SecretsProviderConnectionRepository);
const createMockConnection = (
overrides: Partial<SecretsProviderConnection> = {},
): SecretsProviderConnection => {
return mock<SecretsProviderConnection>({
id: random(1, Number.MAX_SAFE_INTEGER),
providerKey: 'myVault',
type: 'vault',
encryptedSettings: '',
isEnabled: false,
projectAccess: [],
...overrides,
});
};
beforeEach(() => {
jest.resetAllMocks();
});
describe('findAll', () => {
const createMockConnection = (
overrides: Partial<SecretsProviderConnection> = {},
): SecretsProviderConnection => {
return mock<SecretsProviderConnection>({
id: random(1, Number.MAX_SAFE_INTEGER),
providerKey: 'myVault',
type: 'vault',
encryptedSettings: '',
isEnabled: false,
projectAccess: [],
...overrides,
});
};
it('should return all secrets provider connections', async () => {
const mockConnections = [createMockConnection(), createMockConnection()];
@@ -49,4 +48,49 @@ describe('SecretsProviderConnectionRepository', () => {
expect(result).toEqual([]);
});
});
describe('findIdByProviderKey', () => {
it('returns id when provider exists', async () => {
entityManager.findOne.mockResolvedValueOnce(createMockConnection({ id: 42 }));
const result = await repository.findIdByProviderKey('myVault');
expect(entityManager.findOne).toHaveBeenCalledWith(SecretsProviderConnection, {
select: ['id'],
where: { providerKey: 'myVault' },
});
expect(result).toBe('42');
});
it('returns null when provider does not exist', async () => {
entityManager.findOne.mockResolvedValueOnce(null);
const result = await repository.findIdByProviderKey('missing');
expect(result).toBeNull();
});
});
describe('findIdsByProviderKeys', () => {
it('returns ids as strings for matching providers', async () => {
entityManager.find.mockResolvedValueOnce([
createMockConnection({ id: 7 }),
createMockConnection({ id: 8 }),
] as SecretsProviderConnection[]);
const result = await repository.findIdsByProviderKeys(['vault-a', 'vault-b']);
expect(entityManager.find).toHaveBeenCalledWith(SecretsProviderConnection, {
select: ['id'],
where: { providerKey: expect.anything() },
});
expect(result).toEqual(['7', '8']);
});
it('returns empty list for empty input', async () => {
const result = await repository.findIdsByProviderKeys([]);
expect(result).toEqual([]);
expect(entityManager.find).not.toHaveBeenCalled();
});
});
});
@@ -0,0 +1,161 @@
import { Service } from '@n8n/di';
import {
DataSource,
In,
Repository,
type EntityManager,
type SelectQueryBuilder,
} from '@n8n/typeorm';
import {
CredentialDependency,
CredentialsEntity,
type CredentialDependencyType,
} from '../entities';
export type CredentialDependencyFilter = {
dependencyType: CredentialDependencyType;
dependencyId: string;
};
/**
* Apply dependency filter to a credential query.
* Expects outer query builder alias to be "credential".
*/
export function addCredentialDependencyExistsFilter(
qb: SelectQueryBuilder<CredentialsEntity>,
filter: CredentialDependencyFilter,
) {
return qb.andWhere(
`EXISTS (
SELECT 1
FROM credential_dependency cd
WHERE cd."credentialId" = credential.id
AND cd."dependencyType" = :dependencyType
AND cd."dependencyId" = :dependencyId
)`,
filter,
);
}
type DependencyMutationOptions = {
credentialId: string;
dependencyType: CredentialDependencyType;
dependencyIds: string[];
entityManager?: EntityManager;
};
type DeleteByDependencyOptions = {
dependencyType: CredentialDependencyType;
dependencyId: string;
entityManager?: EntityManager;
};
type DeleteByDependenciesOptions = {
dependencyType: CredentialDependencyType;
dependencyIds: string[];
entityManager?: EntityManager;
};
@Service()
export class CredentialDependencyRepository extends Repository<CredentialDependency> {
constructor(dataSource: DataSource) {
super(CredentialDependency, dataSource.manager);
}
async findCredentialIdsByDependencyId(
dependencyType: CredentialDependencyType,
dependencyId: string,
): Promise<string[]> {
const results = await this.find({
select: ['credentialId'],
where: { dependencyType, dependencyId },
});
return results.map((result) => result.credentialId);
}
async upsertDependenciesForCredential({
credentialId,
dependencyType,
dependencyIds,
entityManager = this.manager,
}: DependencyMutationOptions): Promise<void> {
if (dependencyIds.length === 0) return;
const uniqueDependencyIds = Array.from(new Set(dependencyIds));
await entityManager
.createQueryBuilder()
.insert()
.into(CredentialDependency)
.values(
uniqueDependencyIds.map((dependencyId) => ({
credentialId,
dependencyType,
dependencyId,
})),
)
.orIgnore()
.execute();
}
async syncDependenciesForCredential({
credentialId,
dependencyType,
dependencyIds,
entityManager = this.manager,
}: DependencyMutationOptions): Promise<void> {
const nextIds = new Set(dependencyIds);
const existing = await entityManager.findBy(CredentialDependency, {
credentialId,
dependencyType,
});
const existingIds = new Set(existing.map(({ dependencyId }) => dependencyId));
const idsToInsert = [...nextIds].filter((id) => !existingIds.has(id));
const idsToDelete = [...existingIds].filter((id) => !nextIds.has(id));
if (idsToDelete.length > 0) {
await entityManager.delete(CredentialDependency, {
credentialId,
dependencyType,
dependencyId: In(idsToDelete),
});
}
if (idsToInsert.length > 0) {
await this.upsertDependenciesForCredential({
credentialId,
dependencyType,
dependencyIds: idsToInsert,
entityManager,
});
}
}
async deleteByDependency({
dependencyType,
dependencyId,
entityManager = this.manager,
}: DeleteByDependencyOptions): Promise<void> {
await entityManager.delete(CredentialDependency, {
dependencyType,
dependencyId,
});
}
async deleteByDependencies({
dependencyType,
dependencyIds,
entityManager = this.manager,
}: DeleteByDependenciesOptions): Promise<void> {
if (dependencyIds.length === 0) return;
await entityManager.delete(CredentialDependency, {
dependencyType,
dependencyId: In(dependencyIds),
});
}
}
@@ -3,8 +3,11 @@ import type { Scope } from '@n8n/permissions';
import type { FindManyOptions, SelectQueryBuilder } from '@n8n/typeorm';
import { DataSource, In, Like, Repository } from '@n8n/typeorm';
import { CredentialsEntity } from '../entities';
import type { User } from '../entities';
import { CredentialsEntity, type User } from '../entities';
import {
addCredentialDependencyExistsFilter,
type CredentialDependencyFilter,
} from './credential-dependency.repository';
import { SharedCredentialsRepository } from './shared-credentials.repository';
import type { ListQuery } from '../entities/types-db';
@@ -185,14 +188,63 @@ export class CredentialsRepository extends Repository<CredentialsEntity> {
/**
* Find all global credentials
*/
async findAllGlobalCredentials(includeData = false): Promise<CredentialsEntity[]> {
async findAllGlobalCredentials(
options: {
includeData?: boolean;
filters?: {
dependency?: CredentialDependencyFilter;
};
} = {},
): Promise<CredentialsEntity[]> {
const { includeData = false, filters } = options;
const dependencyFilter = filters?.dependency;
if (dependencyFilter) {
return await this.findAllGlobalCredentialsByDependencyFilter({
dependencyFilter,
includeData,
});
}
const findManyOptions = this.toFindManyOptions({ includeData });
findManyOptions.where = { ...findManyOptions.where, isGlobal: true };
return await this.find(findManyOptions);
}
private async findAllGlobalCredentialsByDependencyFilter(options: {
dependencyFilter: CredentialDependencyFilter;
includeData?: boolean;
}): Promise<CredentialsEntity[]> {
const { includeData, dependencyFilter } = options;
const qb = this.createQueryBuilder('credential');
qb.where('credential.isGlobal = :isGlobal', { isGlobal: true });
addCredentialDependencyExistsFilter(qb, dependencyFilter);
const defaultSelect: Array<keyof CredentialsEntity> = [
'id',
'name',
'type',
'isManaged',
'createdAt',
'updatedAt',
'isGlobal',
'isResolvable',
'resolverId',
];
const selectColumns = defaultSelect.map((k) => `credential.${k}`);
if (includeData) {
selectColumns.push('credential.data');
}
qb.select(selectColumns);
qb.leftJoinAndSelect('credential.shared', 'shared');
qb.leftJoinAndSelect('shared.project', 'project');
qb.leftJoinAndSelect('project.projectRelations', 'projectRelations');
return await qb.getMany();
}
/**
* Find all credentials that are owned by a personal project.
*/
@@ -240,6 +292,9 @@ export class CredentialsRepository extends Repository<CredentialsEntity> {
options: ListQuery.Options & {
includeData?: boolean;
order?: FindManyOptions<CredentialsEntity>['order'];
filters?: {
dependency?: CredentialDependencyFilter;
};
} = {},
) {
const query = this.getManyQueryWithSharingSubquery(user, sharingOptions, options);
@@ -277,10 +332,17 @@ export class CredentialsRepository extends Repository<CredentialsEntity> {
options: ListQuery.Options & {
includeData?: boolean;
order?: FindManyOptions<CredentialsEntity>['order'];
filters?: {
dependency?: CredentialDependencyFilter;
};
} = {},
): SelectQueryBuilder<CredentialsEntity> {
const qb = this.createQueryBuilder('credential');
if (options.filters?.dependency) {
addCredentialDependencyExistsFilter(qb, options.filters.dependency);
}
// Pass projectId from options to sharing options
const projectId =
typeof options.filter?.projectId === 'string' ? options.filter.projectId : undefined;
@@ -5,6 +5,7 @@ export { AuthIdentityRepository } from './auth-identity.repository';
export { AuthProviderSyncHistoryRepository } from './auth-provider-sync-history.repository';
export { BinaryDataRepository } from './binary-data.repository';
export { CredentialsRepository } from './credentials.repository';
export { CredentialDependencyRepository } from './credential-dependency.repository';
export { ExecutionAnnotationRepository } from './execution-annotation.repository';
export { ExecutionDataRepository } from './execution-data.repository';
export { ExecutionMetadataRepository } from './execution-metadata.repository';
@@ -1,5 +1,5 @@
import { Service } from '@n8n/di';
import { DataSource, In, Repository } from '@n8n/typeorm';
import { DataSource, EntityManager, In, Repository } from '@n8n/typeorm';
import { ProjectSecretsProviderAccess } from '../entities';
import type { SecretsProviderAccessRole } from '../entities';
@@ -25,8 +25,11 @@ export class ProjectSecretsProviderAccessRepository extends Repository<ProjectSe
});
}
async deleteByConnectionId(secretsProviderConnectionId: number): Promise<void> {
await this.delete({ secretsProviderConnectionId });
async deleteByConnectionId(
secretsProviderConnectionId: number,
entityManager: EntityManager = this.manager,
): Promise<void> {
await entityManager.delete(this.target, { secretsProviderConnectionId });
}
async updateProjectAccess(
@@ -1,5 +1,5 @@
import { Service } from '@n8n/di';
import { Brackets, DataSource, Repository } from '@n8n/typeorm';
import { Brackets, DataSource, In, Repository } from '@n8n/typeorm';
import { SecretsProviderConnection, SharedCredentials } from '../entities';
@@ -13,6 +13,26 @@ export class SecretsProviderConnectionRepository extends Repository<SecretsProvi
return await this.find();
}
async findIdByProviderKey(providerKey: string): Promise<string | null> {
const connection = await this.findOne({
select: ['id'],
where: { providerKey },
});
return connection ? connection.id.toString() : null;
}
async findIdsByProviderKeys(providerKeys: string[]): Promise<string[]> {
if (providerKeys.length === 0) return [];
const connections = await this.find({
select: ['id'],
where: { providerKey: In(providerKeys) },
});
return connections.map(({ id }) => id.toString());
}
async hasGlobalProvider(providerKey: string): Promise<boolean> {
const count = await this.manager
.createQueryBuilder(SecretsProviderConnection, 'connection')
@@ -210,21 +230,4 @@ export class SecretsProviderConnectionRepository extends Repository<SecretsProvi
.andWhere('projectAccess.projectId = :projectId', { projectId })
.getOne();
}
/**
* Remove a connection by its providerKey, but only if it is assigned to the specified project.
* Returns the removed connection, or null if no matching connection was found.
*/
async removeByProviderKeyAndProjectId(
providerKey: string,
projectId: string,
): Promise<SecretsProviderConnection | null> {
const connection = await this.findByProviderKeyAndProjectId(providerKey, projectId);
if (!connection) {
return null;
}
return await this.remove(connection);
}
}
@@ -0,0 +1,217 @@
import type { CredentialDependencyRepository, SecretsProviderConnectionRepository } from '@n8n/db';
import { In } from '@n8n/typeorm';
import type { EntityManager } from '@n8n/typeorm';
import { mock } from 'jest-mock-extended';
import {
CredentialDependencyService,
EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
} from '@/credentials/credential-dependency.service';
describe('CredentialDependencyService', () => {
const credentialDependencyRepository = mock<CredentialDependencyRepository>();
const secretsProviderConnectionRepository = mock<SecretsProviderConnectionRepository>();
const service = new CredentialDependencyService(
credentialDependencyRepository,
secretsProviderConnectionRepository,
);
beforeEach(() => {
jest.resetAllMocks();
});
describe('resolveExternalSecretsStoreDependencyFilter', () => {
it('returns dependency filter when provider exists', async () => {
secretsProviderConnectionRepository.findIdByProviderKey.mockResolvedValue('42');
const result = await service.resolveExternalSecretsStoreDependencyFilter('vault');
expect(secretsProviderConnectionRepository.findIdByProviderKey).toHaveBeenCalledWith('vault');
expect(result).toEqual({
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyId: '42',
});
});
it('returns undefined when provider does not exist', async () => {
secretsProviderConnectionRepository.findIdByProviderKey.mockResolvedValue(null);
const result = await service.resolveExternalSecretsStoreDependencyFilter('missing');
expect(result).toBeUndefined();
});
});
describe('upsertExternalSecretProviderDependenciesForCredential', () => {
it('resolves provider ids and upserts dependencies', async () => {
secretsProviderConnectionRepository.findIdsByProviderKeys.mockResolvedValue(['7', '8']);
const entityManager = mock<EntityManager>();
await service.upsertExternalSecretProviderDependenciesForCredential({
credentialId: 'cred-1',
decryptedCredentialData: {
apiKey: '={{ $secrets.vault.apiKey }}',
token: '={{ $secrets["aws-secrets-manager"].token }}',
},
entityManager,
});
expect(secretsProviderConnectionRepository.findIdsByProviderKeys).toHaveBeenCalledWith([
'vault',
'aws-secrets-manager',
]);
expect(credentialDependencyRepository.upsertDependenciesForCredential).toHaveBeenCalledWith({
credentialId: 'cred-1',
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyIds: ['7', '8'],
entityManager,
});
});
it('handles credential data without external providers', async () => {
secretsProviderConnectionRepository.findIdsByProviderKeys.mockResolvedValue([]);
const entityManager = mock<EntityManager>();
await service.upsertExternalSecretProviderDependenciesForCredential({
credentialId: 'cred-1',
decryptedCredentialData: { apiKey: 'plain-value' },
entityManager,
});
expect(secretsProviderConnectionRepository.findIdsByProviderKeys).toHaveBeenCalledWith([]);
expect(credentialDependencyRepository.upsertDependenciesForCredential).toHaveBeenCalledWith({
credentialId: 'cred-1',
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyIds: [],
entityManager,
});
});
});
describe('syncExternalSecretProviderDependenciesForCredential', () => {
it('resolves provider ids and syncs dependencies', async () => {
secretsProviderConnectionRepository.findIdsByProviderKeys.mockResolvedValue(['7', '8']);
const entityManager = mock<EntityManager>();
await service.syncExternalSecretProviderDependenciesForCredential({
credentialId: 'cred-1',
decryptedCredentialData: {
apiKey: '={{ $secrets.vault.apiKey }}',
token: '={{ $secrets["aws-secrets-manager"].token }}',
},
entityManager,
});
expect(secretsProviderConnectionRepository.findIdsByProviderKeys).toHaveBeenCalledWith([
'vault',
'aws-secrets-manager',
]);
expect(credentialDependencyRepository.syncDependenciesForCredential).toHaveBeenCalledWith({
credentialId: 'cred-1',
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyIds: ['7', '8'],
entityManager,
});
});
it('handles credential data without external providers', async () => {
secretsProviderConnectionRepository.findIdsByProviderKeys.mockResolvedValue([]);
const entityManager = mock<EntityManager>();
await service.syncExternalSecretProviderDependenciesForCredential({
credentialId: 'cred-1',
decryptedCredentialData: { apiKey: 'plain-value' },
entityManager,
});
expect(secretsProviderConnectionRepository.findIdsByProviderKeys).toHaveBeenCalledWith([]);
expect(credentialDependencyRepository.syncDependenciesForCredential).toHaveBeenCalledWith({
credentialId: 'cred-1',
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyIds: [],
entityManager,
});
});
});
describe('deleteDependencyById', () => {
it('deletes through entity manager when provided', async () => {
const entityManager = mock<EntityManager>();
await service.deleteDependencyById({
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyId: 'provider-1',
entityManager,
});
expect(entityManager.delete).toHaveBeenCalledWith(credentialDependencyRepository.target, {
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyId: 'provider-1',
});
expect(credentialDependencyRepository.delete).not.toHaveBeenCalled();
});
it('deletes through repository when entity manager is not provided', async () => {
const manager = mock<EntityManager>();
Object.defineProperty(credentialDependencyRepository, 'manager', {
value: manager,
configurable: true,
});
await service.deleteDependencyById({
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyId: 'provider-1',
});
expect(manager.delete).toHaveBeenCalledWith(credentialDependencyRepository.target, {
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyId: 'provider-1',
});
});
});
describe('deleteDependenciesByIds', () => {
it('returns early when dependency ids are empty', async () => {
await service.deleteDependenciesByIds({
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyIds: [],
});
expect(credentialDependencyRepository.delete).not.toHaveBeenCalled();
});
it('deletes through entity manager when provided', async () => {
const entityManager = mock<EntityManager>();
await service.deleteDependenciesByIds({
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyIds: ['provider-1', 'provider-2'],
entityManager,
});
expect(entityManager.delete).toHaveBeenCalledWith(credentialDependencyRepository.target, {
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyId: In(['provider-1', 'provider-2']),
});
expect(credentialDependencyRepository.delete).not.toHaveBeenCalled();
});
it('deletes through repository when entity manager is not provided', async () => {
const manager = mock<EntityManager>();
Object.defineProperty(credentialDependencyRepository, 'manager', {
value: manager,
configurable: true,
});
await service.deleteDependenciesByIds({
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyIds: ['provider-1', 'provider-2'],
});
expect(manager.delete).toHaveBeenCalledWith(credentialDependencyRepository.target, {
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyId: In(['provider-1', 'provider-2']),
});
});
});
});
@@ -5,26 +5,30 @@ jest.mock('@/generic-helpers', () => ({
import type { LicenseState } from '@n8n/backend-common';
import type {
AuthenticatedRequest,
ICredentialsDb,
Project,
SharedCredentials,
SharedCredentialsRepository,
CredentialsEntity,
CredentialsRepository,
} from '@n8n/db';
import { GLOBAL_OWNER_ROLE, GLOBAL_MEMBER_ROLE } from '@n8n/db';
import type { Scope } from '@n8n/permissions';
import { mock } from 'jest-mock-extended';
import { createRawProjectData } from '@/__tests__/project.test-data';
import type { EventService } from '@/events/event.service';
import type { CredentialRequest } from '@/requests';
import { createNewCredentialsPayload, createdCredentialsWithScopes } from './credentials.test-data';
import type { CredentialDependencyService } from '../credential-dependency.service';
import type { CredentialsFinderService } from '../credentials-finder.service';
import { CredentialsController } from '../credentials.controller';
import { CredentialsService } from '../credentials.service';
import * as validation from '../validation';
import * as checkAccess from '@/permissions.ee/check-access';
import { createNewCredentialsPayload, createdCredentialsWithScopes } from './credentials.test-data';
import type { CredentialRequest } from '@/requests';
describe('CredentialsController', () => {
const eventService = mock<EventService>();
type ControllerEventService = ConstructorParameters<typeof CredentialsController>[9];
const eventService = mock<ControllerEventService>();
const sharedCredentialsRepository = mock<SharedCredentialsRepository>();
const credentialsFinderService = mock<CredentialsFinderService>();
const licenseState = mock<LicenseState>();
@@ -35,6 +39,7 @@ describe('CredentialsController', () => {
// real CredentialsService instance with mocked dependencies
const credentialsService = new CredentialsService(
credentialsRepository,
mock<CredentialDependencyService>(),
mock(), // sharedCredentialsRepository
mock(), // ownershipService
mock(), // logger
@@ -55,12 +60,16 @@ describe('CredentialsController', () => {
// Spy on methods that need to be mocked in tests
// This allows us to mock specific behavior while keeping real implementations
// for isChangingExternalSecretExpression and validateExternalSecretsPermissions
jest.spyOn(credentialsService, 'decrypt');
jest.spyOn(credentialsService, 'prepareUpdateData');
jest.spyOn(credentialsService, 'createEncryptedData');
jest.spyOn(credentialsService, 'getCredentialScopes');
jest.spyOn(credentialsService, 'update');
jest.spyOn(credentialsService, 'createUnmanagedCredential');
const decryptSpy = jest.spyOn(credentialsService, 'decrypt');
const createEncryptedDataSpy = jest.spyOn(credentialsService, 'createEncryptedData');
const getCredentialScopesSpy = jest.spyOn(credentialsService, 'getCredentialScopes');
const updateSpy = jest.spyOn(credentialsService, 'update');
const createUnmanagedCredentialSpy = jest.spyOn(credentialsService, 'createUnmanagedCredential');
const findCredentialOwningProjectSpy = jest.spyOn(
sharedCredentialsRepository,
'findCredentialOwningProject',
);
const emitSpy = jest.spyOn(eventService, 'emit');
const credentialsController = new CredentialsController(
mock(),
@@ -98,17 +107,14 @@ describe('CredentialsController', () => {
const createdCredentials = createdCredentialsWithScopes(payloadWithoutData);
const projectOwningCredentialData = createRawProjectData({
const projectOwningCredentialData = mock<Project>({
id: newCredentialsPayload.projectId,
type: 'team',
});
jest
.mocked(credentialsService.createUnmanagedCredential)
.mockResolvedValue(createdCredentials);
createUnmanagedCredentialSpy.mockResolvedValue(createdCredentials);
sharedCredentialsRepository.findCredentialOwningProject.mockResolvedValue(
projectOwningCredentialData,
);
findCredentialOwningProjectSpy.mockResolvedValue(projectOwningCredentialData);
// Act
@@ -120,15 +126,12 @@ describe('CredentialsController', () => {
// Assert
expect(credentialsService.createUnmanagedCredential).toHaveBeenCalledWith(
newCredentialsPayload,
req.user,
);
expect(sharedCredentialsRepository.findCredentialOwningProject).toHaveBeenCalledWith(
createdCredentials.id,
);
expect(eventService.emit).toHaveBeenCalledWith('credentials-created', {
user: expect.objectContaining({ id: req.user.id }),
expect(createUnmanagedCredentialSpy).toHaveBeenCalledWith(newCredentialsPayload, req.user);
expect(findCredentialOwningProjectSpy).toHaveBeenCalledWith(createdCredentials.id);
expect(emitSpy).toHaveBeenCalledTimes(1);
const [eventName, eventPayload] = emitSpy.mock.calls[0];
expect(eventName).toBe('credentials-created');
expect(eventPayload).toMatchObject({
credentialId: createdCredentials.id,
credentialType: createdCredentials.type,
projectId: projectOwningCredentialData.id,
@@ -137,6 +140,7 @@ describe('CredentialsController', () => {
uiContext: newCredentialsPayload.uiContext,
isDynamic: false,
});
expect((eventPayload as { user: { id: string } }).user.id).toBe('123');
expect(newApiKey).toEqual(createdCredentials);
});
@@ -156,24 +160,27 @@ describe('CredentialsController', () => {
role: 'credential:owner',
projectId: 'WHwt9vP3keCUvmB5',
credentialsId: credentialId,
} as any,
} as SharedCredentials,
],
});
const getEncryptedCredential = (isResolvable = false): ICredentialsDb => ({
name: 'Updated Credential',
type: 'apiKey',
data: 'encrypted-data',
id: credentialId,
createdAt: new Date(),
updatedAt: new Date(),
isResolvable,
});
beforeEach(() => {
jest.mocked(credentialsService.decrypt).mockReturnValue({ apiKey: 'test-key' });
jest.mocked(credentialsService.createEncryptedData).mockReturnValue({
name: 'Updated Credential',
type: 'apiKey',
data: 'encrypted-data',
id: 'cred-123',
createdAt: new Date(),
updatedAt: new Date(),
isResolvable: false,
} as any);
jest
.mocked(credentialsService.getCredentialScopes)
.mockResolvedValue(['credential:read', 'credential:update'] as any);
decryptSpy.mockReturnValue({ apiKey: 'test-key' });
createEncryptedDataSpy.mockReturnValue(getEncryptedCredential());
getCredentialScopesSpy.mockResolvedValue([
'credential:read' as Scope,
'credential:update' as Scope,
]);
});
it('should not allow owner to set isGlobal to true if not licensed', async () => {
@@ -199,7 +206,7 @@ describe('CredentialsController', () => {
);
// ASSERT
expect(credentialsService.update).not.toHaveBeenCalled();
expect(updateSpy).not.toHaveBeenCalled();
});
it('should allow owner to set isGlobal to true if licensed', async () => {
@@ -218,7 +225,7 @@ describe('CredentialsController', () => {
licenseState.isSharingLicensed.mockReturnValue(true);
credentialsFinderService.findCredentialForUser.mockResolvedValue(existingCredential);
jest.mocked(credentialsService.update).mockResolvedValue({
updateSpy.mockResolvedValue({
...existingCredential,
name: 'Updated Credential',
isGlobal: true,
@@ -228,13 +235,14 @@ describe('CredentialsController', () => {
await credentialsController.updateCredentials(ownerReq);
// ASSERT
expect(credentialsService.update).toHaveBeenCalledWith(
expect(updateSpy).toHaveBeenCalledWith(
credentialId,
expect.objectContaining({
isGlobal: true,
}),
expect.any(Object),
);
expect(eventService.emit).toHaveBeenCalledWith('credentials-updated', {
expect(emitSpy).toHaveBeenCalledWith('credentials-updated', {
user: ownerReq.user,
credentialType: existingCredential.type,
credentialId: existingCredential.id,
@@ -262,7 +270,7 @@ describe('CredentialsController', () => {
licenseState.isSharingLicensed.mockReturnValue(true);
credentialsFinderService.findCredentialForUser.mockResolvedValue(globalCredential);
jest.mocked(credentialsService.update).mockResolvedValue({
updateSpy.mockResolvedValue({
...globalCredential,
isGlobal: false,
});
@@ -271,11 +279,12 @@ describe('CredentialsController', () => {
await credentialsController.updateCredentials(ownerReq);
// ASSERT
expect(credentialsService.update).toHaveBeenCalledWith(
expect(updateSpy).toHaveBeenCalledWith(
credentialId,
expect.objectContaining({
isGlobal: false,
}),
expect.any(Object),
);
});
@@ -302,7 +311,7 @@ describe('CredentialsController', () => {
);
// ASSERT
expect(credentialsService.update).not.toHaveBeenCalled();
expect(updateSpy).not.toHaveBeenCalled();
});
it('should prevent non-owner from changing isGlobal to true', async () => {
@@ -331,7 +340,7 @@ describe('CredentialsController', () => {
);
// ASSERT
expect(credentialsService.update).not.toHaveBeenCalled();
expect(updateSpy).not.toHaveBeenCalled();
});
it('should update credential without changing isGlobal when not provided', async () => {
@@ -348,7 +357,7 @@ describe('CredentialsController', () => {
} as unknown as CredentialRequest.Update;
credentialsFinderService.findCredentialForUser.mockResolvedValue(existingCredential);
jest.mocked(credentialsService.update).mockResolvedValue({
updateSpy.mockResolvedValue({
...existingCredential,
name: 'Updated Credential',
});
@@ -358,12 +367,9 @@ describe('CredentialsController', () => {
// ASSERT
// Should not include isGlobal in update when not provided
expect(credentialsService.update).toHaveBeenCalledWith(
credentialId,
expect.not.objectContaining({
isGlobal: expect.anything(),
}),
);
expect(updateSpy).toHaveBeenCalledWith(credentialId, expect.any(Object), expect.any(Object));
const updatePayload = updateSpy.mock.calls[0][1];
expect(updatePayload).not.toHaveProperty('isGlobal');
});
it('should update isResolvable when provided', async () => {
@@ -387,16 +393,8 @@ describe('CredentialsController', () => {
credentialsFinderService.findCredentialForUser.mockResolvedValue(
existingCredentialWithResolvable,
);
jest.mocked(credentialsService.createEncryptedData).mockReturnValue({
name: 'Updated Credential',
type: 'apiKey',
data: 'encrypted-data',
id: 'cred-123',
createdAt: new Date(),
updatedAt: new Date(),
isResolvable: true,
} as any);
jest.mocked(credentialsService.update).mockResolvedValue({
createEncryptedDataSpy.mockReturnValue(getEncryptedCredential(true));
updateSpy.mockResolvedValue({
...existingCredentialWithResolvable,
name: 'Updated Credential',
isResolvable: true,
@@ -406,11 +404,12 @@ describe('CredentialsController', () => {
await credentialsController.updateCredentials(ownerReq);
// ASSERT
expect(credentialsService.update).toHaveBeenCalledWith(
expect(updateSpy).toHaveBeenCalledWith(
credentialId,
expect.objectContaining({
isResolvable: true,
}),
expect.any(Object),
);
});
@@ -435,16 +434,8 @@ describe('CredentialsController', () => {
credentialsFinderService.findCredentialForUser.mockResolvedValue(
existingCredentialWithResolvable,
);
jest.mocked(credentialsService.createEncryptedData).mockReturnValue({
name: 'Updated Credential',
type: 'apiKey',
data: 'encrypted-data',
id: 'cred-123',
createdAt: new Date(),
updatedAt: new Date(),
isResolvable: true,
} as any);
jest.mocked(credentialsService.update).mockResolvedValue({
createEncryptedDataSpy.mockReturnValue(getEncryptedCredential(true));
updateSpy.mockResolvedValue({
...existingCredentialWithResolvable,
name: 'Updated Credential',
});
@@ -453,11 +444,12 @@ describe('CredentialsController', () => {
await credentialsController.updateCredentials(ownerReq);
// ASSERT
expect(credentialsService.update).toHaveBeenCalledWith(
expect(updateSpy).toHaveBeenCalledWith(
credentialId,
expect.objectContaining({
isResolvable: true, // Should keep the existing value
}),
expect.any(Object),
);
});
@@ -481,7 +473,7 @@ describe('CredentialsController', () => {
existingCredentialWithSecret,
);
// Mock setup: existing credential already has a secret expression
jest.mocked(credentialsService.decrypt).mockReturnValue({ apiKey: '$secrets.oldKey' });
decryptSpy.mockReturnValue({ apiKey: '$secrets.oldKey' });
await expect(credentialsController.updateCredentials(memberReq)).rejects.toThrow(
'Lacking permissions to reference external secrets in credentials',
@@ -492,7 +484,7 @@ describe('CredentialsController', () => {
dataToSave: memberReq.body.data,
decryptedExistingData: { apiKey: '$secrets.oldKey' },
});
expect(credentialsService.update).not.toHaveBeenCalled();
expect(updateSpy).not.toHaveBeenCalled();
});
it('should throw error when adding new external secret expression without permission', async () => {
@@ -511,7 +503,7 @@ describe('CredentialsController', () => {
// Mock setup: existing credential has no external secret yet
credentialsFinderService.findCredentialForUser.mockResolvedValue(existingCredential);
jest.mocked(credentialsService.decrypt).mockReturnValue({ apiKey: 'regular-key' });
decryptSpy.mockReturnValue({ apiKey: 'regular-key' });
await expect(credentialsController.updateCredentials(memberReq)).rejects.toThrow(
'Lacking permissions to reference external secrets in credentials',
@@ -522,7 +514,7 @@ describe('CredentialsController', () => {
dataToSave: memberReq.body.data,
decryptedExistingData: { apiKey: 'regular-key' },
});
expect(credentialsService.update).not.toHaveBeenCalled();
expect(updateSpy).not.toHaveBeenCalled();
});
});
});
@@ -16,7 +16,11 @@ import {
type ICredentialDataDecryptedObject,
type ICredentialType,
} from 'n8n-workflow';
import { mockExistingCredential } from './credentials.test-data';
import type { CredentialTypes } from '@/credential-types';
import type { CredentialDependencyService } from '@/credentials/credential-dependency.service';
import type { CredentialsFinderService } from '@/credentials/credentials-finder.service';
import { CredentialsService } from '@/credentials/credentials.service';
import * as validation from '@/credentials/validation';
@@ -30,8 +34,6 @@ import type { OwnershipService } from '@/services/ownership.service';
import type { ProjectService } from '@/services/project.service.ee';
import type { RoleService } from '@/services/role.service';
import { mockExistingCredential } from './credentials.test-data';
const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
const memberUser = mock<User>({ id: 'member-id', role: GLOBAL_MEMBER_ROLE });
@@ -57,6 +59,7 @@ describe('CredentialsService', () => {
const errorReporter = mock<ErrorReporter>();
const credentialTypes = mock<CredentialTypes>();
const credentialsRepository = mock<CredentialsRepository>();
const credentialDependencyService = mock<CredentialDependencyService>();
const sharedCredentialsRepository = mock<SharedCredentialsRepository>();
const ownershipService = mock<OwnershipService>();
const logger = mock<Logger>();
@@ -73,6 +76,7 @@ describe('CredentialsService', () => {
const service = new CredentialsService(
credentialsRepository,
credentialDependencyService,
sharedCredentialsRepository,
ownershipService,
logger,
@@ -92,6 +96,13 @@ describe('CredentialsService', () => {
beforeEach(() => {
jest.resetAllMocks();
credentialDependencyService.resolveExternalSecretsStoreDependencyFilter.mockResolvedValue(
undefined,
);
credentialDependencyService.syncExternalSecretProviderDependenciesForCredential.mockResolvedValue(
undefined,
);
ownershipService.addOwnedByAndSharedWith.mockImplementation((credential: any) => credential);
// Mock the subquery method used by member users and admin users with onlySharedWithMe
credentialsRepository.getManyAndCountWithSharingSubquery.mockResolvedValue({
credentials: [],
@@ -956,7 +967,9 @@ describe('CredentialsService', () => {
projectRoles: expect.any(Array),
credentialRoles: expect.any(Array),
}),
{},
expect.objectContaining({
filters: { dependency: undefined },
}),
);
});
@@ -1136,7 +1149,9 @@ describe('CredentialsService', () => {
expect.objectContaining({
onlySharedWithMe: true,
}),
{},
expect.objectContaining({
filters: { dependency: undefined },
}),
);
expect(sharedCredentialsRepository.getAllRelationsForCredentials).toHaveBeenCalledWith([
'cred-1',
@@ -1191,7 +1206,10 @@ describe('CredentialsService', () => {
// ASSERT
expect(credentialsRepository.findMany).toHaveBeenCalled();
expect(credentialsRepository.findAllGlobalCredentials).toHaveBeenCalledWith(false);
expect(credentialsRepository.findAllGlobalCredentials).toHaveBeenCalledWith({
includeData: false,
filters: { dependency: undefined },
});
expect(result).toHaveLength(2);
expect(result).toEqual(
expect.arrayContaining([
@@ -1216,7 +1234,10 @@ describe('CredentialsService', () => {
// ASSERT
expect(credentialsRepository.getManyAndCountWithSharingSubquery).toHaveBeenCalled();
expect(credentialsRepository.findAllGlobalCredentials).toHaveBeenCalledWith(false);
expect(credentialsRepository.findAllGlobalCredentials).toHaveBeenCalledWith({
includeData: false,
filters: { dependency: undefined },
});
expect(result).toHaveLength(2);
expect(result).toEqual(
expect.arrayContaining([
@@ -1272,7 +1293,10 @@ describe('CredentialsService', () => {
});
// ASSERT
expect(credentialsRepository.findAllGlobalCredentials).toHaveBeenCalledWith(true);
expect(credentialsRepository.findAllGlobalCredentials).toHaveBeenCalledWith({
includeData: true,
filters: { dependency: undefined },
});
});
});
@@ -1312,68 +1336,62 @@ describe('CredentialsService', () => {
});
describe('with externalSecretsStore', () => {
const createCredentialWithEncryptedData = (id: string, apiKey: string) =>
mock<CredentialsEntity>({
id,
data: service.createEncryptedData({ ...regularCredential, data: { apiKey } }).data,
});
it('should filter credentials by external secrets store using dot notation', async () => {
it('should resolve dependency filter and pass it to repository query', async () => {
// ARRANGE
const credentialWithExternalSecret1 = createCredentialWithEncryptedData(
'cred-with-external-secret-1',
'{{ $secrets.myProvider.apiKey }}',
);
const credentialWithExternalSecret2 = createCredentialWithEncryptedData(
'cred-with-external-secret-2',
'{{ $secrets.anotherProvider.apiKey }}',
ownershipService.addOwnedByAndSharedWith.mockImplementation(
(credential: any) => credential,
);
const credentialWithExternalSecret1 = {
id: 'cred-with-external-secret-1',
name: 'Credential with secret 1',
type: 'apiKey',
isGlobal: false,
shared: [],
} as Partial<CredentialsEntity> as CredentialsEntity;
credentialDependencyService.resolveExternalSecretsStoreDependencyFilter.mockResolvedValue({
dependencyType: 'externalSecretProvider',
dependencyId: '123',
});
credentialsRepository.getManyAndCountWithSharingSubquery.mockResolvedValue({
credentials: [
regularCredential,
credentialWithExternalSecret1,
credentialWithExternalSecret2,
],
count: 3,
credentials: [credentialWithExternalSecret1],
count: 1,
});
// ACT
const result = await service.getMany(memberUser, {
externalSecretsStore: 'myProvider',
filters: { externalSecretsStore: 'myProvider' },
listQueryOptions: { select: { id: true } } as any,
});
// ASSERT
expect(result).toHaveLength(1);
expect(result[0].id).toBe(credentialWithExternalSecret1.id);
expect(credentialsRepository.getManyAndCountWithSharingSubquery).toHaveBeenCalledWith(
memberUser,
expect.any(Object),
{
select: { id: true },
filters: {
dependency: {
dependencyType: 'externalSecretProvider',
dependencyId: '123',
},
},
},
);
});
it('should filter credentials by external secrets store using square bracket notation', async () => {
// ARRANGE
const credentialWithExternalSecret1 = createCredentialWithEncryptedData(
'cred-with-external-secret-1',
'{{ $secrets["myProvider"]["apiKey"] }}',
it('should return early when no credential dependencies match the external secrets store', async () => {
credentialDependencyService.resolveExternalSecretsStoreDependencyFilter.mockResolvedValue(
undefined,
);
const credentialWithExternalSecret2 = createCredentialWithEncryptedData(
'cred-with-external-secret-2',
'{{ $secrets["anotherProvider"]["apiKey"] }}',
);
credentialsRepository.getManyAndCountWithSharingSubquery.mockResolvedValue({
credentials: [
regularCredential,
credentialWithExternalSecret1,
credentialWithExternalSecret2,
],
count: 3,
});
// ACT
const result = await service.getMany(memberUser, {
externalSecretsStore: 'myProvider',
filters: { externalSecretsStore: 'myProvider' },
});
// ASSERT
expect(result).toHaveLength(1);
expect(result[0].id).toBe(credentialWithExternalSecret1.id);
expect(result).toEqual([]);
expect(credentialsRepository.getManyAndCountWithSharingSubquery).not.toHaveBeenCalled();
});
});
});
@@ -0,0 +1,96 @@
import {
extractProviderKeysFromCredentialData,
extractProviderKeysFromExpression,
getExternalSecretExpressionPaths,
} from '../external-secrets.utils';
describe('External secrets utils', () => {
describe('extractProviderKeysFromExpression', () => {
it('extracts single provider from dot notation', () => {
expect(extractProviderKeysFromExpression('={{ $secrets.vault.myKey }}')).toEqual(['vault']);
});
it('extracts single provider from bracket notation', () => {
expect(extractProviderKeysFromExpression("={{ $secrets['aws']['secret'] }}")).toEqual([
'aws',
]);
});
it('extracts multiple providers from same expression', () => {
const result = extractProviderKeysFromExpression(
'={{ $secrets.vault.myKey + ":" + $secrets.aws.otherKey }}',
);
expect(result.sort()).toEqual(['aws', 'vault']);
});
it('deduplicates repeated provider keys', () => {
expect(
extractProviderKeysFromExpression('={{ $secrets.vault.key1 + $secrets.vault.key2 }}'),
).toEqual(['vault']);
});
it('does not extract partial provider keys from malformed dot notation', () => {
expect(
extractProviderKeysFromExpression(
'={{ $secrets.vault_invalid.key + $secrets.aws.secret }}',
),
).toEqual(['aws']);
});
it('returns empty array when no $secrets references found', () => {
expect(extractProviderKeysFromExpression('={{ $variables.myVar }}')).toEqual([]);
});
it('returns empty array for plain text', () => {
expect(extractProviderKeysFromExpression('some plain text')).toEqual([]);
});
it('returns empty array when $secrets is not inside expression braces', () => {
expect(extractProviderKeysFromExpression('$secrets.vault.key')).toEqual([]);
expect(
extractProviderKeysFromExpression('text with $secrets.vault.key but no braces'),
).toEqual([]);
});
it('only extracts providers from inside expression braces', () => {
expect(
extractProviderKeysFromExpression('$secrets.vault.key and {{ $secrets.aws.secret }}'),
).toEqual(['aws']);
});
it('extracts providers from multiple expression blocks', () => {
const expression = 'hello {{ $secrets.vault.key }} world {{ $secrets.aws.secret }}';
const result = extractProviderKeysFromExpression(expression);
expect(result.sort()).toEqual(['aws', 'vault']);
});
});
describe('getExternalSecretExpressionPaths', () => {
it('returns all paths that contain external secret expressions', () => {
const data = {
a: 'plain',
b: '={{ $secrets.vault.key }}',
nested: {
c: "={{ $secrets['aws']['token'] }}",
},
arr: [{ d: '={{ $secrets.azure.secret }}' }],
};
expect(getExternalSecretExpressionPaths(data).sort()).toEqual(['arr[0].d', 'b', 'nested.c']);
});
});
describe('extractProviderKeysFromCredentialData', () => {
it('extracts unique provider keys across nested credential data', () => {
const data = {
apiKey: '={{ $secrets.vault.key1 }}',
nested: {
token: "={{ $secrets['aws']['token'] }}",
duplicate: '={{ $secrets.vault.key2 }}',
},
};
expect([...extractProviderKeysFromCredentialData(data)].sort()).toEqual(['aws', 'vault']);
});
});
});
@@ -1,15 +1,13 @@
import { GLOBAL_OWNER_ROLE, GLOBAL_MEMBER_ROLE, type User } from '@n8n/db';
import { mock } from 'jest-mock-extended';
import type { SecretsProviderAccessCheckService } from '@/modules/external-secrets.ee/secret-provider-access-check.service.ee';
import * as checkAccess from '@/permissions.ee/check-access';
import {
validateExternalSecretsPermissions,
isChangingExternalSecretExpression,
validateAccessToReferencedSecretProviders,
extractProviderKeys,
} from '../validation';
import type { SecretsProviderAccessCheckService } from '@/modules/external-secrets.ee/secret-provider-access-check.service.ee';
import * as checkAccess from '@/permissions.ee/check-access';
const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
const memberUser = mock<User>({ id: 'member-id', role: GLOBAL_MEMBER_ROLE });
@@ -18,54 +16,6 @@ describe('Credentials Validation', () => {
const projectId = 'project-id';
const errorMessage = 'Lacking permissions to reference external secrets in credentials';
describe('extractProviderKeys', () => {
it('should extract single provider from dot notation', () => {
expect(extractProviderKeys('={{ $secrets.vault.myKey }}')).toEqual(['vault']);
});
it('should extract single provider from bracket notation', () => {
expect(extractProviderKeys("={{ $secrets['aws']['secret'] }}")).toEqual(['aws']);
});
it('should extract multiple providers from same expression', () => {
const result = extractProviderKeys(
'={{ $secrets.vault.myKey + ":" + $secrets.aws.otherKey }}',
);
expect(result.sort()).toEqual(['aws', 'vault']);
});
it('should deduplicate repeated provider keys', () => {
expect(extractProviderKeys('={{ $secrets.vault.key1 + $secrets.vault.key2 }}')).toEqual([
'vault',
]);
});
it('should return empty array when no $secrets references found', () => {
expect(extractProviderKeys('={{ $variables.myVar }}')).toEqual([]);
});
it('should return empty array for plain text', () => {
expect(extractProviderKeys('some plain text')).toEqual([]);
});
it('should return empty array when $secrets is not inside expression braces', () => {
expect(extractProviderKeys('$secrets.vault.key')).toEqual([]);
expect(extractProviderKeys('text with $secrets.vault.key but no braces')).toEqual([]);
});
it('should only extract providers from inside expression braces', () => {
expect(extractProviderKeys('$secrets.vault.key and {{ $secrets.aws.secret }}')).toEqual([
'aws',
]);
});
it('should extract providers from multiple expression blocks', () => {
const expression = 'hello {{ $secrets.vault.key }} world {{ $secrets.aws.secret }}';
const result = extractProviderKeys(expression);
expect(result.sort()).toEqual(['aws', 'vault']);
});
});
describe('validateExternalSecretsPermissions', () => {
beforeEach(() => {
jest.restoreAllMocks();
@@ -0,0 +1,115 @@
import type { CredentialDependencyType } from '@n8n/db';
import { CredentialDependencyRepository, SecretsProviderConnectionRepository } from '@n8n/db';
import { Service } from '@n8n/di';
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
import { In, type EntityManager } from '@n8n/typeorm';
import type { ICredentialDataDecryptedObject } from 'n8n-workflow';
import { extractProviderKeysFromCredentialData } from './external-secrets.utils';
export const EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE = 'externalSecretProvider' as const;
export type CredentialDependencyFilter = {
dependencyType: CredentialDependencyType;
dependencyId: string;
};
@Service()
export class CredentialDependencyService {
constructor(
private readonly credentialDependencyRepository: CredentialDependencyRepository,
private readonly secretsProviderConnectionRepository: SecretsProviderConnectionRepository,
) {}
async resolveExternalSecretsStoreDependencyFilter(
externalSecretsStoreProviderKey: string,
): Promise<CredentialDependencyFilter | undefined> {
const providerId = await this.secretsProviderConnectionRepository.findIdByProviderKey(
externalSecretsStoreProviderKey,
);
if (providerId === null) return undefined;
return {
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyId: providerId,
};
}
private async resolveProviderIdsFromCredentialData(
decryptedCredentialData: ICredentialDataDecryptedObject,
): Promise<string[]> {
const providerKeys = [...extractProviderKeysFromCredentialData(decryptedCredentialData)];
return await this.secretsProviderConnectionRepository.findIdsByProviderKeys(providerKeys);
}
async upsertExternalSecretProviderDependenciesForCredential({
credentialId,
decryptedCredentialData,
entityManager,
}: {
credentialId: string;
decryptedCredentialData: ICredentialDataDecryptedObject;
entityManager: EntityManager;
}): Promise<void> {
const dependencyIds = await this.resolveProviderIdsFromCredentialData(decryptedCredentialData);
await this.credentialDependencyRepository.upsertDependenciesForCredential({
credentialId,
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyIds,
entityManager,
});
}
async syncExternalSecretProviderDependenciesForCredential({
credentialId,
decryptedCredentialData,
entityManager,
}: {
credentialId: string;
decryptedCredentialData: ICredentialDataDecryptedObject;
entityManager: EntityManager;
}): Promise<void> {
const dependencyIds = await this.resolveProviderIdsFromCredentialData(decryptedCredentialData);
await this.credentialDependencyRepository.syncDependenciesForCredential({
credentialId,
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyIds,
entityManager,
});
}
async deleteDependencyById({
dependencyType,
dependencyId,
entityManager,
}: {
dependencyType: CredentialDependencyType;
dependencyId: string;
entityManager?: EntityManager;
}): Promise<void> {
const manager = entityManager ?? this.credentialDependencyRepository.manager;
await manager.delete(this.credentialDependencyRepository.target, {
dependencyType,
dependencyId,
});
}
async deleteDependenciesByIds({
dependencyType,
dependencyIds,
entityManager,
}: {
dependencyType: CredentialDependencyType;
dependencyIds: string[];
entityManager?: EntityManager;
}): Promise<void> {
if (dependencyIds.length === 0) return;
const manager = entityManager ?? this.credentialDependencyRepository.manager;
await manager.delete(this.credentialDependencyRepository.target, {
dependencyType,
dependencyId: In(dependencyIds),
});
}
}
@@ -75,7 +75,9 @@ export class CredentialsController {
includeData: query.includeData,
onlySharedWithMe: query.onlySharedWithMe,
includeGlobal: query.includeGlobal,
externalSecretsStore: query.externalSecretsStore,
filters: {
externalSecretsStore: query.externalSecretsStore,
},
});
credentials.forEach((c) => {
// @ts-expect-error: This is to emulate the old behavior of removing the shared
@@ -263,7 +265,13 @@ export class CredentialsController {
}
newCredentialData.isResolvable = body.isResolvable ?? credential.isResolvable;
const responseData = await this.credentialsService.update(credentialId, newCredentialData);
const responseData = await this.credentialsService.update(
credentialId,
newCredentialData,
body.data
? (preparedCredentialData.data as unknown as ICredentialDataDecryptedObject)
: undefined,
);
if (responseData === null) {
throw new NotFoundError(`Credential ID "${credentialId}" could not be found to be updated.`);
@@ -37,6 +37,16 @@ import {
NodeHelpers,
} from 'n8n-workflow';
import {
CredentialDependencyService,
type CredentialDependencyFilter,
} from './credential-dependency.service';
import { CredentialsFinderService } from './credentials-finder.service';
import {
validateAccessToReferencedSecretProviders,
validateExternalSecretsPermissions,
} from './validation';
import { CredentialTypes } from '@/credential-types';
import { createCredentialsFromCredentialsEntity, CredentialsHelper } from '@/credentials-helper';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
@@ -53,13 +63,6 @@ import { CredentialsTester } from '@/services/credentials-tester.service';
import { OwnershipService } from '@/services/ownership.service';
import { ProjectService } from '@/services/project.service.ee';
import { RoleService } from '@/services/role.service';
import { getAllKeyPaths } from '@/utils';
import { CredentialsFinderService } from './credentials-finder.service';
import {
validateAccessToReferencedSecretProviders,
validateExternalSecretsPermissions,
} from './validation';
export type CredentialsGetSharedOptions =
| { allowGlobalScope: true; globalScope: Scope }
@@ -69,10 +72,21 @@ type CreateCredentialOptions = CreateCredentialDto & {
isManaged: boolean;
};
type GetManyCredentialsOptions = {
listQueryOptions: ListQuery.Options;
includeGlobal: boolean;
includeData: boolean;
onlySharedWithMe: boolean;
filters?: {
dependency?: CredentialDependencyFilter;
};
};
@Service()
export class CredentialsService {
constructor(
private readonly credentialsRepository: CredentialsRepository,
private readonly credentialDependencyService: CredentialDependencyService,
private readonly sharedCredentialsRepository: SharedCredentialsRepository,
private readonly ownershipService: OwnershipService,
private readonly logger: Logger,
@@ -93,9 +107,12 @@ export class CredentialsService {
private async addGlobalCredentials(
credentials: CredentialsEntity[],
includeData: boolean,
dependencyFilter?: CredentialDependencyFilter,
): Promise<CredentialsEntity[]> {
const globalCredentials =
await this.credentialsRepository.findAllGlobalCredentials(includeData);
const globalCredentials = await this.credentialsRepository.findAllGlobalCredentials({
includeData,
filters: { dependency: dependencyFilter },
});
// Merge and deduplicate based on credential ID
const credentialIds = new Set(credentials.map((c) => c.id));
@@ -107,23 +124,27 @@ export class CredentialsService {
async getMany(
user: User,
options: {
listQueryOptions?: ListQuery.Options & { includeData?: boolean };
listQueryOptions?: ListQuery.Options;
includeScopes?: boolean;
includeData: true;
onlySharedWithMe?: boolean;
includeGlobal?: boolean;
externalSecretsStore?: string;
filters?: {
externalSecretsStore?: string;
};
},
): Promise<Array<ICredentialsDecrypted<ICredentialDataDecryptedObject>>>;
async getMany(
user: User,
options?: {
listQueryOptions?: ListQuery.Options & { includeData?: boolean };
listQueryOptions?: ListQuery.Options;
includeScopes?: boolean;
includeData?: boolean;
onlySharedWithMe?: boolean;
includeGlobal?: boolean;
externalSecretsStore?: string;
filters?: {
externalSecretsStore?: string;
};
},
): Promise<CredentialsEntity[]>;
async getMany(
@@ -134,47 +155,54 @@ export class CredentialsService {
includeData = false,
onlySharedWithMe = false,
includeGlobal = false,
externalSecretsStore,
filters = {},
}: {
listQueryOptions?: ListQuery.Options & { includeData?: boolean };
listQueryOptions?: ListQuery.Options;
includeScopes?: boolean;
includeData?: boolean;
onlySharedWithMe?: boolean;
includeGlobal?: boolean;
externalSecretsStore?: string;
filters?: {
externalSecretsStore?: string;
};
} = {},
): Promise<Array<ICredentialsDecrypted<ICredentialDataDecryptedObject>> | CredentialsEntity[]> {
const { externalSecretsStore } = filters;
const returnAll = hasGlobalScope(user, 'credential:list');
const isDefaultSelect = !listQueryOptions.select;
const dependencyFilter = externalSecretsStore
? await this.credentialDependencyService.resolveExternalSecretsStoreDependencyFilter(
externalSecretsStore,
)
: undefined;
if (externalSecretsStore && !dependencyFilter) {
return [];
}
// Auto-enable includeScopes when includeData is requested
if (includeData) {
includeScopes = true;
listQueryOptions.includeData = true;
}
let credentials: CredentialsEntity[];
if (returnAll) {
credentials = await this.getManyForAdminUser(
user,
credentials = await this.getManyForAdminUser(user, {
listQueryOptions,
includeGlobal,
includeData,
onlySharedWithMe,
);
filters: { dependency: dependencyFilter },
});
} else {
credentials = await this.getManyForMemberUser(
user,
credentials = await this.getManyForMemberUser(user, {
listQueryOptions,
includeGlobal,
includeData,
onlySharedWithMe,
);
}
if (externalSecretsStore) {
credentials = this.filterByExternalSecretsStore(credentials, externalSecretsStore);
filters: { dependency: dependencyFilter },
});
}
return await this.enrichCredentials(
@@ -188,55 +216,37 @@ export class CredentialsService {
);
}
private filterByExternalSecretsStore(
credentials: CredentialsEntity[],
externalSecretsStore: string,
): CredentialsEntity[] {
// matches either dot notation ($secrets.providerKey) or square bracket notation ($secrets['providerKey'])
const providerRegex = /\$secrets(?:\.([A-Za-z0-9_-]+)|\[['"]([^'"]+)['"]\])/g;
credentials = credentials.filter((credential) => {
const decryptedData = this.decrypt(credential, true);
const matchingSecretPaths = getAllKeyPaths(decryptedData, '', [], (value) => {
if (!value.includes('$secrets')) {
return false;
}
let match: RegExpExecArray | null;
while ((match = providerRegex.exec(value)) !== null) {
const providerKey = match[1] ?? match[2];
if (providerKey === externalSecretsStore) {
return true;
}
}
return false;
});
return matchingSecretPaths.length > 0;
});
return credentials;
}
private async getManyForAdminUser(
user: User,
listQueryOptions: ListQuery.Options & { includeData?: boolean },
includeGlobal: boolean,
includeData: boolean,
onlySharedWithMe: boolean,
{
listQueryOptions,
includeGlobal,
includeData,
onlySharedWithMe,
filters,
}: GetManyCredentialsOptions,
): Promise<CredentialsEntity[]> {
// If onlySharedWithMe is requested, use the subquery approach even for admin users
if (onlySharedWithMe) {
const { dependency: dependencyFilter } = filters ?? {};
// If onlySharedWithMe or dependency filtering is requested, use subquery approach.
if (onlySharedWithMe || dependencyFilter) {
const sharingOptions = {
onlySharedWithMe: true,
...(onlySharedWithMe ? { onlySharedWithMe: true } : {}),
};
const { credentials } = await this.credentialsRepository.getManyAndCountWithSharingSubquery(
user,
sharingOptions,
listQueryOptions,
{
...listQueryOptions,
...(includeData ? { includeData: true } : {}),
filters: {
dependency: dependencyFilter,
},
},
);
if (includeGlobal) {
return await this.addGlobalCredentials(credentials, includeData);
return await this.addGlobalCredentials(credentials, includeData, dependencyFilter);
}
return credentials;
@@ -244,10 +254,13 @@ export class CredentialsService {
await this.applyPersonalProjectFilter(listQueryOptions);
let credentials = await this.credentialsRepository.findMany(listQueryOptions);
let credentials = await this.credentialsRepository.findMany({
...listQueryOptions,
...(includeData ? { includeData: true } : {}),
});
if (includeGlobal) {
credentials = await this.addGlobalCredentials(credentials, includeData);
credentials = await this.addGlobalCredentials(credentials, includeData, dependencyFilter);
}
return credentials;
@@ -255,11 +268,16 @@ export class CredentialsService {
private async getManyForMemberUser(
user: User,
listQueryOptions: ListQuery.Options & { includeData?: boolean },
includeGlobal: boolean,
includeData: boolean,
onlySharedWithMe: boolean,
{
listQueryOptions,
includeGlobal,
includeData,
onlySharedWithMe,
filters,
}: GetManyCredentialsOptions,
): Promise<CredentialsEntity[]> {
const { dependency: dependencyFilter } = filters ?? {};
let isPersonalProject = false;
let personalProjectOwnerId: string | null = null;
@@ -309,19 +327,23 @@ export class CredentialsService {
const { credentials } = await this.credentialsRepository.getManyAndCountWithSharingSubquery(
user,
sharingOptions,
listQueryOptions,
{
...listQueryOptions,
...(includeData ? { includeData: true } : {}),
filters: {
dependency: dependencyFilter,
},
},
);
if (includeGlobal) {
return await this.addGlobalCredentials(credentials, includeData);
return await this.addGlobalCredentials(credentials, includeData, dependencyFilter);
}
return credentials;
}
private async applyPersonalProjectFilter(
listQueryOptions: ListQuery.Options & { includeData?: boolean },
): Promise<void> {
private async applyPersonalProjectFilter(listQueryOptions: ListQuery.Options): Promise<void> {
const projectId =
typeof listQueryOptions.filter?.projectId === 'string'
? listQueryOptions.filter.projectId
@@ -350,7 +372,7 @@ export class CredentialsService {
isDefaultSelect: boolean,
includeScopes: boolean,
includeData: true,
listQueryOptions: ListQuery.Options & { includeData?: boolean },
listQueryOptions: ListQuery.Options,
onlySharedWithMe: boolean,
): Promise<Array<ICredentialsDecrypted<ICredentialDataDecryptedObject>>>;
private async enrichCredentials(
@@ -359,7 +381,7 @@ export class CredentialsService {
isDefaultSelect: boolean,
includeScopes: boolean,
includeData: boolean,
listQueryOptions: ListQuery.Options & { includeData?: boolean },
listQueryOptions: ListQuery.Options,
onlySharedWithMe: boolean,
): Promise<CredentialsEntity[]>;
private async enrichCredentials(
@@ -368,7 +390,7 @@ export class CredentialsService {
isDefaultSelect: boolean,
includeScopes: boolean,
includeData: boolean,
listQueryOptions: ListQuery.Options & { includeData?: boolean },
listQueryOptions: ListQuery.Options,
onlySharedWithMe: boolean,
): Promise<Array<ICredentialsDecrypted<ICredentialDataDecryptedObject>> | CredentialsEntity[]> {
if (isDefaultSelect) {
@@ -397,7 +419,7 @@ export class CredentialsService {
private async populateSharedRelations(
credentials: CredentialsEntity[],
listQueryOptions: ListQuery.Options & { includeData?: boolean },
listQueryOptions: ListQuery.Options,
onlySharedWithMe: boolean,
): Promise<CredentialsEntity[]> {
const needsRelations =
@@ -494,8 +516,9 @@ export class CredentialsService {
}
async findAllGlobalCredentialIds(includeData: boolean = false): Promise<CredentialsEntity[]> {
const globalCredentials =
await this.credentialsRepository.findAllGlobalCredentials(includeData);
const globalCredentials = await this.credentialsRepository.findAllGlobalCredentials({
includeData,
});
return globalCredentials;
}
@@ -656,15 +679,29 @@ export class CredentialsService {
}
}
async update(credentialId: string, newCredentialData: ICredentialsDb) {
async update(
credentialId: string,
newCredentialData: ICredentialsDb,
decryptedCredentialData?: ICredentialDataDecryptedObject,
) {
await this.externalHooks.run('credentials.update', [newCredentialData]);
// Update the credentials in DB
await this.credentialsRepository.update(credentialId, newCredentialData);
return await this.credentialsRepository.manager.transaction(async (transactionManager) => {
// Update the credentials in DB
await transactionManager.update(CredentialsEntity, credentialId, newCredentialData);
// We sadly get nothing back from "update". Neither if it updated a record
// nor the new value. So query now the updated entry.
return await this.credentialsRepository.findOneBy({ id: credentialId });
if (decryptedCredentialData) {
await this.credentialDependencyService.syncExternalSecretProviderDependenciesForCredential({
credentialId,
decryptedCredentialData,
entityManager: transactionManager,
});
}
// We sadly get nothing back from "update". Neither if it updated a record
// nor the new value. So query now the updated entry.
return await transactionManager.findOneBy(CredentialsEntity, { id: credentialId });
});
}
async save(
@@ -672,6 +709,7 @@ export class CredentialsService {
encryptedData: ICredentialsDb,
user: User,
projectId?: string,
decryptedCredentialData?: ICredentialDataDecryptedObject,
) {
// To avoid side effects
const newCredential = new CredentialsEntity();
@@ -716,6 +754,16 @@ export class CredentialsService {
await transactionManager.save<SharedCredentials>(newSharedCredential);
if (decryptedCredentialData) {
await this.credentialDependencyService.upsertExternalSecretProviderDependenciesForCredential(
{
credentialId: savedCredential.id,
decryptedCredentialData,
entityManager: transactionManager,
},
);
}
return savedCredential;
});
this.logger.debug('New credential created', {
@@ -1143,6 +1191,7 @@ export class CredentialsService {
encryptedCredential,
user,
opts.projectId,
opts.data as ICredentialDataDecryptedObject,
);
const scopes = await this.getCredentialScopes(user, credential.id);
@@ -0,0 +1,76 @@
import get from 'lodash/get';
import type { ICredentialDataDecryptedObject } from 'n8n-workflow';
import { getAllKeyPaths } from '@/utils';
/**
* Regular expression pattern for valid provider keys.
* Keep this in sync with the regex implemented in CreateSecretsProviderConnectionDto.
*/
const PROVIDER_KEY_PATTERN = '[A-Za-z0-9]+(?:-[A-Za-z0-9]+)*';
/**
* Checks if a string value contains an external secret expression.
* Detects both dot notation ($secrets.vault.key) and bracket notation ($secrets['vault']['key']).
*/
function containsExternalSecretExpression(value: string): boolean {
const containsExpression = value.includes('{{') && value.includes('}}');
if (!containsExpression) {
return false;
}
return value.includes('$secrets.') || value.includes('$secrets[');
}
export function getExternalSecretExpressionPaths(data: unknown): string[] {
return getAllKeyPaths(data, '', [], containsExternalSecretExpression);
}
/**
* Extracts the provider keys from an expression string.
* Supports both dot notation ($secrets.vault.key) and bracket notation ($secrets['vault']['key']).
* Only extracts provider keys from $secrets references inside {{ }} expression braces.
*/
export function extractProviderKeysFromExpression(expression: string): string[] {
const providerKeys = new Set<string>();
const expressionBlocks = expression.matchAll(/\{\{(.*?)\}\}/gs);
for (const expression of expressionBlocks) {
const expressionContent = expression[1]; // Content inside {{ }}
// Match all dot notation occurrences: $secrets.providerKey
const dotMatches = expressionContent.matchAll(
new RegExp(`\\$secrets\\.(${PROVIDER_KEY_PATTERN})(?=\\.)`, 'g'),
);
for (const match of dotMatches) {
providerKeys.add(match[1]);
}
// Match all bracket notation occurrences: $secrets['providerKey'] or $secrets["providerKey"]
const bracketMatches = expressionContent.matchAll(
new RegExp(`\\$secrets\\[['"](${PROVIDER_KEY_PATTERN})['"]\\]`, 'g'),
);
for (const match of bracketMatches) {
providerKeys.add(match[1]);
}
}
return Array.from(providerKeys);
}
export function extractProviderKeysFromCredentialData(
data: ICredentialDataDecryptedObject,
): ReadonlySet<string> {
const secretPaths = getExternalSecretExpressionPaths(data);
const providerKeys = new Set<string>();
for (const path of secretPaths) {
const expressionString = get(data, path);
if (typeof expressionString !== 'string') continue;
for (const providerKey of extractProviderKeysFromExpression(expressionString)) {
providerKeys.add(providerKey);
}
}
return providerKeys;
}
+9 -66
View File
@@ -2,79 +2,22 @@ import type { User } from '@n8n/db';
import get from 'lodash/get';
import { type ICredentialDataDecryptedObject } from 'n8n-workflow';
import {
extractProviderKeysFromExpression,
getExternalSecretExpressionPaths,
} from './external-secrets.utils';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import type { SecretsProviderAccessCheckService } from '@/modules/external-secrets.ee/secret-provider-access-check.service.ee';
import { userHasScopes } from '@/permissions.ee/check-access';
import { getAllKeyPaths } from '@/utils';
// #region External Secrets
/**
* Regular expression pattern for valid provider keys.
* Keep this in sync with the regex implemented in CreateSecretsProviderConnectionDto.
*/
const PROVIDER_KEY_PATTERN = '[A-Za-z0-9]+(?:-[A-Za-z0-9]+)*';
/**
* Checks if a string value contains an external secret expression.
* Detects both dot notation ($secrets.vault.key) and bracket notation ($secrets['vault']['key']).
*/
export function containsExternalSecretExpression(value: string): boolean {
const containsExpression = value.includes('{{') && value.includes('}}');
if (!containsExpression) {
return false;
}
return value.includes('$secrets.') || value.includes('$secrets[');
}
/**
* Extracts the provider keys from an expression string.
* Supports both dot notation ($secrets.vault.key) and bracket notation ($secrets['vault']['key']).
* Only extracts provider keys from $secrets references inside {{ }} expression braces.
*
* @param expression - The expression string containing $secrets reference
* @returns Array of unique provider keys, or empty array if none found
*
* @example
* extractProviderKeys("={{ $secrets.vault.myKey }}") // returns ["vault"]
* extractProviderKeys("={{ $secrets['aws']['secret'] }}") // returns ["aws"]
* extractProviderKeys("={{ $secrets.vault.myKey + ':' + $secrets.aws.otherKey }}") // returns ["vault", "aws"]
* extractProviderKeys("$secrets.vault.key") // returns [] (not inside braces)
*/
export function extractProviderKeys(expression: string): string[] {
const providerKeys = new Set<string>();
const expressionBlocks = expression.matchAll(/\{\{(.*?)\}\}/gs);
for (const expression of expressionBlocks) {
const expressionContent = expression[1]; // Content inside {{ }}
// Match all dot notation occurrences: $secrets.providerKey
const dotMatches = expressionContent.matchAll(
new RegExp(`\\$secrets\\.(${PROVIDER_KEY_PATTERN})`, 'g'),
);
for (const match of dotMatches) {
providerKeys.add(match[1]);
}
// Match all bracket notation occurrences: $secrets['providerKey'] or $secrets["providerKey"]
const bracketMatches = expressionContent.matchAll(
new RegExp(`\\$secrets\\[['"](${PROVIDER_KEY_PATTERN})['"]\\]`, 'g'),
);
for (const match of bracketMatches) {
providerKeys.add(match[1]);
}
}
return Array.from(providerKeys);
}
/**
* Checks if credential data contains any external secret expressions ($secrets)
*/
function containsExternalSecrets(data: ICredentialDataDecryptedObject): boolean {
const secretPaths = getAllKeyPaths(data, '', [], containsExternalSecretExpression);
return secretPaths.length > 0;
return getExternalSecretExpressionPaths(data).length > 0;
}
/**
@@ -85,7 +28,7 @@ export function isChangingExternalSecretExpression(
existingData: ICredentialDataDecryptedObject,
): boolean {
// Find all paths in newData that contain external secret expressions
const newSecretPaths = getAllKeyPaths(newData, '', [], containsExternalSecretExpression);
const newSecretPaths = getExternalSecretExpressionPaths(newData);
// Check if any of these paths represent a change from existingData
for (const path of newSecretPaths) {
@@ -150,7 +93,7 @@ export async function validateAccessToReferencedSecretProviders(
externalSecretsProviderAccessCheckService: SecretsProviderAccessCheckService,
source: 'create' | 'update' | 'transfer',
) {
const secretPaths = getAllKeyPaths(data, '', [], containsExternalSecretExpression);
const secretPaths = getExternalSecretExpressionPaths(data);
if (secretPaths.length === 0) {
return; // No external secrets referenced, nothing to check
}
@@ -161,7 +104,7 @@ export async function validateAccessToReferencedSecretProviders(
for (const credentialProperty of secretPaths) {
const expressionString = get(data, credentialProperty);
if (typeof expressionString === 'string') {
const providerKeys = extractProviderKeys(expressionString);
const providerKeys = extractProviderKeysFromExpression(expressionString);
if (providerKeys.length === 0) {
throw new BadRequestError(
`Could not find a valid external secret vault name inside "${expressionString}" used in "${credentialProperty}"`,
@@ -144,7 +144,7 @@ describe('CredentialsRepository', () => {
entityManager.find.mockResolvedValueOnce([globalCred]);
// ACT
const credentials = await repository.findAllGlobalCredentials(true);
const credentials = await repository.findAllGlobalCredentials({ includeData: true });
// ASSERT
expect(entityManager.find).toHaveBeenCalledWith(
@@ -176,7 +176,7 @@ describe('CredentialsRepository', () => {
entityManager.find.mockResolvedValueOnce([globalCred]);
// ACT
const credentials = await repository.findAllGlobalCredentials(false);
const credentials = await repository.findAllGlobalCredentials({ includeData: false });
// ASSERT
expect(entityManager.find).toHaveBeenCalledWith(
@@ -8,8 +8,10 @@ import type {
import { In } from '@n8n/typeorm';
import { mock } from 'jest-mock-extended';
import { CREDENTIAL_BLANKING_VALUE, type IDataObject, type INodeProperties } from 'n8n-workflow';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import type { EventService } from '@/events/event.service';
import type { CredentialDependencyService } from '@/credentials/credential-dependency.service';
import type { ExternalSecretsManager } from '@/modules/external-secrets.ee/external-secrets-manager.ee';
import type { ExternalSecretsProviderRegistry } from '@/modules/external-secrets.ee/provider-registry.service';
import type { RedactionService } from '@/modules/external-secrets.ee/redaction.service.ee';
@@ -18,6 +20,7 @@ import type { SecretsProvider } from '@/modules/external-secrets.ee/types';
describe('SecretsProvidersConnectionsService', () => {
const mockRepository = mock<SecretsProviderConnectionRepository>();
const mockProjectAccessRepository = mock<ProjectSecretsProviderAccessRepository>();
const mockCredentialDependencyService = mock<CredentialDependencyService>();
const mockExternalSecretsManager = mock<ExternalSecretsManager>();
const mockRedactionService = mock<RedactionService>();
const mockProviderRegistry = mock<ExternalSecretsProviderRegistry>();
@@ -31,6 +34,7 @@ describe('SecretsProvidersConnectionsService', () => {
mockLogger(),
mockRepository,
mockProjectAccessRepository,
mockCredentialDependencyService,
mockProviderRegistry,
mockCipher as any,
mockExternalSecretsManager,
@@ -387,11 +391,31 @@ describe('SecretsProvidersConnectionsService', () => {
});
it('should sync provider connection after deleteConnection', async () => {
const entityManager = {
delete: jest.fn().mockResolvedValue(undefined),
};
const transaction = jest.fn(
async (fn: (em: typeof entityManager) => Promise<void>) => await fn(entityManager),
);
Object.defineProperty(mockRepository, 'manager', {
value: { transaction },
configurable: true,
});
mockRepository.findOne.mockResolvedValueOnce(savedConnection);
mockRepository.remove.mockResolvedValueOnce(savedConnection);
await service.deleteConnection('my-aws', 'user-123');
expect(mockProjectAccessRepository.deleteByConnectionId).toHaveBeenCalledWith(
1,
entityManager,
);
expect(mockCredentialDependencyService.deleteDependencyById).toHaveBeenCalledWith({
dependencyType: 'externalSecretProvider',
dependencyId: '1',
entityManager,
});
expect(entityManager.delete).toHaveBeenCalledWith(mockRepository.target, { id: 1 });
expect(mockExternalSecretsManager.syncProviderConnection).toHaveBeenCalledWith('my-aws');
});
});
@@ -429,10 +453,15 @@ describe('SecretsProvidersConnectionsService', () => {
await service.cleanupConnectionsForProjectDeletion('project-1');
expect(transaction).toHaveBeenCalledTimes(1);
expect(entityManager.delete).toHaveBeenNthCalledWith(1, mockRepository.target, {
expect(mockCredentialDependencyService.deleteDependenciesByIds).toHaveBeenCalledWith({
dependencyType: 'externalSecretProvider',
dependencyIds: ['1'],
entityManager,
});
expect(entityManager.delete).toHaveBeenCalledWith(mockRepository.target, {
id: In([1]),
});
expect(entityManager.delete).toHaveBeenNthCalledWith(2, mockProjectAccessRepository.target, {
expect(entityManager.delete).toHaveBeenCalledWith(mockProjectAccessRepository.target, {
projectId: 'project-1',
secretsProviderConnectionId: In([2]),
});
@@ -449,6 +478,72 @@ describe('SecretsProvidersConnectionsService', () => {
expect(mockExternalSecretsManager.syncProviderConnection).toHaveBeenCalledWith('provider-a');
expect(mockExternalSecretsManager.syncProviderConnection).toHaveBeenCalledWith('provider-b');
});
it('deletes credential dependencies for owner connections in a single bulk call', async () => {
const entityManager = {
delete: jest.fn().mockResolvedValue(undefined),
update: jest.fn().mockResolvedValue(undefined),
};
const transaction = jest.fn(
async (callback: (em: typeof entityManager) => Promise<void>) =>
await callback(entityManager),
);
Object.defineProperty(mockRepository, 'manager', {
value: { transaction },
configurable: true,
});
mockProjectAccessRepository.findByProjectId.mockResolvedValue([
mock<ProjectSecretsProviderAccess>({
projectId: 'project-1',
role: 'secretsProviderConnection:owner',
secretsProviderConnectionId: 10,
secretsProviderConnection: { providerKey: 'provider-a' },
}),
mock<ProjectSecretsProviderAccess>({
projectId: 'project-1',
role: 'secretsProviderConnection:owner',
secretsProviderConnectionId: 11,
secretsProviderConnection: { providerKey: 'provider-b' },
}),
]);
await service.cleanupConnectionsForProjectDeletion('project-1');
expect(mockCredentialDependencyService.deleteDependenciesByIds).toHaveBeenCalledWith({
dependencyType: 'externalSecretProvider',
dependencyIds: ['10', '11'],
entityManager,
});
});
it('does not delete credential dependencies when there are no owner connections', async () => {
const entityManager = {
delete: jest.fn().mockResolvedValue(undefined),
update: jest.fn().mockResolvedValue(undefined),
};
const transaction = jest.fn(
async (callback: (em: typeof entityManager) => Promise<void>) =>
await callback(entityManager),
);
Object.defineProperty(mockRepository, 'manager', {
value: { transaction },
configurable: true,
});
mockProjectAccessRepository.findByProjectId.mockResolvedValue([
mock<ProjectSecretsProviderAccess>({
projectId: 'project-1',
role: 'secretsProviderConnection:user',
secretsProviderConnectionId: 12,
secretsProviderConnection: { providerKey: 'provider-c' },
}),
]);
await service.cleanupConnectionsForProjectDeletion('project-1');
expect(mockCredentialDependencyService.deleteDependenciesByIds).not.toHaveBeenCalled();
});
});
describe('event emissions', () => {
@@ -678,31 +773,38 @@ describe('SecretsProvidersConnectionsService', () => {
} as unknown as SecretsProviderConnection;
it('should delete connection and sync provider when found', async () => {
mockRepository.removeByProviderKeyAndProjectId.mockResolvedValue(deletedConnection);
mockRepository.findByProviderKeyAndProjectId.mockResolvedValue(deletedConnection);
mockRepository.delete.mockResolvedValue({} as never);
const result = await service.deleteConnectionForProject('my-aws', 'project-1');
expect(result).toBe(deletedConnection);
expect(mockRepository.removeByProviderKeyAndProjectId).toHaveBeenCalledWith(
expect(mockRepository.findByProviderKeyAndProjectId).toHaveBeenCalledWith(
'my-aws',
'project-1',
);
expect(mockCredentialDependencyService.deleteDependencyById).toHaveBeenCalledWith({
dependencyType: 'externalSecretProvider',
dependencyId: '1',
});
expect(mockProjectAccessRepository.deleteByConnectionId).toHaveBeenCalledWith(1);
expect(mockRepository.delete).toHaveBeenCalledWith({ id: 1 });
expect(mockExternalSecretsManager.syncProviderConnection).toHaveBeenCalledWith('my-aws');
});
it('should throw NotFoundError when connection does not exist', async () => {
mockRepository.removeByProviderKeyAndProjectId.mockResolvedValue(null);
mockRepository.findByProviderKeyAndProjectId.mockResolvedValue(null);
await expect(service.deleteConnectionForProject('missing', 'project-1')).rejects.toThrow(
NotFoundError,
);
expect(mockCredentialDependencyService.deleteDependencyById).not.toHaveBeenCalled();
expect(mockProjectAccessRepository.deleteByConnectionId).not.toHaveBeenCalled();
expect(mockExternalSecretsManager.syncProviderConnection).not.toHaveBeenCalled();
});
it('should throw NotFoundError when connection does not belong to the project', async () => {
mockRepository.removeByProviderKeyAndProjectId.mockResolvedValue(null);
mockRepository.findByProviderKeyAndProjectId.mockResolvedValue(null);
await expect(service.deleteConnectionForProject('my-aws', 'other-project')).rejects.toThrow(
NotFoundError,
@@ -21,6 +21,12 @@ import { Cipher } from 'n8n-core';
import type { IDataObject } from 'n8n-workflow';
import { jsonParse } from 'n8n-workflow';
import { ExternalSecretsProviderRegistry } from './provider-registry.service';
import {
CredentialDependencyService,
EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
} from '@/credentials/credential-dependency.service';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { EventService } from '@/events/event.service';
@@ -28,14 +34,13 @@ import type { ProjectSummary } from '@/events/maps/relay.event-map';
import { ExternalSecretsManager } from '@/modules/external-secrets.ee/external-secrets-manager.ee';
import { RedactionService } from '@/modules/external-secrets.ee/redaction.service.ee';
import { ExternalSecretsProviderRegistry } from './provider-registry.service';
@Service()
export class SecretsProvidersConnectionsService {
constructor(
private readonly logger: Logger,
private readonly repository: SecretsProviderConnectionRepository,
private readonly projectAccessRepository: ProjectSecretsProviderAccessRepository,
private readonly credentialDependencyService: CredentialDependencyService,
private readonly providerRegistry: ExternalSecretsProviderRegistry,
private readonly cipher: Cipher,
private readonly externalSecretsManager: ExternalSecretsManager,
@@ -201,9 +206,17 @@ export class SecretsProvidersConnectionsService {
async deleteConnection(providerKey: string, userId: string): Promise<SecretsProviderConnection> {
const connection = await this.findConnectionOrFail(providerKey);
const projectInfo = this.extractProjectInfo(connection);
const dependencyId = connection.id.toString();
await this.projectAccessRepository.deleteByConnectionId(connection.id);
await this.repository.remove(connection);
await this.repository.manager.transaction(async (entityManager) => {
await this.projectAccessRepository.deleteByConnectionId(connection.id, entityManager);
await this.credentialDependencyService.deleteDependencyById({
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyId,
entityManager,
});
await entityManager.delete(this.repository.target, { id: connection.id });
});
await this.externalSecretsManager.syncProviderConnection(providerKey);
@@ -406,6 +419,12 @@ export class SecretsProvidersConnectionsService {
// Wrap deletion + update ops in a transaction for consistency
await this.repository.manager.transaction(async (entityManager) => {
if (ownerConnectionIds.size > 0) {
await this.credentialDependencyService.deleteDependenciesByIds({
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyIds: [...ownerConnectionIds].map((id) => id.toString()),
entityManager,
});
// Delete owned connections entirely; DB cascade removes access entries
await entityManager.delete(this.repository.target, { id: In([...ownerConnectionIds]) });
}
@@ -466,16 +485,19 @@ export class SecretsProvidersConnectionsService {
providerKey: string,
projectId: string,
): Promise<SecretsProviderConnection> {
const connection = await this.repository.removeByProviderKeyAndProjectId(
providerKey,
projectId,
);
const connection = await this.repository.findByProviderKeyAndProjectId(providerKey, projectId);
if (!connection) {
throw new NotFoundError(`Connection with key "${providerKey}" not found`);
}
await this.projectAccessRepository.deleteByConnectionId(connection.id);
const connectionId = connection.id;
await this.credentialDependencyService.deleteDependencyById({
dependencyType: EXTERNAL_SECRET_PROVIDER_DEPENDENCY_TYPE,
dependencyId: connectionId.toString(),
});
await this.projectAccessRepository.deleteByConnectionId(connectionId);
await this.repository.delete({ id: connectionId });
await this.externalSecretsManager.syncProviderConnection(providerKey);
return connection;
@@ -6,14 +6,14 @@ import { Cipher } from 'n8n-core';
import type { InstanceSettings } from 'n8n-core';
import type { GenericValue, IDataObject, INodeProperties } from 'n8n-workflow';
import { buildSharedForCredential, toJsonSchema, updateCredential } from '../credentials.service';
import { CredentialsService } from '@/credentials/credentials.service';
import { ExternalSecretsConfig } from '@/modules/external-secrets.ee/external-secrets.config';
import { SecretsProviderAccessCheckService } from '@/modules/external-secrets.ee/secret-provider-access-check.service.ee';
import * as checkAccess from '@/permissions.ee/check-access';
import type { IDependency } from '@/public-api/types';
import { buildSharedForCredential, toJsonSchema, updateCredential } from '../credentials.service';
// Set up real Cipher with mocked InstanceSettings for encryption
const cipher = new Cipher(mock<InstanceSettings>({ encryptionKey: 'test-encryption-key' }));
Container.set(Cipher, cipher);
@@ -417,6 +417,7 @@ describe('CredentialsService', () => {
const credentialsService = new CredentialsService(
mock(), // credentialsRepository
mock(),
mock(), // sharedCredentialsRepository
mock(), // ownershipService
mock(), // logger
@@ -492,7 +493,7 @@ describe('CredentialsService', () => {
await expect(
updateCredential('cred-id', memberUser, {
data: { apiKey: '{{ $secrets.myKey }}' },
data: { apiKey: '{{ $secrets.vault.myKey }}' },
}),
).rejects.toThrow('Lacking permissions to reference external secrets in credentials');
});
@@ -513,11 +514,11 @@ describe('CredentialsService', () => {
// Mock credential that already has secret expression
jest
.mocked(credentialsService.decrypt)
.mockReturnValue({ apiKey: '{{ $secrets.oldKey }}' });
.mockReturnValue({ apiKey: '{{ $secrets.vault.oldKey }}' });
await expect(
updateCredential('cred-id', memberUser, {
data: { apiKey: '{{ $secrets.newKey }}' },
data: { apiKey: '{{ $secrets.vault.newKey }}' },
}),
).rejects.toThrow('Lacking permissions to reference external secrets in credentials');
});
@@ -566,7 +567,9 @@ describe('CredentialsService', () => {
credentialsRepository.update = jest.fn().mockResolvedValue(undefined);
// Mock credential that has existing secret expression
jest.mocked(credentialsService.decrypt).mockReturnValue({ apiKey: '{{ $secrets.myKey }}' });
jest
.mocked(credentialsService.decrypt)
.mockReturnValue({ apiKey: '{{ $secrets.vault.myKey }}' });
credentialsRepository.update = jest.fn().mockResolvedValue(undefined);
@@ -615,7 +618,7 @@ describe('CredentialsService', () => {
credentialsRepository.update = jest.fn().mockResolvedValue(undefined);
await updateCredential('cred-id', ownerUser, {
data: { apiKey: '{{ $secrets.myKey }}' },
data: { apiKey: '{{ $secrets.vault.myKey }}' },
});
});
});
@@ -105,6 +105,46 @@ describe('CredentialsTester', () => {
expect(redactedMessage.message).toBe('Test failed for apiKey *****key');
});
it('should redact secrets for bracket-notation external secret expressions', async () => {
mockTestFunction.mockResolvedValue({
status: 'Error',
message: 'Test failed for apiKey secret_api_key',
});
const computedCredentialsData = {
testNestedData: {
access_token: 'abc123',
secretData: {
apiKey: 'secret_api_key',
},
},
};
credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue(computedCredentialsData);
const rawCredentialsData = {
...computedCredentialsData,
testNestedData: {
...computedCredentialsData.testNestedData,
secretData: {
apiKey: "={{ $secrets['vault']['apiKey'] }}",
},
},
};
const redactedMessage = await credentialsTester.testCredentials(
'user-id',
'testCredentials',
{
id: 'credential-id',
name: 'credential-name',
type: 'oAuth2Api',
data: rawCredentialsData,
},
);
expect(redactedMessage.status).toBe('Error');
expect(redactedMessage.message).toBe('Test failed for apiKey *****key');
});
it('should not redact secrets with value shorter than 3 characters', async () => {
mockTestFunction.mockResolvedValue({
status: 'Error',
@@ -35,11 +35,11 @@ import {
} from 'n8n-workflow';
import { RESPONSE_ERROR_MESSAGES } from '../constants';
import { getExternalSecretExpressionPaths } from '../credentials/external-secrets.utils';
import { CredentialsHelper } from '../credentials-helper';
import { CredentialTypes } from '@/credential-types';
import { NodeTypes } from '@/node-types';
import { getAllKeyPaths } from '@/utils';
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
const { OAUTH2_CREDENTIAL_TEST_SUCCEEDED, OAUTH2_CREDENTIAL_TEST_FAILED } = RESPONSE_ERROR_MESSAGES;
@@ -211,9 +211,7 @@ export class CredentialsTester {
});
// Keep all credentials data keys which have a secret value
credentialsDataSecretKeys = getAllKeyPaths(credentialsDecrypted.data, '', [], (value) =>
value.includes('$secrets.'),
);
credentialsDataSecretKeys = getExternalSecretExpressionPaths(credentialsDecrypted.data);
credentialsDecrypted.data = await this.credentialsHelper.applyDefaultsAndOverwrites(
additionalData,
credentialsDecrypted.data,
@@ -0,0 +1,217 @@
import {
createTestMigrationContext,
initDbUpToMigration,
runSingleMigration,
undoLastSingleMigration,
type TestMigrationContext,
} from '@n8n/backend-test-utils';
import { DbConnection } from '@n8n/db';
import { Container } from '@n8n/di';
import { DataSource } from '@n8n/typeorm';
import { Cipher } from 'n8n-core';
import { randomUUID } from 'node:crypto';
const MIGRATION_NAME = 'CreateCredentialDependencyTable1773000000000';
const DEPENDENCY_TYPE = 'externalSecretProvider';
type CredentialDependencyRow = {
credentialId: string;
dependencyType: string;
dependencyId: string;
};
describe('CreateCredentialDependencyTable Migration', () => {
let dataSource: DataSource;
let cipher: Cipher;
jest.setTimeout(20_000);
async function withContext<T>(fn: (context: TestMigrationContext) => Promise<T>): Promise<T> {
const context = createTestMigrationContext(dataSource);
try {
return await fn(context);
} finally {
await context.queryRunner.release();
}
}
beforeAll(async () => {
const dbConnection = Container.get(DbConnection);
await dbConnection.init();
dataSource = Container.get(DataSource);
cipher = Container.get(Cipher);
});
beforeEach(async () => {
await withContext(async (context) => {
await context.queryRunner.clearDatabase();
});
await initDbUpToMigration(MIGRATION_NAME);
});
afterAll(async () => {
const dbConnection = Container.get(DbConnection);
await dbConnection.close();
});
async function insertProviderConnection(
context: TestMigrationContext,
data: { providerKey: string; type?: string },
): Promise<void> {
const tableName = context.escape.tableName('secrets_provider_connection');
const now = new Date();
await context.runQuery(
`INSERT INTO ${tableName} ("providerKey", "type", "encryptedSettings", "isEnabled", "createdAt", "updatedAt")
VALUES (:providerKey, :type, :encryptedSettings, :isEnabled, :createdAt, :updatedAt)`,
{
providerKey: data.providerKey,
type: data.type ?? data.providerKey,
encryptedSettings: cipher.encrypt(JSON.stringify({ region: 'us-east-1' })),
isEnabled: true,
createdAt: now,
updatedAt: now,
},
);
}
async function getProviderIdByKey(
context: TestMigrationContext,
providerKey: string,
): Promise<string | undefined> {
const tableName = context.escape.tableName('secrets_provider_connection');
const rows = await context.runQuery<Array<{ id: number }>>(
`SELECT "id" AS "id" FROM ${tableName} WHERE "providerKey" = :providerKey`,
{ providerKey },
);
return rows[0]?.id.toString();
}
async function insertCredential(
context: TestMigrationContext,
data: { id: string; encryptedData: string },
): Promise<void> {
const tableName = context.escape.tableName('credentials_entity');
const now = new Date();
await context.runQuery(
`INSERT INTO ${tableName} ("id", "name", "data", "type", "createdAt", "updatedAt")
VALUES (:id, :name, :data, :type, :createdAt, :updatedAt)`,
{
id: data.id,
name: `cred-${data.id.slice(0, 8)}`,
data: data.encryptedData,
type: 'testApi',
createdAt: now,
updatedAt: now,
},
);
}
async function getDependencies(
context: TestMigrationContext,
): Promise<CredentialDependencyRow[]> {
const tableName = context.escape.tableName('credential_dependency');
return await context.runQuery<CredentialDependencyRow[]>(
`SELECT "credentialId" AS "credentialId", "dependencyType" AS "dependencyType", "dependencyId" AS "dependencyId"
FROM ${tableName}
ORDER BY "credentialId", "dependencyId"`,
);
}
describe('up migration', () => {
it('should create table and backfill dependencies from credential expressions', async () => {
const credentialWithSecretsId = randomUUID();
const credentialWithoutSecretsId = randomUUID();
const credentialUnknownProviderId = randomUUID();
const { vaultProviderId, awsProviderId } = await withContext(async (context) => {
await insertProviderConnection(context, { providerKey: 'vault' });
await insertProviderConnection(context, { providerKey: 'aws-secrets-manager' });
const vaultProviderId = await getProviderIdByKey(context, 'vault');
const awsProviderId = await getProviderIdByKey(context, 'aws-secrets-manager');
expect(vaultProviderId).toBeDefined();
expect(awsProviderId).toBeDefined();
await insertCredential(context, {
id: credentialWithSecretsId,
encryptedData: cipher.encrypt(
JSON.stringify({
apiKey: '={{ $secrets.vault.primaryKey }}',
nested: {
token: "={{ $secrets['aws-secrets-manager']['token'] }}",
},
repeated: '={{ $secrets.vault.secondary + ":" + $secrets.vault.third }}',
}),
),
});
await insertCredential(context, {
id: credentialWithoutSecretsId,
encryptedData: cipher.encrypt(JSON.stringify({ apiKey: 'plain-value' })),
});
await insertCredential(context, {
id: credentialUnknownProviderId,
encryptedData: cipher.encrypt(JSON.stringify({ apiKey: '={{ $secrets.unknown.key }}' })),
});
return { vaultProviderId, awsProviderId };
});
await runSingleMigration(MIGRATION_NAME);
dataSource = Container.get(DataSource);
const dependencies = await withContext(async (context) => await getDependencies(context));
expect(dependencies).toHaveLength(2);
expect(dependencies).toEqual(
expect.arrayContaining([
{
credentialId: credentialWithSecretsId,
dependencyType: DEPENDENCY_TYPE,
dependencyId: vaultProviderId!,
},
{
credentialId: credentialWithSecretsId,
dependencyType: DEPENDENCY_TYPE,
dependencyId: awsProviderId!,
},
]),
);
});
it('should skip credentials that cannot be decrypted', async () => {
const invalidCredentialId = randomUUID();
await withContext(async (context) => {
await insertProviderConnection(context, { providerKey: 'vault' });
await insertCredential(context, {
id: invalidCredentialId,
encryptedData: 'not-encrypted-data',
});
});
await runSingleMigration(MIGRATION_NAME);
dataSource = Container.get(DataSource);
const dependencies = await withContext(async (context) => await getDependencies(context));
expect(dependencies).toHaveLength(0);
});
});
describe('down migration', () => {
it('should drop credential_dependency table', async () => {
await runSingleMigration(MIGRATION_NAME);
await undoLastSingleMigration();
await withContext(async (context) => {
const dependencyTableName = `${context.tablePrefix}credential_dependency`;
const hasDependencyTable = await context.queryRunner.hasTable(dependencyTableName);
expect(hasDependencyTable).toBe(false);
});
});
});
});