Skip to content

Fix MCP discovery: path-aware well-known URL and protocol version#19766

Merged
FelixMalfait merged 6 commits intomainfrom
mcp-fix-discovery-path-and-protocol-version
Apr 16, 2026
Merged

Fix MCP discovery: path-aware well-known URL and protocol version#19766
FelixMalfait merged 6 commits intomainfrom
mcp-fix-discovery-path-and-protocol-version

Conversation

@FelixMalfait
Copy link
Copy Markdown
Member

Summary

Adding https://api.twenty.com/mcp as an MCP server in Claude fails with Couldn't reach the MCP server before OAuth can start. Two independent bugs cause this:

  1. Missing path-aware well-known route. The latest MCP spec instructs clients to probe /.well-known/oauth-protected-resource/mcp before /.well-known/oauth-protected-resource. Only the root path was registered, so the path-aware request fell through to ServeStaticModule and returned the SPA's index.html with HTTP 200. Strict clients (Claude.ai) tried to parse it as JSON and gave up. Fixed by registering both paths on the same handler.
  2. Stale protocol version. Server advertised 2024-11-05, which predates Streamable HTTP. We've implemented Streamable HTTP (SSE response format was added in Add SSE streaming support on POST /mcp (Phase 2) #19528), so bumped to 2025-06-18.

Reproduction before the fix:

$ curl -s -o /dev/null -w "%{http_code} %{content_type}\n" https://api.twenty.com/.well-known/oauth-protected-resource/mcp
200 text/html; charset=UTF-8

After the fix this returns application/json with the RFC 9728 metadata document.

Note: this is separate from #19755 (host-aware resource URL for multi-host deployments).

Test plan

  • npx jest oauth-discovery.controller — 2/2 tests pass, including one asserting both routes are registered
  • npx nx lint:diff-with-main twenty-server passes
  • After deploy, curl https://api.twenty.com/.well-known/oauth-protected-resource/mcp returns JSON (not HTML)
  • Adding https://api.twenty.com/mcp in Claude reaches the OAuth authorization screen

🤖 Generated with Claude Code

@FelixMalfait FelixMalfait enabled auto-merge April 16, 2026 14:23
@FelixMalfait FelixMalfait disabled auto-merge April 16, 2026 14:23
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

@twenty-ci-bot-public
Copy link
Copy Markdown

twenty-ci-bot-public bot commented Apr 16, 2026

📊 API Changes Report

GraphQL Schema Changes

GraphQL Schema Changes

[error] Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-schema-introspection.json: Not valid JSON content
at JsonFileLoader.handleFileContent (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:147:19)
at /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:95:43
at async Promise.all (index 0)
at async JsonFileLoader.load (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:88:9)
at async /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:15:39
at async Promise.all (index 4)
at async loadFile (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:13:9)
at async /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/collect-sources.js:200:25
Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-schema-introspection.json: Not valid JSON content
at JsonFileLoader.handleFileContent (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:147:19)
at /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:95:43
at async Promise.all (index 0)
at async JsonFileLoader.load (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:88:9)
at async /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:15:39
at async Promise.all (index 4)
at async loadFile (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:13:9)
at async /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/collect-sources.js:200:25
⚠️ Breaking changes or errors detected in GraphQL schema

[error] Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-schema-introspection.json: Not valid JSON content
    at JsonFileLoader.handleFileContent (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:147:19)
    at /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:95:43
    at async Promise.all (index 0)
    at async JsonFileLoader.load (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:88:9)
    at async /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:15:39
    at async Promise.all (index 4)
    at async loadFile (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:13:9)
    at async /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/collect-sources.js:200:25
Error generating diff

GraphQL Metadata Schema Changes

GraphQL Metadata Schema Changes

[error] Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-metadata-schema-introspection.json: Not valid JSON content
at JsonFileLoader.handleFileContent (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:147:19)
at /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:95:43
at async Promise.all (index 0)
at async JsonFileLoader.load (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:88:9)
at async /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:15:39
at async Promise.all (index 4)
at async loadFile (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:13:9)
at async /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/collect-sources.js:200:25
Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-metadata-schema-introspection.json: Not valid JSON content
at JsonFileLoader.handleFileContent (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:147:19)
at /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:95:43
at async Promise.all (index 0)
at async JsonFileLoader.load (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:88:9)
at async /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:15:39
at async Promise.all (index 4)
at async loadFile (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:13:9)
at async /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/collect-sources.js:200:25
⚠️ Breaking changes or errors detected in GraphQL metadata schema

