Oauth rfc9207 issuer identification - #9
Merged
Conversation
Adds `iss` to the authorization response and advertises `authorization_response_iss_parameter_supported` in the authorization server metadata. RFC 9207 is mixed-up-authorization-server defence: it names which server produced a response, so a client registered with several cannot be induced to redeem a code at the wrong one. It is also the fix for hosted clients that choose their redirect URI from whether the server supports it. ChatGPT connectors use the stable `https://chatgpt.com/connector_platform_oauth_redirect` when the server conforms, and a per-connector `https://chatgpt.com/connector/oauth/{callback_id}` when it does not — a URI no exact-match allowlist can express. Confirmed in production: 9 rejections over two days, all naming the per-connector form. No control is relaxed. The redirect allowlist is untouched and still exact-match (RFC 9700 §2.1); conforming makes the client ask for a URI that is already on it. The alternative — matching the vendor's callbacks by pattern — would let an attacker name their own destination inside the prefix, so the README now records why that is not the fix. - `iss` is response-owned alongside `code`/`state`: a caller that seeds one into its own redirect_uri has it replaced, not appended. - `iss` must byte-match the advertised `issuer` (clients compare by exact string). Both derive from `mcp_oauth_issuer`, which reads `request.base_url` and so honours X-Forwarded-Host — a spec pins the redirect against the discovery document rather than a literal, so any divergence fails here. - `approve` is the only response this bridge redirects; every error path renders (400/422), so there is no error response for `iss` to be absent from. 619 examples/0 failures (all 4 new assertions verified red without the change), RuboCop 65/0, Brakeman 0.
The rationale (mixed-up authorization server, RFC 9207 §2's MUST, why `approve` is the only redirect that can carry `iss`) was written into the source as a 12-line block minutes after working it out. It is already in the previous commit message and the CHANGELOG, and the code shows what matters: `iss` is `mcp_oauth_issuer`, the same method the metadata renders. Also renames the loopback describe, which said "its own code" while now covering a seeded `iss` too. 24 added comment lines -> 2.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.