diff --git a/packages/@n8n/decorators/src/credential-resolver/credential-resolver.ts b/packages/@n8n/decorators/src/credential-resolver/credential-resolver.ts index df1a0ea43aa..53dddd055b9 100644 --- a/packages/@n8n/decorators/src/credential-resolver/credential-resolver.ts +++ b/packages/@n8n/decorators/src/credential-resolver/credential-resolver.ts @@ -50,6 +50,7 @@ export interface ICredentialResolver { credentialId: string, context: ICredentialContext, handle: CredentialResolverHandle, + executionId?: string, ): Promise; /** @@ -61,6 +62,7 @@ export interface ICredentialResolver { context: ICredentialContext, data: ICredentialDataDecryptedObject, handle: CredentialResolverHandle, + executionId?: string, ): Promise; /** @@ -72,6 +74,7 @@ export interface ICredentialResolver { credentialId: string, context: ICredentialContext, handle: CredentialResolverHandle, + executionId?: string, ): Promise; /** @@ -94,7 +97,11 @@ export interface ICredentialResolver { * @param context - The identity of the entity to validate access for * @throws {CredentialResolverAccessValidationError} When access is invalid */ - validateIdentity?(context: ICredentialContext, handle: CredentialResolverHandle): Promise; + validateIdentity?( + context: ICredentialContext, + handle: CredentialResolverHandle, + executionId?: string, + ): Promise; /** * Returns the n8n user id the resolved credentials belong to, when this @@ -109,6 +116,7 @@ export interface ICredentialResolver { resolveOwningUserId?( context: ICredentialContext, handle: CredentialResolverHandle, + executionId?: string, ): Promise; /** diff --git a/packages/@n8n/nodes-langchain/nodes/mcp/McpTrigger/McpTrigger.node.ts b/packages/@n8n/nodes-langchain/nodes/mcp/McpTrigger/McpTrigger.node.ts index 2dbb3c748d3..5af77bacc0b 100644 --- a/packages/@n8n/nodes-langchain/nodes/mcp/McpTrigger/McpTrigger.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/mcp/McpTrigger/McpTrigger.node.ts @@ -250,7 +250,11 @@ export class McpTrigger extends Node { if (authResult === 'handled') { return { noWebhookResponse: true }; } - await context.establishTriggerIdentity(authResult.token, authResult.resource); + await context.establishTriggerIdentity( + authResult.token, + authResult.resource, + authResult.user.id, + ); authedUser = authResult.user; } else { try { diff --git a/packages/cli/src/__tests__/credentials-helper.test.ts b/packages/cli/src/__tests__/credentials-helper.test.ts index b2de8e40e22..8a432cf285c 100644 --- a/packages/cli/src/__tests__/credentials-helper.test.ts +++ b/packages/cli/src/__tests__/credentials-helper.test.ts @@ -1298,6 +1298,7 @@ describe('CredentialsHelper', () => { { apiKey: 'static-key' }, mockAdditionalData.executionContext, mockAdditionalData.workflowSettings, + undefined, // executeData ); expect(result).toEqual(resolvedData); }); diff --git a/packages/cli/src/credentials-helper.ts b/packages/cli/src/credentials-helper.ts index e0302cfdf01..86ba1db64e0 100644 --- a/packages/cli/src/credentials-helper.ts +++ b/packages/cli/src/credentials-helper.ts @@ -562,6 +562,7 @@ export class CredentialsHelper extends ICredentialsHelper { decryptedDataOriginal, additionalData.executionContext, additionalData.workflowSettings, + additionalData.executionId, ); decryptedDataOriginal = resolveResult.data; if (resolveResult.isDynamic) { diff --git a/packages/cli/src/credentials/__tests__/dynamic-credentials-proxy.test.ts b/packages/cli/src/credentials/__tests__/dynamic-credentials-proxy.test.ts index 39a86e538c3..5de41dcf9e4 100644 --- a/packages/cli/src/credentials/__tests__/dynamic-credentials-proxy.test.ts +++ b/packages/cli/src/credentials/__tests__/dynamic-credentials-proxy.test.ts @@ -89,6 +89,7 @@ describe('DynamicCredentialsProxy', () => { staticData, undefined, undefined, + undefined, ); }); @@ -120,6 +121,7 @@ describe('DynamicCredentialsProxy', () => { staticData, executionContext, workflowSettings, + undefined, ); }); }); diff --git a/packages/cli/src/credentials/credential-resolution-provider.interface.ts b/packages/cli/src/credentials/credential-resolution-provider.interface.ts index 1d7984904fb..c3517432d40 100644 --- a/packages/cli/src/credentials/credential-resolution-provider.interface.ts +++ b/packages/cli/src/credentials/credential-resolution-provider.interface.ts @@ -46,6 +46,7 @@ export interface ICredentialResolutionProvider { staticData: ICredentialDataDecryptedObject, executionContext?: IExecutionContext, workflowSettings?: IWorkflowSettings, + executionId?: string, ): Promise; /** diff --git a/packages/cli/src/credentials/dynamic-credentials-proxy.ts b/packages/cli/src/credentials/dynamic-credentials-proxy.ts index ed9a10f4666..210a1095f09 100644 --- a/packages/cli/src/credentials/dynamic-credentials-proxy.ts +++ b/packages/cli/src/credentials/dynamic-credentials-proxy.ts @@ -66,6 +66,7 @@ export class DynamicCredentialsProxy staticData: ICredentialDataDecryptedObject, executionContext?: IExecutionContext, workflowSettings?: IWorkflowSettings, + executionId?: string, ): Promise { if (!this.resolvingProvider) { if (credentialsResolveMetadata.isResolvable) { @@ -81,6 +82,7 @@ export class DynamicCredentialsProxy staticData, executionContext, workflowSettings, + executionId, ); } diff --git a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/__tests__/n8n-credential-resolver.test.ts b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/__tests__/n8n-credential-resolver.test.ts index 04cdd9059f9..051f6e34d25 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/__tests__/n8n-credential-resolver.test.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/__tests__/n8n-credential-resolver.test.ts @@ -126,7 +126,7 @@ describe('N8NCredentialResolver', () => { await resolver.getSecret(credentialId, context, handle); // Verify N8NIdentifier was called with correct context - expect(mockIdentifier.resolve).toHaveBeenCalledWith(context, {}); + expect(mockIdentifier.resolve).toHaveBeenCalledWith(context, {}, undefined); // Verify storage was queried with resolved user ID expect(mockStorage.getCredentialData).toHaveBeenCalledWith( credentialId, @@ -188,7 +188,7 @@ describe('N8NCredentialResolver', () => { await resolver.setSecret(credentialId, context, data, handle); // Verify user ID was resolved first - expect(mockIdentifier.resolve).toHaveBeenCalledWith(context, {}); + expect(mockIdentifier.resolve).toHaveBeenCalledWith(context, {}, undefined); // Verify storage uses resolved user ID expect(mockStorage.setCredentialData).toHaveBeenCalledWith( credentialId, @@ -210,7 +210,7 @@ describe('N8NCredentialResolver', () => { await resolver.deleteSecret(credentialId, context, handle); - expect(mockIdentifier.resolve).toHaveBeenCalledWith(context, {}); + expect(mockIdentifier.resolve).toHaveBeenCalledWith(context, {}, undefined); expect(mockStorage.deleteCredentialData).toHaveBeenCalledWith( credentialId, 'user-to-delete-456', diff --git a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/identifiers/__tests__/n8n-identifier.test.ts b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/identifiers/__tests__/n8n-identifier.test.ts index 5892eef577e..66d773ed386 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/identifiers/__tests__/n8n-identifier.test.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/identifiers/__tests__/n8n-identifier.test.ts @@ -1,5 +1,5 @@ import type { Mocked } from 'vitest'; -import type { User } from '@n8n/db'; +import type { User, UserRepository } from '@n8n/db'; import { CredentialResolverError } from '@n8n/decorators'; import { mock } from 'vitest-mock-extended'; @@ -18,14 +18,18 @@ describe('N8NIdentifier', () => { let identifier: N8NIdentifier; let mockAuthService: Mocked; let mockOAuthVerifier: Mocked; + let mockUserRepository: Mocked; const mockUser = mock({ id: 'user-123' }); beforeEach(() => { mockAuthService = mock(); mockOAuthVerifier = mock(); + mockOAuthVerifier.authorizeSealedGrant.mockResolvedValue(true); + mockUserRepository = mock(); + mockUserRepository.findOneBy.mockResolvedValue(mock({ id: 'user-123', disabled: false })); - identifier = new N8NIdentifier(mockAuthService, mockOAuthVerifier); + identifier = new N8NIdentifier(mockAuthService, mockOAuthVerifier, mockUserRepository); }); afterEach(() => { @@ -415,6 +419,123 @@ describe('N8NIdentifier', () => { await expect(identifier.resolve(context, {})).rejects.toThrow(CredentialResolverError); }); }); + + describe('n8n-oauth branch — sealed (subject present)', () => { + const sealedContext = (metaOverrides: Record = {}) => ({ + identity: 'oauth-access-token', + version: 1 as const, + metadata: { + source: 'n8n-oauth' as const, + resource: 'https://host/mcp/wf', + subject: 'user-123', + establishedAt: 1, + executionPath: ['exec-root'], + ...metaOverrides, + }, + }); + + it('returns the subject when the execution is in the path, without verifying the token', async () => { + const result = await identifier.resolve(sealedContext(), {}, 'exec-root'); + + expect(result).toBe('user-123'); + expect(mockOAuthVerifier.verifyOAuthAccessToken).not.toHaveBeenCalled(); + }); + + it('accepts a sub-workflow execution present in the inherited path', async () => { + const result = await identifier.resolve( + sealedContext({ executionPath: ['exec-root', 'exec-child'] }), + {}, + 'exec-child', + ); + + expect(result).toBe('user-123'); + }); + + it('rejects a seal used in a different execution of the same workflow', async () => { + await expect(identifier.resolve(sealedContext(), {}, 'exec-other')).rejects.toThrow( + CredentialResolverError, + ); + }); + + it('fails closed when a bound seal reaches resolution with no execution id', async () => { + await expect(identifier.resolve(sealedContext(), {}, undefined)).rejects.toThrow( + CredentialResolverError, + ); + }); + + it('rejects when the sealed principal is disabled', async () => { + mockUserRepository.findOneBy.mockResolvedValue( + mock({ id: 'user-123', disabled: true }), + ); + + await expect(identifier.resolve(sealedContext(), {}, 'exec-root')).rejects.toThrow( + CredentialResolverError, + ); + }); + + it('rejects when the sealed principal no longer exists', async () => { + mockUserRepository.findOneBy.mockResolvedValue(null); + + await expect(identifier.resolve(sealedContext(), {}, 'exec-root')).rejects.toThrow( + CredentialResolverError, + ); + }); + + it('resolves however long after establishment (no token, no TTL)', async () => { + const result = await identifier.resolve( + sealedContext({ establishedAt: 0 }), + {}, + 'exec-root', + ); + + expect(result).toBe('user-123'); + expect(mockOAuthVerifier.verifyOAuthAccessToken).not.toHaveBeenCalled(); + }); + + it('returns the subject ungated for an unbound seal (empty path, pre-execution probe)', async () => { + const result = await identifier.resolve( + sealedContext({ executionPath: [] }), + {}, + undefined, + ); + + expect(result).toBe('user-123'); + expect(mockOAuthVerifier.verifyOAuthAccessToken).not.toHaveBeenCalled(); + }); + + it('rejects an unbound seal (empty path) when resolving inside an execution', async () => { + await expect( + identifier.resolve(sealedContext({ executionPath: [] }), {}, 'exec-root'), + ).rejects.toThrow(CredentialResolverError); + }); + + it('re-takes the sealed grant and returns the subject when still authorized', async () => { + const grant = { audiences: ['https://host/mcp/wf'], executeAccessWorkflowId: 'wf' }; + + const result = await identifier.resolve(sealedContext({ grant }), {}, 'exec-root'); + + expect(result).toBe('user-123'); + expect(mockOAuthVerifier.authorizeSealedGrant).toHaveBeenCalledWith('user-123', grant); + expect(mockOAuthVerifier.verifyOAuthAccessToken).not.toHaveBeenCalled(); + }); + + it('rejects when the sealed grant is no longer authorized', async () => { + mockOAuthVerifier.authorizeSealedGrant.mockResolvedValue(false); + const grant = { audiences: ['https://host/mcp/wf'], executeAccessWorkflowId: 'wf' }; + + await expect(identifier.resolve(sealedContext({ grant }), {}, 'exec-root')).rejects.toThrow( + CredentialResolverError, + ); + }); + + it('skips the grant re-take for a grant-less seal and checks the principal locally', async () => { + const result = await identifier.resolve(sealedContext(), {}, 'exec-root'); + + expect(result).toBe('user-123'); + expect(mockOAuthVerifier.authorizeSealedGrant).not.toHaveBeenCalled(); + expect(mockUserRepository.findOneBy).toHaveBeenCalledWith({ id: 'user-123' }); + }); + }); }); }); diff --git a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/identifiers/identifier-interface.ts b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/identifiers/identifier-interface.ts index cc260c703d6..26e4cea3fc9 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/identifiers/identifier-interface.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/identifiers/identifier-interface.ts @@ -22,10 +22,15 @@ export interface ITokenIdentifier { * * @param context - Credential context with execution details * @param identifierOptions - Implementation-specific options + * @param executionId - Optional execution ID for context * @returns Unique identifier string * @throws {IdentifierValidationError} When validation or resolution fails */ - resolve(context: ICredentialContext, identifierOptions: Record): Promise; + resolve( + context: ICredentialContext, + identifierOptions: Record, + executionId?: string, + ): Promise; /** * Validates identifier options before use diff --git a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/identifiers/n8n-identifier.ts b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/identifiers/n8n-identifier.ts index ac4a2fea486..4fa1ece0ce1 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/identifiers/n8n-identifier.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/identifiers/n8n-identifier.ts @@ -5,6 +5,7 @@ import { AuthService } from '@/auth/auth.service'; import { z } from 'zod'; import { CredentialResolverError } from '@n8n/decorators'; import { OAuthTokenVerifierProxy } from '@/services/oauth-token-verifier-proxy.service'; +import { UserRepository } from '@n8n/db'; /** * The `source` values this identifier accepts, declared once so the schemas below and @@ -40,6 +41,14 @@ const N8nOAuthMetadataSchema = z.object({ resource: z.string(), /** Absent for contexts sealed before grants existed, and for long-lived resources. */ grant: OAuthResourceGrantSchema.optional(), + /** + * The resolved n8n user, sealed at establishment. When present, resolution trusts it + * (bound to `executionPath`, principal re-checked) instead of re-verifying the token. + * Absent on legacy / grant-only carriers, which fall back to token verification. + */ + subject: z.string().optional(), + establishedAt: z.number().optional(), + executionPath: z.array(z.string()).optional(), }); /** Exported for the drift test that keeps {@link N8N_IDENTITY_SOURCES} in step with it. */ @@ -94,13 +103,18 @@ export class N8NIdentifier implements ITokenIdentifier { constructor( private readonly authService: AuthService, private readonly oauthTokenVerifierProxy: OAuthTokenVerifierProxy, + private readonly userRepository: UserRepository, ) {} async validateOptions(_: Record): Promise { return; } - async resolve(context: ICredentialContext, _: Record): Promise { + async resolve( + context: ICredentialContext, + _: Record, + executionId?: string, + ): Promise { const metadataResult = N8NIdentifierMetadataSchema.safeParse(context.metadata); if (!metadataResult.success) { throw new CredentialResolverError( @@ -115,8 +129,54 @@ export class N8NIdentifier implements ITokenIdentifier { } if (metadataResult.data.source === 'n8n-oauth') { - // Looked up afresh on every access, so by now the resource may be gone — the - // sealed grant carries what the gate needs in that case. + // Sealed identity: trust the resolved user, bound to its execution, without + // re-verifying the token (so it resolves past the token's TTL, e.g. after a Wait). + if (metadataResult.data.subject) { + const userId = metadataResult.data.subject; + + const executionPath = metadataResult.data.executionPath ?? []; + if (executionId) { + // Resolving inside an execution: the seal must be bound to it (or a sub-workflow + // of it). An unbound seal (empty path) is not valid once an execution exists. + if (!executionPath.includes(executionId)) { + throw new CredentialResolverError('Sealed identity is not valid for this execution'); + } + } else if (executionPath.length > 0) { + // Bound seal but no execution to check it against → fail closed. + throw new CredentialResolverError('Sealed identity is not valid for this execution'); + } + + const grant = metadataResult.data.grant; + if (grant) { + // Re-take the grant's live workflow:execute decision (token-independent) so a + // principal whose access was revoked after sealing stops resolving. This also + // covers existence/disabled — a denied user resolves to `false`. + const authorized = await this.oauthTokenVerifierProxy.authorizeSealedGrant(userId, grant); + if (!authorized) { + throw new CredentialResolverError( + `Invalid OAuth token for resource ${metadataResult.data.resource}`, + ); + } + return userId; + } + + // Grant-less sealed carrier (legacy): no execute gate to re-take, so confirm the + // principal still exists and is enabled. Keep "missing" and "disabled" + // indistinguishable to the caller (no account enumeration) and never surface the + // internal user id in the error. + const user = await this.userRepository.findOneBy({ id: userId }); + if (!user || user.disabled) { + throw new CredentialResolverError( + `Invalid OAuth token for resource ${metadataResult.data.resource}`, + ); + } + + return user.id; + } + + // No sealed subject: verify the token. The grant is looked up afresh on every + // access, so by now the resource may be gone — the sealed grant carries what the + // gate needs in that case. const user = await this.oauthTokenVerifierProxy.verifyOAuthAccessToken( context.identity, metadataResult.data.resource, diff --git a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/n8n-credential-resolver.ts b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/n8n-credential-resolver.ts index 94124bd8f5e..17d5f294d02 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/n8n-credential-resolver.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/n8n-credential-resolver.ts @@ -41,8 +41,9 @@ export class N8NCredentialResolver implements ICredentialResolver { credentialId: string, context: ICredentialContext, handle: CredentialResolverHandle, + executionId?: string, ): Promise { - const key = await this.resolveIdentifier(context, handle.configuration); + const key = await this.resolveIdentifier(context, handle.configuration, executionId); const data = await this.storage.getCredentialData( credentialId, @@ -70,8 +71,9 @@ export class N8NCredentialResolver implements ICredentialResolver { context: ICredentialContext, data: ICredentialDataDecryptedObject, handle: CredentialResolverHandle, + executionId?: string, ): Promise { - const key = await this.resolveIdentifier(context, handle.configuration); + const key = await this.resolveIdentifier(context, handle.configuration, executionId); const encryptedData = await this.cipher.encryptV2(data); @@ -89,8 +91,9 @@ export class N8NCredentialResolver implements ICredentialResolver { credentialId: string, context: ICredentialContext, handle: CredentialResolverHandle, + executionId?: string, ): Promise { - const key = await this.resolveIdentifier(context, handle.configuration); + const key = await this.resolveIdentifier(context, handle.configuration, executionId); await this.storage.deleteCredentialData( credentialId, key, @@ -110,8 +113,9 @@ export class N8NCredentialResolver implements ICredentialResolver { private async resolveIdentifier( context: ICredentialContext, configuration: CredentialResolverConfiguration, + executionId?: string, ): Promise { - return await this.n8nIdentifier.resolve(context, configuration); + return await this.n8nIdentifier.resolve(context, configuration, executionId); } /** @@ -121,14 +125,16 @@ export class N8NCredentialResolver implements ICredentialResolver { async resolveOwningUserId( context: ICredentialContext, handle: CredentialResolverHandle, + executionId?: string, ): Promise { - return await this.resolveIdentifier(context, handle.configuration); + return await this.resolveIdentifier(context, handle.configuration, executionId); } async validateIdentity( context: ICredentialContext, handle: CredentialResolverHandle, + executionId?: string, ): Promise { - await this.resolveIdentifier(context, handle.configuration); + await this.resolveIdentifier(context, handle.configuration, executionId); } } diff --git a/packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/dynamic-credential.service.test.ts b/packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/dynamic-credential.service.test.ts index 6bd423c1184..48aed58fd56 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/dynamic-credential.service.test.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/dynamic-credential.service.test.ts @@ -693,13 +693,18 @@ describe('DynamicCredentialService', () => { apiKey: 'dynamic-key', // From dynamic (overridden) refreshToken: 'dynamic-refresh-token', // From dynamic (new field) }); - expect(mockResolver.getSecret).toHaveBeenCalledWith('cred-123', credentialContext, { - resolverId: resolverEntity.id, - resolverName: resolverEntity.type, - configuration: { - prefix: 'test', + expect(mockResolver.getSecret).toHaveBeenCalledWith( + 'cred-123', + credentialContext, + { + resolverId: resolverEntity.id, + resolverName: resolverEntity.type, + configuration: { + prefix: 'test', + }, }, - }); + undefined, + ); expect(mockLogger.debug).toHaveBeenCalledWith( 'Successfully resolved dynamic credentials', expect.objectContaining({ @@ -738,11 +743,16 @@ describe('DynamicCredentialService', () => { undefined, ); - expect(mockResolver.getSecret).toHaveBeenCalledWith('cred-123', credentialContext, { - resolverId: resolverEntity.id, - resolverName: resolverEntity.type, - configuration: customConfig, - }); + expect(mockResolver.getSecret).toHaveBeenCalledWith( + 'cred-123', + credentialContext, + { + resolverId: resolverEntity.id, + resolverName: resolverEntity.type, + configuration: customConfig, + }, + undefined, + ); }); it('credential context with metadata is properly decrypted', async () => { @@ -777,6 +787,7 @@ describe('DynamicCredentialService', () => { 'cred-123', credentialContext, expect.any(Object), + undefined, ); expect(mockLogger.debug).toHaveBeenCalledWith( 'Successfully resolved dynamic credentials', @@ -854,11 +865,16 @@ describe('DynamicCredentialService', () => { undefined, ); - expect(mockResolver.getSecret).toHaveBeenCalledWith('cred-123', credentialContext, { - resolverId: resolverEntity.id, - resolverName: resolverEntity.type, - configuration: {}, - }); + expect(mockResolver.getSecret).toHaveBeenCalledWith( + 'cred-123', + credentialContext, + { + resolverId: resolverEntity.id, + resolverName: resolverEntity.type, + configuration: {}, + }, + undefined, + ); }); }); @@ -1022,14 +1038,19 @@ describe('DynamicCredentialService', () => { ); // Verify the resolver was called with resolved config - expect(mockResolver.getSecret).toHaveBeenCalledWith('cred-123', credentialContext, { - resolverId: resolverEntity.id, - resolverName: resolverEntity.type, - configuration: { - apiKey: 'secret-api-key-123', // $vars expression resolved - prefix: 'cred', + expect(mockResolver.getSecret).toHaveBeenCalledWith( + 'cred-123', + credentialContext, + { + resolverId: resolverEntity.id, + resolverName: resolverEntity.type, + configuration: { + apiKey: 'secret-api-key-123', // $vars expression resolved + prefix: 'cred', + }, }, - }); + undefined, + ); // Cleanup delete (global as any).testVars; @@ -1115,16 +1136,21 @@ describe('DynamicCredentialService', () => { ); // Verify only global expressions were resolved, runtime expressions remain as-is - expect(mockResolver.getSecret).toHaveBeenCalledWith('cred-123', credentialContext, { - resolverId: resolverEntity.id, - resolverName: resolverEntity.type, - configuration: { - prefix: '={{$execution.id}}', // NOT resolved (runtime data) - envValue: 'env-value', // Resolved (global data) - mode: '={{$execution.mode}}', // NOT resolved (runtime data) - staticValue: 'no-expression', + expect(mockResolver.getSecret).toHaveBeenCalledWith( + 'cred-123', + credentialContext, + { + resolverId: resolverEntity.id, + resolverName: resolverEntity.type, + configuration: { + prefix: '={{$execution.id}}', // NOT resolved (runtime data) + envValue: 'env-value', // Resolved (global data) + mode: '={{$execution.mode}}', // NOT resolved (runtime data) + staticValue: 'no-expression', + }, }, - }); + undefined, + ); // Cleanup delete (global as any).testVars; @@ -1159,14 +1185,19 @@ describe('DynamicCredentialService', () => { ); // Verify config passed as-is (expression not resolved) - expect(mockResolver.getSecret).toHaveBeenCalledWith('cred-123', credentialContext, { - resolverId: resolverEntity.id, - resolverName: resolverEntity.type, - configuration: { - prefix: 'cred', - executionId: '={{$execution.id}}', // Expression NOT resolved + expect(mockResolver.getSecret).toHaveBeenCalledWith( + 'cred-123', + credentialContext, + { + resolverId: resolverEntity.id, + resolverName: resolverEntity.type, + configuration: { + prefix: 'cred', + executionId: '={{$execution.id}}', // Expression NOT resolved + }, }, - }); + undefined, + ); }); }); diff --git a/packages/cli/src/modules/dynamic-credentials.ee/services/dynamic-credential.service.ts b/packages/cli/src/modules/dynamic-credentials.ee/services/dynamic-credential.service.ts index eafca2e9811..03c53e79feb 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/services/dynamic-credential.service.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/services/dynamic-credential.service.ts @@ -66,6 +66,7 @@ export class DynamicCredentialService implements ICredentialResolutionProvider { staticData: ICredentialDataDecryptedObject, executionContext?: IExecutionContext, workflowSettings?: IWorkflowSettings, + executionId?: string, ): Promise { // Determine which resolver ID to use: credential's own resolver or workflow's fallback // (explicit workflow override, or the seeded system resolver looked up via the proxy). @@ -144,6 +145,7 @@ export class DynamicCredentialService implements ICredentialResolutionProvider { credentialsResolveMetadata.id, credentialContext, handle, + executionId, ); this.logger.debug('Successfully resolved dynamic credentials', { @@ -166,7 +168,11 @@ export class DynamicCredentialService implements ICredentialResolutionProvider { // execution simply stays unattributed (redacted for everyone). let resolvedUserId: string | undefined; try { - resolvedUserId = await resolver.resolveOwningUserId?.(credentialContext, handle); + resolvedUserId = await resolver.resolveOwningUserId?.( + credentialContext, + handle, + executionId, + ); } catch (error) { this.logger.debug('Could not resolve owning user for dynamic credentials', { credentialId: credentialsResolveMetadata.id, diff --git a/packages/cli/src/modules/oauth-server/__tests__/oauth-token.service.test.ts b/packages/cli/src/modules/oauth-server/__tests__/oauth-token.service.test.ts index 7ea531c77f3..20903893c4b 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/oauth-token.service.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/oauth-token.service.test.ts @@ -480,6 +480,48 @@ describe('OAuthTokenService', () => { }); }); + describe('authorizeSealedGrant', () => { + const grant = { audiences: ['https://host/mcp/wf'], executeAccessWorkflowId: 'wf' }; + + it('returns false when the user no longer exists', async () => { + userRepository.findOne.mockResolvedValue(null); + + expect(await service.authorizeSealedGrant('user-123', grant)).toBe(false); + expect(workflowFinderService.findWorkflowIdsWithScopeForUser).not.toHaveBeenCalled(); + }); + + it('returns false when the user is disabled', async () => { + userRepository.findOne.mockResolvedValue(mock({ id: 'user-123', disabled: true })); + + expect(await service.authorizeSealedGrant('user-123', grant)).toBe(false); + expect(workflowFinderService.findWorkflowIdsWithScopeForUser).not.toHaveBeenCalled(); + }); + + it('grants when the user still holds workflow:execute on the bound workflow', async () => { + const user = mock({ id: 'user-123', disabled: false }); + userRepository.findOne.mockResolvedValue(user); + workflowFinderService.findWorkflowIdsWithScopeForUser.mockResolvedValue(new Set(['wf'])); + + expect(await service.authorizeSealedGrant('user-123', grant)).toBe(true); + expect(userRepository.findOne).toHaveBeenCalledWith({ + where: { id: 'user-123' }, + relations: ['role'], + }); + expect(workflowFinderService.findWorkflowIdsWithScopeForUser).toHaveBeenCalledWith( + ['wf'], + user, + ['workflow:execute'], + ); + }); + + it('denies when the user no longer holds workflow:execute on the bound workflow', async () => { + userRepository.findOne.mockResolvedValue(mock({ id: 'user-123', disabled: false })); + workflowFinderService.findWorkflowIdsWithScopeForUser.mockResolvedValue(new Set()); + + expect(await service.authorizeSealedGrant('user-123', grant)).toBe(false); + }); + }); + describe('verifyOAuthAccessToken audience resolution', () => { it('should deny when a resource-scoped audience cannot be resolved', async () => { // Fail closed: the token carries an audience but no resource resolves for diff --git a/packages/cli/src/modules/oauth-server/oauth-token.service.ts b/packages/cli/src/modules/oauth-server/oauth-token.service.ts index f2bd4d1cc2f..49b7b55dc6b 100644 --- a/packages/cli/src/modules/oauth-server/oauth-token.service.ts +++ b/packages/cli/src/modules/oauth-server/oauth-token.service.ts @@ -313,6 +313,17 @@ export class OAuthTokenService implements OAuthTokenVerifier { } } + /** + * Re-take a sealed grant's decision for a user, without the token. The sealed-identity + * credential path calls this on every resolve, so a revoked `workflow:execute` stops + * resolution mid-run. Loads the user with its role (a bare id lookup carries no scopes). + */ + async authorizeSealedGrant(userId: string, grant: OAuthResourceGrant): Promise { + const user = await this.userRepository.findOne({ where: { id: userId }, relations: ['role'] }); + if (!user || user.disabled) return false; + return await authorizeAgainstGrant(this.workflowFinderService, grant, user); + } + /** Deletes every access and refresh token a user holds for a client. */ async revokeAllTokensForGrant(clientId: string, userId: string): Promise { await Promise.all([ diff --git a/packages/cli/src/services/__tests__/oauth-token-verifier-proxy.service.test.ts b/packages/cli/src/services/__tests__/oauth-token-verifier-proxy.service.test.ts index 330bc252520..942b559ea34 100644 --- a/packages/cli/src/services/__tests__/oauth-token-verifier-proxy.service.test.ts +++ b/packages/cli/src/services/__tests__/oauth-token-verifier-proxy.service.test.ts @@ -51,4 +51,24 @@ describe('OAuthTokenVerifierProxy', () => { grant, ); }); + + describe('authorizeSealedGrant', () => { + const grant = { audiences: ['https://n8n.example.com/x'], executeAccessWorkflowId: 'wf' }; + + it('should fail closed when no provider is registered', async () => { + const proxy = new OAuthTokenVerifierProxy(); + + expect(await proxy.authorizeSealedGrant('user-1', grant)).toBe(false); + }); + + it('should delegate to the registered provider', async () => { + const proxy = new OAuthTokenVerifierProxy(); + const provider = mock(); + provider.authorizeSealedGrant.mockResolvedValue(true); + proxy.registerProvider(provider); + + expect(await proxy.authorizeSealedGrant('user-1', grant)).toBe(true); + expect(provider.authorizeSealedGrant).toHaveBeenCalledWith('user-1', grant); + }); + }); }); diff --git a/packages/cli/src/services/oauth-token-verifier-proxy.service.ts b/packages/cli/src/services/oauth-token-verifier-proxy.service.ts index bd6587bcec3..fd342a665b3 100644 --- a/packages/cli/src/services/oauth-token-verifier-proxy.service.ts +++ b/packages/cli/src/services/oauth-token-verifier-proxy.service.ts @@ -56,6 +56,14 @@ export interface OAuthTokenVerifier { expectedAudience?: string, grant?: OAuthResourceGrant, ): Promise; + + /** + * Re-take a sealed grant's `workflow:execute` decision for `userId`, without the token. + * Used by the sealed-identity credential path to keep the check live: a caller whose + * execute access was revoked after the identity was sealed must stop resolving. Returns + * `false` when the user is gone/disabled or lacks the access the grant requires. + */ + authorizeSealedGrant(userId: string, grant: OAuthResourceGrant): Promise; } /** @@ -92,4 +100,12 @@ export class OAuthTokenVerifierProxy implements OAuthTokenVerifier { } return await this.provider.verifyOAuthAccessToken(token, expectedAudience, grant); } + + async authorizeSealedGrant(userId: string, grant: OAuthResourceGrant): Promise { + // Fail closed: with no verifier registered there is nothing to re-authorize against. + if (!this.provider) { + return false; + } + return await this.provider.authorizeSealedGrant(userId, grant); + } } diff --git a/packages/cli/src/webhooks/__tests__/webhook-helpers.test.ts b/packages/cli/src/webhooks/__tests__/webhook-helpers.test.ts index 87136373c6e..b73f04283c9 100644 --- a/packages/cli/src/webhooks/__tests__/webhook-helpers.test.ts +++ b/packages/cli/src/webhooks/__tests__/webhook-helpers.test.ts @@ -1317,6 +1317,9 @@ describe('executeWebhook establishTriggerIdentity', () => { workflowRunner.run.mockResolvedValue(EXECUTION_ID); activeExecutions.getPostExecutePromise.mockReturnValue(new Promise(() => {})); executionContextService.buildTriggerIdentityCredentials.mockResolvedValue('sealed-context'); + // `establishExecutionContext` binds the execution id onto the sealed context; with no + // execution id yet (or no sealed subject) it hands the context straight back. + executionContextService.maybeBindExecutionId.mockImplementation(async (context) => context); // `establishExecutionContext` runs the hook pass over the seeded stack. executionContextService.augmentExecutionContextWithHooks.mockImplementation( async (_workflow, _startItem, context) => ({ context, triggerItems: null }), @@ -1391,6 +1394,7 @@ describe('executeWebhook establishTriggerIdentity', () => { 'caller-token', RESOURCE_URL, GRANT, + undefined, ); expect(additionalData.encryptedRunnerIdentity).toBe('sealed-context'); }); @@ -1413,6 +1417,7 @@ describe('executeWebhook establishTriggerIdentity', () => { 'caller-token', RESOURCE_URL, undefined, + undefined, ); }); @@ -1423,6 +1428,7 @@ describe('executeWebhook establishTriggerIdentity', () => { 'caller-token', RESOURCE_URL, undefined, + undefined, ); }); }); diff --git a/packages/cli/src/webhooks/webhook-helpers.ts b/packages/cli/src/webhooks/webhook-helpers.ts index 5b891a69a45..8757fd59b2e 100644 --- a/packages/cli/src/webhooks/webhook-helpers.ts +++ b/packages/cli/src/webhooks/webhook-helpers.ts @@ -623,7 +623,11 @@ export async function executeWebhook( }; }; - additionalData.establishTriggerIdentity = async (token: string, resource: string) => { + additionalData.establishTriggerIdentity = async ( + token: string, + resource: string, + subject?: string, + ) => { // The run re-verifies this token after the trigger stops listening, so it carries // the gate with it. Fall back to a lookup for callers that establish an identity // without going through `validateN8nOAuth2Token`. @@ -642,7 +646,7 @@ export async function executeWebhook( additionalData.encryptedRunnerIdentity = await Container.get( ExecutionContextService, - ).buildTriggerIdentityCredentials(token, resource, grant); + ).buildTriggerIdentityCredentials(token, resource, grant, subject); if (runExecutionData) { await establishExecutionContext(workflow, runExecutionData, additionalData, executionMode); } diff --git a/packages/core/src/execution-engine/__tests__/execution-context.service.test.ts b/packages/core/src/execution-engine/__tests__/execution-context.service.test.ts index 701582ab03f..facc2a53ca2 100644 --- a/packages/core/src/execution-engine/__tests__/execution-context.service.test.ts +++ b/packages/core/src/execution-engine/__tests__/execution-context.service.test.ts @@ -373,18 +373,26 @@ describe('ExecutionContextService', () => { }); describe('buildTriggerIdentityCredentials()', () => { - it('should encrypt the credential context with the token as identity and resource in metadata', async () => { + it('keeps the token as identity and seals the subject + establishedAt in metadata', async () => { mockCipher.encryptV2.mockResolvedValue('encrypted-credential-blob'); const result = await service.buildTriggerIdentityCredentials( 'oauth-token-jwt', 'https://api.example.com/resource', + undefined, + 'user-123', ); expect(mockCipher.encryptV2).toHaveBeenCalledWith({ version: 1, identity: 'oauth-token-jwt', - metadata: { source: 'n8n-oauth', resource: 'https://api.example.com/resource' }, + metadata: { + source: 'n8n-oauth', + subject: 'user-123', + resource: 'https://api.example.com/resource', + establishedAt: expect.any(Number), + executionPath: [], + }, }); expect(result).toBe('encrypted-credential-blob'); }); @@ -409,17 +417,159 @@ describe('ExecutionContextService', () => { metadata: { source: 'n8n-oauth', resource: 'https://api.example.com/resource', + establishedAt: expect.any(Number), + executionPath: [], grant, }, }); }); + it('omits the subject and grant when neither is provided (legacy token-only carrier)', async () => { + mockCipher.encryptV2.mockResolvedValue('encrypted-credential-blob'); + + await service.buildTriggerIdentityCredentials('oauth-token-jwt', 'https://api/r'); + + expect(mockCipher.encryptV2).toHaveBeenCalledWith({ + version: 1, + identity: 'oauth-token-jwt', + metadata: { + source: 'n8n-oauth', + resource: 'https://api/r', + establishedAt: expect.any(Number), + executionPath: [], + }, + }); + }); + it('should propagate errors raised by the cipher', async () => { mockCipher.encryptV2.mockRejectedValue(new Error('encryption key missing')); - await expect(service.buildTriggerIdentityCredentials('token', 'resource')).rejects.toThrow( - 'encryption key missing', + await expect( + service.buildTriggerIdentityCredentials('token', 'resource', undefined, 'user-123'), + ).rejects.toThrow('encryption key missing'); + }); + }); + + describe('maybeBindExecutionId()', () => { + beforeEach(() => { + // Symmetric fakes: decrypt is identity, encrypt is JSON.stringify, so we can + // read the re-encrypted credentials back with JSON.parse. + mockCipher.decryptV2.mockImplementation(async (data: string) => data); + mockCipher.encryptV2.mockImplementation(async (data: unknown) => JSON.stringify(data)); + toCredentialContext.mockImplementation((data: string) => JSON.parse(data)); + }); + + const contextWith = (metadata: Record): IExecutionContext => ({ + version: 1, + establishedAt: 1, + source: 'webhook', + credentials: JSON.stringify({ version: 1, identity: 'oauth-token', metadata }), + }); + + const pathOf = (context: IExecutionContext) => + JSON.parse(context.credentials as string).metadata.executionPath; + + it('stamps the current execution id onto a freshly sealed carrier', async () => { + const bound = await service.maybeBindExecutionId( + contextWith({ source: 'n8n-oauth', resource: 'r', subject: 'user-123' }), + 'exec-root', ); + + expect(pathOf(bound)).toEqual(['exec-root']); + }); + + it('appends a child execution id, preserving the inherited path', async () => { + const bound = await service.maybeBindExecutionId( + contextWith({ + source: 'n8n-oauth', + resource: 'r', + subject: 'user-123', + executionPath: ['exec-root'], + }), + 'exec-child', + { allowInherit: true }, + ); + + expect(pathOf(bound)).toEqual(['exec-root', 'exec-child']); + }); + + it('leaves a populated path untouched for an unrelated execution', async () => { + const context = contextWith({ + source: 'n8n-oauth', + resource: 'r', + subject: 'user-123', + executionPath: ['exec-other'], + }); + + const bound = await service.maybeBindExecutionId(context, 'exec-current'); + + expect(bound).toBe(context); + expect(mockCipher.encryptV2).not.toHaveBeenCalled(); + }); + + it('is idempotent when the execution id is already in the path (retry/resume)', async () => { + const context = contextWith({ + source: 'n8n-oauth', + resource: 'r', + subject: 'user-123', + executionPath: ['exec-root'], + }); + + const bound = await service.maybeBindExecutionId(context, 'exec-root'); + + expect(bound).toBe(context); + expect(mockCipher.encryptV2).not.toHaveBeenCalled(); + }); + + it('is a no-op for a non-sealed carrier (no subject)', async () => { + const context = contextWith({ source: 'n8n-oauth', resource: 'r' }); + + const bound = await service.maybeBindExecutionId(context, 'exec-root'); + + expect(bound).toBe(context); + expect(mockCipher.encryptV2).not.toHaveBeenCalled(); + }); + + it('is a no-op when no execution id is provided', async () => { + const context = contextWith({ source: 'n8n-oauth', resource: 'r', subject: 'user-123' }); + + const bound = await service.maybeBindExecutionId(context, undefined); + + expect(bound).toBe(context); + expect(mockCipher.decryptV2).not.toHaveBeenCalled(); + }); + + it('is a no-op for a non-n8n-oauth carrier (schema mismatch)', async () => { + const context = contextWith({ source: 'manual-execution' }); + + const bound = await service.maybeBindExecutionId(context, 'exec-root'); + + expect(bound).toBe(context); + expect(mockCipher.encryptV2).not.toHaveBeenCalled(); + }); + + it('is a no-op for a carrier without metadata', async () => { + const context: IExecutionContext = { + version: 1, + establishedAt: 1, + source: 'webhook', + credentials: JSON.stringify({ version: 1, identity: 'oauth-token' }), + }; + + const bound = await service.maybeBindExecutionId(context, 'exec-root'); + + expect(bound).toBe(context); + expect(mockCipher.encryptV2).not.toHaveBeenCalled(); + }); + + it('is a no-op for a context without credentials', async () => { + const context: IExecutionContext = { version: 1, establishedAt: 1, source: 'webhook' }; + + const bound = await service.maybeBindExecutionId(context, 'exec-root'); + + expect(bound).toBe(context); + expect(mockCipher.decryptV2).not.toHaveBeenCalled(); + expect(mockCipher.encryptV2).not.toHaveBeenCalled(); }); }); diff --git a/packages/core/src/execution-engine/__tests__/execution-context.test.ts b/packages/core/src/execution-engine/__tests__/execution-context.test.ts index 3dd716e7970..71f8904639c 100644 --- a/packages/core/src/execution-engine/__tests__/execution-context.test.ts +++ b/packages/core/src/execution-engine/__tests__/execution-context.test.ts @@ -1,9 +1,11 @@ +import type { Logger } from '@n8n/backend-common'; import { Container } from '@n8n/di'; import { createEmptyRunExecutionData, createRunExecutionData, UnexpectedError, type IExecutionContext, + type IN8NOAuthMetadata, type INode, type IWorkflowExecuteAdditionalData, type RelatedExecution, @@ -12,7 +14,10 @@ import { } from 'n8n-workflow'; import { mock } from 'vitest-mock-extended'; +import type { Cipher } from '@/encryption'; + import { establishExecutionContext } from '../execution-context'; +import type { ExecutionContextHookRegistry } from '../execution-context-hook-registry.service'; import { ExecutionContextService } from '../execution-context.service'; describe('establishExecutionContext', () => { @@ -21,6 +26,9 @@ describe('establishExecutionContext', () => { webhookWaitingBaseUrl: 'http://localhost:5678/webhook-waiting', formWaitingBaseUrl: 'http://localhost:5678/form-waiting', encryptedRunnerIdentity: undefined, + // No executionId at establishment time; the real (unmocked) maybeBindExecutionId + // is then a no-op and never tries to decrypt these tests' fake credentials. + executionId: undefined, }); const mockMode: WorkflowExecuteMode = 'manual'; @@ -769,6 +777,11 @@ describe('establishExecutionContext', () => { beforeEach(() => { mockExecutionContextService = mock(); + // maybeBindExecutionId runs on every branch; pass through so this describe only + // asserts sub-execution augmentation (binding is covered in its own tests). + mockExecutionContextService.maybeBindExecutionId.mockImplementation( + async (context) => context, + ); Container.set(ExecutionContextService, mockExecutionContextService); }); @@ -1223,6 +1236,11 @@ describe('establishExecutionContext', () => { triggerItems: null, }), ); + // maybeBindExecutionId runs on every branch; pass through so these tests assert + // only the manual-injection branch (binding is covered in its own tests). + mockExecutionContextService.maybeBindExecutionId.mockImplementation( + async (context) => context, + ); Container.set(ExecutionContextService, mockExecutionContextService); }); @@ -1303,4 +1321,178 @@ describe('establishExecutionContext', () => { expect(runExecutionData.executionData!.runtimeData).toEqual(existingContext); }); }); + + // End-to-end binding through the REAL service (with a symmetric fake cipher so the + // carrier round-trips): proves that a sealed carrier established before the real + // executionId existed actually gets bound to it, so credential resolution can gate + // on executionPath. Without the bind-on-early-return fix, executionPath stays empty. + describe('execution id binding (real service)', () => { + // Symmetric fake: encrypt is JSON.stringify, decrypt is identity — the real + // toCredentialContext then parses/validates the JSON back into a context. + const fakeCipher = { + encryptV2: async (data: unknown) => JSON.stringify(data), + decryptV2: async (data: string) => data, + } as unknown as Cipher; + + let service: ExecutionContextService; + + beforeEach(() => { + service = new ExecutionContextService( + mock(), + mock(), + fakeCipher, + ); + Container.set(ExecutionContextService, service); + }); + + afterEach(() => { + Container.reset(); + }); + + const pathOf = async (context: IExecutionContext) => { + const decrypted = await service.decryptExecutionContext(context); + return (decrypted.credentials!.metadata as IN8NOAuthMetadata).executionPath; + }; + + it('binds the real execution id onto a seal established before it existed', async () => { + const sealed = await service.buildTriggerIdentityCredentials( + 'oauth-token', + 'https://host/mcp/wf', + undefined, + 'user-123', + ); + const runExecutionData = createRunExecutionData({ + startData: {}, + resultData: { runData: {} }, + executionData: { + contextData: {}, + nodeExecutionStack: [], + metadata: {}, + waitingExecution: {}, + waitingExecutionSource: {}, + }, + }); + // Pre-established carrier, as the webhook mint leaves it (unbound, path []). + runExecutionData.executionData!.runtimeData = { + version: 1, + establishedAt: 1, + source: 'webhook', + credentials: sealed, + }; + const additionalData = mock({ + executionId: 'exec-real', + encryptedRunnerIdentity: undefined, + }); + + await establishExecutionContext(mockWorkflow, runExecutionData, additionalData, 'webhook'); + + expect(await pathOf(runExecutionData.executionData!.runtimeData)).toEqual(['exec-real']); + }); + + it('does not extend a populated path when the carrier belongs to another execution', async () => { + const sealed = await service.buildTriggerIdentityCredentials( + 'oauth-token', + 'https://host/mcp/wf', + undefined, + 'user-123', + ); + const runExecutionData = createRunExecutionData({ + startData: {}, + resultData: { runData: {} }, + executionData: { + contextData: {}, + nodeExecutionStack: [], + metadata: {}, + waitingExecution: {}, + waitingExecutionSource: {}, + }, + }); + // Carrier already bound to a different execution, re-attached to this run. + const carried = await service.maybeBindExecutionId( + { version: 1, establishedAt: 1, source: 'webhook', credentials: sealed }, + 'exec-other', + ); + runExecutionData.executionData!.runtimeData = carried; + const additionalData = mock({ + executionId: 'exec-real', + encryptedRunnerIdentity: undefined, + }); + + await establishExecutionContext(mockWorkflow, runExecutionData, additionalData, 'webhook'); + + // Path stays pinned to its own execution, so resolution rejects this run. + expect(await pathOf(runExecutionData.executionData!.runtimeData)).toEqual(['exec-other']); + }); + + it('appends the retry execution id, keeping the carrier resolvable on retry', async () => { + const sealed = await service.buildTriggerIdentityCredentials( + 'oauth-token', + 'https://host/mcp/wf', + undefined, + 'user-123', + ); + const runExecutionData = createRunExecutionData({ + startData: {}, + resultData: { runData: {} }, + executionData: { + contextData: {}, + nodeExecutionStack: [], + metadata: {}, + waitingExecution: {}, + waitingExecutionSource: {}, + }, + }); + // Original run's carrier, bound to its own execution, reloaded verbatim for the retry. + const carried = await service.maybeBindExecutionId( + { version: 1, establishedAt: 1, source: 'webhook', credentials: sealed }, + 'exec-root', + ); + runExecutionData.executionData!.runtimeData = carried; + const additionalData = mock({ + executionId: 'exec-retry', + encryptedRunnerIdentity: undefined, + }); + + await establishExecutionContext(mockWorkflow, runExecutionData, additionalData, 'retry'); + + // Retry id joins the path, so resolution accepts the run. + expect(await pathOf(runExecutionData.executionData!.runtimeData)).toEqual([ + 'exec-root', + 'exec-retry', + ]); + }); + + it('leaves a legacy (subject-less) carrier untouched', async () => { + const legacy = await service.buildTriggerIdentityCredentials( + 'oauth-token', + 'https://host/mcp/wf', + ); + const runExecutionData = createRunExecutionData({ + startData: {}, + resultData: { runData: {} }, + executionData: { + contextData: {}, + nodeExecutionStack: [], + metadata: {}, + waitingExecution: {}, + waitingExecutionSource: {}, + }, + }); + runExecutionData.executionData!.runtimeData = { + version: 1, + establishedAt: 1, + source: 'webhook', + credentials: legacy, + }; + const additionalData = mock({ + executionId: 'exec-real', + encryptedRunnerIdentity: undefined, + }); + + await establishExecutionContext(mockWorkflow, runExecutionData, additionalData, 'webhook'); + + // No subject → not sealed → executionPath stays as minted (empty), never gated. + expect(await pathOf(runExecutionData.executionData!.runtimeData)).toEqual([]); + }); + }); }); diff --git a/packages/core/src/execution-engine/__tests__/workflow-execute-node-error-reporting.test.ts b/packages/core/src/execution-engine/__tests__/workflow-execute-node-error-reporting.test.ts index 457ded8a0b2..af138513a8f 100644 --- a/packages/core/src/execution-engine/__tests__/workflow-execute-node-error-reporting.test.ts +++ b/packages/core/src/execution-engine/__tests__/workflow-execute-node-error-reporting.test.ts @@ -43,7 +43,10 @@ describe('WorkflowExecute node error forwarding to ErrorReporter', () => { let mockErrorReporter: { error: ReturnType }; let mockNodeTypes: Mocked; let mockLogger: { error: ReturnType; warn: ReturnType }; - let mockExecutionContextService: { augmentExecutionContextWithHooks: ReturnType }; + let mockExecutionContextService: { + augmentExecutionContextWithHooks: ReturnType; + maybeBindExecutionId: ReturnType; + }; beforeEach(() => { vi.clearAllMocks(); @@ -59,6 +62,9 @@ describe('WorkflowExecute node error forwarding to ErrorReporter', () => { context: { version: 1, establishedAt: Date.now(), source: 'manual' }, triggerItems: undefined, }), + // establishExecutionContext also binds the execution id onto an already + // established context; pass through so the run isn't aborted. + maybeBindExecutionId: vi.fn(async (context) => context), }; mockContainer.get.mockImplementation((token) => { diff --git a/packages/core/src/execution-engine/execution-context.service.ts b/packages/core/src/execution-engine/execution-context.service.ts index 6b2028c2945..e8cc1a85ebf 100644 --- a/packages/core/src/execution-engine/execution-context.service.ts +++ b/packages/core/src/execution-engine/execution-context.service.ts @@ -4,7 +4,9 @@ import { ICredentialContext, IExecuteData, IExecutionContext, + IN8NOAuthMetadata, INodeExecutionData, + N8NOAuthMetadataSchema, OAuthResourceGrant, PlaintextExecutionContext, toCredentialContext, @@ -99,20 +101,64 @@ export class ExecutionContextService { ); } + async maybeBindExecutionId( + context: IExecutionContext, + executionId: string | undefined, + { allowInherit = false }: { allowInherit?: boolean } = {}, + ): Promise { + if (!executionId) { + return context; + } + const decryptedContext = await this.decryptExecutionContext(context); + if (decryptedContext.credentials) { + if (decryptedContext.credentials.metadata) { + const metadata = N8NOAuthMetadataSchema.safeParse(decryptedContext.credentials.metadata); + // Only sealed carriers (a resolved subject) need execution binding; a + // non-sealed n8n-oauth carrier has nothing that reads its executionPath. + if (metadata.success && metadata.data.subject) { + const executionPath = metadata.data.executionPath ?? []; + // Seed an empty path at mint, or extend it only for a legitimate re-run of + // this carrier's own execution data — an inherited child (sub-workflow / + // error workflow) or a retry. A non-empty path that lacks this id on a + // non-inherit bind means the carrier was attached to an execution it was + // not minted for — leave it, so credential resolution rejects it. + const mayBind = + !executionPath.includes(executionId) && (executionPath.length === 0 || allowInherit); + if (mayBind) { + metadata.data.executionPath = [...executionPath, executionId]; + decryptedContext.credentials.metadata = metadata.data; + return await this.encryptExecutionContext(decryptedContext); + } + } + } + } + return context; + } + /** - * Seals the token a trigger authenticated its caller with, plus the grant it was - * accepted under, so the run can re-verify itself for as long as it lasts. See - * {@link OAuthResourceGrant}. + * Seals the identity a trigger authenticated its caller with. The token stays in + * `identity` as evidence; `grant` lets the run re-verify that token after the + * protected resource stops resolving (see {@link OAuthResourceGrant}), and `subject` + * seals the resolved n8n user so a bound run resolves without re-verifying the token. */ async buildTriggerIdentityCredentials( token: string, resource: string, grant?: OAuthResourceGrant, + subject?: string, ): Promise { + const metadata: IN8NOAuthMetadata = { + source: 'n8n-oauth', + resource, + establishedAt: Date.now(), + executionPath: [], + ...(grant ? { grant } : {}), + ...(subject ? { subject } : {}), + }; const payload: ICredentialContext = { version: 1, identity: token, - metadata: { source: 'n8n-oauth', resource, ...(grant ? { grant } : {}) }, + metadata, }; return await this.cipher.encryptV2(payload); } diff --git a/packages/core/src/execution-engine/execution-context.ts b/packages/core/src/execution-engine/execution-context.ts index cc9d5c389c9..3455d7980a9 100644 --- a/packages/core/src/execution-engine/execution-context.ts +++ b/packages/core/src/execution-engine/execution-context.ts @@ -111,9 +111,23 @@ export const establishExecutionContext = async ( const executionData = runExecutionData.executionData; + // Call the execution context service to augment the context with any hook-based data + const executionContextService = Container.get(ExecutionContextService); + if (executionData.runtimeData) { - // Context is already established, no further action needed. - // This can happen, when a workflow is resumed from the database. + // Context is already established (e.g. established at webhook mint time, + // before the real executionId existed, or resumed from the database). + // Bind the now-known executionId to any sealed carrier so credential + // resolution can gate on executionPath. No-op for legacy (subject-less) + // carriers and when executionId is undefined; idempotent on resume. + // A retry reloads the original run's data under a NEW executionId, so its + // id must join the sealed carrier's path (allowInherit) or resolution would + // reject it. Resume keeps the same id, so the bind stays a no-op regardless. + executionData.runtimeData = await executionContextService.maybeBindExecutionId( + executionData.runtimeData, + additionalData?.executionId, + { allowInherit: mode === 'retry' }, + ); return; } @@ -128,6 +142,12 @@ export const establishExecutionContext = async ( if (additionalData?.encryptedRunnerIdentity) { executionData.runtimeData.credentials = additionalData.encryptedRunnerIdentity; + if (executionData.runtimeData.credentials) { + executionData.runtimeData = await executionContextService.maybeBindExecutionId( + executionData.runtimeData, + additionalData.executionId, + ); + } } if (runExecutionData.parentExecution) { @@ -142,6 +162,12 @@ export const establishExecutionContext = async ( parentExecutionId: runExecutionData.parentExecution.executionId, }; + executionData.runtimeData = await executionContextService.maybeBindExecutionId( + executionData.runtimeData, + additionalData?.executionId, + { allowInherit: true }, + ); + // The child inherits the parent's context, but its OWN execution record must // still reflect context derived from the child workflow — most importantly // its redaction policy (a policy'd child called by a policy-less parent must @@ -188,12 +214,17 @@ export const establishExecutionContext = async ( ...executionData.runtimeData, parentExecutionId: startItem.metadata.parentExecution.executionId, }; + + // Bind this execution's id to any inherited sealed carrier so it stays + // resolvable within its own execution (mirrors the parentExecution branch). + executionData.runtimeData = await executionContextService.maybeBindExecutionId( + executionData.runtimeData, + additionalData?.executionId, + { allowInherit: true }, + ); return; } - // Call the execution context service to augment the context with any hook-based data - const executionContextService = Container.get(ExecutionContextService); - try { const { context, triggerItems } = await executionContextService.augmentExecutionContextWithHooks( diff --git a/packages/core/src/execution-engine/node-execution-context/webhook-context.ts b/packages/core/src/execution-engine/node-execution-context/webhook-context.ts index c643083452c..a5fc2e11c6f 100644 --- a/packages/core/src/execution-engine/node-execution-context/webhook-context.ts +++ b/packages/core/src/execution-engine/node-execution-context/webhook-context.ts @@ -209,11 +209,11 @@ export class WebhookContext extends NodeExecutionContext implements IWebhookFunc return await this.additionalData.validateN8nOAuth2Token(token, resourceUrl); } - async establishTriggerIdentity(token: string, resource: string): Promise { + async establishTriggerIdentity(token: string, resource: string, subject?: string): Promise { if (!this.additionalData.establishTriggerIdentity) { throw new UnexpectedError('Trigger identity establishment is not available'); } - await this.additionalData.establishTriggerIdentity(token, resource); + await this.additionalData.establishTriggerIdentity(token, resource, subject); } async checkTriggerCredentialStatus(): Promise { diff --git a/packages/workflow/src/execution-context.ts b/packages/workflow/src/execution-context.ts index db75ace737a..aab350416e9 100644 --- a/packages/workflow/src/execution-context.ts +++ b/packages/workflow/src/execution-context.ts @@ -253,6 +253,36 @@ export const ExecutionContextSchema = z */ export type IExecutionContext = z.output; +/** + * Metadata shape for the `n8n-oauth` credential-context source. + * + * `subject` (the resolved n8n user id) and `executionPath` (the execution ids the + * seal is valid for) turn the carrier into a verify-once "sealed" identity: when a + * subject is present, resolution trusts it and binds to `executionPath` instead of + * re-verifying the stored token. Absent `subject` = the legacy token-verify carrier; + * `establishedAt`/`executionPath` are optional so those legacy carriers still parse. + * + * `grant` (see {@link OAuthResourceGrant}) is carried by grant-based triggers so a run + * can re-verify its token after the protected resource stops resolving. It is listed + * here so `maybeBindExecutionId` preserves it through its parse-and-re-encrypt round-trip; + * the identifier validates it against its own local schema. + */ +export const N8NOAuthMetadataSchema = z.object({ + source: z.literal('n8n-oauth'), + subject: z.string().optional(), + resource: z.string(), + establishedAt: z.number().optional(), + executionPath: z.array(z.string()).optional(), + grant: z + .object({ + audiences: z.array(z.string()).min(1), + executeAccessWorkflowId: z.string().optional(), + }) + .optional(), +}); + +export type IN8NOAuthMetadata = z.output; + /** * Runtime representation of execution context with decrypted credential data. * diff --git a/packages/workflow/src/interfaces.ts b/packages/workflow/src/interfaces.ts index aee219ef851..b7c8788be45 100644 --- a/packages/workflow/src/interfaces.ts +++ b/packages/workflow/src/interfaces.ts @@ -1543,7 +1543,7 @@ export interface IWebhookFunctions extends FunctionsBaseWithRequiredKeys<'getMod * Call only after the token is validated. The identity persists for the whole * execution (including across a Wait), within the token's validity window. */ - establishTriggerIdentity(token: string, resource: string): Promise; + establishTriggerIdentity(token: string, resource: string, subject?: string): Promise; /** * Checks the status of the triggering identity's resolvable (end-user) credentials * for this workflow, using the execution context established by @@ -3727,7 +3727,7 @@ export interface IWorkflowExecuteAdditionalData { token: string, resourceUrl: string, ) => Promise; - establishTriggerIdentity?(token: string, resource: string): Promise; + establishTriggerIdentity?(token: string, resource: string, subject?: string): Promise; checkTriggerCredentialStatus?(): Promise; currentNodeExecutionIndex: number; httpResponse?: express.Response;