[error] Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-metadata-schema-introspection.json: Not valid JSON content
    at JsonFileLoader.handleFileContent (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:147:19)
    at /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:95:43
    at async Promise.all (index 0)
    at async JsonFileLoader.load (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/json-file-loader/cjs/index.js:88:9)
    at async /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:15:39
    at async Promise.all (index 4)
    at async loadFile (/opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:13:9)
    at async /opt/hostedtoolcache/node/24.14.1/x64/lib/node_modules/@graphql-inspector/cli/node_modules/@graphql-tools/load/cjs/load-typedefs/collect-sources.js:200:25
Error generating diff

REST API Analysis Error

⚠️ Error occurred while analyzing REST API changes

Error Output

REST Metadata API Analysis Error

⚠️ Error occurred while analyzing REST Metadata API changes

Error Output

⚠️ Please review these API changes carefully before merging.

@twenty-ci-bot-public
Copy link
Copy Markdown

twenty-ci-bot-public bot commented Apr 16, 2026

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:46340

This environment will automatically shut down after 5 hours.

FelixMalfait and others added 3 commits April 16, 2026 16:52
The latest MCP spec instructs clients to probe the resource-specific
well-known URL first (e.g. `/.well-known/oauth-protected-resource/mcp`)
before falling back to the root path. Without a route for that path the
request fell through to ServeStaticModule and returned the SPA's
index.html with HTTP 200, which strict clients (e.g. Claude.ai) tried
to parse as JSON and failed with "Couldn't reach the MCP server".

Also bump the advertised MCP protocol version from `2024-11-05` to
`2025-06-18` to match the actual transport (Streamable HTTP) the server
implements. The old version predates Streamable HTTP and caused some
clients to reject the handshake.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rewrite the discovery-route comments so they explain what the code is
(RFC 9728 defines both well-known URL forms; we serve both) rather
than narrating the bug they were introduced to fix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test asserted a hardcoded protocol version, so any future bump to
the constant breaks the suite. Reference the constant directly so the
assertion stays in sync with what the server actually advertises.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@FelixMalfait FelixMalfait force-pushed the mcp-fix-discovery-path-and-protocol-version branch from dceb750 to 4db2871 Compare April 16, 2026 14:53
FelixMalfait and others added 3 commits April 16, 2026 17:02
`application-oauth.module.ts` imported `DomainServerConfigModule` twice
in the `imports` array. Drive-by cleanup of a pre-existing duplicate.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@FelixMalfait
Copy link
Copy Markdown
Member Author

Added two commits on top of the original PR:

  1. Revert chore(server): drop api-host branch in OAuth discovery #19768 — reinstates the isApiHost branch on authorization_endpoint. Removing it broke the flow for any deployment where /mcp resolves on an API-only origin:

    • Local dev (localhost:3000/mcp paste → /authorize 404s on NestJS; React lives on :3001)
    • Cloud api.twenty.com/mcp (same issue — frontend is served from app.twenty.com via a separate ingress + S3)
    • Any self-host that splits API and frontend origins

    The branch is precisely what RFC 8414 separates issuer from authorization_endpoint for. For the common self-host monolith (SERVER_URL == FRONTEND_URL, single origin), the branch is a no-op — authorizeBase === issuer on every host — so it costs nothing.

  2. Drop duplicate DomainServerConfigModule import in application-oauth.module.ts. Pre-existing from fix(server): make OAuth discovery and MCP auth metadata host-aware #19755.

Verified every deployment × paste-URL pairing:

Deployment Paste URL authorization_endpoint OK
Cloud api.twenty.com api.twenty.com/mcp app.twenty.com/authorize
Cloud app.twenty.com app.twenty.com/mcp app.twenty.com/authorize
Cloud workspace subdomain <ws>.twenty.com/mcp <ws>.twenty.com/authorize
Cloud custom domain crm.acme.com/mcp crm.acme.com/authorize (SPA served via /s$uri)
Self-host monolith twenty.example.com/mcp same origin
Self-host split api.twenty.example.com/mcp twenty.example.com/authorize (from FRONTEND_URL)
Local dev localhost:3000/mcp localhost:3001/authorize (Vite)

