mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-01 15:47:41 +08:00
fix(core): Keep resolving end-user credentials after a trigger stops listening (#35973)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
parent
a723e791f1
commit
a447ac422e
+48
@@ -339,11 +339,59 @@ describe('N8NIdentifier', () => {
|
||||
expect(mockOAuthVerifier.verifyOAuthAccessToken).toHaveBeenCalledWith(
|
||||
'oauth-access-token',
|
||||
'https://host/mcp/workflow-a',
|
||||
undefined,
|
||||
);
|
||||
expect(mockAuthService.authenticateUserByCookie).not.toHaveBeenCalled();
|
||||
expect(mockAuthService.authenticateUserBasedOnToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should pass the sealed grant through, for a run that outlived its trigger', async () => {
|
||||
mockOAuthVerifier.verifyOAuthAccessToken.mockResolvedValue({ user: mockUser });
|
||||
|
||||
const grant = {
|
||||
audiences: ['https://host/mcp/workflow-a'],
|
||||
executeAccessWorkflowId: 'workflow-a',
|
||||
};
|
||||
|
||||
const result = await identifier.resolve(
|
||||
{
|
||||
identity: 'oauth-access-token',
|
||||
version: 1 as const,
|
||||
metadata: {
|
||||
source: 'n8n-oauth' as const,
|
||||
resource: 'https://host/mcp/workflow-a',
|
||||
grant,
|
||||
},
|
||||
},
|
||||
{},
|
||||
);
|
||||
|
||||
expect(result).toBe('user-123');
|
||||
expect(mockOAuthVerifier.verifyOAuthAccessToken).toHaveBeenCalledWith(
|
||||
'oauth-access-token',
|
||||
'https://host/mcp/workflow-a',
|
||||
grant,
|
||||
);
|
||||
});
|
||||
|
||||
it('should reject a grant that names no audience', async () => {
|
||||
await expect(
|
||||
identifier.resolve(
|
||||
{
|
||||
identity: 'oauth-access-token',
|
||||
version: 1 as const,
|
||||
metadata: {
|
||||
source: 'n8n-oauth' as const,
|
||||
resource: 'https://host/mcp/workflow-a',
|
||||
grant: { audiences: [] },
|
||||
},
|
||||
},
|
||||
{},
|
||||
),
|
||||
).rejects.toThrow(CredentialResolverError);
|
||||
expect(mockOAuthVerifier.verifyOAuthAccessToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should throw CredentialResolverError when the token resolves to no user', async () => {
|
||||
mockOAuthVerifier.verifyOAuthAccessToken.mockResolvedValue({
|
||||
user: null,
|
||||
|
||||
+16
-1
@@ -1,5 +1,5 @@
|
||||
import { Service } from '@n8n/di';
|
||||
import type { ICredentialContext } from 'n8n-workflow';
|
||||
import type { ICredentialContext, OAuthResourceGrant } from 'n8n-workflow';
|
||||
import { ITokenIdentifier } from './identifier-interface';
|
||||
import { AuthService } from '@/auth/auth.service';
|
||||
import { z } from 'zod';
|
||||
@@ -17,9 +17,21 @@ const RequestBoundMetadataSchema = z.object({
|
||||
browserId: z.string().optional(),
|
||||
});
|
||||
|
||||
/**
|
||||
* Declared here rather than shared from `n8n-workflow`: that package is on a different
|
||||
* zod major, so its schemas cannot compose into the union below. `satisfies` keeps this
|
||||
* in step with the {@link OAuthResourceGrant} type it validates.
|
||||
*/
|
||||
const OAuthResourceGrantSchema = z.object({
|
||||
audiences: z.array(z.string()).min(1),
|
||||
executeAccessWorkflowId: z.string().optional(),
|
||||
}) satisfies z.ZodType<OAuthResourceGrant, z.ZodTypeDef, unknown>;
|
||||
|
||||
const N8nOAuthMetadataSchema = z.object({
|
||||
source: z.literal('n8n-oauth'),
|
||||
resource: z.string(),
|
||||
/** Absent for contexts sealed before grants existed, and for long-lived resources. */
|
||||
grant: OAuthResourceGrantSchema.optional(),
|
||||
});
|
||||
|
||||
const N8NIdentifierMetadataSchema = z.discriminatedUnion('source', [
|
||||
@@ -68,9 +80,12 @@ 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.
|
||||
const user = await this.oauthTokenVerifierProxy.verifyOAuthAccessToken(
|
||||
context.identity,
|
||||
metadataResult.data.resource,
|
||||
metadataResult.data.grant,
|
||||
);
|
||||
if (!user?.user) {
|
||||
throw new CredentialResolverError(
|
||||
|
||||
@@ -19,6 +19,7 @@ import type { McpConfig } from '@/modules/mcp/mcp.config';
|
||||
import type { McpSettingsService } from '@/modules/mcp/mcp.settings.service';
|
||||
import { ProtectedResourceRegistry } from '@/services/protected-resource.registry';
|
||||
import type { UrlService } from '@/services/url.service';
|
||||
import type { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
|
||||
const instanceSettings = mock<InstanceSettings>({ encryptionKey: 'test-key' });
|
||||
const jwtService = new JwtService(instanceSettings, mock());
|
||||
@@ -29,6 +30,7 @@ let accessTokenRepository: Mocked<AccessTokenRepository>;
|
||||
let refreshTokenRepository: Mocked<RefreshTokenRepository>;
|
||||
let service: OAuthTokenService;
|
||||
let txRunner: MockProxy<TransactionRunner>;
|
||||
const workflowFinderService = mock<WorkflowFinderService>();
|
||||
|
||||
const TEST_BASE_URL = 'https://n8n.example.com';
|
||||
const TEST_RESOURCE_URL = `${TEST_BASE_URL}/mcp-server/http`;
|
||||
@@ -66,6 +68,7 @@ describe('OAuthTokenService', () => {
|
||||
refreshTokenRepository,
|
||||
registry,
|
||||
txRunner,
|
||||
workflowFinderService,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -587,6 +590,7 @@ describe('OAuthTokenService', () => {
|
||||
refreshTokenRepository,
|
||||
multiResourceRegistry,
|
||||
txRunner,
|
||||
workflowFinderService,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -669,6 +673,7 @@ describe('OAuthTokenService', () => {
|
||||
refreshTokenRepository,
|
||||
scopedRegistry,
|
||||
txRunner,
|
||||
workflowFinderService,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -785,6 +790,7 @@ describe('OAuthTokenService', () => {
|
||||
refreshTokenRepository,
|
||||
configuredRegistry,
|
||||
txRunner,
|
||||
workflowFinderService,
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
import type { User } from '@n8n/db';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import type { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
|
||||
import { authorizeAgainstGrant, triggerResourceGate } from '../resource-gate';
|
||||
|
||||
const user = mock<User>({ id: 'user-1' });
|
||||
const workflowFinderService = mock<WorkflowFinderService>();
|
||||
|
||||
const withExecuteAccessTo = (...workflowIds: string[]) => {
|
||||
workflowFinderService.findWorkflowIdsWithScopeForUser.mockImplementation(
|
||||
async (requested) => new Set(requested.filter((id) => workflowIds.includes(id))),
|
||||
);
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe('authorizeAgainstGrant', () => {
|
||||
it('allows a holder who still has execute access on the named workflow', async () => {
|
||||
withExecuteAccessTo('wf-1');
|
||||
|
||||
await expect(
|
||||
authorizeAgainstGrant(
|
||||
workflowFinderService,
|
||||
{ audiences: ['aud'], executeAccessWorkflowId: 'wf-1' },
|
||||
user,
|
||||
),
|
||||
).resolves.toBe(true);
|
||||
});
|
||||
|
||||
it('denies a holder who has lost it', async () => {
|
||||
withExecuteAccessTo();
|
||||
|
||||
await expect(
|
||||
authorizeAgainstGrant(
|
||||
workflowFinderService,
|
||||
{ audiences: ['aud'], executeAccessWorkflowId: 'wf-1' },
|
||||
user,
|
||||
),
|
||||
).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it('names no workflow when the trigger does not require execute access', async () => {
|
||||
await expect(
|
||||
authorizeAgainstGrant(workflowFinderService, { audiences: ['aud'] }, user),
|
||||
).resolves.toBe(true);
|
||||
expect(workflowFinderService.findWorkflowIdsWithScopeForUser).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('triggerResourceGate', () => {
|
||||
const grant = { audiences: ['aud-a', 'aud-b'], executeAccessWorkflowId: 'wf-1' };
|
||||
|
||||
it('seals the grant it was built from', () => {
|
||||
expect(triggerResourceGate(workflowFinderService, grant).getGrant?.()).toEqual(grant);
|
||||
});
|
||||
|
||||
// The invariant the whole grant mechanism rests on: whichever side of the resource's
|
||||
// lifetime the check happens on, it is the same check.
|
||||
it.each([
|
||||
['grants', ['wf-1'], true],
|
||||
['denies', [], false],
|
||||
])('%s alike whether asked live or via the sealed grant', async (_, accessible, expected) => {
|
||||
withExecuteAccessTo(...accessible);
|
||||
const gate = triggerResourceGate(workflowFinderService, grant);
|
||||
|
||||
await expect(gate.authorize(user)).resolves.toBe(expected);
|
||||
await expect(
|
||||
authorizeAgainstGrant(workflowFinderService, gate.getGrant!(), user),
|
||||
).resolves.toBe(expected);
|
||||
});
|
||||
});
|
||||
+333
@@ -0,0 +1,333 @@
|
||||
import { createWorkflowWithHistory, testDb } from '@n8n/backend-test-utils';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import type { User } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import { Cipher } from 'n8n-core';
|
||||
import type { IHttpRequestMethods, INode, IWebhookData, IWorkflowBase } from 'n8n-workflow';
|
||||
import { toCredentialContext, WEBHOOK_NODE_TYPE } from 'n8n-workflow';
|
||||
import { randomUUID } from 'node:crypto';
|
||||
|
||||
import { createOwner, createMember } from '@test-integration/db/users';
|
||||
import { setupTestServer } from '@test-integration/utils';
|
||||
|
||||
import { OAuthClientRepository } from '@/modules/oauth-server/database/repositories/oauth-client.repository';
|
||||
import { OAuthTokenService } from '@/modules/oauth-server/oauth-token.service';
|
||||
import { CacheService } from '@/services/cache/cache.service';
|
||||
import { ProtectedResourceRegistry } from '@/services/protected-resource.registry';
|
||||
import { UrlService } from '@/services/url.service';
|
||||
import { TestWebhookRegistrationsService } from '@/webhooks/test-webhook-registrations.service';
|
||||
|
||||
/**
|
||||
* A run re-verifies its token on every dynamic-credential access, and outlives the
|
||||
* resource descriptor that gate reads. These tests pin that the sealed grant keeps the
|
||||
* gate working once the resource is gone, without becoming a way around it.
|
||||
*/
|
||||
setupTestServer({ modules: ['oauth-server', 'mcp'], endpointGroups: ['mcp'] });
|
||||
|
||||
let owner: User;
|
||||
let member: User;
|
||||
let webhookTestEndpoint: string;
|
||||
let registrations: TestWebhookRegistrationsService;
|
||||
let clientId: string;
|
||||
|
||||
const testWebhookBaseUrl = () =>
|
||||
Container.get(UrlService).getTestWebhookBaseUrl().replace(/\/$/, '');
|
||||
|
||||
const resourceUrlFor = (webhookPath: string, method: IHttpRequestMethods = 'POST') =>
|
||||
`${testWebhookBaseUrl()}/${webhookTestEndpoint}/${webhookPath}?method=${method}`;
|
||||
|
||||
const oauth2WebhookNode = (requireExecuteAccess?: boolean): INode => ({
|
||||
id: randomUUID(),
|
||||
name: 'Webhook',
|
||||
type: WEBHOOK_NODE_TYPE,
|
||||
typeVersion: 2.1,
|
||||
position: [0, 0],
|
||||
parameters: {
|
||||
path: 'unused',
|
||||
httpMethod: 'POST',
|
||||
authentication: 'n8nOAuth2',
|
||||
...(requireExecuteAccess === undefined ? {} : { requireExecuteAccess }),
|
||||
},
|
||||
});
|
||||
|
||||
/** Mirrors what `TestWebhooks.needsWebhook` writes when the editor starts listening. */
|
||||
const registerTestWebhook = async (
|
||||
webhookPath: string,
|
||||
node: INode,
|
||||
workflow: { id: string; name: string },
|
||||
methods: IHttpRequestMethods[] = ['POST'],
|
||||
) => {
|
||||
for (const httpMethod of methods) {
|
||||
await registrations.register({
|
||||
version: 1,
|
||||
workflowEntity: {
|
||||
id: workflow.id,
|
||||
name: workflow.name,
|
||||
active: false,
|
||||
nodes: [node],
|
||||
connections: {},
|
||||
} as IWorkflowBase,
|
||||
webhook: {
|
||||
httpMethod,
|
||||
path: webhookPath,
|
||||
node: node.name,
|
||||
workflowId: workflow.id,
|
||||
} as IWebhookData,
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
const mintTokenFor = async (resourceUrl: string, user: User) => {
|
||||
const tokenService = Container.get(OAuthTokenService);
|
||||
const pair = tokenService.generateTokenPair(user.id, clientId, resourceUrl, []);
|
||||
await tokenService.saveTokenPair(pair.accessToken, pair.refreshToken, clientId, user.id, []);
|
||||
return pair.accessToken;
|
||||
};
|
||||
|
||||
/**
|
||||
* The grant as the run sees it — round-tripped through the encrypted context, so the
|
||||
* cipher and metadata schema are covered. Mirrors `establishTriggerIdentity`.
|
||||
*/
|
||||
const sealAndReadBackGrant = async (resourceUrl: string) => {
|
||||
const resource = await Container.get(ProtectedResourceRegistry).getByResourceUrl(resourceUrl);
|
||||
const grant = resource?.getGrant?.();
|
||||
|
||||
const sealed = await Container.get(Cipher).encryptV2({
|
||||
version: 1,
|
||||
identity: 'unused',
|
||||
metadata: { source: 'n8n-oauth', resource: resourceUrl, ...(grant ? { grant } : {}) },
|
||||
});
|
||||
|
||||
const context = toCredentialContext(await Container.get(Cipher).decryptV2(sealed));
|
||||
return (context.metadata as { grant?: { audiences: string[]; executeAccessWorkflowId?: string } })
|
||||
.grant;
|
||||
};
|
||||
|
||||
/** Teardown of the registration, as `TestWebhooks.executeWebhook` performs it. */
|
||||
const deregisterTrigger = async (
|
||||
webhookPath: string,
|
||||
methods: IHttpRequestMethods[] = ['POST'],
|
||||
) => {
|
||||
for (const httpMethod of methods) {
|
||||
await registrations.deregister(registrations.toKey({ httpMethod, path: webhookPath }));
|
||||
}
|
||||
};
|
||||
|
||||
beforeAll(async () => {
|
||||
process.env.N8N_ENV_FEAT_WEBHOOK_PRIVATE_CREDENTIALS = 'true';
|
||||
owner = await createOwner();
|
||||
member = await createMember();
|
||||
webhookTestEndpoint = Container.get(GlobalConfig).endpoints.webhookTest;
|
||||
registrations = Container.get(TestWebhookRegistrationsService);
|
||||
|
||||
clientId = randomUUID();
|
||||
await Container.get(OAuthClientRepository).insert({
|
||||
id: clientId,
|
||||
name: 'resource-lifetime-tests',
|
||||
redirectUris: ['https://example.com/callback'],
|
||||
grantTypes: ['authorization_code'],
|
||||
});
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
delete process.env.N8N_ENV_FEAT_WEBHOOK_PRIVATE_CREDENTIALS;
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await Container.get(CacheService).reset();
|
||||
await testDb.truncate([
|
||||
'AccessToken',
|
||||
'RefreshToken',
|
||||
'WebhookEntity',
|
||||
'SharedWorkflow',
|
||||
'WorkflowEntity',
|
||||
'WorkflowHistory',
|
||||
]);
|
||||
});
|
||||
|
||||
describe('protected-resource grants outliving the trigger', () => {
|
||||
test('keeps verifying the run token after the registration is torn down', async () => {
|
||||
const webhookPath = randomUUID();
|
||||
const node = oauth2WebhookNode();
|
||||
const workflow = await createWorkflowWithHistory({ active: false, nodes: [node] }, owner);
|
||||
await registerTestWebhook(webhookPath, node, workflow);
|
||||
|
||||
const resourceUrl = resourceUrlFor(webhookPath);
|
||||
const token = await mintTokenFor(resourceUrl, owner);
|
||||
const tokenService = Container.get(OAuthTokenService);
|
||||
|
||||
// The gate the triggering request itself passes.
|
||||
await expect(tokenService.verifyOAuthAccessToken(token, resourceUrl)).resolves.toMatchObject({
|
||||
user: expect.objectContaining({ id: owner.id }),
|
||||
});
|
||||
|
||||
const grant = await sealAndReadBackGrant(resourceUrl);
|
||||
await deregisterTrigger(webhookPath);
|
||||
|
||||
// Without a grant the gate fails closed once the resource stops resolving.
|
||||
await expect(tokenService.verifyOAuthAccessToken(token, resourceUrl)).resolves.toMatchObject({
|
||||
user: null,
|
||||
context: expect.objectContaining({ reason: 'insufficient_scope' }),
|
||||
});
|
||||
|
||||
// The check the run makes for every dynamic credential it touches, however late.
|
||||
await expect(
|
||||
tokenService.verifyOAuthAccessToken(token, resourceUrl, grant),
|
||||
).resolves.toMatchObject({ user: expect.objectContaining({ id: owner.id }) });
|
||||
});
|
||||
|
||||
test('covers every method of a multi-method trigger', async () => {
|
||||
const webhookPath = randomUUID();
|
||||
const node = oauth2WebhookNode();
|
||||
const workflow = await createWorkflowWithHistory({ active: false, nodes: [node] }, owner);
|
||||
await registerTestWebhook(webhookPath, node, workflow, ['GET', 'POST']);
|
||||
|
||||
const resourceUrl = resourceUrlFor(webhookPath, 'POST');
|
||||
const grant = await sealAndReadBackGrant(resourceUrl);
|
||||
const token = await mintTokenFor(resourceUrlFor(webhookPath, 'GET'), owner);
|
||||
|
||||
// The sealed audiences are the ones the live resource serves, so what a token is
|
||||
// checked against doesn't shift when the resource stops resolving.
|
||||
const live = await Container.get(ProtectedResourceRegistry).getByResourceUrl(resourceUrl);
|
||||
expect(grant?.audiences).toEqual(live?.getAudiences());
|
||||
|
||||
await deregisterTrigger(webhookPath, ['GET', 'POST']);
|
||||
|
||||
await expect(
|
||||
Container.get(OAuthTokenService).verifyOAuthAccessToken(token, resourceUrl, grant),
|
||||
).resolves.toMatchObject({ user: expect.objectContaining({ id: owner.id }) });
|
||||
});
|
||||
|
||||
describe('does not widen what the resource allowed', () => {
|
||||
test('re-checks execute access, so revoking it stops the run', async () => {
|
||||
const webhookPath = randomUUID();
|
||||
const node = oauth2WebhookNode();
|
||||
const workflow = await createWorkflowWithHistory({ active: false, nodes: [node] }, owner);
|
||||
await registerTestWebhook(webhookPath, node, workflow);
|
||||
|
||||
const resourceUrl = resourceUrlFor(webhookPath);
|
||||
// `member` has no execute access, so the grant's live check must reject them.
|
||||
const token = await mintTokenFor(resourceUrl, member);
|
||||
const grant = await sealAndReadBackGrant(resourceUrl);
|
||||
await deregisterTrigger(webhookPath);
|
||||
|
||||
await expect(
|
||||
Container.get(OAuthTokenService).verifyOAuthAccessToken(token, resourceUrl, grant),
|
||||
).resolves.toMatchObject({
|
||||
user: null,
|
||||
context: expect.objectContaining({ reason: 'insufficient_scope' }),
|
||||
});
|
||||
});
|
||||
|
||||
test('rejects a token minted for a different resource', async () => {
|
||||
const webhookPath = randomUUID();
|
||||
const otherPath = randomUUID();
|
||||
const node = oauth2WebhookNode();
|
||||
const workflow = await createWorkflowWithHistory({ active: false, nodes: [node] }, owner);
|
||||
await registerTestWebhook(webhookPath, node, workflow);
|
||||
|
||||
const grant = await sealAndReadBackGrant(resourceUrlFor(webhookPath));
|
||||
const foreignToken = await mintTokenFor(resourceUrlFor(otherPath), owner);
|
||||
await deregisterTrigger(webhookPath);
|
||||
|
||||
await expect(
|
||||
Container.get(OAuthTokenService).verifyOAuthAccessToken(
|
||||
foreignToken,
|
||||
resourceUrlFor(webhookPath),
|
||||
grant,
|
||||
),
|
||||
).resolves.toMatchObject({ user: null });
|
||||
});
|
||||
|
||||
test('stops at the sealed token expiring, which the grant does not extend', async () => {
|
||||
const webhookPath = randomUUID();
|
||||
const node = oauth2WebhookNode();
|
||||
const workflow = await createWorkflowWithHistory({ active: false, nodes: [node] }, owner);
|
||||
await registerTestWebhook(webhookPath, node, workflow);
|
||||
|
||||
const resourceUrl = resourceUrlFor(webhookPath);
|
||||
const token = await mintTokenFor(resourceUrl, owner);
|
||||
const grant = await sealAndReadBackGrant(resourceUrl);
|
||||
await deregisterTrigger(webhookPath);
|
||||
|
||||
const tokenService = Container.get(OAuthTokenService);
|
||||
await expect(
|
||||
tokenService.verifyOAuthAccessToken(token, resourceUrl, grant),
|
||||
).resolves.toMatchObject({ user: expect.objectContaining({ id: owner.id }) });
|
||||
|
||||
// This is the real ceiling on how long a run can keep resolving credentials: the
|
||||
// grant outlives the resource, but not the access token it was sealed with. A run
|
||||
// parked past `getAccessTokenExpirySeconds()` — a Wait node, a long queue backlog —
|
||||
// fails here, not at the resource lookup.
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(
|
||||
new Date(Date.now() + (tokenService.getAccessTokenExpirySeconds() + 60) * 1000),
|
||||
);
|
||||
try {
|
||||
await expect(
|
||||
tokenService.verifyOAuthAccessToken(token, resourceUrl, grant),
|
||||
).resolves.toMatchObject({
|
||||
user: null,
|
||||
context: expect.objectContaining({ reason: 'invalid_token' }),
|
||||
});
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
test('rejects a revoked token', async () => {
|
||||
const webhookPath = randomUUID();
|
||||
const node = oauth2WebhookNode();
|
||||
const workflow = await createWorkflowWithHistory({ active: false, nodes: [node] }, owner);
|
||||
await registerTestWebhook(webhookPath, node, workflow);
|
||||
|
||||
const resourceUrl = resourceUrlFor(webhookPath);
|
||||
const token = await mintTokenFor(resourceUrl, owner);
|
||||
const grant = await sealAndReadBackGrant(resourceUrl);
|
||||
await deregisterTrigger(webhookPath);
|
||||
|
||||
await testDb.truncate(['AccessToken']); // as `revokeAccessToken` would leave it
|
||||
|
||||
await expect(
|
||||
Container.get(OAuthTokenService).verifyOAuthAccessToken(token, resourceUrl, grant),
|
||||
).resolves.toMatchObject({
|
||||
user: null,
|
||||
context: expect.objectContaining({ reason: 'token_not_found_in_db' }),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
test('omits the execute check when the trigger does not require it', async () => {
|
||||
const webhookPath = randomUUID();
|
||||
const node = oauth2WebhookNode(false);
|
||||
const workflow = await createWorkflowWithHistory({ active: false, nodes: [node] }, owner);
|
||||
await registerTestWebhook(webhookPath, node, workflow);
|
||||
|
||||
const resourceUrl = resourceUrlFor(webhookPath);
|
||||
const token = await mintTokenFor(resourceUrl, member);
|
||||
const grant = await sealAndReadBackGrant(resourceUrl);
|
||||
await deregisterTrigger(webhookPath);
|
||||
|
||||
expect(grant?.executeAccessWorkflowId).toBeUndefined();
|
||||
await expect(
|
||||
Container.get(OAuthTokenService).verifyOAuthAccessToken(token, resourceUrl, grant),
|
||||
).resolves.toMatchObject({ user: expect.objectContaining({ id: member.id }) });
|
||||
});
|
||||
|
||||
test('prefers the live resource while the trigger is still registered', async () => {
|
||||
const webhookPath = randomUUID();
|
||||
const node = oauth2WebhookNode();
|
||||
const workflow = await createWorkflowWithHistory({ active: false, nodes: [node] }, owner);
|
||||
await registerTestWebhook(webhookPath, node, workflow);
|
||||
|
||||
const resourceUrl = resourceUrlFor(webhookPath);
|
||||
const token = await mintTokenFor(resourceUrl, member);
|
||||
|
||||
// A grant naming no workflow would pass if it took precedence over the resource.
|
||||
await expect(
|
||||
Container.get(OAuthTokenService).verifyOAuthAccessToken(token, resourceUrl, {
|
||||
audiences: [resourceUrl],
|
||||
}),
|
||||
).resolves.toMatchObject({ user: null });
|
||||
});
|
||||
});
|
||||
@@ -3,9 +3,10 @@ import { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js';
|
||||
import { OAuthTokens } from '@modelcontextprotocol/sdk/shared/auth.js';
|
||||
import { Logger } from '@n8n/backend-common';
|
||||
import { Time } from '@n8n/constants';
|
||||
import { TransactionRunner, UserRepository } from '@n8n/db';
|
||||
import { TransactionRunner, User, UserRepository } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { ensureError } from '@n8n/utils/errors/ensure-error';
|
||||
import type { OAuthResourceGrant } from 'n8n-workflow';
|
||||
import { UnexpectedError } from 'n8n-workflow';
|
||||
import { randomBytes, randomUUID } from 'node:crypto';
|
||||
|
||||
@@ -16,10 +17,12 @@ import type {
|
||||
} from '@/services/oauth-token-verifier-proxy.service';
|
||||
import type { ProtectedResource } from '@/services/protected-resource.registry';
|
||||
import { ProtectedResourceRegistry } from '@/services/protected-resource.registry';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
|
||||
import { AccessTokenRepository } from './database/repositories/oauth-access-token.repository';
|
||||
import { RefreshTokenRepository } from './database/repositories/oauth-refresh-token.repository';
|
||||
import { AccessTokenNotFoundError, JWTVerificationError } from './oauth.errors';
|
||||
import { authorizeAgainstGrant } from './resource-gate';
|
||||
|
||||
/**
|
||||
* Manages the OAuth 2.1 token lifecycle for the shared OAuth server.
|
||||
@@ -42,6 +45,7 @@ export class OAuthTokenService implements OAuthTokenVerifier {
|
||||
private readonly refreshTokenRepository: RefreshTokenRepository,
|
||||
private readonly resourceRegistry: ProtectedResourceRegistry,
|
||||
private readonly txRunner: TransactionRunner,
|
||||
private readonly workflowFinderService: WorkflowFinderService,
|
||||
) {}
|
||||
|
||||
getAccessTokenExpirySeconds(): number {
|
||||
@@ -236,20 +240,30 @@ export class OAuthTokenService implements OAuthTokenVerifier {
|
||||
return scopeClaim === '' ? [] : scopeClaim.split(' ');
|
||||
}
|
||||
|
||||
async verifyOAuthAccessToken(token: string, expectedAudience?: string): Promise<UserWithContext> {
|
||||
async verifyOAuthAccessToken(
|
||||
token: string,
|
||||
expectedAudience?: string,
|
||||
grant?: OAuthResourceGrant,
|
||||
): Promise<UserWithContext> {
|
||||
try {
|
||||
const resource = await this.getResourceByAudience(expectedAudience);
|
||||
|
||||
// Fail closed: a token bearing a resource-scoped audience whose resource
|
||||
// can't be resolved (deleted, or a transient resolver failure the registry
|
||||
// swallows to `undefined`) must NOT bypass the authorize gate below.
|
||||
if (expectedAudience && !resource) {
|
||||
//
|
||||
// A sealed grant stands in for the resource rather than bypassing it — it is
|
||||
// minted by this instance only once this gate has passed, and supplies the
|
||||
// same audiences and execute check the resource would have.
|
||||
if (expectedAudience && !resource && !grant) {
|
||||
return { user: null, context: { reason: 'insufficient_scope', auth_type: 'oauth' } };
|
||||
}
|
||||
|
||||
const authInfo = await this.verifyTokenWithAudiences(
|
||||
token,
|
||||
this.audiencesForResource(resource, expectedAudience),
|
||||
resource || !grant
|
||||
? this.audiencesForResource(resource, expectedAudience)
|
||||
: grant.audiences,
|
||||
);
|
||||
|
||||
const userId =
|
||||
@@ -269,7 +283,7 @@ export class OAuthTokenService implements OAuthTokenVerifier {
|
||||
return { user: null, context: { reason: 'user_not_found', auth_type: 'oauth' } };
|
||||
}
|
||||
|
||||
if (resource && !(await resource.authorize(user))) {
|
||||
if (!(await this.isAuthorized(user, resource, grant))) {
|
||||
this.logger.warn('OAuth token denied: user lacks execute access', {
|
||||
userId: user.id,
|
||||
expectedAudience,
|
||||
@@ -277,7 +291,9 @@ export class OAuthTokenService implements OAuthTokenVerifier {
|
||||
return { user: null, context: { reason: 'insufficient_scope', auth_type: 'oauth' } };
|
||||
}
|
||||
|
||||
return { user, authType: 'oauth', scopes: authInfo.scopes };
|
||||
// Handed back so a caller whose work outlives this resource can seal the gate it
|
||||
// was just admitted by.
|
||||
return { user, authType: 'oauth', scopes: authInfo.scopes, grant: resource?.getGrant?.() };
|
||||
} catch (error) {
|
||||
const errorForSure = ensureError(error);
|
||||
const reason =
|
||||
@@ -370,6 +386,22 @@ export class OAuthTokenService implements OAuthTokenVerifier {
|
||||
return this.resourceRegistry.getAllAudiences();
|
||||
}
|
||||
|
||||
/**
|
||||
* The resource's own gate while it resolves, otherwise the sealed grant — through the
|
||||
* same function that gate is built from, so a grant can't allow more than it did.
|
||||
*/
|
||||
private async isAuthorized(
|
||||
user: User,
|
||||
resource: ProtectedResource | undefined,
|
||||
grant: OAuthResourceGrant | undefined,
|
||||
): Promise<boolean> {
|
||||
if (resource) return await resource.authorize(user);
|
||||
|
||||
if (!grant) return true;
|
||||
|
||||
return await authorizeAgainstGrant(this.workflowFinderService, grant, user);
|
||||
}
|
||||
|
||||
private async getResourceByAudience(
|
||||
expectedAudience?: string,
|
||||
): Promise<ProtectedResource | undefined> {
|
||||
|
||||
+8
-14
@@ -1,6 +1,6 @@
|
||||
import { Logger } from '@n8n/backend-common';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import { User, WorkflowRepository } from '@n8n/db';
|
||||
import { WorkflowRepository } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { FORM_TRIGGER_NODE_TYPE } from 'n8n-workflow';
|
||||
|
||||
@@ -10,6 +10,7 @@ import { UrlService } from '@/services/url.service';
|
||||
import { WebhookService } from '@/webhooks/webhook.service';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
|
||||
import { triggerResourceGate } from '../resource-gate';
|
||||
import {
|
||||
FORM_TRIGGER_SCOPES,
|
||||
resourceUrlToWebhookPath,
|
||||
@@ -81,26 +82,19 @@ export class FormTriggerResourceResolver implements ProtectedResourceResolver {
|
||||
// existing any-authenticated-user behaviour, so turning the feature flag on does
|
||||
// not change who may submit an already-published form. Don't "align" these.
|
||||
const requireExecute = node.parameters.requireExecuteAccess === true;
|
||||
const audiences = [resourceUrl];
|
||||
return {
|
||||
id: 'workflow-form:' + workflow.id,
|
||||
isFirstParty: true,
|
||||
getResourceUrl: () => resourceUrl,
|
||||
getAudiences: () => [resourceUrl],
|
||||
getAudiences: () => audiences,
|
||||
getAllowedRedirectUris: async () => [resourceUrl],
|
||||
scopes: FORM_TRIGGER_SCOPES,
|
||||
displayName: workflow.name,
|
||||
authorize: async (user: User) => {
|
||||
if (requireExecute) {
|
||||
return (
|
||||
await this.workflowFinderService.findWorkflowIdsWithScopeForUser(
|
||||
[workflow.id],
|
||||
user,
|
||||
['workflow:execute'],
|
||||
)
|
||||
).has(workflow.id);
|
||||
}
|
||||
return true;
|
||||
},
|
||||
...triggerResourceGate(this.workflowFinderService, {
|
||||
audiences,
|
||||
executeAccessWorkflowId: requireExecute ? workflow.id : undefined,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
+7
-14
@@ -1,6 +1,5 @@
|
||||
import { Logger } from '@n8n/backend-common';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import { User } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { FORM_TRIGGER_NODE_TYPE } from 'n8n-workflow';
|
||||
|
||||
@@ -10,6 +9,7 @@ import { UrlService } from '@/services/url.service';
|
||||
import { TestWebhookRegistrationsService } from '@/webhooks/test-webhook-registrations.service';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
|
||||
import { triggerResourceGate } from '../resource-gate';
|
||||
import {
|
||||
FORM_TRIGGER_SCOPES,
|
||||
resourceUrlToWebhookPath,
|
||||
@@ -84,26 +84,19 @@ export class FormTriggerTestResourceResolver implements ProtectedResourceResolve
|
||||
// existing any-authenticated-user behaviour, so turning the feature flag on does
|
||||
// not change who may submit an already-published form. Don't "align" these.
|
||||
const requireExecute = node.parameters.requireExecuteAccess === true;
|
||||
const audiences = [resourceUrl];
|
||||
return {
|
||||
id: 'workflow-form:' + workflowEntity.id,
|
||||
isFirstParty: true,
|
||||
getResourceUrl: () => resourceUrl,
|
||||
getAudiences: () => [resourceUrl],
|
||||
getAudiences: () => audiences,
|
||||
getAllowedRedirectUris: async () => [resourceUrl],
|
||||
scopes: FORM_TRIGGER_SCOPES,
|
||||
displayName: workflowEntity.name,
|
||||
authorize: async (user: User) => {
|
||||
if (requireExecute) {
|
||||
return (
|
||||
await this.workflowFinderService.findWorkflowIdsWithScopeForUser(
|
||||
[workflowEntity.id],
|
||||
user,
|
||||
['workflow:execute'],
|
||||
)
|
||||
).has(workflowEntity.id);
|
||||
}
|
||||
return true;
|
||||
},
|
||||
...triggerResourceGate(this.workflowFinderService, {
|
||||
audiences,
|
||||
executeAccessWorkflowId: requireExecute ? workflowEntity.id : undefined,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
+7
-14
@@ -7,13 +7,13 @@ import { Logger } from '@n8n/backend-common';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import { Service } from '@n8n/di';
|
||||
|
||||
import { triggerResourceGate } from '../resource-gate';
|
||||
import {
|
||||
WORKFLOW_MCP_TRIGGER_SCOPES,
|
||||
resourceUrlToWebhookPath,
|
||||
trimSlashes,
|
||||
trimTrailingSlash,
|
||||
} from './utils';
|
||||
import { User } from '@n8n/db';
|
||||
|
||||
@Service()
|
||||
export class WorkflowMcpTestTriggerResourceResolver implements ProtectedResourceResolver {
|
||||
@@ -78,24 +78,17 @@ export class WorkflowMcpTestTriggerResourceResolver implements ProtectedResource
|
||||
) {
|
||||
const resourceUrl = `${trimTrailingSlash(this.urlService.getWebhookBaseUrl())}/${this.config.endpoints.mcpTest}/${path}`;
|
||||
const requireExecute = node.parameters.requireExecuteAccess !== false;
|
||||
const audiences = [resourceUrl];
|
||||
return {
|
||||
id: 'workflow-mcp-test:' + workflowEntity.id,
|
||||
getResourceUrl: () => resourceUrl,
|
||||
getAudiences: () => [resourceUrl],
|
||||
getAudiences: () => audiences,
|
||||
scopes: WORKFLOW_MCP_TRIGGER_SCOPES,
|
||||
displayName: workflowEntity.name,
|
||||
authorize: async (user: User) => {
|
||||
if (requireExecute) {
|
||||
return (
|
||||
await this.workflowFinderService.findWorkflowIdsWithScopeForUser(
|
||||
[workflowEntity.id],
|
||||
user,
|
||||
['workflow:execute'],
|
||||
)
|
||||
).has(workflowEntity.id);
|
||||
}
|
||||
return true;
|
||||
},
|
||||
...triggerResourceGate(this.workflowFinderService, {
|
||||
audiences,
|
||||
executeAccessWorkflowId: requireExecute ? workflowEntity.id : undefined,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
+8
-14
@@ -5,9 +5,10 @@ import { WebhookService } from '@/webhooks/webhook.service';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
import { Logger } from '@n8n/backend-common';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import { User, WorkflowRepository } from '@n8n/db';
|
||||
import { WorkflowRepository } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
|
||||
import { triggerResourceGate } from '../resource-gate';
|
||||
import {
|
||||
WORKFLOW_MCP_TRIGGER_SCOPES,
|
||||
resourceUrlToWebhookPath,
|
||||
@@ -97,24 +98,17 @@ export class WorkflowMcpTriggerResourceResolver implements ProtectedResourceReso
|
||||
) {
|
||||
const resourceUrl = `${trimTrailingSlash(this.urlService.getWebhookBaseUrl())}/${this.config.endpoints.mcp}/${path}`;
|
||||
const requireExecute = node.parameters.requireExecuteAccess !== false;
|
||||
const audiences = [resourceUrl];
|
||||
return {
|
||||
id: 'workflow-mcp:' + workflow.id,
|
||||
getResourceUrl: () => resourceUrl,
|
||||
getAudiences: () => [resourceUrl],
|
||||
getAudiences: () => audiences,
|
||||
scopes: WORKFLOW_MCP_TRIGGER_SCOPES,
|
||||
displayName: workflow.name,
|
||||
authorize: async (user: User) => {
|
||||
if (requireExecute) {
|
||||
return (
|
||||
await this.workflowFinderService.findWorkflowIdsWithScopeForUser(
|
||||
[workflow.id],
|
||||
user,
|
||||
['workflow:execute'],
|
||||
)
|
||||
).has(workflow.id);
|
||||
}
|
||||
return true;
|
||||
},
|
||||
...triggerResourceGate(this.workflowFinderService, {
|
||||
audiences,
|
||||
executeAccessWorkflowId: requireExecute ? workflow.id : undefined,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
+10
-14
@@ -1,6 +1,5 @@
|
||||
import { Logger } from '@n8n/backend-common';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import { User } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { WEBHOOK_NODE_TYPE } from 'n8n-workflow';
|
||||
|
||||
@@ -9,6 +8,8 @@ import type {
|
||||
ProtectedResource,
|
||||
ProtectedResourceResolver,
|
||||
} from '@/services/protected-resource.registry';
|
||||
|
||||
import { triggerResourceGate } from '../resource-gate';
|
||||
import { UrlService } from '@/services/url.service';
|
||||
import { TestWebhooks } from '@/webhooks/test-webhooks';
|
||||
import type { TestWebhookRegistration } from '@/webhooks/test-webhook-registrations.service';
|
||||
@@ -143,24 +144,19 @@ export class WorkflowWebhookTestTriggerResourceResolver implements ProtectedReso
|
||||
const urlFor = (method: string) => `${baseUrl}${methodQueryString(method)}`;
|
||||
const methods = [...new Set(triggerMethods.map((method) => method.toUpperCase()))].sort();
|
||||
const requireExecute = node.parameters.requireExecuteAccess !== false;
|
||||
// One list, served live and sealed into the grant, so the audiences a run is
|
||||
// verified against don't change when the registration goes away.
|
||||
const audiences = methods.map(urlFor);
|
||||
return {
|
||||
id: `workflow-webhook-test:${workflowEntity.id}:${resourcePath}`,
|
||||
getResourceUrl: () => urlFor(requestedMethod),
|
||||
getAudiences: () => methods.map(urlFor),
|
||||
getAudiences: () => audiences,
|
||||
scopes: WEBHOOK_TRIGGER_SCOPES,
|
||||
displayName: workflowEntity.name,
|
||||
authorize: async (user: User) => {
|
||||
if (requireExecute) {
|
||||
return (
|
||||
await this.workflowFinderService.findWorkflowIdsWithScopeForUser(
|
||||
[workflowEntity.id],
|
||||
user,
|
||||
['workflow:execute'],
|
||||
)
|
||||
).has(workflowEntity.id);
|
||||
}
|
||||
return true;
|
||||
},
|
||||
...triggerResourceGate(this.workflowFinderService, {
|
||||
audiences,
|
||||
executeAccessWorkflowId: requireExecute ? workflowEntity.id : undefined,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
+10
-14
@@ -1,6 +1,6 @@
|
||||
import { Logger } from '@n8n/backend-common';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import { User, WorkflowRepository } from '@n8n/db';
|
||||
import { WorkflowRepository } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { WEBHOOK_NODE_TYPE } from 'n8n-workflow';
|
||||
|
||||
@@ -13,6 +13,7 @@ import { UrlService } from '@/services/url.service';
|
||||
import { WebhookService } from '@/webhooks/webhook.service';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
|
||||
import { triggerResourceGate } from '../resource-gate';
|
||||
import {
|
||||
WEBHOOK_TRIGGER_SCOPES,
|
||||
methodQueryString,
|
||||
@@ -158,6 +159,9 @@ export class WorkflowWebhookTriggerResourceResolver implements ProtectedResource
|
||||
const urlFor = (method: string) => `${baseUrl}${methodQueryString(method)}`;
|
||||
const methods = [...new Set(triggerMethods.map((method) => method.toUpperCase()))].sort();
|
||||
const requireExecute = node.parameters.requireExecuteAccess !== false;
|
||||
// One list, served live and sealed into the grant, so the audiences a run is
|
||||
// verified against don't change when the resource stops resolving.
|
||||
const audiences = methods.map(urlFor);
|
||||
return {
|
||||
// Identity = the trigger, so the method is deliberately absent: editing the
|
||||
// node's method list must not rotate the id and drop the user's consent.
|
||||
@@ -170,21 +174,13 @@ export class WorkflowWebhookTriggerResourceResolver implements ProtectedResource
|
||||
// A token minted for any of this trigger's methods is accepted at all of
|
||||
// them; cross-trigger replay stays impossible because the list is built
|
||||
// only from rows sharing this (workflowId, node).
|
||||
getAudiences: () => methods.map(urlFor),
|
||||
getAudiences: () => audiences,
|
||||
scopes: WEBHOOK_TRIGGER_SCOPES,
|
||||
displayName: workflow.name,
|
||||
authorize: async (user: User) => {
|
||||
if (requireExecute) {
|
||||
return (
|
||||
await this.workflowFinderService.findWorkflowIdsWithScopeForUser(
|
||||
[workflow.id],
|
||||
user,
|
||||
['workflow:execute'],
|
||||
)
|
||||
).has(workflow.id);
|
||||
}
|
||||
return true;
|
||||
},
|
||||
...triggerResourceGate(this.workflowFinderService, {
|
||||
audiences,
|
||||
executeAccessWorkflowId: requireExecute ? workflow.id : undefined,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
import type { User } from '@n8n/db';
|
||||
import type { OAuthResourceGrant } from 'n8n-workflow';
|
||||
|
||||
import type { ProtectedResource } from '@/services/protected-resource.registry';
|
||||
import type { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
|
||||
/**
|
||||
* Re-takes the grant's decision for `user`. Used by the live resource's `authorize` and,
|
||||
* once the resource is gone, against the grant sealed into the run — so the check is the
|
||||
* same one either way.
|
||||
*/
|
||||
export async function authorizeAgainstGrant(
|
||||
workflowFinderService: WorkflowFinderService,
|
||||
grant: OAuthResourceGrant,
|
||||
user: User,
|
||||
): Promise<boolean> {
|
||||
if (!grant.executeAccessWorkflowId) return true;
|
||||
|
||||
const allowed = await workflowFinderService.findWorkflowIdsWithScopeForUser(
|
||||
[grant.executeAccessWorkflowId],
|
||||
user,
|
||||
['workflow:execute'],
|
||||
);
|
||||
|
||||
return allowed.has(grant.executeAccessWorkflowId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds a trigger resource's grant and authorize members from one grant, so the sealed
|
||||
* copy can't allow more than the live resource does. Spread into a resolver's descriptor
|
||||
* alongside its identity (`id`, `getResourceUrl`, `getAudiences`, `scopes`, …).
|
||||
*
|
||||
* Not for resources whose gate a grant can't express — the instance MCP server reads a
|
||||
* live instance setting, so it keeps its own `authorize` and offers no grant.
|
||||
*/
|
||||
export function triggerResourceGate(
|
||||
workflowFinderService: WorkflowFinderService,
|
||||
grant: OAuthResourceGrant,
|
||||
): Pick<ProtectedResource, 'getGrant' | 'authorize'> {
|
||||
return {
|
||||
getGrant: () => grant,
|
||||
authorize: async (user: User) =>
|
||||
await authorizeAgainstGrant(workflowFinderService, grant, user),
|
||||
};
|
||||
}
|
||||
@@ -30,7 +30,25 @@ describe('OAuthTokenVerifierProxy', () => {
|
||||
expect(provider.verifyOAuthAccessToken).toHaveBeenCalledWith(
|
||||
'some-token',
|
||||
'https://n8n.example.com/mcp-server/http',
|
||||
undefined,
|
||||
);
|
||||
expect(result).toEqual({ user, authType: 'oauth' });
|
||||
});
|
||||
|
||||
it('should pass a sealed resource grant through to the provider', async () => {
|
||||
const proxy = new OAuthTokenVerifierProxy();
|
||||
const provider = mock<OAuthTokenVerifier>();
|
||||
provider.verifyOAuthAccessToken.mockResolvedValue({ user: mock<User>(), authType: 'oauth' });
|
||||
proxy.registerProvider(provider);
|
||||
|
||||
const grant = { audiences: ['https://n8n.example.com/webhook-test/abc?method=POST'] };
|
||||
|
||||
await proxy.verifyOAuthAccessToken('some-token', 'https://n8n.example.com/x', grant);
|
||||
|
||||
expect(provider.verifyOAuthAccessToken).toHaveBeenCalledWith(
|
||||
'some-token',
|
||||
'https://n8n.example.com/x',
|
||||
grant,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { User } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import type { OAuthResourceGrant } from 'n8n-workflow';
|
||||
|
||||
export type AuthFailureReason =
|
||||
| 'missing_authorization_header'
|
||||
@@ -28,6 +29,12 @@ export type UserWithContext = {
|
||||
authType?: Mcpauth_type;
|
||||
/** OAuth scopes granted to the token. `undefined` = not scope-bearing (e.g. API key) → full access. */
|
||||
scopes?: string[];
|
||||
/**
|
||||
* Sealable form of the gate this call was admitted by, for callers that keep
|
||||
* re-verifying after the resource stops resolving. Absent when the gate can't be
|
||||
* expressed as a grant, or the call was rejected.
|
||||
*/
|
||||
grant?: OAuthResourceGrant;
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -39,8 +46,16 @@ export interface OAuthTokenVerifier {
|
||||
* Verify an OAuth access token against the audiences of the protected
|
||||
* resource identified by `expectedAudience` (its canonical resource URL),
|
||||
* and resolve the token's user.
|
||||
*
|
||||
* `grant` is the gate sealed into an execution (see `OAuthResourceGrant`), consulted
|
||||
* only when the resource no longer resolves. Callers verifying a live request must
|
||||
* not pass it.
|
||||
*/
|
||||
verifyOAuthAccessToken(token: string, expectedAudience?: string): Promise<UserWithContext>;
|
||||
verifyOAuthAccessToken(
|
||||
token: string,
|
||||
expectedAudience?: string,
|
||||
grant?: OAuthResourceGrant,
|
||||
): Promise<UserWithContext>;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -60,7 +75,11 @@ export class OAuthTokenVerifierProxy implements OAuthTokenVerifier {
|
||||
this.provider = provider;
|
||||
}
|
||||
|
||||
async verifyOAuthAccessToken(token: string, expectedAudience?: string): Promise<UserWithContext> {
|
||||
async verifyOAuthAccessToken(
|
||||
token: string,
|
||||
expectedAudience?: string,
|
||||
grant?: OAuthResourceGrant,
|
||||
): Promise<UserWithContext> {
|
||||
if (!this.provider) {
|
||||
return {
|
||||
user: null,
|
||||
@@ -71,6 +90,6 @@ export class OAuthTokenVerifierProxy implements OAuthTokenVerifier {
|
||||
},
|
||||
};
|
||||
}
|
||||
return await this.provider.verifyOAuthAccessToken(token, expectedAudience);
|
||||
return await this.provider.verifyOAuthAccessToken(token, expectedAudience, grant);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ import { Logger } from '@n8n/backend-common';
|
||||
import type { User } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { ensureError } from '@n8n/utils/errors/ensure-error';
|
||||
import type { OAuthResourceGrant } from 'n8n-workflow';
|
||||
|
||||
/**
|
||||
* Descriptor for an OAuth 2.1 protected resource served by this instance.
|
||||
@@ -75,6 +76,14 @@ export interface ProtectedResource {
|
||||
* @returns A promise that resolves to a boolean indicating whether the user is authorized
|
||||
**/
|
||||
authorize(user: User): Promise<boolean>;
|
||||
|
||||
/**
|
||||
* Serializable form of this resource's gate, sealed into the executions it grants
|
||||
* access to — see {@link OAuthResourceGrant}. Implement it on any resource derived
|
||||
* from something shorter-lived than an execution; omitting it makes those runs
|
||||
* depend on the resource still resolving at every credential access.
|
||||
*/
|
||||
getGrant?(): OAuthResourceGrant;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -4,6 +4,7 @@ import type express from 'express';
|
||||
import {
|
||||
BinaryDataService,
|
||||
ErrorReporter,
|
||||
ExecutionContextService,
|
||||
getHtmlSandboxCSP,
|
||||
isWebhookHtmlSandboxingDisabled,
|
||||
} from 'n8n-core';
|
||||
@@ -58,6 +59,8 @@ import { ActiveExecutions } from '@/active-executions';
|
||||
import { AuthService } from '@/auth/auth.service';
|
||||
import { EventService } from '@/events/event.service';
|
||||
import { OwnershipService } from '@/services/ownership.service';
|
||||
import type { ProtectedResource } from '@/services/protected-resource.registry';
|
||||
import { ProtectedResourceRegistry } from '@/services/protected-resource.registry';
|
||||
import { WorkflowStatisticsService } from '@/services/workflow-statistics.service';
|
||||
import { WorkflowRunner } from '@/workflow-runner';
|
||||
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
|
||||
@@ -1075,18 +1078,23 @@ describe('getWebhookErrorMessage', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// Shared by the two `executeWebhook` blocks below: `mockInstance` overwrites the
|
||||
// container binding, so registering these per-describe would leave the first block
|
||||
// holding a mock the code under test no longer resolves.
|
||||
const ownershipService = mockInstance(OwnershipService);
|
||||
const webhookService = mockInstance(WebhookService);
|
||||
const workflowRunner = mockInstance(WorkflowRunner);
|
||||
const activeExecutions = mockInstance(ActiveExecutions);
|
||||
const resourceRegistry = mockInstance(ProtectedResourceRegistry);
|
||||
const executionContextService = mockInstance(ExecutionContextService);
|
||||
mockInstance(AuthService);
|
||||
mockInstance(EventService);
|
||||
mockInstance(WorkflowStatisticsService);
|
||||
|
||||
const WORKFLOW_ID = 'wf-1';
|
||||
const EXECUTION_ID = 'exec-1';
|
||||
|
||||
describe('executeWebhook credential-status gate', () => {
|
||||
const ownershipService = mockInstance(OwnershipService);
|
||||
const webhookService = mockInstance(WebhookService);
|
||||
const workflowRunner = mockInstance(WorkflowRunner);
|
||||
const activeExecutions = mockInstance(ActiveExecutions);
|
||||
mockInstance(AuthService);
|
||||
mockInstance(EventService);
|
||||
mockInstance(WorkflowStatisticsService);
|
||||
|
||||
const WORKFLOW_ID = 'wf-1';
|
||||
const EXECUTION_ID = 'exec-1';
|
||||
|
||||
const missingGateResult: CredentialCheckResult = {
|
||||
readyToExecute: false,
|
||||
credentials: [
|
||||
@@ -1281,3 +1289,137 @@ describe('executeWebhook credential-status gate', () => {
|
||||
expect(workflowRunner.run).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('executeWebhook establishTriggerIdentity', () => {
|
||||
const RESOURCE_URL = 'https://n8n.test/webhook-test/abc?method=POST';
|
||||
const GRANT = { audiences: [RESOURCE_URL], executeAccessWorkflowId: WORKFLOW_ID };
|
||||
|
||||
const resourceWithoutGrant: ProtectedResource = {
|
||||
id: `workflow-webhook-test:${WORKFLOW_ID}:abc`,
|
||||
getResourceUrl: () => RESOURCE_URL,
|
||||
getAudiences: () => [RESOURCE_URL],
|
||||
scopes: [],
|
||||
authorize: async () => true,
|
||||
};
|
||||
|
||||
const resourceWithGrant: ProtectedResource = { ...resourceWithoutGrant, getGrant: () => GRANT };
|
||||
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
vi.clearAllMocks();
|
||||
|
||||
ownershipService.getWorkflowProjectCached.mockResolvedValue(
|
||||
mock<Project>({ id: 'project-1', name: 'Project 1' }),
|
||||
);
|
||||
workflowRunner.run.mockResolvedValue(EXECUTION_ID);
|
||||
activeExecutions.getPostExecutePromise.mockReturnValue(new Promise(() => {}));
|
||||
executionContextService.buildTriggerIdentityCredentials.mockResolvedValue('sealed-context');
|
||||
// `establishExecutionContext` runs the hook pass over the seeded stack.
|
||||
executionContextService.augmentExecutionContextWithHooks.mockImplementation(
|
||||
async (_workflow, _startItem, context) => ({ context, triggerItems: null }),
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* Drives `executeWebhook` for an `n8nOAuth2` Webhook node whose `webhook()` seeds the
|
||||
* run with the caller it just authenticated — what `n8nOAuth2Auth` plus
|
||||
* `context.establishTriggerIdentity` do in the node.
|
||||
*/
|
||||
const runWithTriggerIdentity = async (resource: ProtectedResource | undefined) => {
|
||||
resourceRegistry.getByResourceUrl.mockResolvedValue(resource);
|
||||
|
||||
const additionalData = {
|
||||
webhookWaitingBaseUrl: 'https://n8n.test/webhook-waiting',
|
||||
formWaitingBaseUrl: 'https://n8n.test/form-waiting',
|
||||
} as unknown as IWorkflowExecuteAdditionalData;
|
||||
vi.spyOn(WorkflowExecuteAdditionalData, 'getBase').mockResolvedValue(additionalData);
|
||||
|
||||
webhookService.runWebhook.mockImplementation(async (_workflow, _webhookData, _node, data) => {
|
||||
await data.establishTriggerIdentity!('caller-token', RESOURCE_URL);
|
||||
return { workflowData: [[{ json: {} }]] };
|
||||
});
|
||||
|
||||
const workflowStartNode = mock<INode>({
|
||||
name: 'Webhook',
|
||||
type: WEBHOOK_NODE_TYPE,
|
||||
typeVersion: 2,
|
||||
parameters: { authentication: 'n8nOAuth2' },
|
||||
});
|
||||
|
||||
const workflow = mock<Workflow>({
|
||||
id: WORKFLOW_ID,
|
||||
name: 'Test Workflow',
|
||||
nodeTypes: {
|
||||
getByNameAndVersion: vi
|
||||
.fn()
|
||||
.mockReturnValue(mock<INodeType>({ description: { name: 'webhook' } })),
|
||||
},
|
||||
expression: {
|
||||
getSimpleParameterValue: vi.fn().mockReturnValue('onReceived'),
|
||||
getComplexParameterValue: vi.fn().mockReturnValue('firstEntryJson'),
|
||||
},
|
||||
});
|
||||
|
||||
await executeWebhook(
|
||||
workflow,
|
||||
{
|
||||
webhookDescription: { name: 'default' },
|
||||
workflowId: WORKFLOW_ID,
|
||||
} as unknown as IWebhookData,
|
||||
mock<IWorkflowBase>({ id: WORKFLOW_ID, name: 'Test Workflow' }),
|
||||
workflowStartNode,
|
||||
'manual',
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
mock<WebhookRequest>({ method: 'POST', contentType: undefined }),
|
||||
mock<express.Response>({ headersSent: false }),
|
||||
vi.fn(),
|
||||
);
|
||||
|
||||
return additionalData;
|
||||
};
|
||||
|
||||
it('seals the resource grant, so the run can still verify itself once the trigger is gone', async () => {
|
||||
const additionalData = await runWithTriggerIdentity(resourceWithGrant);
|
||||
|
||||
expect(resourceRegistry.getByResourceUrl).toHaveBeenCalledWith(RESOURCE_URL);
|
||||
expect(executionContextService.buildTriggerIdentityCredentials).toHaveBeenCalledWith(
|
||||
'caller-token',
|
||||
RESOURCE_URL,
|
||||
GRANT,
|
||||
);
|
||||
expect(additionalData.encryptedRunnerIdentity).toBe('sealed-context');
|
||||
});
|
||||
|
||||
it('hands the sealed context to the runner, so it survives the queue hop', async () => {
|
||||
await runWithTriggerIdentity(resourceWithGrant);
|
||||
|
||||
const [runData] = workflowRunner.run.mock.calls[0];
|
||||
|
||||
// Both the field the worker reads and the context persisted with the execution.
|
||||
expect(runData.encryptedRunnerIdentity).toBe('sealed-context');
|
||||
expect(runData.executionData?.executionData?.runtimeData?.credentials).toBe('sealed-context');
|
||||
expect(runData.executionData?.resultData.error).toBeUndefined();
|
||||
});
|
||||
|
||||
it('seals no grant for a resource whose gate cannot be expressed as one', async () => {
|
||||
await runWithTriggerIdentity(resourceWithoutGrant);
|
||||
|
||||
expect(executionContextService.buildTriggerIdentityCredentials).toHaveBeenCalledWith(
|
||||
'caller-token',
|
||||
RESOURCE_URL,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it('seals no grant when the resource has already stopped resolving', async () => {
|
||||
await runWithTriggerIdentity(undefined);
|
||||
|
||||
expect(executionContextService.buildTriggerIdentityCredentials).toHaveBeenCalledWith(
|
||||
'caller-token',
|
||||
RESOURCE_URL,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -33,6 +33,7 @@ import type {
|
||||
IWorkflowExecuteAdditionalData,
|
||||
WebhookResponseMode,
|
||||
OAuth2FailureReason,
|
||||
OAuthResourceGrant,
|
||||
Workflow,
|
||||
WorkflowExecuteMode,
|
||||
IWorkflowExecutionDataProcess,
|
||||
@@ -72,6 +73,7 @@ import {
|
||||
} from '@/services/oauth-token-verifier-proxy.service';
|
||||
import { OAuth2FlowProxy } from '@/services/oauth2-flow-proxy.service';
|
||||
import { OwnershipService } from '@/services/ownership.service';
|
||||
import { ProtectedResourceRegistry } from '@/services/protected-resource.registry';
|
||||
import { WorkflowStatisticsService } from '@/services/workflow-statistics.service';
|
||||
import { WaitTracker } from '@/wait-tracker';
|
||||
import { WebhookExecutionContext } from '@/webhooks/webhook-execution-context';
|
||||
@@ -593,10 +595,15 @@ export async function executeWebhook(
|
||||
additionalData.completeN8nOAuth2Flow = async (code: string, state: string) =>
|
||||
await Container.get(OAuth2FlowProxy).complete(code, state);
|
||||
|
||||
// Captured here so `establishTriggerIdentity` seals the gate that admitted this
|
||||
// request, instead of resolving the resource a second time.
|
||||
let admittedBy: { resource: string; grant?: OAuthResourceGrant } | undefined;
|
||||
|
||||
additionalData.validateN8nOAuth2Token = async (token: string, resourceUrl: string) => {
|
||||
const oauthTokenVerifierProxy = Container.get(OAuthTokenVerifierProxy);
|
||||
const result = await oauthTokenVerifierProxy.verifyOAuthAccessToken(token, resourceUrl);
|
||||
if (result.user) {
|
||||
admittedBy = { resource: resourceUrl, grant: result.grant };
|
||||
return {
|
||||
valid: true,
|
||||
user: {
|
||||
@@ -615,9 +622,25 @@ export async function executeWebhook(
|
||||
};
|
||||
|
||||
additionalData.establishTriggerIdentity = async (token: string, resource: 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`.
|
||||
const grant =
|
||||
admittedBy?.resource === resource
|
||||
? admittedBy.grant
|
||||
: (await Container.get(ProtectedResourceRegistry).getByResourceUrl(resource))?.getGrant?.();
|
||||
|
||||
if (!grant) {
|
||||
// Not fatal now, but this is the state a queued or parked run later fails in.
|
||||
Container.get(Logger).warn(
|
||||
'Established a trigger identity without a resource grant; this run will depend on the protected resource still resolving',
|
||||
{ workflowId: workflow.id, resource },
|
||||
);
|
||||
}
|
||||
|
||||
additionalData.encryptedRunnerIdentity = await Container.get(
|
||||
ExecutionContextService,
|
||||
).buildTriggerIdentityCredentials(token, resource);
|
||||
).buildTriggerIdentityCredentials(token, resource, grant);
|
||||
if (runExecutionData) {
|
||||
await establishExecutionContext(workflow, runExecutionData, additionalData, executionMode);
|
||||
}
|
||||
|
||||
@@ -346,6 +346,31 @@ describe('ExecutionContextService', () => {
|
||||
expect(result).toBe('encrypted-credential-blob');
|
||||
});
|
||||
|
||||
it('should seal the resource grant so the run can verify itself after the trigger is gone', async () => {
|
||||
mockCipher.encryptV2.mockResolvedValue('encrypted-credential-blob');
|
||||
|
||||
const grant = {
|
||||
audiences: ['https://api.example.com/resource?method=POST'],
|
||||
executeAccessWorkflowId: 'workflow-1',
|
||||
};
|
||||
|
||||
await service.buildTriggerIdentityCredentials(
|
||||
'oauth-token-jwt',
|
||||
'https://api.example.com/resource',
|
||||
grant,
|
||||
);
|
||||
|
||||
expect(mockCipher.encryptV2).toHaveBeenCalledWith({
|
||||
version: 1,
|
||||
identity: 'oauth-token-jwt',
|
||||
metadata: {
|
||||
source: 'n8n-oauth',
|
||||
resource: 'https://api.example.com/resource',
|
||||
grant,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('should propagate errors raised by the cipher', async () => {
|
||||
mockCipher.encryptV2.mockRejectedValue(new Error('encryption key missing'));
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
IExecuteData,
|
||||
IExecutionContext,
|
||||
INodeExecutionData,
|
||||
OAuthResourceGrant,
|
||||
PlaintextExecutionContext,
|
||||
toCredentialContext,
|
||||
toExecutionContextEstablishmentHookParameter,
|
||||
@@ -58,11 +59,20 @@ export class ExecutionContextService {
|
||||
return await this.cipher.encryptV2(payload);
|
||||
}
|
||||
|
||||
async buildTriggerIdentityCredentials(token: string, resource: string): Promise<string> {
|
||||
/**
|
||||
* 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}.
|
||||
*/
|
||||
async buildTriggerIdentityCredentials(
|
||||
token: string,
|
||||
resource: string,
|
||||
grant?: OAuthResourceGrant,
|
||||
): Promise<string> {
|
||||
const payload: ICredentialContext = {
|
||||
version: 1,
|
||||
identity: token,
|
||||
metadata: { source: 'n8n-oauth', resource },
|
||||
metadata: { source: 'n8n-oauth', resource, ...(grant ? { grant } : {}) },
|
||||
};
|
||||
return await this.cipher.encryptV2(payload);
|
||||
}
|
||||
|
||||
@@ -60,6 +60,21 @@ export const SecureArtifactsSchema = z
|
||||
*/
|
||||
export type ISecureArtifacts = z.output<typeof SecureArtifactsSchema>;
|
||||
|
||||
/**
|
||||
* What a run needs to keep verifying its token once the OAuth protected resource it was
|
||||
* granted access to can no longer be looked up. Resource descriptors are derived from
|
||||
* what routes the request, which stops existing when the trigger stops listening; a run
|
||||
* outlives that, so it carries the facts with it.
|
||||
*
|
||||
* Holds no authorization *decision* — only its inputs, so every check stays live.
|
||||
*/
|
||||
export interface OAuthResourceGrant {
|
||||
/** `aud` values a token issued for this resource may carry. */
|
||||
audiences: string[];
|
||||
/** Workflow the holder must keep `workflow:execute` on. Absent if none is required. */
|
||||
executeAccessWorkflowId?: string;
|
||||
}
|
||||
|
||||
const CredentialContextSchemaV1 = z.object({
|
||||
version: z.literal(1),
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user