diff --git a/docs/generated/postgres-schema/README.md b/docs/generated/postgres-schema/README.md index 90da8f2b5c2..773be6d53a3 100644 --- a/docs/generated/postgres-schema/README.md +++ b/docs/generated/postgres-schema/README.md @@ -93,7 +93,7 @@ Auto-generated from the PostgreSQL migrations in @n8n/db. Do not edit by hand. | [public.oauth_access_tokens](public.oauth_access_tokens.md) | 3 | | BASE TABLE | | [public.oauth_authorization_codes](public.oauth_authorization_codes.md) | 13 | | BASE TABLE | | [public.oauth_clients](public.oauth_clients.md) | 10 | | BASE TABLE | -| [public.oauth_refresh_tokens](public.oauth_refresh_tokens.md) | 7 | | BASE TABLE | +| [public.oauth_refresh_tokens](public.oauth_refresh_tokens.md) | 8 | | BASE TABLE | | [public.oauth_user_consents](public.oauth_user_consents.md) | 5 | | BASE TABLE | | [public.poller_state](public.poller_state.md) | 7 | | BASE TABLE | | [public.processed_data](public.processed_data.md) | 5 | | BASE TABLE | @@ -1213,6 +1213,7 @@ erDiagram varchar clientId FK timestamp_3__with_time_zone createdAt bigint expiresAt + varchar resource json scope varchar_255_ token timestamp_3__with_time_zone updatedAt diff --git a/docs/generated/postgres-schema/public.oauth_clients.md b/docs/generated/postgres-schema/public.oauth_clients.md index 60895179557..47c0397a956 100644 --- a/docs/generated/postgres-schema/public.oauth_clients.md +++ b/docs/generated/postgres-schema/public.oauth_clients.md @@ -81,6 +81,7 @@ erDiagram varchar clientId FK timestamp_3__with_time_zone createdAt bigint expiresAt + varchar resource json scope varchar_255_ token timestamp_3__with_time_zone updatedAt diff --git a/docs/generated/postgres-schema/public.oauth_refresh_tokens.md b/docs/generated/postgres-schema/public.oauth_refresh_tokens.md index edde80d3771..716c8e1dc25 100644 --- a/docs/generated/postgres-schema/public.oauth_refresh_tokens.md +++ b/docs/generated/postgres-schema/public.oauth_refresh_tokens.md @@ -7,6 +7,7 @@ | clientId | varchar | | false | | [public.oauth_clients](public.oauth_clients.md) | | | createdAt | timestamp(3) with time zone | CURRENT_TIMESTAMP(3) | false | | | | | expiresAt | bigint | | false | | | Unix timestamp in milliseconds | +| resource | varchar | | false | | | RFC 8707 resource indicator the grant was approved for (e.g. https://n8n.example.com/mcp-server/http); the audience of every access token minted from this refresh token. | | scope | json | '["tool:listWorkflows","tool:getWorkflowDetails"]'::json | false | | | OAuth scopes granted for this refresh token | | token | varchar(255) | | false | | | | | updatedAt | timestamp(3) with time zone | CURRENT_TIMESTAMP(3) | false | | | | @@ -22,6 +23,7 @@ | oauth_refresh_tokens_clientId_not_null | n | NOT NULL "clientId" | | oauth_refresh_tokens_createdAt_not_null | n | NOT NULL "createdAt" | | oauth_refresh_tokens_expiresAt_not_null | n | NOT NULL "expiresAt" | +| oauth_refresh_tokens_resource_not_null | n | NOT NULL resource | | oauth_refresh_tokens_scope_not_null | n | NOT NULL scope | | oauth_refresh_tokens_token_not_null | n | NOT NULL token | | oauth_refresh_tokens_updatedAt_not_null | n | NOT NULL "updatedAt" | @@ -45,6 +47,7 @@ erDiagram varchar clientId FK timestamp_3__with_time_zone createdAt bigint expiresAt + varchar resource json scope varchar_255_ token timestamp_3__with_time_zone updatedAt diff --git a/docs/generated/postgres-schema/public.user.md b/docs/generated/postgres-schema/public.user.md index fe1abf3b330..87fe21bf3b0 100644 --- a/docs/generated/postgres-schema/public.user.md +++ b/docs/generated/postgres-schema/public.user.md @@ -266,6 +266,7 @@ erDiagram varchar clientId FK timestamp_3__with_time_zone createdAt bigint expiresAt + varchar resource json scope varchar_255_ token timestamp_3__with_time_zone updatedAt diff --git a/docs/generated/sqlite-schema/README.md b/docs/generated/sqlite-schema/README.md index e16c8d2089a..5a254ca8855 100644 --- a/docs/generated/sqlite-schema/README.md +++ b/docs/generated/sqlite-schema/README.md @@ -93,7 +93,7 @@ Auto-generated from the SQLite migrations in @n8n/db. Do not edit by hand. | [oauth_access_tokens](oauth_access_tokens.md) | 3 | | table | | [oauth_authorization_codes](oauth_authorization_codes.md) | 13 | | table | | [oauth_clients](oauth_clients.md) | 10 | | table | -| [oauth_refresh_tokens](oauth_refresh_tokens.md) | 7 | | table | +| [oauth_refresh_tokens](oauth_refresh_tokens.md) | 8 | | table | | [oauth_user_consents](oauth_user_consents.md) | 5 | | table | | [poller_state](poller_state.md) | 7 | | table | | [processed_data](processed_data.md) | 5 | | table | @@ -266,8 +266,8 @@ erDiagram "oauth_access_tokens" }o--|| "oauth_clients" : "FOREIGN KEY (clientId) REFERENCES oauth_clients (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE" "oauth_authorization_codes" }o--|| "oauth_clients" : "FOREIGN KEY (clientId) REFERENCES oauth_clients (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE" "oauth_authorization_codes" }o--|| "user" : "FOREIGN KEY (userId) REFERENCES user (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE" -"oauth_refresh_tokens" }o--|| "oauth_clients" : "FOREIGN KEY (clientId) REFERENCES oauth_clients (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE" "oauth_refresh_tokens" }o--|| "user" : "FOREIGN KEY (userId) REFERENCES user (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE" +"oauth_refresh_tokens" }o--|| "oauth_clients" : "FOREIGN KEY (clientId) REFERENCES oauth_clients (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE" "oauth_user_consents" }o--|| "user" : "FOREIGN KEY (userId) REFERENCES user (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE" "oauth_user_consents" }o--|| "oauth_clients" : "FOREIGN KEY (clientId) REFERENCES oauth_clients (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE" "poller_state" |o--|| "workflow_entity" : "FOREIGN KEY (workflowId) REFERENCES workflow_entity (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE" @@ -1200,6 +1200,7 @@ erDiagram varchar clientId FK datetime_3_ createdAt bigint expiresAt + varchar resource TEXT scope varchar_255_ token PK datetime_3_ updatedAt diff --git a/docs/generated/sqlite-schema/oauth_clients.md b/docs/generated/sqlite-schema/oauth_clients.md index 69a9df541a1..615a60acfae 100644 --- a/docs/generated/sqlite-schema/oauth_clients.md +++ b/docs/generated/sqlite-schema/oauth_clients.md @@ -85,6 +85,7 @@ erDiagram varchar clientId FK datetime_3_ createdAt bigint expiresAt + varchar resource TEXT scope varchar_255_ token PK datetime_3_ updatedAt diff --git a/docs/generated/sqlite-schema/oauth_refresh_tokens.md b/docs/generated/sqlite-schema/oauth_refresh_tokens.md index b4dc6c8a780..5c9944581d5 100644 --- a/docs/generated/sqlite-schema/oauth_refresh_tokens.md +++ b/docs/generated/sqlite-schema/oauth_refresh_tokens.md @@ -6,7 +6,7 @@ Table Definition ```sql -CREATE TABLE "oauth_refresh_tokens" ("token" varchar(255) PRIMARY KEY NOT NULL, "clientId" varchar NOT NULL, "userId" varchar NOT NULL, "expiresAt" bigint NOT NULL, "createdAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "updatedAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "scope" text NOT NULL DEFAULT ('["tool:listWorkflows","tool:getWorkflowDetails"]'), CONSTRAINT "FK_a699f3ed9fd0c1b19bc2608ac53" FOREIGN KEY ("userId") REFERENCES "user" ("id") ON DELETE CASCADE ON UPDATE NO ACTION, CONSTRAINT "FK_b388696ce4d8be7ffbe8d3e4b69" FOREIGN KEY ("clientId") REFERENCES "oauth_clients" ("id") ON DELETE CASCADE ON UPDATE NO ACTION) +CREATE TABLE "oauth_refresh_tokens" ("token" varchar(255) PRIMARY KEY NOT NULL, "clientId" varchar NOT NULL, "userId" varchar NOT NULL, "expiresAt" bigint NOT NULL, "createdAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "updatedAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "scope" text NOT NULL DEFAULT ('["tool:listWorkflows","tool:getWorkflowDetails"]'), "resource" varchar NOT NULL, CONSTRAINT "FK_b388696ce4d8be7ffbe8d3e4b69" FOREIGN KEY ("clientId") REFERENCES "oauth_clients" ("id") ON DELETE CASCADE ON UPDATE NO ACTION, CONSTRAINT "FK_a699f3ed9fd0c1b19bc2608ac53" FOREIGN KEY ("userId") REFERENCES "user" ("id") ON DELETE CASCADE ON UPDATE NO ACTION) ``` @@ -18,6 +18,7 @@ CREATE TABLE "oauth_refresh_tokens" ("token" varchar(255) PRIMARY KEY NOT NULL, | clientId | varchar | | false | | [oauth_clients](oauth_clients.md) | | | createdAt | datetime(3) | STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW') | false | | | | | expiresAt | bigint | | false | | | | +| resource | varchar | | false | | | | | scope | TEXT | '["tool:listWorkflows","tool:getWorkflowDetails"]' | false | | | | | token | varchar(255) | | false | | | | | updatedAt | datetime(3) | STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW') | false | | | | @@ -27,8 +28,8 @@ CREATE TABLE "oauth_refresh_tokens" ("token" varchar(255) PRIMARY KEY NOT NULL, | Name | Type | Definition | | ---- | ---- | ---------- | -| - (Foreign key ID: 0) | FOREIGN KEY | FOREIGN KEY (clientId) REFERENCES oauth_clients (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE | -| - (Foreign key ID: 1) | FOREIGN KEY | FOREIGN KEY (userId) REFERENCES user (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE | +| - (Foreign key ID: 0) | FOREIGN KEY | FOREIGN KEY (userId) REFERENCES user (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE | +| - (Foreign key ID: 1) | FOREIGN KEY | FOREIGN KEY (clientId) REFERENCES oauth_clients (id) ON UPDATE NO ACTION ON DELETE CASCADE MATCH NONE | | sqlite_autoindex_oauth_refresh_tokens_1 | PRIMARY KEY | PRIMARY KEY (token) | | token | PRIMARY KEY | PRIMARY KEY (token) | @@ -50,6 +51,7 @@ erDiagram varchar clientId FK datetime_3_ createdAt bigint expiresAt + varchar resource TEXT scope varchar_255_ token PK datetime_3_ updatedAt diff --git a/docs/generated/sqlite-schema/user.md b/docs/generated/sqlite-schema/user.md index af2a05a3998..86b940a62fc 100644 --- a/docs/generated/sqlite-schema/user.md +++ b/docs/generated/sqlite-schema/user.md @@ -272,6 +272,7 @@ erDiagram varchar clientId FK datetime_3_ createdAt bigint expiresAt + varchar resource TEXT scope varchar_255_ token PK datetime_3_ updatedAt diff --git a/packages/@n8n/db/src/migrations/common/1787739515257-AddResourceToOAuthRefreshTokens.ts b/packages/@n8n/db/src/migrations/common/1787739515257-AddResourceToOAuthRefreshTokens.ts new file mode 100644 index 00000000000..eb83a8d4e64 --- /dev/null +++ b/packages/@n8n/db/src/migrations/common/1787739515257-AddResourceToOAuthRefreshTokens.ts @@ -0,0 +1,45 @@ +import type { MigrationContext, ReversibleMigration } from '../migration-types'; + +const TABLE_NAME = 'oauth_refresh_tokens'; +const COLUMN_NAME = 'resource'; + +/** + * Binds every refresh token to the protected resource its grant was approved for, + * so rotation reissues the same audience instead of taking one from the request. + * + * Existing rows are cleared first: they predate the column and carry no record of + * the resource they were granted for, so there is nothing to backfill them with. + * Defaulting them to the instance MCP resource would widen those grants, so they + * are dropped instead — clients re-run the authorization flow on their next + * refresh. Access tokens are left alone; they are already audience-bound and + * expire within the hour, so live sessions are unaffected. + * + * Reversible on purpose, following `DisallowOrphanExecutions1693554410387`: the column + * is NOT NULL with no default, so a downgrade needs it dropped for the older code to + * insert tokens again. Reversal excludes restoring deleted rows. + */ +export class AddResourceToOAuthRefreshTokens1787739515257 implements ReversibleMigration { + async up({ schemaBuilder: { addColumns, column }, escape, runQuery }: MigrationContext) { + await runQuery(`DELETE FROM ${escape.tableName(TABLE_NAME)}`); + + // No default on purpose: an insert that forgets the resource must fail instead + // of silently binding the grant to something the user never approved. + await addColumns( + TABLE_NAME, + [ + column(COLUMN_NAME) + .varchar() + .notNull.comment( + 'RFC 8707 resource indicator the grant was approved for (e.g. https://n8n.example.com/mcp-server/http); ' + + 'the audience of every access token minted from this refresh token.', + ), + ], + { recreatesOnSqlite: true }, + ); + } + + /** Reversal excludes restoring the cleared refresh tokens. */ + async down({ schemaBuilder: { dropColumns } }: MigrationContext) { + await dropColumns(TABLE_NAME, [COLUMN_NAME], { recreatesOnSqlite: true }); + } +} diff --git a/packages/cli/src/modules/oauth-server/__tests__/chat-trigger-resource.resolver.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/chat-trigger-resource.resolver.api.test.ts index 86da42ba6cb..5b233f8793d 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/chat-trigger-resource.resolver.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/chat-trigger-resource.resolver.api.test.ts @@ -369,7 +369,14 @@ describe('runtime gate: verifyOAuthAccessToken enforces workflow:execute', () => tokenEndpointAuthMethod: 'none', }); const pair = tokenService.generateTokenPair(userId, clientId, resourceUrl, []); - await tokenService.saveTokenPair(pair.accessToken, pair.refreshToken, clientId, userId, []); + await tokenService.saveTokenPair( + pair.accessToken, + pair.refreshToken, + clientId, + userId, + [], + pair.audience, + ); return pair.accessToken; }; diff --git a/packages/cli/src/modules/oauth-server/__tests__/chat-trigger-test-resource.resolver.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/chat-trigger-test-resource.resolver.api.test.ts index d4d912a4a22..87b78292512 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/chat-trigger-test-resource.resolver.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/chat-trigger-test-resource.resolver.api.test.ts @@ -313,7 +313,14 @@ describe('runtime gate: verifyOAuthAccessToken enforces workflow:execute', () => tokenEndpointAuthMethod: 'none', }); const pair = tokenService.generateTokenPair(userId, clientId, resourceUrl, []); - await tokenService.saveTokenPair(pair.accessToken, pair.refreshToken, clientId, userId, []); + await tokenService.saveTokenPair( + pair.accessToken, + pair.refreshToken, + clientId, + userId, + [], + pair.audience, + ); return pair.accessToken; }; @@ -383,7 +390,14 @@ describe('test vs production chat resources', () => { const mint = async (resourceUrl: string) => { const pair = tokenService.generateTokenPair(owner.id, clientId, resourceUrl, []); - await tokenService.saveTokenPair(pair.accessToken, pair.refreshToken, clientId, owner.id, []); + await tokenService.saveTokenPair( + pair.accessToken, + pair.refreshToken, + clientId, + owner.id, + [], + pair.audience, + ); return pair.accessToken; }; diff --git a/packages/cli/src/modules/oauth-server/__tests__/form-trigger-resource.resolver.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/form-trigger-resource.resolver.api.test.ts index cf6db766bb7..54847ced843 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/form-trigger-resource.resolver.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/form-trigger-resource.resolver.api.test.ts @@ -315,7 +315,14 @@ describe('runtime gate: verifyOAuthAccessToken enforces workflow:execute', () => tokenEndpointAuthMethod: 'none', }); const pair = tokenService.generateTokenPair(userId, clientId, resourceUrl, []); - await tokenService.saveTokenPair(pair.accessToken, pair.refreshToken, clientId, userId, []); + await tokenService.saveTokenPair( + pair.accessToken, + pair.refreshToken, + clientId, + userId, + [], + pair.audience, + ); return pair.accessToken; }; diff --git a/packages/cli/src/modules/oauth-server/__tests__/form-trigger-test-resource.resolver.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/form-trigger-test-resource.resolver.api.test.ts index ff4855cbc37..86914af8266 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/form-trigger-test-resource.resolver.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/form-trigger-test-resource.resolver.api.test.ts @@ -323,7 +323,14 @@ describe('test vs production form resources', () => { const mint = async (resourceUrl: string) => { const pair = tokenService.generateTokenPair(owner.id, clientId, resourceUrl, []); - await tokenService.saveTokenPair(pair.accessToken, pair.refreshToken, clientId, owner.id, []); + await tokenService.saveTokenPair( + pair.accessToken, + pair.refreshToken, + clientId, + owner.id, + [], + pair.audience, + ); return pair.accessToken; }; diff --git a/packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts b/packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts index b0138dd2c31..242a2871f50 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts @@ -873,6 +873,7 @@ describe('OAuthServerService', () => { tokenService.generateTokenPair.mockReturnValue({ accessToken: 'access-token-123', refreshToken: 'refresh-token-456', + audience: TEST_RESOURCE_URL, }); tokenService.saveTokenPair.mockResolvedValue(); tokenService.getAccessTokenExpirySeconds.mockReturnValue(3600); @@ -904,6 +905,7 @@ describe('OAuthServerService', () => { 'client-123', 'user-456', ['workflow:read'], + TEST_RESOURCE_URL, ); expect(result).toEqual({ access_token: 'access-token-123', @@ -978,6 +980,7 @@ describe('OAuthServerService', () => { tokenService.generateTokenPair.mockReturnValue({ accessToken: 'access-token-123', refreshToken: 'refresh-token-456', + audience: formResourceUrl, }); tokenService.saveTokenPair.mockResolvedValue(); tokenService.getAccessTokenExpirySeconds.mockReturnValue(3600); @@ -1016,6 +1019,7 @@ describe('OAuthServerService', () => { tokenService.generateTokenPair.mockReturnValue({ accessToken: 'access-token-123', refreshToken: 'refresh-token-456', + audience: TEST_RESOURCE_URL, }); await service.exchangeAuthorizationCode(client, 'auth-code-123', 'verifier-123'); @@ -1028,14 +1032,133 @@ describe('OAuthServerService', () => { expect(authorizationCodeService.markAuthorizationCodeAsUsed).toHaveBeenCalledWith( 'auth-code-123', ); + // A code carrying no resource was approved against the default resource, + // so that is what the token is minted for. expect(tokenService.generateTokenPair).toHaveBeenCalledWith( 'user-456', 'client-123', - undefined, + TEST_RESOURCE_URL, ['workflow:read'], ); }); + describe('resource binding', () => { + const client = { + client_id: 'client-123', + client_name: 'Test Client', + redirect_uris: ['https://example.com/callback'], + grant_types: ['authorization_code'], + token_endpoint_auth_method: 'none', + response_types: ['code'], + scope: 'read', + logo_uri: undefined, + tos_uri: undefined, + }; + + // A second registered resource the code was never approved for. + const otherResourceUrl = 'https://n8n.example.com/mcp/other'; + let boundService: OAuthServerService; + + beforeEach(() => { + const registry = new ProtectedResourceRegistry(mock()); + registry.register({ + id: 'instance-mcp', + getResourceUrl: () => TEST_RESOURCE_URL, + getAudiences: () => [TEST_RESOURCE_URL], + scopes: SUPPORTED_SCOPES, + isDefault: true, + authorize: async () => true, + }); + registry.register({ + id: 'other-resource', + getResourceUrl: () => otherResourceUrl, + getAudiences: () => [otherResourceUrl], + scopes: [], + authorize: async () => true, + }); + + boundService = new OAuthServerService( + logger, + mockInstance(GlobalConfig), + oauthSessionService, + oauthClientRepository, + tokenService, + authorizationCodeService, + userConsentRepository, + registry, + mailer, + urlServiceMock, + eventService, + mock(), + mock(), + ); + + tokenService.generateTokenPair.mockReturnValue({ + accessToken: 'access-token-123', + refreshToken: 'refresh-token-456', + audience: TEST_RESOURCE_URL, + }); + tokenService.getAccessTokenExpirySeconds.mockReturnValue(3600); + }); + + const mockCode = (resource: string | null) => + authorizationCodeService.findAuthorizationCode.mockResolvedValue({ + userId: 'user-456', + clientId: 'client-123', + resource, + scope: [] as string[], + } as AuthorizationCode); + + it('should reject a token request naming a resource the code was not approved for', async () => { + mockCode(TEST_RESOURCE_URL); + + await expect( + boundService.exchangeAuthorizationCode( + client, + 'auth-code-123', + 'verifier-123', + 'https://example.com/callback', + new URL(otherResourceUrl), + ), + ).rejects.toThrow(InvalidTargetError); + + expect(authorizationCodeService.markAuthorizationCodeAsUsed).not.toHaveBeenCalled(); + }); + + it('should reject a resource introduced at token time when the code carries none', async () => { + mockCode(null); + + await expect( + boundService.exchangeAuthorizationCode( + client, + 'auth-code-123', + 'verifier-123', + 'https://example.com/callback', + new URL(otherResourceUrl), + ), + ).rejects.toThrow(InvalidTargetError); + }); + + it('should accept the default resource at token time when the code carries none', async () => { + mockCode(null); + + await boundService.exchangeAuthorizationCode( + client, + 'auth-code-123', + 'verifier-123', + 'https://example.com/callback', + new URL(TEST_RESOURCE_URL), + ); + + expect(tokenService.generateTokenPair).toHaveBeenCalledWith( + 'user-456', + 'client-123', + TEST_RESOURCE_URL, + [], + ); + }); + }); + it('should propagate InvalidGrantError when markAuthorizationCodeAsUsed detects concurrent consumption', async () => { const client = { client_id: 'client-123', 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 3b356a004c6..d0a03c90360 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 @@ -1,3 +1,4 @@ +import { InvalidTargetError } from '@modelcontextprotocol/sdk/server/auth/errors.js'; import type { Mocked } from 'vitest'; import { Logger, type LicenseState, type ModuleRegistry } from '@n8n/backend-common'; import { mockInstance } from '@n8n/backend-test-utils'; @@ -35,6 +36,7 @@ const workflowFinderService = mock(); const TEST_BASE_URL = 'https://n8n.example.com'; const TEST_RESOURCE_URL = `${TEST_BASE_URL}/mcp-server/http`; const LEGACY_AUDIENCE = 'mcp-server-api'; +const OTHER_RESOURCE_URL = `${TEST_BASE_URL}/mcp/workflow-b`; const registry = new ProtectedResourceRegistry(mock()); registry.register({ @@ -132,6 +134,21 @@ describe('OAuthTokenService', () => { expect(decoded.scope).toBe(''); }); + it('should return the resolved audience so the grant can be persisted', () => { + const withResource = service.generateTokenPair( + 'user-123', + 'client-456', + OTHER_RESOURCE_URL, + [], + ); + expect(withResource.audience).toBe(OTHER_RESOURCE_URL); + + // Resource-less grants resolve to the default resource, so the returned + // audience is always a concrete URL. + const withoutResource = service.generateTokenPair('user-123', 'client-456', undefined, []); + expect(withoutResource.audience).toBe(TEST_RESOURCE_URL); + }); + it('should generate different tokens on each call', () => { const userId = 'user-123'; const clientId = 'client-456'; @@ -151,7 +168,14 @@ describe('OAuthTokenService', () => { const clientId = 'client-123'; const userId = 'user-456'; - await service.saveTokenPair(accessToken, refreshToken, clientId, userId, ['workflow:read']); + await service.saveTokenPair( + accessToken, + refreshToken, + clientId, + userId, + ['workflow:read'], + TEST_RESOURCE_URL, + ); expect(txRunner.run).toHaveBeenCalled(); @@ -167,6 +191,7 @@ describe('OAuthTokenService', () => { userId, expiresAt: expect.any(Number), scope: ['workflow:read'], + resource: TEST_RESOURCE_URL, }, expect.anything(), ); @@ -183,6 +208,7 @@ describe('OAuthTokenService', () => { userId: 'user-456', expiresAt: Date.now() + 1000000, // Valid scope: [], + resource: TEST_RESOURCE_URL, }); refreshTokenRepository.findByToken.mockResolvedValue(refreshTokenRecord); @@ -215,6 +241,7 @@ describe('OAuthTokenService', () => { userId: 'user-456', expiresAt: Date.now() + 1000000, scope: [], + resource: TEST_RESOURCE_URL, }); refreshTokenRepository.findByToken.mockResolvedValue(refreshTokenRecord); @@ -241,6 +268,7 @@ describe('OAuthTokenService', () => { userId: 'user-456', expiresAt: Date.now() + 1000000, scope: ['workflow:read', 'execution:read'], + resource: TEST_RESOURCE_URL, } as RefreshToken; refreshTokenRepository.findByToken.mockResolvedValue(refreshTokenRecord); @@ -281,6 +309,120 @@ describe('OAuthTokenService', () => { }); }); + describe('refresh-token resource binding', () => { + const GRANTED_URL = `${TEST_BASE_URL}/mcp/granted`; + // Second URL the same resource is reachable by, e.g. through the instance hostname. + const GRANTED_ALIAS_URL = 'https://alias.example.com/mcp/granted'; + const ANOTHER_URL = `${TEST_BASE_URL}/mcp/another`; + const REFRESH_TOKEN = 'stored-refresh-token'; + const CLIENT_ID = 'client-123'; + + let boundService: OAuthTokenService; + + beforeAll(() => { + const boundRegistry = new ProtectedResourceRegistry(mock()); + boundRegistry.register({ + id: 'instance-mcp', + getResourceUrl: () => TEST_RESOURCE_URL, + getAudiences: () => [TEST_RESOURCE_URL, LEGACY_AUDIENCE], + scopes: [], + isDefault: true, + authorize: async () => true, + }); + boundRegistry.register({ + id: 'granted-resource', + getResourceUrl: () => GRANTED_URL, + getResourceUrls: () => [GRANTED_URL, GRANTED_ALIAS_URL], + getAudiences: () => [GRANTED_URL, GRANTED_ALIAS_URL], + scopes: [], + authorize: async () => true, + }); + boundRegistry.register({ + id: 'another-resource', + getResourceUrl: () => ANOTHER_URL, + getAudiences: () => [ANOTHER_URL], + scopes: [], + authorize: async () => true, + }); + + boundService = new OAuthTokenService( + logger, + jwtService, + userRepository, + accessTokenRepository, + refreshTokenRepository, + boundRegistry, + txRunner, + workflowFinderService, + ); + }); + + beforeEach(() => { + refreshTokenRepository.findByToken.mockResolvedValue({ + token: REFRESH_TOKEN, + clientId: CLIENT_ID, + userId: 'user-456', + expiresAt: Date.now() + 1000000, + scope: [] as string[], + resource: GRANTED_URL, + } as RefreshToken); + refreshTokenRepository.deleteValidByToken.mockResolvedValue(1); + }); + + it('reuses the granted resource when the request names none', async () => { + const result = await boundService.validateAndRotateRefreshToken(REFRESH_TOKEN, CLIENT_ID); + + // Not the default resource, which is what a resource-less mint would fall back to. + expect(jwtService.decode(result.access_token).aud).toBe(GRANTED_URL); + }); + + it('accepts a request naming the granted resource', async () => { + const result = await boundService.validateAndRotateRefreshToken( + REFRESH_TOKEN, + CLIENT_ID, + GRANTED_URL, + ); + + expect(jwtService.decode(result.access_token).aud).toBe(GRANTED_URL); + }); + + it('accepts an equivalent spelling of the granted resource', async () => { + const result = await boundService.validateAndRotateRefreshToken( + REFRESH_TOKEN, + CLIENT_ID, + GRANTED_ALIAS_URL, + ); + + // Minted from the stored spelling, which the resource also accepts. + expect(jwtService.decode(result.access_token).aud).toBe(GRANTED_URL); + }); + + it('rejects a request naming a different resource, leaving the token usable', async () => { + await expect( + boundService.validateAndRotateRefreshToken(REFRESH_TOKEN, CLIENT_ID, ANOTHER_URL), + ).rejects.toThrow(InvalidTargetError); + + expect(refreshTokenRepository.deleteValidByToken).not.toHaveBeenCalled(); + expect(refreshTokenRepository.insertToken).not.toHaveBeenCalled(); + expect(accessTokenRepository.insertToken).not.toHaveBeenCalled(); + }); + + it('rejects a request naming the default resource for a grant made elsewhere', async () => { + await expect( + boundService.validateAndRotateRefreshToken(REFRESH_TOKEN, CLIENT_ID, TEST_RESOURCE_URL), + ).rejects.toThrow(InvalidTargetError); + }); + + it('carries the granted resource onto the rotated refresh token', async () => { + await boundService.validateAndRotateRefreshToken(REFRESH_TOKEN, CLIENT_ID, GRANTED_ALIAS_URL); + + expect(refreshTokenRepository.insertToken).toHaveBeenCalledWith( + expect.objectContaining({ resource: GRANTED_URL }), + expect.anything(), + ); + }); + }); + describe('verifyAccessToken', () => { it('should verify valid access token and return auth info', async () => { const userId = 'user-123'; @@ -728,6 +870,7 @@ describe('OAuthTokenService', () => { userId: 'user-456', expiresAt: Date.now() + 1000000, scope: ['workflow:read'], + resource: TEST_RESOURCE_URL, } as RefreshToken; refreshTokenRepository.findByToken.mockResolvedValue(refreshTokenRecord); diff --git a/packages/cli/src/modules/oauth-server/__tests__/test-trigger-resource-lifetime.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/test-trigger-resource-lifetime.api.test.ts index 052a51bc1c9..58b45e06823 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/test-trigger-resource-lifetime.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/test-trigger-resource-lifetime.api.test.ts @@ -80,7 +80,14 @@ const registerTestWebhook = async ( 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, []); + await tokenService.saveTokenPair( + pair.accessToken, + pair.refreshToken, + clientId, + user.id, + [], + pair.audience, + ); return pair.accessToken; }; diff --git a/packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-test-trigger-resource.resolver.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-test-trigger-resource.resolver.api.test.ts index 1db87b5859f..4f838f9d14b 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-test-trigger-resource.resolver.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-test-trigger-resource.resolver.api.test.ts @@ -253,6 +253,7 @@ describe('test vs production resources', () => { clientId, owner.id, [], + testToken.audience, ); const productionToken = tokenService.generateTokenPair( owner.id, @@ -266,6 +267,7 @@ describe('test vs production resources', () => { clientId, owner.id, [], + productionToken.audience, ); await expect( diff --git a/packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-trigger-authorization.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-trigger-authorization.api.test.ts index 99d6cd86958..1ed4ba4d2ca 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-trigger-authorization.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-trigger-authorization.api.test.ts @@ -82,13 +82,13 @@ const createProtectedWorkflow = async (workflowName: string, requireExecuteAcces return { workflow, resourceUrl: productionResourceUrl(webhookPath) }; }; -const registerClient = async () => +const registerClient = async (grantTypes: string[] = ['authorization_code']) => ( await oauthClientRepository.save({ id: `client-${randomUUID()}`, name: 'Test OAuth Client', redirectUris: ['https://example.com/callback'], - grantTypes: ['authorization_code'], + grantTypes, tokenEndpointAuthMethod: 'none', }) ).id; @@ -96,7 +96,14 @@ const registerClient = async () => const mintAccessToken = async (userId: string, resourceUrl: string) => { const clientId = await registerClient(); const pair = tokenService.generateTokenPair(userId, clientId, resourceUrl, []); - await tokenService.saveTokenPair(pair.accessToken, pair.refreshToken, clientId, userId, []); + await tokenService.saveTokenPair( + pair.accessToken, + pair.refreshToken, + clientId, + userId, + [], + pair.audience, + ); return pair.accessToken; }; @@ -222,3 +229,76 @@ describe('consent gate: workflow name and authorization code require execute acc expect(String(response.body.data?.redirectUrl ?? '')).not.toContain('code='); }); }); + +/** + * A grant is approved for one protected resource, so every token the token endpoint + * mints from it — including after rotation — carries that resource as its audience. + */ +describe('token endpoint: refresh grants stay on their approved resource', () => { + const mintGrantFor = async (resourceUrl: string) => { + const clientId = await registerClient(['authorization_code', 'refresh_token']); + const pair = tokenService.generateTokenPair(owner.id, clientId, resourceUrl, []); + await tokenService.saveTokenPair( + pair.accessToken, + pair.refreshToken, + clientId, + owner.id, + [], + pair.audience, + ); + return { clientId, refreshToken: pair.refreshToken }; + }; + + const refresh = async (clientId: string, refreshToken: string, resource?: string) => + await testServer.restlessAgent + .post('/oauth/token') + .type('form') + .send({ + grant_type: 'refresh_token', + refresh_token: refreshToken, + client_id: clientId, + ...(resource ? { resource } : {}), + }); + + test('refuses a resource the grant was not approved for, and keeps the token usable', async () => { + const { resourceUrl: resourceA } = await createProtectedWorkflow('Workflow A'); + const { resourceUrl: resourceB } = await createProtectedWorkflow('Workflow B'); + const { clientId, refreshToken } = await mintGrantFor(resourceA); + + const denied = await refresh(clientId, refreshToken, resourceB); + + expect(denied.statusCode).toBe(400); + expect(denied.body.error).toBe('invalid_target'); + + // The refused request must not have consumed the refresh token. + const retry = await refresh(clientId, refreshToken, resourceA); + expect(retry.statusCode).toBe(200); + }); + + test('reuses the approved resource when the request names none', async () => { + const { resourceUrl } = await createProtectedWorkflow('Workflow A'); + const { clientId, refreshToken } = await mintGrantFor(resourceUrl); + + const response = await refresh(clientId, refreshToken); + + expect(response.statusCode).toBe(200); + expect(jwtService.decode(response.body.access_token).aud).toBe(resourceUrl); + }); + + test('keeps the resource on the rotated refresh token', async () => { + const { resourceUrl: resourceA } = await createProtectedWorkflow('Workflow A'); + const { resourceUrl: resourceB } = await createProtectedWorkflow('Workflow B'); + const { clientId, refreshToken } = await mintGrantFor(resourceA); + + const rotated = await refresh(clientId, refreshToken, resourceA); + expect(rotated.statusCode).toBe(200); + + const crossResource = await refresh(clientId, rotated.body.refresh_token, resourceB); + expect(crossResource.statusCode).toBe(400); + expect(crossResource.body.error).toBe('invalid_target'); + + const reused = await refresh(clientId, rotated.body.refresh_token); + expect(reused.statusCode).toBe(200); + expect(jwtService.decode(reused.body.access_token).aud).toBe(resourceA); + }); +}); diff --git a/packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-trigger-resource.resolver.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-trigger-resource.resolver.api.test.ts index 4aa0755fb0f..09ff5ca98fb 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-trigger-resource.resolver.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-trigger-resource.resolver.api.test.ts @@ -303,13 +303,13 @@ describe('token audience', () => { const clientId = await registerOAuthClient(); const tokenService = Container.get(OAuthTokenService); - const { accessToken, refreshToken } = tokenService.generateTokenPair( + const { accessToken, refreshToken, audience } = tokenService.generateTokenPair( owner.id, clientId, resourceUrlFor(pathA), [], ); - await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, []); + await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, [], audience); expect(decodeJwtPayload(accessToken).aud).toBe(resourceUrlFor(pathA)); @@ -330,13 +330,13 @@ describe('token audience', () => { const tokenService = Container.get(OAuthTokenService); // no resource indicator -> falls back to the default resource (instance MCP) - const { accessToken, refreshToken } = tokenService.generateTokenPair( + const { accessToken, refreshToken, audience } = tokenService.generateTokenPair( owner.id, clientId, undefined, [], ); - await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, []); + await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, [], audience); expect(decodeJwtPayload(accessToken).aud).not.toBe(resourceUrlFor(webhookPath)); await expect( diff --git a/packages/cli/src/modules/oauth-server/__tests__/workflow-webhook-test-trigger-resource.resolver.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/workflow-webhook-test-trigger-resource.resolver.api.test.ts index e33ce6d67ae..7f5a0b129a4 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/workflow-webhook-test-trigger-resource.resolver.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/workflow-webhook-test-trigger-resource.resolver.api.test.ts @@ -395,6 +395,7 @@ describe('test vs production resources', () => { clientId, owner.id, [], + testToken.audience, ); const productionToken = tokenService.generateTokenPair( owner.id, @@ -408,6 +409,7 @@ describe('test vs production resources', () => { clientId, owner.id, [], + productionToken.audience, ); await expect( diff --git a/packages/cli/src/modules/oauth-server/__tests__/workflow-webhook-trigger-resource.resolver.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/workflow-webhook-trigger-resource.resolver.api.test.ts index 33ce889b8b2..f56ec274548 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/workflow-webhook-trigger-resource.resolver.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/workflow-webhook-trigger-resource.resolver.api.test.ts @@ -452,13 +452,13 @@ describe('protected resource metadata for webhook triggers', () => { const clientId = await registerOAuthClient(); const resourceA = resourceUrlFor(`${webhookIdA}/user/:id`); - const { accessToken, refreshToken } = tokenService.generateTokenPair( + const { accessToken, refreshToken, audience } = tokenService.generateTokenPair( owner.id, clientId, resourceA, [], ); - await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, []); + await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, [], audience); expect(decodeJwtPayload(accessToken).aud).toBe(resourceA); @@ -549,13 +549,13 @@ describe('token audience', () => { const tokenService = Container.get(OAuthTokenService); const clientId = await registerOAuthClient(); - const { accessToken, refreshToken } = tokenService.generateTokenPair( + const { accessToken, refreshToken, audience } = tokenService.generateTokenPair( owner.id, clientId, resourceUrlFor(pathA), [], ); - await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, []); + await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, [], audience); expect(decodeJwtPayload(accessToken).aud).toBe(resourceUrlFor(pathA)); @@ -583,13 +583,13 @@ describe('token audience', () => { const clientId = await registerOAuthClient(); const getResource = resourceUrlFor(sharedPath, 'GET'); - const { accessToken, refreshToken } = tokenService.generateTokenPair( + const { accessToken, refreshToken, audience } = tokenService.generateTokenPair( owner.id, clientId, getResource, [], ); - await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, []); + await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, [], audience); expect(decodeJwtPayload(accessToken).aud).toBe(getResource); @@ -611,13 +611,13 @@ describe('token audience', () => { const tokenService = Container.get(OAuthTokenService); const clientId = await registerOAuthClient(); - const { accessToken, refreshToken } = tokenService.generateTokenPair( + const { accessToken, refreshToken, audience } = tokenService.generateTokenPair( owner.id, clientId, resourceUrlFor(webhookPath, 'GET'), [], ); - await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, []); + await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, [], audience); await expect( tokenService.verifyAccessToken(accessToken, resourceUrlFor(webhookPath, 'POST')), @@ -653,13 +653,13 @@ describe('token audience', () => { const clientId = await registerOAuthClient(); const getResource = resourceUrlFor(webhookPath, 'GET'); - const { accessToken, refreshToken } = tokenService.generateTokenPair( + const { accessToken, refreshToken, audience } = tokenService.generateTokenPair( owner.id, clientId, getResource, [], ); - await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, []); + await tokenService.saveTokenPair(accessToken, refreshToken, clientId, owner.id, [], audience); // the user adds POST to the node; the GET-scoped token must survive await insertWebhookRow(workflow.id, webhookPath, 'Webhook', 'POST'); diff --git a/packages/cli/src/modules/oauth-server/database/entities/oauth-refresh-token.entity.ts b/packages/cli/src/modules/oauth-server/database/entities/oauth-refresh-token.entity.ts index 8746ef027af..7dd565d73ef 100644 --- a/packages/cli/src/modules/oauth-server/database/entities/oauth-refresh-token.entity.ts +++ b/packages/cli/src/modules/oauth-server/database/entities/oauth-refresh-token.entity.ts @@ -33,4 +33,12 @@ export class RefreshToken extends WithTimestamps { /** OAuth scopes of the originating grant, reissued on every rotation. */ @JsonColumn() scope: string[]; + + /** + * RFC 8707 resource the grant was approved for — the audience of every access + * token minted from it, carried unchanged across rotations. Never null: the + * mint path resolves the default resource before the row is written. + */ + @Column({ type: 'varchar' }) + resource: string; } diff --git a/packages/cli/src/modules/oauth-server/database/repositories/oauth-refresh-token.repository.ts b/packages/cli/src/modules/oauth-server/database/repositories/oauth-refresh-token.repository.ts index 3d77c7aaaaf..a0540644e2d 100644 --- a/packages/cli/src/modules/oauth-server/database/repositories/oauth-refresh-token.repository.ts +++ b/packages/cli/src/modules/oauth-server/database/repositories/oauth-refresh-token.repository.ts @@ -10,6 +10,7 @@ type NewRefreshToken = { userId: string; expiresAt: number; scope: string[]; + resource: string; }; @Service() diff --git a/packages/cli/src/modules/oauth-server/oauth-server.service.ts b/packages/cli/src/modules/oauth-server/oauth-server.service.ts index 52c598a28ee..279d8c02126 100644 --- a/packages/cli/src/modules/oauth-server/oauth-server.service.ts +++ b/packages/cli/src/modules/oauth-server/oauth-server.service.ts @@ -38,6 +38,7 @@ import { OAuthConsentService } from './oauth-consent.service'; import { OAuthSessionPayload, OAuthSessionService } from './oauth-session.service'; import { OAuthTokenService } from './oauth-token.service'; import { OAuthClientLimitReachedError } from './oauth.errors'; +import { isSameProtectedResource } from './resource-identity'; /** Maximum number of redirect URIs per client */ const MAX_REDIRECT_URIS = 10; @@ -434,29 +435,31 @@ export class OAuthServerService implements OAuthServerProvider { const resourceStr = resource?.toString(); const tokenResource = await this.resolveAndValidateResourceIndicator(resourceStr); - // RFC 8707: if both the token request and the auth code specify a resource, they must match - // (token substitution defense). Otherwise either supplies the other, falling back to the - // registry's default resource. - let finalResource: string | undefined; - const codeResource = authRecord.resource ?? undefined; + // RFC 8707 §2.2: the authorization request's resource applies to the whole grant, so + // the token request may only name the resource the user approved. A code carrying no + // resource was approved against the registry's default resource — that is what the + // consent screen showed — so the token request may only name that one. + const approvedResource = + authRecord.resource ?? this.resourceRegistry.getDefaultResource()?.getResourceUrl(); - if (tokenResource && codeResource) { - if (tokenResource !== codeResource) { - throw new InvalidResourceIndicatorError(tokenResource, codeResource); - } - finalResource = tokenResource; - } else { - finalResource = tokenResource ?? codeResource; + if ( + tokenResource && + !( + approvedResource && + (await isSameProtectedResource(this.resourceRegistry, tokenResource, approvedResource)) + ) + ) { + throw new InvalidResourceIndicatorError(tokenResource, approvedResource ?? 'none'); } await this.authorizationCodeService.markAuthorizationCodeAsUsed(authorizationCode); const grantedScopes = authRecord.scope; - const { accessToken, refreshToken } = this.tokenService.generateTokenPair( + const { accessToken, refreshToken, audience } = this.tokenService.generateTokenPair( authRecord.userId, client.client_id, - finalResource, + approvedResource, grantedScopes, ); @@ -466,14 +469,15 @@ export class OAuthServerService implements OAuthServerProvider { client.client_id, authRecord.userId, grantedScopes, + audience, ); // Completion of the authorization-code grant is the point at which the user // has finished the OAuth flow for this client. The authorization server is // shared by every protected resource on the instance (MCP, forms, ...), so // only grants targeting the instance MCP server count as MCP usage. - const grantedResource = finalResource - ? await this.resourceRegistry.getByResourceUrl(finalResource) + const grantedResource = approvedResource + ? await this.resourceRegistry.getByResourceUrl(approvedResource) : this.resourceRegistry.getDefaultResource(); if (grantedResource?.id === INSTANCE_MCP_RESOURCE_ID) { this.eventService.emit('mcp-oauth-completed', { @@ -494,9 +498,10 @@ export class OAuthServerService implements OAuthServerProvider { }; } - // `resource` (when present) is normalized and validated before rotation; if omitted, - // the token service falls back to the default protected resource. `_scopes` is part of - // the SDK contract but unused — OAuth 2.1 refresh tokens reuse the original grant's scopes. + // `resource` (when present) is normalized and validated against the registry here, then + // against the grant's own resource by the token service; if omitted, the token service + // reuses the grant's resource. `_scopes` is part of the SDK contract but unused — OAuth + // 2.1 refresh tokens reuse the original grant's scopes. async exchangeRefreshToken( client: OAuthClientInformationFull, refreshToken: string, 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 c3c9692c4f6..35b6753ac50 100644 --- a/packages/cli/src/modules/oauth-server/oauth-token.service.ts +++ b/packages/cli/src/modules/oauth-server/oauth-token.service.ts @@ -1,4 +1,7 @@ -import { InvalidGrantError } from '@modelcontextprotocol/sdk/server/auth/errors.js'; +import { + InvalidGrantError, + InvalidTargetError, +} from '@modelcontextprotocol/sdk/server/auth/errors.js'; import { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js'; import { OAuthTokens } from '@modelcontextprotocol/sdk/shared/auth.js'; import { Logger } from '@n8n/backend-common'; @@ -23,6 +26,7 @@ import { AccessTokenRepository } from './database/repositories/oauth-access-toke import { RefreshTokenRepository } from './database/repositories/oauth-refresh-token.repository'; import { AccessTokenNotFoundError, JWTVerificationError } from './oauth.errors'; import { authorizeAgainstGrant } from './resource-gate'; +import { isSameProtectedResource } from './resource-identity'; /** * Manages the OAuth 2.1 token lifecycle for the shared OAuth server. @@ -52,12 +56,17 @@ export class OAuthTokenService implements OAuthTokenVerifier { return this.ACCESS_TOKEN_EXPIRY_SECONDS; } + /** + * Mints an access/refresh pair. Returns the `audience` it resolved, so the + * caller can persist the grant's resource alongside the refresh token — the + * resolved value, not the (optional) request one, is what the token is bound to. + */ generateTokenPair( userId: string, clientId: string, resource: string | undefined, scopes: string[], - ): { accessToken: string; refreshToken: string } { + ): { accessToken: string; refreshToken: string; audience: string } { // Pre-RFC-8707 clients omit the resource indicator; fall back to the // registry's default resource (the instance MCP server). const audience = resource ?? this.resourceRegistry.getDefaultResource()?.getResourceUrl(); @@ -85,7 +94,7 @@ export class OAuthTokenService implements OAuthTokenVerifier { const refreshToken = randomBytes(32).toString('hex'); - return { accessToken, refreshToken }; + return { accessToken, refreshToken, audience }; } async saveTokenPair( @@ -94,6 +103,7 @@ export class OAuthTokenService implements OAuthTokenVerifier { clientId: string, userId: string, scopes: string[], + resource: string, ): Promise { await this.txRunner.run({}, async (ctx) => { await this.accessTokenRepository.insertToken({ token: accessToken, clientId, userId }, ctx); @@ -104,12 +114,18 @@ export class OAuthTokenService implements OAuthTokenVerifier { userId, expiresAt: Date.now() + this.REFRESH_TOKEN_EXPIRY_MS, scope: scopes, + resource, }, ctx, ); }); } + /** + * Rotates a refresh token into a fresh pair, reissuing the grant's scopes and + * its resource. `resource`, when the client sends one, must name the resource + * the grant was approved for (RFC 8707 §2.2). + */ async validateAndRotateRefreshToken( refreshToken: string, clientId: string, @@ -130,6 +146,23 @@ export class OAuthTokenService implements OAuthTokenVerifier { throw new InvalidGrantError('Invalid refresh token'); } + // The resource approved at authorization bounds every later token request on + // the grant (RFC 8707 §2.2), so a request naming another one is refused and a + // request naming none reuses the grant's own — never the default resource. + // Checked before the row is consumed, so a refused request leaves the refresh + // token usable. + const grantedResource = refreshTokenRecord.resource; + if ( + resource && + !(await isSameProtectedResource(this.resourceRegistry, resource, grantedResource)) + ) { + this.logger.warn('Refresh token request denied: resource is outside the grant', { + clientId, + userId: refreshTokenRecord.userId, + }); + throw new InvalidTargetError('Requested resource is not part of this grant'); + } + const numAffected = await this.refreshTokenRepository.deleteValidByToken( refreshToken, clientId, @@ -142,10 +175,12 @@ export class OAuthTokenService implements OAuthTokenVerifier { const scopes = refreshTokenRecord.scope; + // Minted from the stored resource, not the requested spelling, so the binding + // cannot drift across rotations. const { accessToken, refreshToken: newRefreshToken } = this.generateTokenPair( refreshTokenRecord.userId, clientId, - resource, + grantedResource, scopes, ); @@ -161,6 +196,7 @@ export class OAuthTokenService implements OAuthTokenVerifier { userId: refreshTokenRecord.userId, expiresAt: now + this.REFRESH_TOKEN_EXPIRY_MS, scope: scopes, + resource: grantedResource, }, ctx, ); diff --git a/packages/cli/src/modules/oauth-server/resource-identity.ts b/packages/cli/src/modules/oauth-server/resource-identity.ts new file mode 100644 index 00000000000..a385a573f7b --- /dev/null +++ b/packages/cli/src/modules/oauth-server/resource-identity.ts @@ -0,0 +1,31 @@ +import type { ProtectedResourceRegistry } from '@/services/protected-resource.registry'; + +/** + * Whether two RFC 8707 resource indicators name the same protected resource. + * + * String equality alone is too strict: the token endpoint deliberately echoes back + * the caller's spelling when it names one of a resource's declared URLs (the + * instance MCP server publishes several), so one grant can legitimately be referred + * to by more than one URL. + * + * Fails closed — a URL that no longer resolves cannot be shown to name the resource + * that was approved, so a deleted or unavailable resource never matches. + */ +export async function isSameProtectedResource( + registry: ProtectedResourceRegistry, + requested: string, + granted: string, +): Promise { + if (requested === granted) return true; + + const [requestedResource, grantedResource] = await Promise.all([ + registry.getByResourceUrl(requested), + registry.getByResourceUrl(granted), + ]); + + return ( + requestedResource !== undefined && + grantedResource !== undefined && + requestedResource.id === grantedResource.id + ); +} diff --git a/packages/cli/test/integration/dynamic-credentials.ee/form-trigger-submit-gate.api.test.ts b/packages/cli/test/integration/dynamic-credentials.ee/form-trigger-submit-gate.api.test.ts index 35c54becb24..e9c96426a8b 100644 --- a/packages/cli/test/integration/dynamic-credentials.ee/form-trigger-submit-gate.api.test.ts +++ b/packages/cli/test/integration/dynamic-credentials.ee/form-trigger-submit-gate.api.test.ts @@ -102,7 +102,14 @@ const mintAccessToken = async (userId: string, resourceUrl: string) => { tokenEndpointAuthMethod: 'none', }); const pair = tokenService.generateTokenPair(userId, clientId, resourceUrl, []); - await tokenService.saveTokenPair(pair.accessToken, pair.refreshToken, clientId, userId, []); + await tokenService.saveTokenPair( + pair.accessToken, + pair.refreshToken, + clientId, + userId, + [], + pair.audience, + ); return pair.accessToken; };