mirror of
https://github.com/n8n-io/n8n.git
synced 2026-06-19 07:36:52 +00:00
fix(core): Check resolver-less credentials under each workflow resolver (no-changelog)
A sub-workflow resolves credentials using its own credentialResolverId, so a credential without its own resolverId can resolve differently in a parent vs a sub-workflow. Dedup the status check by (credentialId, effective resolver) instead of credentialId alone, checking such a credential once per distinct fallback resolver to avoid a false "configured" status. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+50
@@ -791,6 +791,56 @@ describe('CredentialResolverWorkflowService', () => {
|
||||
expect(result[0].resolverId).toBe('resolver-2');
|
||||
});
|
||||
|
||||
it('checks a shared resolver-less credential under each workflow effective resolver', async () => {
|
||||
// Same credential (no own resolverId) used in a parent and a sub-workflow that
|
||||
// override the resolver differently: it must be checked under both, since at
|
||||
// execution time each workflow resolves it with its own effective resolver.
|
||||
mockWorkflowsById({
|
||||
'workflow-1': createMockWorkflow({
|
||||
id: 'workflow-1',
|
||||
nodes: [
|
||||
nodeWithCredential('cred-shared'),
|
||||
createExecuteWorkflowNode({ value: 'sub-1' }),
|
||||
],
|
||||
settings: { credentialResolverId: 'resolver-1' },
|
||||
}),
|
||||
'sub-1': createMockWorkflow({
|
||||
id: 'sub-1',
|
||||
nodes: [nodeWithCredential('cred-shared')],
|
||||
settings: { credentialResolverId: 'resolver-2' },
|
||||
}),
|
||||
});
|
||||
mockFindReturning([createMockCredential({ id: 'cred-shared', resolverId: null })]);
|
||||
mockResolverRepository.findOneBy.mockImplementation(async ({ id }: { id: string }) =>
|
||||
createMockResolver({ id, type: 'test.resolver' }),
|
||||
);
|
||||
// Configured under resolver-1, missing under resolver-2.
|
||||
mockResolverImplementation.getSecret.mockImplementation(async (_id, _ctx, opts) => {
|
||||
if (opts.resolverId === 'resolver-2') {
|
||||
throw new Error('Secret not found');
|
||||
}
|
||||
return 'secret' as any;
|
||||
});
|
||||
|
||||
const result = await service.getWorkflowStatus('workflow-1', credentialContext);
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
credentialId: 'cred-shared',
|
||||
resolverId: 'resolver-1',
|
||||
status: 'configured',
|
||||
}),
|
||||
expect.objectContaining({
|
||||
credentialId: 'cred-shared',
|
||||
resolverId: 'resolver-2',
|
||||
status: 'missing',
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it('enforces user access on the root but resolves sub-workflows by id', async () => {
|
||||
const user = { id: 'user-1' } as unknown as Parameters<typeof service.getWorkflowStatus>[2];
|
||||
mockWorkflowFinderService.findWorkflowForUser.mockResolvedValue(
|
||||
|
||||
+24
-9
@@ -96,9 +96,11 @@ export class CredentialResolverWorkflowService {
|
||||
user?: User,
|
||||
): Promise<CredentialStatus[]> {
|
||||
const visited = new Set<string>();
|
||||
// credentialId -> effective resolverId of the workflow it was first found in (fallback only;
|
||||
// a credential-level resolverId still takes priority during the status check).
|
||||
const credentialFallbackResolvers = new Map<string, string | null>();
|
||||
// credentialId -> set of distinct effective resolverIds of the workflows it appears in
|
||||
// (fallback only; a credential-level resolverId still takes priority during the status
|
||||
// check). A credential without its own resolverId resolves under the resolver of whichever
|
||||
// workflow contains it, so it must be checked once per distinct fallback.
|
||||
const credentialFallbackResolvers = new Map<string, Set<string | null>>();
|
||||
|
||||
await this.collectResolvableCredentials(
|
||||
workflowId,
|
||||
@@ -122,10 +124,20 @@ export class CredentialResolverWorkflowService {
|
||||
// Fetch/decrypt each distinct resolver at most once across the whole tree.
|
||||
const resolverCache = new Map<string, ResolvedResolver>();
|
||||
|
||||
const credentialStatusPromises = credentials.map(
|
||||
async (credential) =>
|
||||
// A credential with its own resolverId always resolves the same way, so check it once.
|
||||
// Otherwise check it once per distinct fallback resolver, since the same credential can
|
||||
// resolve differently in a parent vs a sub-workflow that override the resolver.
|
||||
const credentialChecks = credentials.flatMap((credential) => {
|
||||
const fallbacks = credential.resolverId
|
||||
? [null]
|
||||
: [...(credentialFallbackResolvers.get(credential.id) ?? [null])];
|
||||
return fallbacks.map((fallbackResolverId) => ({ credential, fallbackResolverId }));
|
||||
});
|
||||
|
||||
const credentialStatusPromises = credentialChecks.map(
|
||||
async ({ credential, fallbackResolverId }) =>
|
||||
await this.checkCredentialStatus(credential, {
|
||||
fallbackResolverId: credentialFallbackResolvers.get(credential.id) ?? null,
|
||||
fallbackResolverId,
|
||||
credentialContext,
|
||||
resolverCache,
|
||||
}),
|
||||
@@ -141,7 +153,7 @@ export class CredentialResolverWorkflowService {
|
||||
*/
|
||||
private async collectResolvableCredentials(
|
||||
workflowId: string,
|
||||
acc: Map<string, string | null>,
|
||||
acc: Map<string, Set<string | null>>,
|
||||
visited: Set<string>,
|
||||
user: User | undefined,
|
||||
isRoot: boolean,
|
||||
@@ -178,9 +190,12 @@ export class CredentialResolverWorkflowService {
|
||||
for (const node of workflow.nodes ?? []) {
|
||||
for (const credentialName in node.credentials ?? {}) {
|
||||
const credentialId = node.credentials?.[credentialName]?.id;
|
||||
if (credentialId && !acc.has(credentialId)) {
|
||||
acc.set(credentialId, resolverId);
|
||||
if (!credentialId) {
|
||||
continue;
|
||||
}
|
||||
const fallbacks = acc.get(credentialId) ?? new Set<string | null>();
|
||||
fallbacks.add(resolverId);
|
||||
acc.set(credentialId, fallbacks);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user