Allow auth providers to override protected resource base URLs#3900
Allow auth providers to override protected resource base URLs#3900
Conversation
marvin-context-protocol
Bot
added
enhancement
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c964a83454
ℹ️ 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".
| The full URL of the protected resource | ||
| """ | ||
| if self.base_url is None: | ||
| resource_base_url = self.resource_base_url or self.base_url |
There was a problem hiding this comment.
This change makes _get_resource_url prefer self.resource_base_url, but MultiAuth still initializes only base_url from its wrapped server. In create_sse_app and create_streamable_http_app, the framework calls auth._get_resource_url(...) on the outer auth object to build resource_metadata_url, so MultiAuth(server=OAuthProxy(..., resource_base_url=...)) now emits metadata/challenge URLs based on base_url while the server routes and JWT audience are based on resource_base_url. That mismatch can send clients to the wrong protected-resource metadata endpoint and break auth discovery/invalid_target flows in MultiAuth deployments.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. MultiAuth was still only inheriting base_url, so the outer auth layer could advertise the wrong protected-resource metadata URL when the wrapped server set resource_base_url.
I pushed a follow-up that propagates resource_base_url through MultiAuth by default, with an explicit override if needed, and added coverage for the auth challenge path so resource_metadata now lines up with the wrapped server’s advertised resource URL.
The base AuthProvider docstring claimed 'audience checks are derived from this URL' which is wrong — the base class does no audience checking. Clarify that resource_base_url advertises the resource URL in protected resource metadata, that OAuthProxy-style providers additionally use it as the minted downstream token audience, and that upstream token audience validation is configured separately on the verifier.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 489cd1e83b
ℹ️ 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".
| super().__init__( | ||
| base_url=effective_base_url, | ||
| resource_base_url=effective_resource_base_url, | ||
| required_scopes=effective_scopes, |
There was a problem hiding this comment.
When resource_base_url is passed to MultiAuth, this constructor applies it only to the outer MultiAuth instance, but get_routes() still delegates to self.server, whose resource URL settings are unchanged. In configurations like MultiAuth(server=RemoteAuthProvider(base_url='https://auth.example.com/proxy'), resource_base_url='https://api.example.com'), the WWW-Authenticate challenge advertises metadata under https://api.example.com/... while the served protected-resource route is still derived from the server (/.well-known/oauth-protected-resource/proxy/mcp), which can break OAuth discovery and resource targeting. The override needs to be propagated to the wrapped server (or the route generation needs to use the outer override) so advertised and served metadata stay consistent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed this. MultiAuth now propagates an explicit resource_base_url override into the wrapped server as well, so the served protected-resource metadata stays aligned with the outer auth challenge. I also added coverage for that override path.
When resource_base_url was set on MultiAuth but not on the wrapped server, the outer WWW-Authenticate challenge advertised metadata at the override URL while the server's get_routes() still built metadata from its own base_url — clients would see inconsistent resource values between the challenge and the served metadata. Now an explicit override on MultiAuth is written back to the wrapped server so served routes stay in sync with the advertised challenge.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93ce21be60
ℹ️ 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".
| base_url: AnyHttpUrl | str | None = None, | ||
| resource_base_url: AnyHttpUrl | str | None = None, | ||
| required_scopes: list[str] | None = None, |
There was a problem hiding this comment.
Adding resource_base_url before required_scopes changes positional binding for existing subclasses that still call super().__init__(base_url, required_scopes). In that call, the scopes list is now interpreted as resource_base_url and required_scopes falls back to [], so RequireAuthMiddleware receives no required scopes and scope checks are skipped. This is a silent security regression for custom verifiers using the previous constructor contract.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Updated this to preserve the original positional constructor behavior. required_scopes stays in its existing slot, and resource_base_url is additive after it. I also added a regression test around the legacy super().__init__(base_url, required_scopes) call shape.
| base_url: AnyHttpUrl | str, | ||
| resource_base_url: AnyHttpUrl | str | None = None, | ||
| scopes_supported: list[str] | None = None, |
There was a problem hiding this comment.
Inserting resource_base_url before scopes_supported breaks existing positional calls like RemoteAuthProvider(verifier, servers, base_url, ['read']): the scope list is now stored as resource_base_url, _scopes_supported stays None, and _get_resource_url('/mcp') later tries to build a URL from that list, triggering URL validation errors when auth routes/challenges are generated. Keeping backward-compatible parameter order (or forcing keyword-only args) avoids this runtime regression.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Adjusted this too. RemoteAuthProvider now keeps scopes_supported in its original positional slot, with resource_base_url added after it so existing positional call sites keep working. There’s a regression test covering the legacy RemoteAuthProvider(verifier, servers, base_url, ['read']) shape.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: Jeremiah Lowin <[email protected]>
2 participants
Summary
resource_base_urlacross remote auth and OAuth/OIDC proxy providersresource_base_urlwhen it is set while keeping operational OAuth routes rooted atbase_urlTesting
uv run --project /tmp/fastmcp pytest /tmp/fastmcp/tests/server/auth/test_remote_auth_provider.py /tmp/fastmcp/tests/server/auth/oauth_proxy/test_config.py /tmp/fastmcp/tests/server/auth/providers/test_github.py