Skip to content

Conversation

@halllo
Copy link

@halllo halllo commented Sep 21, 2025

There currently is a problem with MCP servers that rely on a remote authorization server at a different host. During OAuth login the /authorize request is made to the correct authorization server, but the /token request is made against the MCP server.

Due to creating multiple transports and recursive invocations of connectToRemoteServer(), the acquired resource metadata (containing the correct authorization server) was forgotten between /authorize and /token calls.

Unfortunately the transport has the resource metadata in a private field, so I inject it by bypassing the typesystem. Going forward there should be only a single transport used for everything and no recursions. For now I consider this fix the bare minimum to get spec compliant OAuth to working at all.

…quired resource metadata (containing the correct authorization server) were forgotten between /authorize and /token calls. Now the metadata is remembered throughout the entire login flow.
Copy link
Contributor

@clouatre clouatre left a comment

Choose a reason for hiding this comment

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

Reviewed and tested locally on macOS.

Testing:

  • ✅ All tests pass (49/49)
  • ✅ Build succeeds without errors
  • ✅ Functionally solves the problem

Request Changes:

This PR uses a workaround when a simple proper fix exists.

Current approach:

  • Creates two transports, discards the one with metadata
  • Hacks private _resourceMetadataUrl field with any type
  • Adds 283 lines of tests for the workaround
  • Author admits: "Extremely hacky", "will break when refactored"

Proper fix (simpler, no hacks):

Instead of creating two transports and copying private fields, reuse the transport that discovered the metadata:

// In utils.ts, around line 320-330:
if (!sseTransport && !client) {
  const testClient = new Client({ name: 'mcp-remote-test', version: '0.0.0' }, { capabilities: {} })
  await testClient.connect(transport) // transport discovers metadata during connection
  return transport // has metadata ✅
}

This eliminates:

  • Private field access
  • Type system bypass with any
  • The need for 283-line test file
  • The hack that "will break when refactored"

Recommendation:

Please implement the proper fix. It's simpler, cleaner, and doesn't require hacks or extensive test infrastructure.

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.

3 participants