feat: add Static Client Registration (#3085)#3086
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7c6526fb8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
WalkthroughThe OAuth provider in src/fastmcp/client/auth/oauth.py now accepts optional client_id and client_secret. When a client_id is provided, the provider constructs a static OAuthClientInformationFull, stores it per-MCP-endpoint in the token storage, and bypasses Dynamic Client Registration by injecting the static client info during _bind/_initialize. Redirect and auth flows now pre-flight-check static credentials and surface ClientNotFoundError for invalid static credentials instead of retrying DCR. Token storage and context handling were updated to align static client info with the MCP server URL. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
Test Failure AnalysisSummary: Multiple test failures in OAuth client tests due to an Root Cause: The The Suggested Solution: Initialize In # Use full URL for token storage to properly separate tokens per MCP endpoint
self.token_storage_adapter: TokenStorageAdapter = TokenStorageAdapter(
async_key_value=token_storage, server_url=mcp_url
)
# Initialize static client info (will be set if credentials are provided)
self._static_client_info: OAuthClientInformationFull | None = None
# Handle static client credentials (bypasses dynamic client registration)
# If client_id is provided, we construct OAuthClientInformationFull and inject
# it during initialization to prevent the SDK from performing dynamic registration
if client_id and client_secret:
logger.info(
"Using static OAuth client credentials (client_id provided)"
)
# Create the full client information for static credentials
self._static_client_info = OAuthClientInformationFull(
client_id=client_id,
client_secret=client_secret,
)
# Inject static client info into token storage
anyio.run(
self.token_storage_adapter.set_client_info,
self._static_client_info
)Detailed AnalysisThe error occurs in these test cases:
All fail with: Stack trace shows the error originates from: # src/fastmcp/client/auth/oauth.py:255
async def _initialize(self) -> None:
await super()._initialize()
if self._static_client_info is not None: # <-- AttributeError here
self.context.client_info = self._static_client_infoThe Additional Issues to ConsiderWhile fixing the immediate AttributeError, there are two other issues mentioned in previous reviews that should be addressed:
Related Files
Updated: 2026-02-05 - Analysis of test failures from workflow run #21711611027 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/fastmcp/client/auth/oauth.py (1)
220-220: Consider documenting behavior when onlyclient_idis provided.The condition
if client_id and client_secretsilently skips static registration if onlyclient_idis provided (withoutclient_secret). This may be intentional for OAuth public clients, but consider either:
- Documenting this behavior in the docstring, or
- Supporting public clients explicitly by allowing
client_secret=None, or- Raising a
ValueErrorifclient_idis provided withoutclient_secret
There was a problem hiding this comment.
The feature makes sense — the implementation has a few issues though:
anyio.run() in init crashes in async contexts, which is the common case. The MCP SDK's own ClientCredentialsOAuthProvider shows the right pattern: store credentials in init, then override _initialize() to set context.client_info directly. There can be no async work in the constructor at all.
_static_client_info AttributeError — the _initialize override references it unconditionally but it's only set when credentials are provided, breaking all existing usage.
Static client info needs the full metadata — just passing client_id/client_secret without redirect_uris, grant_types, etc. will likely fail server-side validation. Since client_metadata is already constructed at that point, just spread it in.
client_secret should be optional — the spec says "client ID (and, if applicable, client credentials)," explicitly accommodating public client pre-registration (no secret, relying on PKCE + redirect URI validation). The current if client_id and client_secret guard silently falls through to DCR when only client_id is provided.
The fix for all four (roughly):
# in __init__
self._static_client_info: OAuthClientInformationFull | None = None
if client_id is not None:
self._static_client_info = OAuthClientInformationFull(
client_id=client_id,
client_secret=client_secret,
**client_metadata.model_dump(exclude_none=True),
)
# override _initialize
async def _initialize(self) -> None:
await super()._initialize()
if self._static_client_info is not None:
self.context.client_info = self._static_client_info
if self.context.current_tokens and self.context.current_tokens.expires_in:
self.context.update_token_expiry(self.context.current_tokens)
Test Failure AnalysisSummary: The static analysis check failed because the code contains trailing whitespace that violates the ruff formatting rules. Root Cause: The PR introduces trailing whitespace on two lines in
The Suggested Solution: Run the formatting tools locally to fix the trailing whitespace: uv run prek run --all-filesThis will automatically remove the trailing whitespace and ensure the code passes all static analysis checks. After running this command, commit the changes and push to update the PR. Detailed AnalysisThe @@ -227,7 +227,7 @@ class OAuth(OAuthClientProvider):
scope=scopes_str,
**(self._additional_client_metadata or {}),
)
-
+
if self._client_id:
# Create the full static client info directly which will avoid DCR
self._static_client_info = OAuthClientInformationFull(
@@ -247,7 +247,7 @@ class OAuth(OAuthClientProvider):
+ "See https://gofastmcp.com/clients/auth/oauth#token-storage for details.",
stacklevel=2,
)
-
+
# Use full URL for token storage to properly separate tokens per MCP endpoint
self.token_storage_adapter: TokenStorageAdapter = TokenStorageAdapter(
async_key_value=token_storage, server_url=mcp_urlThe lines marked with Related Files
|
Test Failure AnalysisSummary: The static analysis check failed due to trailing whitespace in Root Cause: The PR introduces trailing whitespace on lines 231 and 250 of Suggested Solution: Remove the trailing whitespace from these lines:
Run Detailed AnalysisThe CI log shows: The specific changes made by ruff-format: @@ -228,7 +228,7 @@ class OAuth(OAuthClientProvider):
scope=scopes_str,
**(self._additional_client_metadata or {}),
)
-
+
if self._client_id:
# Create the full static client info directly which will avoid DCR
self._static_client_info = OAuthClientInformationFull(
@@ -248,7 +248,7 @@ class OAuth(OAuthClientProvider):
+ "See https://gofastmcp.com/clients/auth/oauth#token-storage for details.",
stacklevel=2,
)
-
+
# Use full URL for token storage to properly separate tokens per MCP endpoint
self.token_storage_adapter: TokenStorageAdapter = TokenStorageAdapter(
async_key_value=token_storage, server_url=mcp_urlThe formatter removed trailing spaces from two blank lines. Related Files
🤖 Analysis by Marvin - triage bot for FastMCP |
I’ve addressed all these points @jlowin, thanks for calling them out. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/fastmcp/client/auth/oauth.py (1)
365-381:⚠️ Potential issue | 🟠 MajorRetry-on-stale-credentials logic is counterproductive with static credentials.
When static credentials are in use and the server rejects them (triggering
ClientNotFoundError), the handler clears the cache and retries. But static credentials are fixed — they won't change on retry. Worse, after clearing the cache, the retry may silently fall back to Dynamic Client Registration, contradicting the user's intent.Consider short-circuiting the retry when
_static_client_infois set:🔧 Proposed fix sketch
except ClientNotFoundError: + if self._static_client_info is not None: + raise ClientNotFoundError( + "Static OAuth client credentials were rejected by the server. " + "Verify your client_id and client_secret are correct." + ) logger.debug( "OAuth client not found on server, clearing cache and retrying..." )
🧹 Nitpick comments (1)
src/fastmcp/client/auth/oauth.py (1)
270-278: Consider persisting static client info to storage.
_initializesetsself.context.client_infoin-memory but never callsawait self.token_storage_adapter.set_client_info(...). If any code path (e.g., the parent SDK) reads client info from storage rather than fromself.context, it will find nothing. Persisting also ensures consistency if_initializeis called multiple times after a cache clear.🔧 Proposed fix
if self._static_client_info is not None: + await self.token_storage_adapter.set_client_info(self._static_client_info) self.context.client_info = self._static_client_info
Include full client_metadata in static client info, short-circuit retry for static credentials, persist static client info to storage, add tests and documentation.
jlowin
left a comment
There was a problem hiding this comment.
@martimfasantos thank you! I've pushed a few doc changes to the branch, this LGTM
Test Failure AnalysisSummary: Two OAuth static client E2E tests are failing with "Authorization failed: 400" errors due to redirect URI mismatch between pre-registered clients and dynamically allocated callback ports. Root Cause: The tests pre-register OAuth clients with Suggested Solution:
Example fix for # Find port before pre-registering
callback_port = find_available_port()
pre_registered = OAuthClientInformationFull(
client_id="pre-registered-client",
client_secret="pre-registered-secret",
redirect_uris=[AnyHttpUrl(f"http://localhost:{callback_port}/callback")], # Use actual port
grant_types=["authorization_code", "refresh_token"],
response_types=["code"],
scope="read write",
)
await provider.register_client(pre_registered)
# Pass the same port to OAuth client
oauth = HeadlessOAuth(
mcp_url=url,
client_id="pre-registered-client",
client_secret="pre-registered-secret",
scopes=["read", "write"],
_callback_port=callback_port, # Use same port
)Apply the same fix to Detailed AnalysisError from logs:Code paths:
Related Files
🤖 Analysis by Marvin test triage bot |
Static client info now defaults token_endpoint_auth_method to "client_secret_post" (with secret) or "none" (without), which is required for the token exchange to work. E2E tests fixed for AnyUrl/AnyHttpUrl type mismatch in MCP SDK's redirect_uri validation.
Context
Some MCP servers do not support OAuth 2.0 Dynamic Client Registration (DCR).
The MCP specification explicitly does not require DCR support:
reference: https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#preregistration
So we should be able to manually register the OAuth client and obtain a
client_idandclient_secret.Summary
This PR adds support for providing a pre-registered
client_idandclient_secretto theOAuthclient provider inFastMCP. When provided, these credentials are used to pre-seed the token storage, effectively bypassing the OAuth 2.0 Dynamic Client Registration (DCR) step.Motivation
The MCP specification suggests that clients and servers SHOULD support RFC 7591 (Dynamic Client Registration). However, some MCP servers (e.g., GitHub Copilot's MCP server) do not support DCR and require clients to be manually registered or use static credentials.
Currently, users wishing to connect to such servers must manually instantiate
OAuthClientInformationFulland inject it into theTokenStoragebefore creating the client, which relies on internal implementation details.This change exposes
client_idandclient_secretas first-class arguments in theOAuthclass, streamlining the connection process for these servers.Proposed Changes
src/fastmcp/client/auth/oauth.pyUpdate
OAuth.__init__to acceptclient_idandclient_secret:Example Usage
Contributors Checklist
Review Checklist