fix: authorization resource for EXTERNAL_OAUTH21_PROVIDER=true - #404
fix: authorization resource for EXTERNAL_OAUTH21_PROVIDER=true#404taylorwilsdon wants to merge 5 commits into
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes the OAuth 2.1 authorization configuration for external OAuth mode, transforming the MCP server into a proper Resource Server that delegates to Google's Authorization Server instead of attempting to act as its own Authorization Server.
Changes:
- Enhanced
ExternalOAuthProviderto implement Resource Server behavior with proper metadata endpoints - Changed server configuration from
server.auth = Nonetoserver.auth = providerto enable OAuth validation - Added
resource_server_urlparameter and metadata routes pointing to Google's Authorization Server
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| core/server.py | Updated server initialization to enable protocol-level auth and pass resource_server_url parameter |
| auth/external_oauth_provider.py | Added get_routes() method, resource_server_url handling, and enhanced documentation for Resource Server role |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self._client_id = client_id | ||
| self._client_secret = client_secret | ||
| if self._resource_server_url: | ||
| self.resource_server_url = AnyHttpUrl(self._resource_server_url) |
There was a problem hiding this comment.
Direct instantiation of AnyHttpUrl is incorrect. Pydantic's AnyHttpUrl is a type annotation, not a constructor. Use pydantic.parse_obj_as(AnyHttpUrl, self._resource_server_url) or simply store the string directly since Pydantic validates it during model construction.
| self.resource_server_url = AnyHttpUrl(self._resource_server_url) | |
| self.resource_server_url = self._resource_server_url |
| # Create protected resource routes that point to Google as the authorization server | ||
| protected_routes = create_protected_resource_routes( | ||
| resource_url=self.resource_server_url, | ||
| authorization_servers=[AnyHttpUrl(GOOGLE_ISSUER_URL)], |
There was a problem hiding this comment.
Same issue as Comment 1: AnyHttpUrl cannot be instantiated directly. Use pydantic.parse_obj_as(AnyHttpUrl, GOOGLE_ISSUER_URL) or pass the string directly if the function accepts strings.
| # Google's OAuth 2.0 Authorization Server | ||
| GOOGLE_ISSUER_URL = "https://accounts.google.com" |
There was a problem hiding this comment.
The URL is missing a trailing slash which may cause inconsistencies with URL path joining operations. Consider using https://accounts.google.com/ for consistency with standard OAuth discovery endpoints.
| # Google's OAuth 2.0 Authorization Server | |
| GOOGLE_ISSUER_URL = "https://accounts.google.com" | |
| # Google's OAuth 2.0 Authorization Server (canonical issuer URL with trailing slash) | |
| GOOGLE_ISSUER_URL = "https://accounts.google.com/" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self._client_id = client_id | ||
| self._client_secret = client_secret | ||
| if self._resource_server_url: | ||
| self.resource_server_url = AnyHttpUrl(self._resource_server_url) |
There was a problem hiding this comment.
Direct assignment of AnyHttpUrl may fail validation. Use Pydantic's validation by calling AnyHttpUrl.validate(self._resource_server_url) or wrap in a try-except block to handle potential validation errors gracefully.
| Returns: | ||
| List of routes - only protected resource metadata | ||
| """ | ||
| from mcp.server.auth.routes import create_protected_resource_routes |
There was a problem hiding this comment.
Import statement is placed inside a method rather than at the module level. Consider moving this import to the top of the file with other imports for better code organization and to avoid repeated import overhead on each method call.
302 from #401 (no edit rights)
Thanks @ryohang
Fixes the authorization resource metadata for EXTERNAL_OAUTH21_PROVIDER=true mode to properly act as a Resource Server that points to Google's Authorization Server.
Problem
When using external OAuth mode (where access tokens are issued by external systems), the MCP server was not properly advertising its role as a Resource Server or pointing to the correct Authorization Server.
Changes
Enhanced ExternalOAuthProvider to implement proper Resource Server behavior:
Added get_routes() method to create protected resource metadata endpoints
Metadata now correctly points to Google's Authorization Server (https://accounts.google.com/)
Added resource_server_url parameter for proper endpoint configuration
Improved documentation explaining Resource Server vs Authorization Server roles
Updated server configuration to properly enable protocol-level auth:
Changed from server.auth = None to server.auth = provider to enable OAuth validation
Added clearer logging to indicate protected resource metadata setup