Skip to content

Commit 634041c

Browse files
afitzekafitzek
authored
feat(core): Track dynamic credential resolution per node execution in ITaskData (#26354)
1 parent d00a55f commit 634041c

11 files changed

Lines changed: 110 additions & 45 deletions

File tree

‎packages/cli/src/__tests__/credentials-helper.test.ts‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,10 @@ describe('CredentialsHelper', () => {
619619
dynamicCredentialProxy.setResolverProvider(mockCredentialResolutionProvider);
620620

621621
const resolvedData = { apiKey: 'dynamic-key' };
622-
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue(resolvedData);
622+
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue({
623+
data: resolvedData,
624+
isDynamic: true,
625+
});
623626
credentialsRepository.findOneByOrFail.mockResolvedValue(mockCredentialEntity);
624627

625628
const result = await credentialsHelper.getDecrypted(
@@ -650,7 +653,10 @@ describe('CredentialsHelper', () => {
650653

651654
test('should pass executionContext from additionalData to resolver', async () => {
652655
dynamicCredentialProxy.setResolverProvider(mockCredentialResolutionProvider);
653-
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue({ apiKey: 'resolved' });
656+
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue({
657+
data: { apiKey: 'resolved' },
658+
isDynamic: false,
659+
});
654660

655661
await credentialsHelper.getDecrypted(
656662
mockAdditionalData,
@@ -668,7 +674,10 @@ describe('CredentialsHelper', () => {
668674

669675
test('should pass workflowSettings from additionalData to resolver', async () => {
670676
dynamicCredentialProxy.setResolverProvider(mockCredentialResolutionProvider);
671-
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue({ apiKey: 'resolved' });
677+
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue({
678+
data: { apiKey: 'resolved' },
679+
isDynamic: false,
680+
});
672681

673682
await credentialsHelper.getDecrypted(
674683
mockAdditionalData,
@@ -713,7 +722,10 @@ describe('CredentialsHelper', () => {
713722
dynamicCredentialProxy.setResolverProvider(mockCredentialResolutionProvider);
714723

715724
const dynamicData = { apiKey: 'dynamic-key', extraField: 'extra-value' };
716-
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue(dynamicData);
725+
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue({
726+
data: dynamicData,
727+
isDynamic: true,
728+
});
717729

718730
const result = await credentialsHelper.getDecrypted(
719731
mockAdditionalData,
@@ -730,7 +742,10 @@ describe('CredentialsHelper', () => {
730742

731743
test('should skip resolution when executionContext is missing', async () => {
732744
dynamicCredentialProxy.setResolverProvider(mockCredentialResolutionProvider);
733-
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue({ apiKey: 'resolved' });
745+
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue({
746+
data: { apiKey: 'resolved' },
747+
isDynamic: false,
748+
});
734749

735750
const additionalDataWithoutContext = {
736751
...mockAdditionalData,
@@ -752,7 +767,10 @@ describe('CredentialsHelper', () => {
752767

753768
test('should skip resolution when credentials context is missing', async () => {
754769
dynamicCredentialProxy.setResolverProvider(mockCredentialResolutionProvider);
755-
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue({ apiKey: 'resolved' });
770+
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue({
771+
data: { apiKey: 'resolved' },
772+
isDynamic: false,
773+
});
756774

757775
const additionalDataWithoutCredentials = {
758776
...mockAdditionalData,
@@ -781,7 +799,10 @@ describe('CredentialsHelper', () => {
781799

782800
test('should handle missing workflowSettings gracefully', async () => {
783801
dynamicCredentialProxy.setResolverProvider(mockCredentialResolutionProvider);
784-
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue({ apiKey: 'resolved' });
802+
mockCredentialResolutionProvider.resolveIfNeeded.mockResolvedValue({
803+
data: { apiKey: 'resolved' },
804+
isDynamic: false,
805+
});
785806

786807
const additionalDataWithoutSettings = {
787808
...mockAdditionalData,

‎packages/cli/src/credentials-helper.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ export class CredentialsHelper extends ICredentialsHelper {
367367
*/
368368
if (additionalData.executionContext?.credentials !== undefined) {
369369
// Resolve dynamic credentials if configured (EE feature)
370-
decryptedDataOriginal = await this.dynamicCredentialsProxy.resolveIfNeeded(
370+
const resolveResult = await this.dynamicCredentialsProxy.resolveIfNeeded(
371371
{
372372
id: credentialsEntity.id,
373373
name: credentialsEntity.name,
@@ -381,6 +381,10 @@ export class CredentialsHelper extends ICredentialsHelper {
381381
additionalData.workflowSettings,
382382
canUseExternalSecrets,
383383
);
384+
decryptedDataOriginal = resolveResult.data;
385+
if (resolveResult.isDynamic) {
386+
additionalData.currentNodeUsedDynamicCredentials = true;
387+
}
384388
}
385389

386390
if (raw === true) {

‎packages/cli/src/credentials/__tests__/dynamic-credentials-proxy.test.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type {
77
} from 'n8n-workflow';
88

99
import type {
10+
CredentialResolutionResult,
1011
CredentialResolveMetadata,
1112
ICredentialResolutionProvider,
1213
} from '../credential-resolution-provider.interface';
@@ -53,7 +54,7 @@ describe('DynamicCredentialsProxy', () => {
5354
it('should return static data when no provider is set and credential is not resolvable', async () => {
5455
const result = await proxy.resolveIfNeeded(credentialMetadata, staticData);
5556

56-
expect(result).toBe(staticData);
57+
expect(result).toEqual({ data: staticData, isDynamic: false });
5758
expect(mockLogger.warn).not.toHaveBeenCalled();
5859
});
5960

@@ -70,14 +71,17 @@ describe('DynamicCredentialsProxy', () => {
7071
});
7172

7273
it('should delegate to provider when set', async () => {
73-
const dynamicData = { token: 'dynamic-token' };
74-
mockResolverProvider.resolveIfNeeded.mockResolvedValue(dynamicData);
74+
const dynamicResult: CredentialResolutionResult = {
75+
data: { token: 'dynamic-token' },
76+
isDynamic: true,
77+
};
78+
mockResolverProvider.resolveIfNeeded.mockResolvedValue(dynamicResult);
7579

7680
proxy.setResolverProvider(mockResolverProvider);
7781

7882
const result = await proxy.resolveIfNeeded(credentialMetadata, staticData);
7983

80-
expect(result).toBe(dynamicData);
84+
expect(result).toBe(dynamicResult);
8185
expect(mockResolverProvider.resolveIfNeeded).toHaveBeenCalledWith(
8286
credentialMetadata,
8387
staticData,
@@ -98,7 +102,10 @@ describe('DynamicCredentialsProxy', () => {
98102
};
99103
const canUseExternalSecrets = true;
100104

101-
mockResolverProvider.resolveIfNeeded.mockResolvedValue(staticData);
105+
mockResolverProvider.resolveIfNeeded.mockResolvedValue({
106+
data: staticData,
107+
isDynamic: false,
108+
});
102109
proxy.setResolverProvider(mockResolverProvider);
103110

104111
await proxy.resolveIfNeeded(

‎packages/cli/src/credentials/credential-resolution-provider.interface.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,12 @@ export type CredentialResolveMetadata = {
1414
isResolvable: boolean;
1515
};
1616

17+
export type CredentialResolutionResult = {
18+
data: ICredentialDataDecryptedObject;
19+
/** True only when the credential was actually resolved via a dynamic resolver (not a fallback to static data). */
20+
isDynamic: boolean;
21+
};
22+
1723
/**
1824
* Interface for credential resolution providers.
1925
* Implementations can provide dynamic credential resolution logic.
@@ -27,13 +33,13 @@ export interface ICredentialResolutionProvider {
2733
* @param staticData The decrypted static credential data
2834
* @param additionalData Additional workflow execution data for context and settings
2935
* @param canUseExternalSecrets Whether the credential can use external secrets for expression resolution
30-
* @returns Resolved credential data (either dynamic or static)
36+
* @returns Resolved credential data and a flag indicating whether dynamic resolution occurred
3137
*/
3238
resolveIfNeeded(
3339
credentialsResolveMetadata: CredentialResolveMetadata,
3440
staticData: ICredentialDataDecryptedObject,
3541
executionContext?: IExecutionContext,
3642
workflowSettings?: IWorkflowSettings,
3743
canUseExternalSecrets?: boolean,
38-
): Promise<ICredentialDataDecryptedObject>;
44+
): Promise<CredentialResolutionResult>;
3945
}

‎packages/cli/src/credentials/dynamic-credentials-proxy.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import type {
1111
import { toCredentialContext, UnexpectedError } from 'n8n-workflow';
1212

1313
import type {
14+
CredentialResolutionResult,
1415
CredentialResolveMetadata,
1516
ICredentialResolutionProvider,
1617
} from './credential-resolution-provider.interface';
@@ -42,15 +43,15 @@ export class DynamicCredentialsProxy
4243
executionContext?: IExecutionContext,
4344
workflowSettings?: IWorkflowSettings,
4445
canUseExternalSecrets?: boolean,
45-
): Promise<ICredentialDataDecryptedObject> {
46+
): Promise<CredentialResolutionResult> {
4647
if (!this.resolvingProvider) {
4748
if (credentialsResolveMetadata.isResolvable) {
4849
this.logger.warn(
4950
`No dynamic credential resolving provider set, but trying to resolve resolvable credential "${credentialsResolveMetadata.name}"`,
5051
);
5152
throw new Error('No dynamic credential resolving provider set');
5253
}
53-
return staticData;
54+
return { data: staticData, isDynamic: false };
5455
}
5556
return await this.resolvingProvider.resolveIfNeeded(
5657
credentialsResolveMetadata,

‎packages/cli/src/events/__tests__/telemetry-event-relay.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,18 +1644,18 @@ describe('TelemetryEventRelay', () => {
16441644
});
16451645

16461646
it('should track successful workflow execution', async () => {
1647-
const runData = mock<IRun>({
1647+
const runData = {
16481648
finished: true,
16491649
status: 'success',
16501650
mode: 'manual',
1651-
data: { resultData: {} },
1652-
});
1651+
data: { resultData: { runData: {} } },
1652+
} as unknown as IRun;
16531653

16541654
const event: RelayEventMap['workflow-post-execute'] = {
16551655
workflow: mockWorkflowBase,
16561656
executionId: 'execution123',
16571657
userId: 'user123',
1658-
runData: runData as unknown as IRun,
1658+
runData,
16591659
};
16601660

16611661
eventService.emit('workflow-post-execute', event);

‎packages/cli/src/events/relays/telemetry.event-relay.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -809,7 +809,9 @@ export class TelemetryEventRelay extends EventRelay {
809809
is_manual: false,
810810
version_cli: N8N_VERSION,
811811
success: false,
812-
used_dynamic_credentials: !!runData?.data?.executionData?.runtimeData?.credentials,
812+
used_dynamic_credentials: Object.values(runData?.data?.resultData?.runData ?? {}).some(
813+
(taskDataList) => taskDataList.some((taskData) => taskData.usedDynamicCredentials),
814+
),
813815
};
814816

815817
if (userId) {

‎packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/dynamic-credential.service.test.ts‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import type {
88
IExecutionContext,
99
} from 'n8n-workflow';
1010

11-
import type { CredentialResolveMetadata } from '@/credentials/credential-resolution-provider.interface';
11+
import type {
12+
CredentialResolutionResult,
13+
CredentialResolveMetadata,
14+
} from '@/credentials/credential-resolution-provider.interface';
1215
import type { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
1316
import { StaticAuthService } from '@/services/static-auth-service';
1417

@@ -215,6 +218,11 @@ describe('DynamicCredentialService', () => {
215218
apiKey: 'static-key',
216219
};
217220

221+
const expectStaticResult = (result: CredentialResolutionResult) => {
222+
expect(result.data).toBe(staticData);
223+
expect(result.isDynamic).toBe(false);
224+
};
225+
218226
describe('should return static credentials when', () => {
219227
it('credential is not resolvable', async () => {
220228
const credentialsEntity = createMockCredentialsMetadata({
@@ -223,7 +231,7 @@ describe('DynamicCredentialService', () => {
223231

224232
const result = await service.resolveIfNeeded(credentialsEntity, staticData, undefined);
225233

226-
expect(result).toBe(staticData);
234+
expectStaticResult(result);
227235
expect(mockResolverRepository.findOneBy).not.toHaveBeenCalled();
228236
});
229237

@@ -236,7 +244,7 @@ describe('DynamicCredentialService', () => {
236244

237245
const result = await service.resolveIfNeeded(credentialsEntity, staticData, undefined);
238246

239-
expect(result).toBe(staticData);
247+
expectStaticResult(result);
240248
expect(mockLogger.debug).toHaveBeenCalledWith(
241249
'Resolver not found, falling back to static credentials',
242250
expect.objectContaining({
@@ -257,7 +265,7 @@ describe('DynamicCredentialService', () => {
257265

258266
const result = await service.resolveIfNeeded(credentialsEntity, staticData, undefined);
259267

260-
expect(result).toBe(staticData);
268+
expectStaticResult(result);
261269
expect(mockLogger.debug).toHaveBeenCalled();
262270
});
263271

@@ -273,7 +281,7 @@ describe('DynamicCredentialService', () => {
273281

274282
const result = await service.resolveIfNeeded(credentialsEntity, staticData, undefined);
275283

276-
expect(result).toBe(staticData);
284+
expectStaticResult(result);
277285
expect(mockLogger.debug).toHaveBeenCalledWith(
278286
'No execution context available, falling back to static credentials',
279287
expect.objectContaining({
@@ -304,7 +312,7 @@ describe('DynamicCredentialService', () => {
304312
undefined,
305313
);
306314

307-
expect(result).toBe(staticData);
315+
expectStaticResult(result);
308316
expect(mockLogger.error).toHaveBeenCalledWith(
309317
'Failed to decrypt credential context from execution context',
310318
expect.any(Object),
@@ -334,7 +342,7 @@ describe('DynamicCredentialService', () => {
334342
undefined,
335343
);
336344

337-
expect(result).toBe(staticData);
345+
expectStaticResult(result);
338346
expect(mockLogger.debug).toHaveBeenCalledWith(
339347
'Dynamic credential resolution failed, falling back to static',
340348
expect.objectContaining({
@@ -368,7 +376,7 @@ describe('DynamicCredentialService', () => {
368376
undefined,
369377
);
370378

371-
expect(result).toBe(staticData);
379+
expectStaticResult(result);
372380
expect(mockLogger.debug).toHaveBeenCalledWith(
373381
'Dynamic credential resolution failed, falling back to static',
374382
expect.objectContaining({
@@ -543,7 +551,8 @@ describe('DynamicCredentialService', () => {
543551
);
544552

545553
// Verify merge: static fields preserved, dynamic fields added/overridden
546-
expect(result).toEqual({
554+
expect(result.isDynamic).toBe(true);
555+
expect(result.data).toEqual({
547556
clientId: 'static-client-id', // From static (preserved)
548557
clientSecret: 'static-client-secret', // From static (preserved)
549558
token: 'dynamic-token', // From dynamic (overridden)
@@ -664,7 +673,7 @@ describe('DynamicCredentialService', () => {
664673
undefined,
665674
);
666675

667-
expect(result).toBe(staticData);
676+
expectStaticResult(result);
668677
expect(mockCipher.decrypt).not.toHaveBeenCalled();
669678
});
670679

@@ -688,7 +697,7 @@ describe('DynamicCredentialService', () => {
688697
undefined,
689698
);
690699

691-
expect(result).toBe(staticData);
700+
expectStaticResult(result);
692701
expect(mockLogger.debug).toHaveBeenCalled();
693702
});
694703

@@ -757,7 +766,8 @@ describe('DynamicCredentialService', () => {
757766
expect(mockResolverRepository.findOneBy).toHaveBeenCalledWith({
758767
id: 'workflow-resolver-789',
759768
});
760-
expect(result).toEqual({ token: 'dynamic-token', apiKey: 'dynamic-key' });
769+
expect(result.isDynamic).toBe(true);
770+
expect(result.data).toEqual({ token: 'dynamic-token', apiKey: 'dynamic-key' });
761771
expect(mockLogger.debug).toHaveBeenCalledWith(
762772
'Successfully resolved dynamic credentials',
763773
expect.objectContaining({
@@ -800,7 +810,8 @@ describe('DynamicCredentialService', () => {
800810
expect(mockResolverRepository.findOneBy).toHaveBeenCalledWith({
801811
id: 'credential-resolver-456',
802812
});
803-
expect(result).toEqual({ token: 'dynamic-token', apiKey: 'dynamic-key' });
813+
expect(result.isDynamic).toBe(true);
814+
expect(result.data).toEqual({ token: 'dynamic-token', apiKey: 'dynamic-key' });
804815
expect(mockLogger.debug).toHaveBeenCalledWith(
805816
'Successfully resolved dynamic credentials',
806817
expect.objectContaining({
@@ -832,7 +843,7 @@ describe('DynamicCredentialService', () => {
832843
additionalData.workflowSettings,
833844
);
834845

835-
expect(result).toBe(staticData);
846+
expectStaticResult(result);
836847
expect(mockLogger.debug).toHaveBeenCalledWith(
837848
'Resolver not found, falling back to static credentials',
838849
expect.objectContaining({

0 commit comments

Comments
 (0)