Skip to content

fix the authorization resource for EXTERNAL_OAUTH21_PROVIDER=true - #401

Closed
ryohang wants to merge 4 commits into
taylorwilsdon:mainfrom
ascending-llc:bug/wellknown-resource
Closed

fix the authorization resource for EXTERNAL_OAUTH21_PROVIDER=true#401
ryohang wants to merge 4 commits into
taylorwilsdon:mainfrom
ascending-llc:bug/wellknown-resource

Conversation

@ryohang

@ryohang ryohang commented Jan 28, 2026

Copy link
Copy Markdown
Contributor

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:

  1. Added get_routes() method to create protected resource metadata endpoints
  2. Metadata now correctly points to Google's Authorization Server (https://accounts.google.com)
  3. Added resource_server_url parameter for proper endpoint configuration
  4. Improved documentation explaining Resource Server vs Authorization Server roles
  5. 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

not sure about Version bump: 1.7.1 → 1.8.0 . please advise

@taylorwilsdon
taylorwilsdon requested review from Copilot and taylorwilsdon and removed request for Copilot January 28, 2026 21:32
@taylorwilsdon taylorwilsdon self-assigned this Jan 28, 2026
@taylorwilsdon taylorwilsdon added the bug Something isn't working label Jan 28, 2026
@taylorwilsdon

Copy link
Copy Markdown
Owner

Hi @ryohang - I lack permissions to push to your fork of the repo, but I have applied the ruff format changes for you here #402

@taylorwilsdon
taylorwilsdon requested a review from Copilot January 28, 2026 21:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes OAuth 2.1 configuration for external provider mode by properly implementing Resource Server behavior that points to Google's Authorization Server.

Changes:

  • Modified ExternalOAuthProvider to act as a Resource Server with metadata endpoints pointing to Google
  • Updated server configuration to enable OAuth validation at the protocol level
  • Enhanced documentation explaining Resource Server vs Authorization Server roles

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
core/server.py Enabled protocol-level auth and updated logging to reflect Resource Server configuration
auth/external_oauth_provider.py Added get_routes() method and resource_server_url parameter to create protected resource metadata

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread core/server.py
Comment thread auth/external_oauth_provider.py Outdated
Comment thread auth/external_oauth_provider.py Outdated
@ryohang

ryohang commented Jan 28, 2026

Copy link
Copy Markdown
Contributor Author

@copilot open a new pull request to apply changes based on the comments in this thread

@ryohang

ryohang commented Jan 28, 2026

Copy link
Copy Markdown
Contributor Author

I updated according to the comments

@taylorwilsdon

Copy link
Copy Markdown
Owner

I don't see any new commits?

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@ryohang

ryohang commented Jan 29, 2026

Copy link
Copy Markdown
Contributor Author

I don't see any new commits?

sorry, I was not too familiar with the process, just submitted.

@taylorwilsdon
taylorwilsdon requested a review from Copilot January 29, 2026 15:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copilot AI Jan 29, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AnyHttpUrl constructor expects a URL string but is being called like a function. Use Pydantic's validation pattern: self.resource_server_url = AnyHttpUrl.validate(self._resource_server_url) or directly assign the string and let Pydantic handle validation through field typing.

Suggested change
self.resource_server_url = AnyHttpUrl(self._resource_server_url)
self.resource_server_url = AnyHttpUrl.validate(self._resource_server_url)

Copilot uses AI. Check for mistakes.
# 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)],

Copilot AI Jan 29, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AnyHttpUrl constructor is being misused. Either use AnyHttpUrl.validate(GOOGLE_ISSUER_URL) or pass the string directly if the function expects URL strings, not Pydantic types.

Suggested change
authorization_servers=[AnyHttpUrl(GOOGLE_ISSUER_URL)],
authorization_servers=[AnyHttpUrl.validate(GOOGLE_ISSUER_URL)],

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants