Add OAuth 2.1 resource-server support#23
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for an OAuth 2.1 deployment profile, enabling browser authorization and downstream token exchange while maintaining compatibility with legacy API keys. Key changes include adding token verification, challenge middleware, and a token exchange cache in a new oauth.py module, alongside configuration updates and comprehensive test coverage. Feedback on these changes highlights a concurrency issue in the token exchange cache where caller cancellation can prematurely clear inflight tasks and waste tokens, as well as a recommendation to relax issuer URL validation to support identity providers hosted on subpaths.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a424ea1937
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| outbound_credential = await exchange_for_tool_token( | ||
| context.client, | ||
| config, | ||
| inbound_credential, | ||
| context.tool_token_cache, |
There was a problem hiding this comment.
Handle token-exchange failures in the tool error path
When OAuth is enabled and the authorization server returns a 4xx/5xx or times out during this exchange, exchange_for_tool_token raises before the try block below, so handle_api_error is bypassed and the MCP tool call surfaces as an unstructured internal exception instead of the repository's retry/no-retry ToolError contract. Put the exchange under the same try/except or catch it here so OAuth users get actionable errors during auth-server outages or revoked exchange grants.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in f77623e: token exchange now runs inside the shared tool error boundary, with a test asserting authorization-server timeouts use the structured ToolError path.
| async with self._lock: | ||
| self._generation += 1 | ||
| self._entries.pop(key, None) | ||
| self._inflight.pop(key, None) |
There was a problem hiding this comment.
Preserve coalescing after concurrent 401s
When several requests use the same cached downstream token and the Tool API starts returning 401, each request invalidates before retrying; this line deletes any _inflight entry for the key, so a later 401 can remove the fresh exchange that an earlier retry just created. In concurrent revocation/downstream-rejection events this fans out into one token-exchange request per caller instead of coalescing, so consider clearing only the rejected generation/token rather than every in-flight exchange for the key.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in f77623e: a downstream 401 evicts only the rejected cached token and never removes a newer in-flight exchange. A concurrent-401 regression test verifies a single fresh exchange.
| headers = [(key, value) for key, value in message.get("headers", []) if key.lower() != b"www-authenticate"] | ||
| headers.append((b"www-authenticate", challenge.encode("ascii"))) |
There was a problem hiding this comment.
Preserve auth errors when rewriting challenges
When a caller sends an expired or otherwise invalid OAuth bearer, FastMCP's auth middleware emits a WWW-Authenticate challenge with error="invalid_token" and an error description, but this replacement drops the existing challenge and returns only resource_metadata/scope. MCP clients commonly use the challenge parameters to decide whether to clear tokens and re-authorize, so preserve the existing error attributes while adding the resource metadata instead of stripping the header.
Useful? React with 👍 / 👎.
| headers={"Cache-Control": "public, max-age=300"}, | ||
| ) | ||
|
|
||
| return [Route(self._metadata_path, protected_resource_metadata, methods=["GET"])] |
There was a problem hiding this comment.
Add CORS support to OAuth metadata route
This custom protected-resource metadata route only handles plain GET responses and does not add any CORS headers or OPTIONS handling. For browser-based MCP/OAuth clients or inspectors running on a different origin, fetching /.well-known/oauth-protected-resource/... is blocked by the browser even though the MCP endpoint itself is intended to support browser authorization, so wrap this route with CORS/OPTIONS support like the standard OAuth metadata routes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in f77623e: PRM GET/OPTIONS returns public CORS headers. A path-aware outer Host/Origin guard permits cross-origin metadata discovery while retaining Host validation and continuing to reject untrusted origins on the protected MCP endpoint.
What changed
Why
Popular MCP clients need a standards-based OAuth 2.1 flow while the Tool API must remain a separate audience and security boundary. This change makes the MCP server a strict resource server and keeps CodeAlive authorization decisions live in the backend.
Companion authorization-server and product-session PR: https://github.com/CodeAlive-AI/backend/pull/501
Verification