feat(mcp): OAuth2 PKCE flow with dynamic client registration for MCP agents - #8264
feat(mcp): OAuth2 PKCE flow with dynamic client registration for MCP agents#8264andypalmi wants to merge 7 commits into
Conversation
7ab6c8c to
311e8ff
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feat/7431-mcp-wellknown-discovery #8264 +/- ##
=====================================================================
+ Coverage 76.11% 76.23% +0.12%
=====================================================================
Files 447 450 +3
Lines 24011 24137 +126
Branches 6405 6442 +37
=====================================================================
+ Hits 18275 18400 +125
- Misses 5736 5737 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…agents Extend the OAuth2 flow so external MCP agents (Claude, Cursor, etc.) can authenticate. Rather than a single hardcoded client id, agents register dynamically per RFC 7591: POST /account/client persists a public AuthClient (type 'mcp', no secret) with its approved redirect URIs and returns a generated client id. The authorize, complete, and token endpoints recognise these clients by looking them up, skip the project/device ownership checks (MCP is user-scoped), and drive an MCP consent step that records the read-only and team selection before issuing a scoped personal access token. Redirect URIs must be loopback http (RFC 8252, port-flexible) or https for hosted clients; token issuance and refresh require no client secret. Also forward the caller scope through the platform automation handler and add a platform_get_active_user tool that reports the calling token's scope. Adds AuthClient.type/name/redirectURIs (migration + model), an AuthClient.createMCPClient controller, and AccessToken.createMCPOAuthToken. Ref #7432
311e8ff to
6747cbd
Compare
An AccessToken row holds both the access token and its refresh token, and getOrExpire destroyed the row when the access token expired, taking the refresh token with it so a client could never refresh (RFC 6749 1.5). Add a separate refreshTokenExpiresAt lifetime: reject an expired access token but keep the row while its refresh token is still valid. The MCP refresh token is stable rather than rotating, and concurrent refreshes coalesce through a shared cache so they reuse the most recently minted access token instead of overwriting the row.
Capture the pre-refresh expiry after lowering it, so the assertion compares against the shortened lifetime rather than the original one and no longer ties when the refresh lands in the same millisecond.
cstns
left a comment
cstns
left a comment
There was a problem hiding this comment.
One thing this makes visible: with dynamic registration, third party agents become the normal case, and right now the audit log can't tell them apart from the Expert.
Every tool call, Expert or third party, goes through the same platform-automation:forge handler, and platformAutomation.js hardcodes source: 'mcp:expert' on the nonce. So a third party agent's actions show up as "via Expert" in the audit log. The frontend already has a source === 'mcp' branch ("via MCP")
but nothing ever emits it.
The client identity is also dropped on the way through. Registration captures client_name onto AuthClient.name, then createMCPOAuthToken hardcodes the token name to 'MCP Agent' and keeps no reference back to the client, so every agent looks identical in both the audit log and the user's token list. The
nonce carries no tokenId either, and inject() swaps in the Expert's platform token, so there's no indirect signal left. Only the direct REST path records a tokenId (as source: 'api').
Would be good to land this branch with the three cases clearly separated in the log:
- mcp:expert for the first party Expert
- mcp for a third party agent, ideally with the registered client name so you can see which agent
- api for a plain token calling the REST API directly
server.js already threads scope down to platformAutomation, so that same seam might be the natural place to carry the caller identity, and storing the client on the token at issuance (or at least using client_name instead of 'MCP Agent') would give the log something to name. Happy to go a different route
if you have one in mind.
|
Good catch, and I agree the three cases should be distinguishable. I have confirmed the gaps: the platform-automation handler hardcodes source 'mcp:expert' for every tool call, the issued token is named 'MCP Agent' with no link back to the AuthClient, and the frontend 'mcp' branch is never emitted. To keep this PR focused on the OAuth mechanics I have opened a dedicated follow-up, #8271, to land the attribution end to end: link the issued token to its AuthClient and name it after the registered client, then separate the runtime source so a third party agent logs as 'mcp' with the client name, 'mcp:expert' stays for the Expert, and a direct token stays 'api'. The runtime part threads caller identity through the gateway and comms seam alongside scope and overlaps with the audit trail work, so it reads cleaner on its own. Let me know if you would rather see any of it in this PR instead. |
|
Good call on this. #8272 now implements the full attribution, stacked on these PRs. The MCP door records the calling client against its session, and the platform-automation handler stamps audit source |
The refresh_token grant returned access_token, expires_in and refresh_token but omitted token_type, which RFC 6749 section 5.1 requires. A spec-compliant client rejects the response and falls back to re-authorization even though the server rotated the token successfully. Return token_type: bearer to match the authorization_code response.
cstns
left a comment
cstns
left a comment
There was a problem hiding this comment.
tested e2e with my local claude code, works as expected; we should sync with @knolleary to validate the ui for the oauth consent page
|
my bad, the consent page was on the next stack up |
|
Verified the refresh flow end to end against a real MCP client, on this PR's code (without the later rotation work). Steps: authorized the client to obtain a token pair, expired the access token in the database while leaving the refresh token valid, then made another tool call. The door rejected the expired access token without destroying the row, the client refreshed via Also confirmed non-MCP OAuth is unaffected: the refresh handling is gated on |
…ache margin Reuse the AuthClients ownerType column with ownerType='mcp' for dynamically registered MCP clients instead of a dedicated type column. Raise the refresh re-mint margin to 5 minutes so the coalescing cache stays effective against the 30 minute access-token lifetime.
2 participants
Summary
Adds the OAuth2 authorization code flow with PKCE for MCP clients, plus dynamic client registration so third-party agents can obtain a
client_idwithout being provisioned ahead of time.POST /account/client(RFC 7591): registers a public MCP client and returns a generatedclient_idwithtoken_endpoint_auth_method: noneand no client secret. Redirect URIs are validated (loopback with port flexibility per RFC 8252, otherwise HTTPS).AuthClientgainstype,nameandredirectURIs(migration + model) with acreateMCPClientcontroller.platform_get_active_usertool so an agent can resolve its own user id and, alongside the profile, read back what its token is allowed to do (token.readOnly,token.allTeams,token.teams). This is the introspection counterpart to the scoped token above, letting an agent check up front whether a requested action is within the session's access. With no scope (the first-party Expert path) it reports the user's own unrestricted access, since Expert acts with the same permissions the user has.The consent UI is added in the next PR in the stack. See the issue for the note on why dynamic registration is used instead of a hardcoded client id.
Stacked on #7431. Part of #7423. Closes #7432.
Important
This PR introduces a database change. It includes a migration that runs on upgrade