Skip to content

fix(sdk): try the OIDC path-appended well-known during OAuth discovery - #1718

Open
mpressiv-vault wants to merge 1 commit into
UsefulSoftwareCo:mainfrom
mpressiv-vault:fix/oauth-discovery-path-appended-well-known
Open

fix(sdk): try the OIDC path-appended well-known during OAuth discovery#1718
mpressiv-vault wants to merge 1 commit into
UsefulSoftwareCo:mainfrom
mpressiv-vault:fix/oauth-discovery-path-appended-well-known

Conversation

@mpressiv-vault

Copy link
Copy Markdown

Problem

discoverAuthorizationServerMetadata only probes the two path-insertion well-known locations:

  • {origin}/.well-known/oauth-authorization-server{path}
  • {origin}/.well-known/openid-configuration{path}

An authorization server whose issuer is mounted under a path may serve its metadata at the path-appended location instead, {issuer}/.well-known/openid-configuration. OpenID Connect Discovery 1.0 §4 defines that location, and the MCP authorization specification requires clients to try it as well. Today discovery returns "no metadata" for such a server even though it is configured correctly, and oauth.probe fails with No OAuth authorization-server metadata found at <endpoint>.

Concrete case: TikTok's Ads MCP server at https://business-api.tiktok.com/open_mcp/tt-ads-mcp-flat. Its 401 correctly points at protected resource metadata, which names the issuer https://business-api.tiktok.com/open_mcp/tt-ads-mcp-flat/oauth. That issuer answers on:

GET {issuer}/.well-known/oauth-authorization-server 200
GET {issuer}/.well-known/openid-configuration 200

and 404s on the path-insertion forms, so discovery fails and the only way through is registering the client by hand with metadata fetched outside Executor.

Change

Replace wellKnownUrlFor with wellKnownCandidatesFor, which returns the existing two URLs plus, when the issuer has a path, the OIDC path-appended URL as a third candidate. Ordering and every other behaviour (per-candidate error tolerance, metadata validation, the surfaced metadataUrl) are unchanged, and issuers without a path produce exactly the same two requests as before.

Verification

  • New test in oauth-discovery.test.ts serves metadata only at the path-appended location and asserts it is found. It fails on main and passes with this change.
  • vitest run src/oauth-discovery.test.ts in packages/core/sdk: 26 passed.
  • oxlint on both files: 0 warnings, 0 errors. oxfmt --check: clean. tsgo --noEmit for the package: no new diagnostics.

Notes for the maintainer

  • The new test follows the withOAuthFixture pattern used by every other case in oauth-discovery.test.ts. AGENTS.md asks for @executor-js/emulate for OAuth/OIDC providers, so happy to move it there if you would rather have the whole file migrate at once.
  • Separate from this change: the loop comment says a candidate that fails "mid-roundtrip (network, parse, issuer mismatch)" still lets the next one be tried, but only the transport step runs inside Effect.result. A candidate that answers 200 with malformed JSON, or whose metadata fails validation, aborts discovery before the remaining candidates are tried. Left alone here to keep this PR to one behaviour change; glad to open a follow-up.

Authorization server metadata was only probed at the two path-insertion
locations, so an issuer mounted under a path that serves its metadata at
`{issuer}/.well-known/openid-configuration` was reported as having no
metadata at all. OIDC Discovery 1.0 section 4 defines that location and the
MCP authorization spec requires clients to try it, so add it as a third
candidate after the existing two.
Real-world case: TikTok's Ads MCP server at
https://business-api.tiktok.com/open_mcp/tt-ads-mcp-flat advertises an
issuer under `/open_mcp/tt-ads-mcp-flat/oauth` and serves metadata only at
the path-appended location. Discovery failed there while the server was
configured correctly, forcing callers to register clients by hand.
Sign up for freeto 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.

1 participant

@mpressiv-vault