feat(oauth): advertise scopes via OAUTH_SCOPES_SUPPORTED - #33
Conversation
|
Thanks — same quality as #32, and the same care in the writeup. I verified the claims against the vendored SDK rather than the docs: The Two things I'd like before merging, plus a rebase: 1. The fix is half-applied. // /.well-known/oauth-protected-resource ← configurable after this PR
"scopes_supported": ["api://<guid>/mcp.access"]
// /.well-known/oauth-authorization-server ← still hardcoded
"scopes_supported": ["openid", "email", "profile"]You're right that the AS document isn't what a client reads to build its authorization request under RFC 9728. But this server does serve it, at a well-known path clients do check, and 2. Comma-only parsing is a trap. 3. Rebase. I'm merging #32 first, which will conflict with this in six files. Most are additive, but three need real attention rather than take-both: the Happy to take the two changes in the same round-trip as the rebase so it's one pass, not three. One unrelated observation while I was in here: |
mcpAuthMetadataRouter accepts a `scopesSupported` option and publishes it as `scopes_supported` in the protected-resource metadata (RFC 9728), but the server never passed it and there was no env var to configure it. The document therefore named no scopes at all, so a client discovering the server through it has nothing to put in the authorization request's `scope` parameter and may omit it — which some IdPs reject outright (Microsoft Entra: AADSTS900144), breaking sign-in before it starts. OAUTH_SCOPES_SUPPORTED is comma-separated. When unset, `advertisedScopes` returns undefined rather than [], so the key is dropped from the serialized document and both well-known responses stay byte-for-byte as before.
…RTED buildOAuthMetadata hardcoded scopes_supported: [openid, email, profile] on the RFC 8414 document. With OAUTH_SCOPES_SUPPORTED driving only the RFC 9728 protected-resource document, an operator configuring scopes for a non-WorkOS IdP would end up with the two well-known documents contradicting each other. Both now come from the same source. When unset the authorization-server document keeps the historic default, so existing deployments are unchanged.
…RTED A scope value can never contain a space (RFC 6749 3.3), so `openid email profile` - the form scopes take everywhere else in OAuth - is unambiguous. Splitting on commas alone turned it into one invalid scope named "openid email profile". Now splits on commas or whitespace.
dfd10a6 to
4b44ea1
Compare
|
Heads-up: I've pushed to this branch rather than making you do another round-trip. Please What I did:
Plus README, I hope pushing directly is alright — it seemed friendlier than asking you to do a conflict resolution I'd already worked through. Tell me if you'd rather I hadn't and I'll leave your branches alone in future. For the record, the |
A `node_modules` symlink pointing at an absolute local path leaked into the #33 merge. Anyone cloning the repo would get a dangling symlink where node_modules belongs, breaking `npm install` / `npm ci` until they deleted it by hand. It slipped past `.gitignore` because the rule was `node_modules/` — a trailing slash matches directories only, never a symlink of the same name. Changed to `node_modules`, which matches both. Verified with `git check-ignore`. `npm ci` in CI happened to mask this: it clears and recreates the directory, so the workflow still passed on a tree that was broken for humans.
OAuth interoperability for IdPs that do not honour the Resource Indicator (OAUTH_AUDIENCE, #32) and scope advertisement in both well-known documents (OAUTH_SCOPES_SUPPORTED, #33), both contributed by @gutencoder. Both are unset by default and change nothing for existing deployments. Also records the dependency security fix already on main (9 advisories, 4 high). Version bumped in all three places: package.json, package-lock.json root, and the McpServer literal in src/server.ts.
Problem
mcpAuthMetadataRouteraccepts ascopesSupportedoption and publishes it asscopes_supportedin the protected-resource metadata. From@modelcontextprotocol/sdk@1.29.0,dist/esm/server/auth/router.d.ts:and its implementation puts the value straight into the document
(
dist/esm/server/auth/router.js):src/server.ts:93-96onmainnever passes it, and there is no env var to configure it:So
/.well-known/oauth-protected-resourcecurrently serves:{"resource":"https://mcp.example.com/","authorization_servers":["https://auth.example.com"]}with no
scopes_supportedat all. A client that discovers the server through thatdocument (per RFC 9728, which is the discovery path the README documents for custom
connectors) is told nothing about what to request, and may omit
scopefrom theauthorization request entirely.
Some IdPs reject that outright. Microsoft Entra fails the request with:
which breaks sign-in before the user ever sees a consent screen.
Note this is specifically the protected-resource document.
buildOAuthMetadataalready setsscopes_supportedon the authorization-servermetadata, which is a different document and not what a client reads to build its
authorization request here.
Solution
A new
OAUTH_SCOPES_SUPPORTEDenv var: comma-separated, parsed insrc/config.tsalongside the other
OAUTH_*variables, and passed through tomcpAuthMetadataRouter.The empty case is handled by a small pure helper in
src/oauth.ts:Returning
undefinedrather than[]is the load-bearing detail: the SDK copies thevalue into the metadata object and
JSON.stringifydrops undefined properties, so withnothing configured the key is absent and the document is unchanged. An empty array would
instead advertise
"scopes_supported": [], which is a different — and misleading —statement to make.
Only the protected-resource document is affected; the authorization-server metadata is
untouched.
What changes for existing users
Nothing, and this is verified rather than asserted. With the variable unset, both
well-known documents are byte-for-byte identical to what
mainserves:With
OAUTH_SCOPES_SUPPORTED=openid,emailthe resource document becomes:{"resource":"https://mcp.example.com/","authorization_servers":["https://auth.example.com"],"scopes_supported":["openid","email"]}Tests
npm testpasses (141 tests, up from 135). New coverage:tests/config.test.ts— parsing (comma-separated, trimmed, blanks dropped) and thedefault empty list.
tests/oauth.test.ts—advertisedScopesreturnsundefinedfor both unset andempty, and the configured list otherwise.
tests/oauth.test.ts— an end-to-end check that mountsmcpAuthMetadataRouterthe wayserver.tsdoes, serves it over a real HTTP listener, fetches the document and assertsthat the
scopes_supportedkey is absent (not merely falsy) when nothing isconfigured, and present with the right value when it is.
npm run buildanddocker buildpass.npm audit --omit=dev --audit-level=highisunchanged from
main(no dependency changes).Docs
README.mdconfig table and.env.exampledocument the variable, including when youwould need it.