fix: authorization resource for EXTERNAL_OAUTH21_PROVIDER=true - #402
fix: authorization resource for EXTERNAL_OAUTH21_PROVIDER=true#402taylorwilsdon wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes the OAuth authorization resource metadata for external OAuth mode (EXTERNAL_OAUTH21_PROVIDER=true), enabling the MCP server to properly act as a Resource Server that validates tokens issued by Google's Authorization Server.
Changes:
- Enhanced
ExternalOAuthProviderto implement Resource Server behavior with metadata endpoints pointing to Google's Authorization Server - Updated server configuration to enable OAuth validation at the protocol level by setting
server.auth = providerinstead ofNone - Improved logging messages to clarify the server's role as a protected resource
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| core/server.py | Updated external OAuth configuration to enable protocol-level auth and added clearer logging |
| auth/external_oauth_provider.py | Added get_routes() method to create protected resource metadata, resource_server_url parameter, and documentation explaining Resource Server role |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self, | ||
| client_id: str, | ||
| client_secret: str, | ||
| resource_server_url: str = None, |
There was a problem hiding this comment.
The parameter resource_server_url should use Optional[str] = None for explicit type annotation instead of str = None.
| resource_server_url: str = None, | |
| resource_server_url: Optional[str] = None, |
| self.resource_server_url = ( | ||
| AnyHttpUrl(self._resource_server_url) | ||
| if isinstance(self._resource_server_url, str) | ||
| else self._resource_server_url | ||
| ) | ||
|
|
There was a problem hiding this comment.
This conditional URL conversion logic is confusing. If resource_server_url parameter is typed as str, the isinstance check and else branch are unnecessary. Consider simplifying to just self.resource_server_url = AnyHttpUrl(self._resource_server_url) or properly typing the parameter as Union[str, AnyHttpUrl].
| self.resource_server_url = ( | |
| AnyHttpUrl(self._resource_server_url) | |
| if isinstance(self._resource_server_url, str) | |
| else self._resource_server_url | |
| ) | |
| self.resource_server_url = AnyHttpUrl(self._resource_server_url) |
@ryohang #401
Summary
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