-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Improve OAuth protected resource metadata URL construction per RFC 9728 #1407
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
base: main
Are you sure you want to change the base?
Improve OAuth protected resource metadata URL construction per RFC 9728 #1407
Conversation
…per RFC 9728 - Replace string manipulation with proper URL parsing using urllib.parse.urlparse - Handle edge case where resource server path is "/" by treating as empty string - Ensure consistent URL construction for both WWW-Authenticate header and metadata endpoint - Add RFC 9728 §3.1 compliance comments for better code documentation
- Moved import of AnyHttpUrl to the appropriate section for clarity - Removed unnecessary duplicate import of urlparse - Cleaned up whitespace for improved code readability
…ed function - Introduced `build_resource_metadata_url` to create RFC 9728 compliant metadata URLs. - Updated `FastMCP` to utilize the new function for constructing resource metadata URLs. - Improved code readability by removing redundant URL parsing logic from `server.py`.
…rces - Renamed `test_metadata_endpoint` to `test_metadata_endpoint_with_path` for clarity. - Added a new test `test_metadata_endpoint_root_path_returns_404` to verify 404 response for root path. - Introduced fixtures `root_resource_app` and `root_resource_client` for testing root-level resources. - Added `test_metadata_endpoint_without_path` to validate metadata retrieval for root-level resources.
Append '/mcp' to resource_server_url and handle trailing slashes to ensure proper OAuth Protected Resource Metadata validation. This prevents the error: "Protected Resource Metadata resource does not match MCP server resolved resource" which occurs when the PRM resource URL doesn't exactly match the server URL. Fixes compatibility with VS Code + MCP authentication per RFC 9728. References: microsoft/vscode#255255
…istency - Introduced `TestMetadataUrlConstruction` to validate URL construction for various resource configurations. - Added `TestRouteConsistency` to ensure consistency between generated metadata URLs and route paths. - Enhanced existing tests for better clarity and coverage of edge cases in URL handling.
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.
@shulkx - I am following this PR and respective issue closely. Thanks for drafting this must have fix for the mcp python sdk which ensures that it closely follows the directed Oauth guidelines.
I am in process of testing the fix outlined here in the PR by starting the resource server with --oauth-strict
flag as mentioned here - I came across another issue, which pertains to token introspection. While performing the token introspection by client - it encountered below error within the resource server.
INFO:httpx:HTTP Request: POST http://localhost:9000/introspect "HTTP/1.1 200 OK"
WARNING:mcp_simple_auth.token_verifier:Token resource validation failed. Expected: http://localhost:8001/
INFO: 127.0.0.1:65498 - "POST /mcp HTTP/1.1" 401 Unauthorized
While triaging this, I figured that Oauth server is sending aud
(for audience) as http://localhost:8001/mcp
but the token verifier is expecting it to be http://localhost:8001/
which is also highlighted within the error above.
In order to successfully perform this token introspection, I made a small change within the token_verifier
configuration within examples/servers/simple-auth/mcp_simple_auth/server.py
as noted below. If you can also verify the change suggested below and incorporate within your fix here, it would be great.
Before:
# Create token verifier for introspection with RFC 8707 resource validation
token_verifier = IntrospectionTokenVerifier(
introspection_endpoint=settings.auth_server_introspection_endpoint,
server_url=str(settings.server_url),
validate_resource=settings.oauth_strict, # Only validate when --oauth-strict is set
)
After:
# Create token verifier for introspection with RFC 8707 resource validation
token_verifier = IntrospectionTokenVerifier(
introspection_endpoint=settings.auth_server_introspection_endpoint,
server_url=f"{str(settings.server_url).rstrip('/')}/mcp", ## this is the change that resolved the introspection issue
validate_resource=settings.oauth_strict, # Only validate when --oauth-strict is set
)
The OAuth server returns `aud` including the `/mcp` path, while the token verifier previously expected only the base URL. This mismatch caused introspection failures under `--oauth-strict`. Updated the verifier configuration to use the correct audience to ensure successful token introspection.
Thanks @shrujanm - I’ve made the change as suggested to align the token verifier’s audience with the OAuth server’s response. Since I am currently traveling and unable to verify the fix myself, I’ve committed the change based on your recommendation. It would be great if you could confirm the resolution and validate it further on your end. Let me know if you run into any issues or if anything needs further adjustments during reviewing. |
Thanks @shulkx - I have just added couple loggers on my own resource server to show that above mentioned fix worked as expected. This one is while the resource server starts and initialize the
This logger is on resource server side while performing the introspection operation - post response is received from Oauth server Before the fix:
After the fix:
|
Hi @felixweinberger - It has been over a week since this PR was opened, and I wanted to follow up to ensure it gets reviewed. The changes address critical compliance issues with RFC 9728, which might cause client discovery failures. Could the assigned reviewers or the relevant teams take a look at this when they get a chance? Your input would be greatly appreciated. |
Hi @shulkx yes we'll get back to this next week - we had a hectic week here with an MCP event which led to less review capacity than we would've liked. Will follow up with the auth codeowners to take another look Monday AM! |
Understood. Thanks for the update! |
Summary
Fixes RFC 9728 compliance issues where the
/.well-known/oauth-protected-resource
endpoint was not correctly constructed for resource identifiers containing path components, causing client discovery failures. (#1400)Motivation and Context
Problem: The current implementation has two critical RFC 9728 compliance issues:
URL Construction Mismatch: When
resource_server_url
contains a path (e.g.,http://localhost:8001/mcp
), the WWW-Authenticate header points tohttp://localhost:8001/mcp/.well-known/oauth-protected-resource
(appending after the path), but the actual route is served athttp://localhost:8001/.well-known/oauth-protected-resource
(root level only).Non-RFC Compliant Path Insertion: RFC 9728 §3.1 specifies that the well-known path should be inserted between the host and resource path, not appended after the full URL.
Example Configuration Issue: The
simple-auth
example had an incorrectresource_server_url
configuration that didn't include the/mcp
path component, causing resource validation errors in VS Code MCP clients.Impact:
Root Cause: String concatenation approach doesn't properly handle URL path components according to RFC 9728 specification.
Changes Made
1. Created Reusable Utility Function
Added
build_resource_metadata_url()
insrc/mcp/server/auth/routes.py
to centralize RFC 9728 compliant URL construction:2. Updated WWW-Authenticate Header Construction
Before (Non-compliant):
After (RFC 9728 Compliant):
3. Updated Route Registration
Before (Non-compliant):
After (RFC 9728 Compliant):
4. Fixed Example Configuration
Updated
examples/servers/simple-auth/mcp_simple_auth/server.py
to include proper trailing slash handling:This ensures the resource identifier correctly matches the MCP server endpoint and prevents VS Code authentication errors.
5. Eliminated Code Duplication
streamable_http_app()
withcreate_protected_resource_routes()
URL Construction Examples:
http://localhost:8001
http://localhost:8001/.well-known/oauth-protected-resource
/.well-known/oauth-protected-resource
http://localhost:8001/
http://localhost:8001/.well-known/oauth-protected-resource
/.well-known/oauth-protected-resource
http://localhost:8001/mcp
http://localhost:8001/.well-known/oauth-protected-resource/mcp
/.well-known/oauth-protected-resource/mcp
How Has This Been Tested?
Breaking Changes
None. This fix maintains backward compatibility for existing deployments where
resource_server_url
has no path component, while fixing the broken behavior for path-based resources.Types of changes
Files Modified
src/mcp/server/fastmcp/server.py
- Updated both SSE and StreamableHTTP apps to use RFC 9728 compliant URL constructionsrc/mcp/server/auth/routes.py
- Added utility function and updatedcreate_protected_resource_routes()
examples/servers/simple-auth/mcp_simple_auth/server.py
- Fixed resource_server_url configuration with proper trailing slash handlingtests/server/auth/test_protected_resource.py
- Added comprehensive tests for new functionalityChecklist
Additional Context
RFC 9728 References:
Section 3.1 - Protected Resource Metadata Request (RFC 9728 §3.1):
Section 3.3 - Protected Resource Metadata Validation (RFC 9728 §3.3):