From 5223c4771df5d8163825cf9ca9731e0cd51cdfa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Sat, 18 Apr 2026 21:28:44 +0200 Subject: [PATCH] fix(server): align OAuth discovery metadata with MCP / RFC 9728 spec (#19838) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Three small spec-compliance fixes called out in an audit against the [MCP authorization spec (draft)](https://modelcontextprotocol.io/specification/draft/basic/authorization) and RFC 9728 / RFC 9207. ### 1. Split Protected Resource Metadata by path (RFC 9728 §3.2) > The `resource` value returned MUST be identical to the protected resource's resource identifier value into which the well-known URI path suffix was inserted. Today a single handler serves both \`/.well-known/oauth-protected-resource\` and \`/.well-known/oauth-protected-resource/mcp\` and returns \`resource: /mcp\` from both. That's wrong for the root form — per RFC 9728 the root URL corresponds to the **origin as resource**, and only the \`/mcp\`-suffixed URL corresponds to \`/mcp\`. After this PR: | Request | `resource` field | |---|---| | `GET /.well-known/oauth-protected-resource` | `https://` | | `GET /.well-known/oauth-protected-resource/mcp` | `https:///mcp` | Both still return the same `authorization_servers`, `scopes_supported`, and `bearer_methods_supported`. Claude's current flow happens to work because our WWW-Authenticate points at the root form and Claude compares `resource` against what it connected to. Strict clients probing the path-aware URL first were rejecting us. ### 2. Advertise `authorization_response_iss_parameter_supported: true` (RFC 9207) Defense against OAuth mix-up attacks. Required by the [OAuth 2.1 security BCP](https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1). Signals that clients receiving an authorization response will find the issuer in the `iss` parameter and can validate it. ### 3. Fix `WWW-Authenticate` challenge: point at path-aware PRM URL, add `scope` param - Was: `Bearer resource_metadata=\"https:///.well-known/oauth-protected-resource\"` - Now: `Bearer resource_metadata=\"https:///.well-known/oauth-protected-resource/mcp\", scope=\"api profile\"` After change (1), only the path-aware URL returns a PRM document whose `resource` matches what the MCP client connected to (\`/mcp\`). Pointing clients at the right URL keeps discovery consistent. The `scope` parameter is a SHOULD in RFC 6750 and lets clients ask for least-privilege scopes on first authorization. ## Not in this PR (queued separately) From the same audit: - **Audit JWT `aud` (audience) validation** — the spec requires the server to reject tokens whose audience doesn't match this resource. Need a read-only code review to confirm; filing as a follow-up. - **Audit PKCE enforcement** — we advertise `code_challenge_methods_supported: [\"S256\"]`; need to confirm the \`/authorize\` flow actually rejects requests missing `code_challenge`. - **403 `insufficient_scope` challenge format** for step-up auth. - **CIMD (Client ID Metadata Documents)** support — newer spec alternative to DCR. ## Test plan - [x] \`yarn jest --testPathPatterns=\"mcp-auth.guard|oauth-discovery.controller\"\` → 4/4 passing - [x] \`tsc --noEmit\` clean on touched files - [ ] After deploy: \`\`\`bash curl -s https:///.well-known/oauth-protected-resource | jq .resource # expect: \"https://\" curl -s https:///.well-known/oauth-protected-resource/mcp | jq .resource # expect: \"https:///mcp\" curl -sI -X POST https:///mcp | grep -i www-authenticate # expect: Bearer resource_metadata=\"…/oauth-protected-resource/mcp\", scope=\"api profile\" \`\`\` ## Related - #19836 — CORS exposes `WWW-Authenticate` + `MCP-Protocol-Version` so browser clients can read them. Pairs with this PR. - #19755 / #19766 / #19824 — the earlier chain that got host-aware discovery and \`TRUST_PROXY\` working. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 --- .../guards/__tests__/mcp-auth.guard.spec.ts | 2 +- .../engine/api/mcp/guards/mcp-auth.guard.ts | 14 ++++---- .../oauth-discovery.controller.spec.ts | 34 +++++++++---------- .../controllers/oauth-discovery.controller.ts | 32 ++++++++++++----- 4 files changed, 49 insertions(+), 33 deletions(-) diff --git a/packages/twenty-server/src/engine/api/mcp/guards/__tests__/mcp-auth.guard.spec.ts b/packages/twenty-server/src/engine/api/mcp/guards/__tests__/mcp-auth.guard.spec.ts index e5a87153b5..80d893cb4a 100644 --- a/packages/twenty-server/src/engine/api/mcp/guards/__tests__/mcp-auth.guard.spec.ts +++ b/packages/twenty-server/src/engine/api/mcp/guards/__tests__/mcp-auth.guard.spec.ts @@ -46,7 +46,7 @@ describe('McpAuthGuard', () => { expect(mockSetHeader).toHaveBeenCalledWith( 'WWW-Authenticate', - 'Bearer resource_metadata="https://acme.twenty.com/.well-known/oauth-protected-resource"', + 'Bearer resource_metadata="https://acme.twenty.com/.well-known/oauth-protected-resource/mcp", scope="api profile"', ); }); }); diff --git a/packages/twenty-server/src/engine/api/mcp/guards/mcp-auth.guard.ts b/packages/twenty-server/src/engine/api/mcp/guards/mcp-auth.guard.ts index 5db8de024e..8df66a919f 100644 --- a/packages/twenty-server/src/engine/api/mcp/guards/mcp-auth.guard.ts +++ b/packages/twenty-server/src/engine/api/mcp/guards/mcp-auth.guard.ts @@ -7,12 +7,13 @@ import { import { type Request, type Response } from 'express'; +import { ALL_OAUTH_SCOPES } from 'src/engine/core-modules/application/application-oauth/constants/oauth-scopes'; import { JwtAuthGuard } from 'src/engine/guards/jwt-auth.guard'; -// RFC 9728: When the MCP endpoint returns 401, include a WWW-Authenticate -// header pointing to the Protected Resource Metadata URL on the same host -// the client connected to — otherwise discovery fails for any host other -// than SERVER_URL (app.twenty.com, workspace subdomains, custom domains). +// RFC 9728 / MCP authorization spec: when the MCP endpoint returns 401, +// include a WWW-Authenticate header pointing to the path-aware Protected +// Resource Metadata URL so the client discovers the correct resource +// identifier. The `scope` parameter tells the client which scopes to request. @Injectable() export class McpAuthGuard implements CanActivate { constructor(private readonly jwtAuthGuard: JwtAuthGuard) {} @@ -23,7 +24,8 @@ export class McpAuthGuard implements CanActivate { if (!isAuthenticated) { const request = context.switchToHttp().getRequest(); const baseUrl = `${request.protocol}://${request.get('host')}`; - const resourceMetadataUrl = `${baseUrl}/.well-known/oauth-protected-resource`; + const resourceMetadataUrl = `${baseUrl}/.well-known/oauth-protected-resource/mcp`; + const scope = ALL_OAUTH_SCOPES.join(' '); // Set the header on the response before throwing, because exception // filters may not preserve custom headers from the exception payload. @@ -31,7 +33,7 @@ export class McpAuthGuard implements CanActivate { response.setHeader( 'WWW-Authenticate', - `Bearer resource_metadata="${resourceMetadataUrl}"`, + `Bearer resource_metadata="${resourceMetadataUrl}", scope="${scope}"`, ); throw new UnauthorizedException(); diff --git a/packages/twenty-server/src/engine/core-modules/application/application-oauth/controllers/__tests__/oauth-discovery.controller.spec.ts b/packages/twenty-server/src/engine/core-modules/application/application-oauth/controllers/__tests__/oauth-discovery.controller.spec.ts index dfb48f6f45..b74494ab69 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-oauth/controllers/__tests__/oauth-discovery.controller.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-oauth/controllers/__tests__/oauth-discovery.controller.spec.ts @@ -1,4 +1,3 @@ -import { PATH_METADATA } from '@nestjs/common/constants'; import { Test, type TestingModule } from '@nestjs/testing'; import { type Request } from 'express'; @@ -46,31 +45,30 @@ describe('OAuthDiscoveryController', () => { controller = module.get(OAuthDiscoveryController); }); + // RFC 9728 §3.2 requires the `resource` value to match the identifier into + // which the well-known path suffix was inserted — so the root maps to the + // origin itself and the /mcp variant maps to /mcp. describe('getProtectedResourceMetadata', () => { - it('echoes the request host in the resource and authorization_servers', () => { + it('root form returns the origin as the resource', () => { const request = buildMockRequest('workspace.twenty.com'); - expect(controller.getProtectedResourceMetadata(request)).toEqual({ - resource: 'https://workspace.twenty.com/mcp', + expect( + controller.getProtectedResourceMetadataRoot(request), + ).toMatchObject({ + resource: 'https://workspace.twenty.com', authorization_servers: ['https://workspace.twenty.com'], - scopes_supported: ['api', 'profile'], - bearer_methods_supported: ['header'], }); }); - // RFC 9728 defines both a root and a resource-specific well-known - // URL; both must resolve to this handler so any conformant client - // can discover the metadata. - it('is registered at both the root and the /mcp path-aware URL', () => { - const paths = Reflect.getMetadata( - PATH_METADATA, - controller.getProtectedResourceMetadata, - ); + it('path-aware /mcp form returns origin/mcp as the resource', () => { + const request = buildMockRequest('workspace.twenty.com'); - expect(paths).toEqual([ - 'oauth-protected-resource', - 'oauth-protected-resource/mcp', - ]); + expect(controller.getProtectedResourceMetadataMcp(request)).toMatchObject( + { + resource: 'https://workspace.twenty.com/mcp', + authorization_servers: ['https://workspace.twenty.com'], + }, + ); }); }); }); diff --git a/packages/twenty-server/src/engine/core-modules/application/application-oauth/controllers/oauth-discovery.controller.ts b/packages/twenty-server/src/engine/core-modules/application/application-oauth/controllers/oauth-discovery.controller.ts index d1d4d1f22f..48d28659f9 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-oauth/controllers/oauth-discovery.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-oauth/controllers/oauth-discovery.controller.ts @@ -54,24 +54,40 @@ export class OAuthDiscoveryController { token_endpoint_auth_methods_supported: ['client_secret_post', 'none'], revocation_endpoint_auth_methods_supported: ['client_secret_post'], introspection_endpoint_auth_methods_supported: ['client_secret_post'], + // RFC 9207: advertise `iss` in authorization responses to defend against + // OAuth mix-up attacks. Required by OAuth 2.1 security BCP. + authorization_response_iss_parameter_supported: true, ...(cliRegistration ? { cli_client_id: cliRegistration.oAuthClientId } : {}), }; } - // RFC 9728: OAuth 2.0 Protected Resource Metadata. - // Exposed at both the root and the resource-specific path (/mcp) because - // RFC 9728 defines the path-aware form and clients may probe either. - // The `resource` value echoes the host the request reached so the metadata - // matches the resource indicator the client used. - @Get(['oauth-protected-resource', 'oauth-protected-resource/mcp']) + // RFC 9728 §3.2: the `resource` value MUST equal the resource identifier + // into which the well-known path suffix was inserted. So the root form maps + // to the origin as-a-resource, and the /mcp-suffixed form maps to + // /mcp. Strict clients probing the path-aware variant will reject + // mismatching metadata. + + @Get('oauth-protected-resource') @UseGuards(PublicEndpointGuard, NoPermissionGuard) - getProtectedResourceMetadata(@Req() request: Request) { + getProtectedResourceMetadataRoot(@Req() request: Request) { const base = this.getRequestBaseUrl(request); + return this.buildProtectedResourceMetadata(base, base); + } + + @Get('oauth-protected-resource/mcp') + @UseGuards(PublicEndpointGuard, NoPermissionGuard) + getProtectedResourceMetadataMcp(@Req() request: Request) { + const base = this.getRequestBaseUrl(request); + + return this.buildProtectedResourceMetadata(base, `${base}/mcp`); + } + + private buildProtectedResourceMetadata(base: string, resource: string) { return { - resource: `${base}/mcp`, + resource, authorization_servers: [base], scopes_supported: ALL_OAUTH_SCOPES, bearer_methods_supported: ['header'],