Skip to content

Fix OAuth: use authorization_servers URL from resource metadata for scope discovery#1133

Open
Copilot wants to merge 2 commits intomainfrom
copilot/investigate-oauth-issue
Open

Fix OAuth: use authorization_servers URL from resource metadata for scope discovery#1133
Copilot wants to merge 2 commits intomainfrom
copilot/investigate-oauth-issue

Conversation

Copy link
Contributor

Copilot AI commented Mar 8, 2026

Summary

discoverScopes always used the MCP server URL for authorization server metadata discovery, ignoring authorization_servers from the protected resource metadata. This breaks OAuth when the auth server is on a different domain than the MCP server (Keycloak, Entra ID, etc.).

Fixes #675

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Refactoring (no functional changes)
  • Test updates
  • Build/CI improvements

Changes Made

The oauth-state-machine.ts metadata_discovery step already correctly extracts authorization_servers[0] from resource metadata. But discoverScopes in auth.ts—called from both the debug and normal OAuth flows—hardcoded new URL("/", serverUrl) as the discovery target.

// Before: always discovers from MCP server domain
const metadata = await discoverAuthorizationServerMetadata(
  new URL("/", serverUrl),
);

// After: uses authorization_servers from resource metadata when available
const authServerUrl =
  resourceMetadata?.authorization_servers?.length
    ? new URL(resourceMetadata.authorization_servers[0])
    : new URL("/", serverUrl);
const metadata = await discoverAuthorizationServerMetadata(authServerUrl);

Related Issues

Addresses the issue where Inspector ignores authorization_servers from protected resource metadata during scope discovery, per MCP spec Section 4.3.

Testing

  • Tested in UI mode
  • Tested in CLI mode
  • Tested with STDIO transport
  • Tested with SSE transport
  • Tested with Streamable HTTP transport
  • Added/updated automated tests
  • Manual testing performed

Test Results and/or Instructions

Added 3 test cases to auth.test.ts:

  • Uses authorization_servers URL from resource metadata for discovery (different domain)
  • Preserves full path in authorization_servers URL (e.g. /realms/my-realm/)
  • Falls back to serverUrl when authorization_servers is empty

All 490 tests pass.

Checklist

  • Code follows the style guidelines (ran npm run prettier-fix)
  • Self-review completed
  • Code is commented where necessary
  • Documentation updated (README, comments, etc.)

Breaking Changes

None.

Additional Context

None.

…uth server discovery

The discoverScopes function always used the MCP server URL to discover
authorization server metadata, ignoring the authorization_servers field
from the protected resource metadata. This caused failures when the
authorization server is on a different domain than the MCP server.

Now checks resourceMetadata.authorization_servers for a URL to use,
falling back to the MCP server URL when not available.

Fixes #675

Co-authored-by: cliffhall <871933+cliffhall@users.noreply.github.com>
Copilot AI changed the title [WIP] Investigate OAuth issue in inspector code Fix OAuth: use authorization_servers URL from resource metadata for scope discovery Mar 8, 2026
@cliffhall cliffhall requested review from BobDickinson, KKonstantinov, olaservo and pcarleton and removed request for KKonstantinov March 8, 2026 21:16
@cliffhall cliffhall marked this pull request as ready for review March 8, 2026 21:19
Copy link
Member

@cliffhall cliffhall left a comment

Choose a reason for hiding this comment

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

@BobDickinson @olaservo Note, this is what we're doing in oauth-state-machine.ts. It is just wrong that we're doing this in two places, V2 should be more unified on this front.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The inspector does not follow the spec according to authorization senarios

2 participants