fix(server): align OAuth discovery metadata with MCP / RFC 9728 spec (#19838)
## 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: <origin>/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 \`<origin>/mcp\`. After this PR: | Request | `resource` field | |---|---| | `GET /.well-known/oauth-protected-resource` | `https://<host>` | | `GET /.well-known/oauth-protected-resource/mcp` | `https://<host>/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://<host>/.well-known/oauth-protected-resource\"` - Now: `Bearer resource_metadata=\"https://<host>/.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 (\`<host>/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://<host>/.well-known/oauth-protected-resource | jq .resource # expect: \"https://<host>\" curl -s https://<host>/.well-known/oauth-protected-resource/mcp | jq .resource # expect: \"https://<host>/mcp\" curl -sI -X POST https://<host>/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 <noreply@anthropic.com>
This commit is contained in:
@@ -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"',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<Request>();
|
||||
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();
|
||||
|
||||
+16
-18
@@ -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 <origin>/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'],
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+24
-8
@@ -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
|
||||
// <origin>/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'],
|
||||
|
||||
Reference in New Issue
Block a user