-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix OAuth metadata endpoint URLs when base_url differs from issuer_url #2353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
OAuth operational endpoints (/authorize, /token) are mounted at base_url, but metadata was incorrectly declaring them at issuer_url. This caused clients following the documented mounting pattern to receive incorrect endpoint URLs in /.well-known/oauth-authorization-server. Fixes #2287
WalkthroughOAuthProvider no longer coerces Changes
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/server/auth/test_oauth_mounting.pyis excluded by none and included by none
📒 Files selected for processing (1)
src/fastmcp/server/auth/auth.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: label-issue-or-pr
- GitHub Check: Run tests: Python 3.10 on windows-latest
🔇 Additional comments (2)
src/fastmcp/server/auth/auth.py (2)
375-375: Verify whether this OAuth metadata separation is intentional or requires alignment.The code shows an intentional design where:
- Line 356 uses
self.base_urlas the issuer in OAuth authorization server metadata (per the comment about operational route mounting)- Line 375 uses
self.issuer_urlfor protected resource authorization serversSince
issuer_urldefaults tobase_urlbut can be explicitly set differently, this creates a potential mismatch: ifissuer_url ≠ base_url, the OAuth metadata and protected resource metadata declare different authorization servers. This could break token validation if token claims reference the issuer from OAuth metadata but protected resources expect the issuer from a separate configuration.Confirm whether this separation is intentional and that token validation will succeed with this setup. If not, line 375 should likely use
self.base_urlfor consistency with line 356.
351-356: Resolve: Review comment based on assumptions not supported by code evidence.The PR change is correct and fully tested. Test
test_oauth_mounting.py(lines 262-267) explicitly validates the new behavior:
- The issuer field should use base_url (where the server is actually running)
- Test asserts
metadata["issuer"]matchesbase_urlpattern, confirming the PR change works as intendedThe test comment and assertions indicate this was a deliberate design decision addressing the exact concern raised in the original review—ensuring metadata declares endpoints where they're actually accessible. The test's presence and passing state confirms OAuth compliance is handled correctly (either by the MCP library's
create_auth_routesor through proper token issuance handling).Line 375's use of
issuer_urlfor protected resources serves a different purpose (authorization server identity) and is appropriately separate from line 356's use ofbase_urlfor operational endpoint URLs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/fastmcp/server/auth/auth.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests: Python 3.10 on windows-latest
|
Fixed the 14 failing provider tests by removing an unnecessary assertion from |
|
Added an INFO log when |
Fixes #2287
When deploying an OAuth-protected FastMCP server under a path prefix (like
/api), users configure separateissuer_urlandbase_urlparameters following the documented pattern:The problem:
/.well-known/oauth-authorization-servermetadata was declaring endpoints atissuer_urlrather thanbase_url, causing a mismatch between where endpoints were advertised and where they actually existed.The fix ensures OAuth metadata declares operational endpoints where they're actually mounted (
base_url), matching the behavior already documented in the HTTP deployment guide.