Skip to content

Document redirect_uri wire-format change in OAuth migration note #2929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Kludex merged 1 commit into from
Jun 20, 2026
+8 −0
Merged

Document redirect_uri wire-format change in OAuth migration note #2929

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/migration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1259,6 +1259,14 @@
URLs constructed in Python from an already-built `AnyHttpUrl` object are unaffected (they were
normalized at construction); only values parsed from strings/JSON change.

This also changes the wire form of `OAuthClientMetadata.redirect_uris`: a path-less redirect URI
passed as a string (e.g. `redirect_uris=['http://localhost:8080']`) now serializes as
`http://localhost:8080` instead of `http://localhost:8080/`, and the client sends it verbatim in
the `/authorize` and token-exchange requests. RFC 6749 §3.1.2.3 requires authorization servers to
match redirect URIs by exact string comparison, so if you registered such a URI with a previous SDK
release (with the trailing slash) and the registration is persisted in `TokenStorage`, re-register
the client so the stored value matches what the SDK now transmits.

Check warning on line 1269 in docs/migration.md

View check run for this annotation

Claude / Claude Code Review

Migration note: 're-register the client' isn't actionable — clarify that the persisted client registration must be cleared from TokenStorage 

The instruction to "re-register the client" isn't actionable as written: the SDK only performs dynamic client registration when `TokenStorage.get_client_info()` returns nothing, so a persisted registration is reused as-is and there is no API to force re-registration. Suggest rewording to tell affected users to clear the stored client registration (and tokens) from their `TokenStorage` so the next flow registers afresh with the slash-less redirect URI.
Comment on lines +1262 to +1269

@claude claude Bot Jun 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 The instruction to "re-register the client" isn't actionable as written: the SDK only performs dynamic client registration when TokenStorage.get_client_info() returns nothing, so a persisted registration is reused as-is and there is no API to force re-registration. Suggest rewording to tell affected users to clear the stored client registration (and tokens) from their TokenStorage so the next flow registers afresh with the slash-less redirect URI.

Extended reasoning...

What the note says vs. what the SDK does. The new paragraph tells users whose registration "is persisted in TokenStorage" to "re-register the client so the stored value matches what the SDK now transmits." However, OAuthClientProvider only performs dynamic client registration when no registration is already persisted: in src/mcp/client/auth/oauth2.py, _initialize() loads client_info from storage.get_client_info() (line 484), and Step 4 of the 401 flow only registers under if not self.context.client_info: (line 596). There is no SDK API to force re-registration while a registration remains in storage, so a reader following the note literally has no way to act on it.

The code path that makes the mismatch real. After PR #2925, OAuthClientMetadata serializes a path-less redirect_uris entry without the trailing slash, and the client sends client_metadata.redirect_uris[0] verbatim in the /authorize request and the token exchange (oauth2.py lines 340 and 399). Meanwhile the authorization server still holds the old slash-suffixed registration created by a previous SDK release, and RFC 6749 §3.1.2.3 mandates exact string comparison — so the flow fails. The note correctly identifies the problem; only the remedy is unclear.

Step-by-step example. (1) A user on the previous SDK registers with redirect_uris=['http://localhost:8080']; the metadata serialized as http://localhost:8080/, and the AS stored that. The resulting OAuthClientInformationFull is persisted via TokenStorage.set_client_info(). (2) The user upgrades and tokens later expire/are cleared, triggering a fresh authorization. (3) _initialize() loads the old client_info from storage, so the registration step is skipped, but the redirect_uri query parameter now goes out as http://localhost:8080 (no slash). (4) The AS exact-matches against its stored http://localhost:8080/ and rejects the request. (5) The user reads the migration note and tries to "re-register" — but as long as get_client_info() keeps returning the old registration, the SDK never re-registers, so the failure persists.

Secondary wording issue. The phrase "so the stored value matches what the SDK now transmits" conflates two stores: the OAuthClientInformationFull persisted in the user's TokenStorage (which gates whether DCR runs at all) and the registration held by the authorization server (which is what the RFC 6749 exact-match comparison actually runs against). Re-registration fixes the AS-side record; clearing TokenStorage is what causes re-registration to happen.

Suggested fix. Reword the final clause to something like: "…clear the stored client registration from your TokenStorage (so get_client_info() returns None) and let the client perform a fresh registration with the new redirect URI — or update the registration at the authorization server out of band." This is a docs-only clarity improvement to a docs-only PR whose entire content is this paragraph; the statement isn't wrong, just not actionable, hence nit severity. All four verifiers independently confirmed the underlying SDK behavior and agreed on this characterization.

### Lowlevel `Server`: `subscribe` capability now correctly reported

Previously, the lowlevel `Server` hardcoded `subscribe=False` in resource capabilities even when a `subscribe_resource()` handler was registered. The `subscribe` capability is now dynamically set to `True` when an `on_subscribe_resource` handler is provided. Clients that previously didn't see `subscribe: true` in capabilities will now see it when a handler is registered, which may change client behavior.
Expand Down
Loading