Tests: 4/4 passing (oauth-discovery.controller + mcp-auth.guard).

@FelixMalfait FelixMalfait merged commit 4f4f723 into main Apr 16, 2026
90 checks passed
@FelixMalfait FelixMalfait deleted the mcp-fix-discovery-path-and-protocol-version branch April 16, 2026 15:32
@twenty-eng-sync
Copy link
Copy Markdown

Hey @FelixMalfait! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

FelixMalfait added a commit that referenced this pull request Apr 18, 2026
…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>
FelixMalfait added a commit that referenced this pull request Apr 18, 2026
…ents (#19836)

## The bug

Claude's MCP connector fails with \"Couldn't reach the MCP server\" on
every URL (\`api.twenty.com/mcp\`, \`app.twenty.com/mcp\`,
\`<workspace>.twenty.com/mcp\`, custom domains). The failure happens
**before** any OAuth flow starts — the client never even reaches the
consent screen.

## Root cause

\`POST /mcp\` unauthenticated returns:

\`\`\`
HTTP/2 401
access-control-allow-origin: *
www-authenticate: Bearer
resource_metadata=\"https://…/.well-known/oauth-protected-resource\"
content-type: application/json; charset=utf-8
(no access-control-expose-headers)
\`\`\`

The [Fetch/CORS
spec](https://fetch.spec.whatwg.org/#cors-safelisted-response-header-name)
defines only six response headers as safelisted — \`Cache-Control\`,
\`Content-Language\`, \`Content-Type\`, \`Expires\`, \`Last-Modified\`,
\`Pragma\`. Every other header is withheld from cross-origin JS unless
the server opts it in via \`Access-Control-Expose-Headers\`.

Result: Claude's browser-side MCP client receives the 401 but
\`response.headers.get('WWW-Authenticate')\` returns \`null\`. No
\`resource_metadata\` URL, no discovery, no OAuth — the client gives up
with the generic \"can't reach server\" error.

The [MCP authorization
spec](https://modelcontextprotocol.io/specification/draft/basic/authorization)
explicitly requires this header to be exposed.

## Fix

One config change in \`main.ts\`:

\`\`\`ts
-    cors: true,
+ // Expose WWW-Authenticate so browser-based MCP clients can read the
+ // resource_metadata pointer on 401. Required by MCP authorization
spec.
+    cors: { exposedHeaders: ['WWW-Authenticate'] },
\`\`\`

NestJS's default \`cors: true\` uses the \`cors\` package defaults,
which don't set \`exposedHeaders\`. Moving to an explicit config keeps
all other defaults (origin \`*\`, standard methods) and adds the single
required expose.

## Why it's safe and generally beneficial

- \`Access-Control-Expose-Headers: WWW-Authenticate\` is sent on every
response but only has an effect when \`WWW-Authenticate\` is actually
present (i.e. 401s). It's an opt-in permission, not a header-setter.
- \`WWW-Authenticate\` itself is still only set by \`McpAuthGuard\` on
401 — this PR doesn't change where or when the header is emitted.
- Covers the entire app, not just \`/mcp\` — any future 401-returning
endpoint will behave correctly for browser clients automatically.
- No change to origin handling, methods, or credentials. All existing
API / GraphQL / REST traffic is unaffected.

## Verification

After deploy:
\`\`\`bash
curl -sI -X POST -H \"Origin: https://claude.ai\"
https://api.twenty.com/mcp \\
  | grep -iE 'access-control-expose|www-authenticate'
# Expect:
#   access-control-expose-headers: WWW-Authenticate
#   www-authenticate: Bearer resource_metadata=\"…\"
\`\`\`

Then re-try adding the MCP connector in Claude — if this was the only
blocker, OAuth should now complete.

## Related

- #19755, #19766, #19824 — prior fixes in the MCP/OAuth discovery chain
(host-aware metadata, path-aware well-known, \`TRUST_PROXY\` for
\`request.protocol\`). This PR completes the CORS side of that work.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant