Skip to content

Conversation

@arjunkmrm
Copy link

@arjunkmrm arjunkmrm commented Jul 30, 2025

Dev from https://smithery.ai here 👋! We noticed a bug when testing our server's OAuth flow where subpaths were being dropped when creating OAuth endpoints. This PR fixes getDefaultMetadataForUrl to preserve them per rfc 8414 section 3: https://datatracker.ietf.org/doc/html/rfc8414#section-3

Authorization servers supporting metadata MUST make a JSON document containing metadata as specified in Section 2 available at a path formed by inserting a well-known URI string into the authorization server's issuer identifier between the host component and the path component, if any. By default, the well-known URI string used is "/.well-known/oauth-authorization-server". This path MUST use the "https" scheme. The syntax and semantics of ".well-known" are defined in RFC 5785 [RFC5785]. The well-known URI suffix used MUST be registered in the IANA "Well-Known URIs" registry [IANA.well-known].

Issue

See #258845

OAuth servers hosted at subpaths (e.g., https://api.example.com/oauth/server) were having their paths dropped:

// Before (broken)
Input: https://api.example.com/oauth/server
Output: https://api.example.com/authorize 

// After (fixed)  
Input: https://api.example.com/oauth/server
Output: https://api.example.com/oauth/server/authorize 

Changes

  • Preserve existing paths in getDefaultMetadataForUrl
  • Add RFC 8414 reference

Fixes the same issue as modelcontextprotocol/typescript-sdk#687.

See tests in src/vs/base/test/common/oauth.test.ts

@arjunkmrm
Copy link
Author

arjunkmrm commented Jul 30, 2025

@microsoft-github-policy-service agree company="Smithery"

@arjunkmrm
Copy link
Author

@joaomoreno closing since the issue has been resolved: #258845

@arjunkmrm arjunkmrm closed this Jul 31, 2025
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Sep 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants