OAuth updates for mcp-agent #547
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Haven't looked at the code yet, so maybe this comment is void, but the issue "OAuth metadata discovery building the wrong URLs" is incorrect. This is working as per the RFC (https://datatracker.ietf.org/doc/html/rfc9728#section-3.1):
(This can be checked by noticing that https://api.githubcopilot.com/.well-known/oauth-protected-resource/mcp is where the metadata is hosted and does not result in a 404, whereas https://api.githubcopilot.com/.well-known/oauth-protected-resource does result in a page not found error. Directly contradicting the "fix".) |
| slack_configs: | ||
| - channel: '#critical-alerts' | ||
| title: '=¨ CRITICAL: MCP Agent Alert' | ||
| title: '=� CRITICAL: MCP Agent Alert' |
There was a problem hiding this comment.
Are these characters expected? �
| candidates = [] | ||
| if path: | ||
| candidates.append( | ||
| str(base.copy_with(path=f"/.well-known/oauth-authorization-server/{path}")) |
There was a problem hiding this comment.
First candidate is per the RFC 👍
src/mcp_agent/server/app_server.py
Outdated
| session_id = getattr(app_context, "session_id", None) | ||
| if session_id: | ||
| current_user = OAuthUserIdentity( | ||
| provider="mcp-session", subject=str(session_id) |
There was a problem hiding this comment.
The session_id in the context is initialised when the server starts and then remains the same. So two users will definitely interfere with each other here. This is why I tried reading the session id from the request parameters. These would be unique
session_id = ctx.request_context.request.query_params.get("session_id") identity = create_default_user_for_preconfigured_tokens(session_id)
src/mcp_agent/oauth/manager.py
Outdated
| await self._token_store.delete(default_key) | ||
| if session_id: | ||
| session_identity = OAuthUserIdentity( | ||
| provider="mcp-session", subject=str(session_id) |
There was a problem hiding this comment.
See also my comment below on app_server, this depends on the session_id being different from one user to another
2 participants
Various fixes for full OAuth 2.1 support to mcp-agent, covering both inbound server authorization (resource metadata, token verification) and outbound delegated authorization to downstream MCP servers.
Removed the mutable Context.current_user field and replaced it with a per-request identity ContextVar plus the existing execution/session registry (_CURRENT_IDENTITY, _RUN_IDENTITY_REGISTRY). Every request now records its identity via _set_current_identity and _register_session, and helpers such as get_current_identity() let other components resolve it safely.
Refactored server request handling (src/mcp_agent/server/app_server.py) and Temporal proxy logic so identities are injected via ContextVar/registry instead of shared state. Workflow pre-auth, status, resume, cancel, and background callbacks all look up the identity by execution ID, preventing cross-user leakage.
Updated the token manager (src/mcp_agent/oauth/manager.py) to accept explicit identities, fall back in order (explicit → ContextVar → session → synthetic), and refuse new OAuth flows without a concrete user. Token invalidation no longer needs session IDs.
Adapted HTTP auth and server registry (src/mcp_agent/oauth/http/auth.py, src/mcp_agent/mcp/mcp_server_registry.py) to use identity resolvers rather than context attributes. Temporal’s SessionProxy now restores identity per execution and only writes to the ContextVar.