Skip to content

Conversation

@jlowin
Copy link
Owner

@jlowin jlowin commented Nov 3, 2025

Fixes #2287

When deploying an OAuth-protected FastMCP server under a path prefix (like /api), users configure separate issuer_url and base_url parameters following the documented pattern:

auth = GitHubProvider(
    client_id="...",
    client_secret="...",
    issuer_url="https://example.com",       # Root level for discovery
    base_url="https://example.com/api",     # Includes mount prefix
)

The problem: /.well-known/oauth-authorization-server metadata was declaring endpoints at issuer_url rather than base_url, causing a mismatch between where endpoints were advertised and where they actually existed.

// Before (incorrect)
{
  "authorization_endpoint": "https://example.com/authorize",  // 404
  "token_endpoint": "https://example.com/token"               // 404
}

// After (correct)
{
  "authorization_endpoint": "https://example.com/api/authorize",  // ✓
  "token_endpoint": "https://example.com/api/token"               // ✓
}

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.

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
@marvin-context-protocol marvin-context-protocol bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. auth Related to authentication (Bearer, JWT, OAuth, WorkOS) for client or server. labels Nov 3, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

Walkthrough

OAuthProvider no longer coerces base_url to AnyHttpUrl in __init__; runtime assertions ensure self.base_url and self.issuer_url are present. get_routes now calls create_auth_routes with issuer_url=self.base_url and uses cast(AnyHttpUrl, ...) for protected-route issuer values. No public signatures changed.

Changes

Cohort / File(s) Summary
OAuthProvider core changes
src/fastmcp/server/auth/auth.py
Removed explicit str→AnyHttpUrl conversion and assignment of base_url in __init__; issuer_url default now relies on self.base_url. Added runtime assert checks for self.base_url and self.issuer_url. In get_routes, call create_auth_routes(issuer_url=self.base_url). Protected-route issuer values are cast with cast(AnyHttpUrl, self.issuer_url).
Typing import
src/fastmcp/server/auth/auth.py
Added cast import from typing and used it where issuer URLs are cast to AnyHttpUrl.

Poem

🐇 I hopped through URLs, tidy and bright,
I stopped forcing strings to pretend they're right.
I whisper asserts where routes like to play,
Issuer and base now agree on the way. ✨

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main bug being fixed: OAuth metadata endpoint URLs declared at issuer_url instead of base_url when they differ.
Description check ✅ Passed The description provides clear context about the issue, the problem, and the fix. It includes issue reference (#2287), example configurations, before/after JSON comparison, and expected behavior.
Linked Issues check ✅ Passed The code changes ensure OAuth metadata declares endpoints at base_url rather than issuer_url, directly addressing issue #2287's requirement that operational routes be advertised at their actual mount location.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the OAuth metadata endpoint URL issue. The modifications to OAuthProvider class directly address the linked issue with no unrelated alterations.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/oauth-metadata-endpoint-urls

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a9c51e and 5ded7f7.

📒 Files selected for processing (1)
  • src/fastmcp/server/auth/auth.py (4 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
🔇 Additional comments (4)
src/fastmcp/server/auth/auth.py (4)

3-3: LGTM: Import addition for type casting.

The cast import is appropriately added to support the type narrowing operation used later in the file (line 375).


299-300: LGTM: Correctly relies on parent initialization.

The change to use self.base_url (set by the parent AuthProvider.__init__ at line 297) instead of the raw base_url parameter ensures consistent type handling, as the parent already performs the str→AnyHttpUrl conversion at lines 60-62.


354-360: Core fix correctly implements the deployment pattern.

Passing self.base_url as issuer_url to create_auth_routes ensures the OAuth metadata document advertises endpoints at their actual operational location. This correctly implements the documented deployment pattern where:

  1. Operational endpoints (/authorize, /token) are mounted at base_url
  2. Auto-generated well-known metadata points to base_url endpoints
  3. Users proxy well-known metadata at issuer_url to point to the same base_url endpoints

The protected resource metadata (line 375) correctly continues to reference self.issuer_url as the canonical authorization server identity, which is conceptually separate from the operational endpoint location.


375-375: LGTM: Type cast safely narrows the type.

The cast(AnyHttpUrl, self.issuer_url) is necessary to satisfy the type checker, as create_protected_resource_routes expects list[AnyHttpUrl]. The runtime assertion at lines 350-352 guarantees self.issuer_url is non-None, making this cast safe.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 6cc9559 and 9efc2b1.

⛔ Files ignored due to path filters (1)
  • tests/server/auth/test_oauth_mounting.py is 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_url as the issuer in OAuth authorization server metadata (per the comment about operational route mounting)
  • Line 375 uses self.issuer_url for protected resource authorization servers

Since issuer_url defaults to base_url but can be explicitly set differently, this creates a potential mismatch: if issuer_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_url for 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"] matches base_url pattern, confirming the PR change works as intended

The 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_routes or through proper token issuance handling).

Line 375's use of issuer_url for protected resources serves a different purpose (authorization server identity) and is appropriately separate from line 356's use of base_url for operational endpoint URLs.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 9efc2b1 and 6a9c51e.

📒 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

@jlowin
Copy link
Owner Author

jlowin commented Nov 4, 2025

Fixed the 14 failing provider tests by removing an unnecessary assertion from OAuthProvider.__init__() that was breaking backward compatibility. Providers can now be initialized without base_url for testing purposes, while the assertion in get_routes() ensures it's set when actually needed.

@jlowin
Copy link
Owner Author

jlowin commented Nov 4, 2025

Added an INFO log when issuer_url and base_url differ to help users ensure proper configuration. The log message points to the documentation for mounting authenticated servers and reminds them to expose well-known routes at the issuer URL.

@jlowin jlowin merged commit 5747cb6 into main Nov 4, 2025
13 of 14 checks passed
@jlowin jlowin deleted the fix/oauth-metadata-endpoint-urls branch November 4, 2025 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auth Related to authentication (Bearer, JWT, OAuth, WorkOS) for client or server. bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OAuth operational routes for auth.providers created at base_url but declared to be at issuer_url in /.well-known/oauth-authorization-server

2 participants