OAuth client: harden SEP-2352/SEP-2350 edge cases; fix conformance comment - #2936
Merged
Claude / Claude Code Review
completed
Jun 22, 2026 in 39m 28s
Code review found 2 potential issues
Found 3 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/mcp/client/auth/oauth2.py:673-682 |
registration_endpoint gate drops issuer stamp for legacy same-origin AS, regressing SEP-2352 auto-recovery |
| 🟣 Pre-existing | src/mcp/client/auth/oauth2.py:480-485 |
Refresh response omitting refresh_token wipes the stored refresh token |
Annotations
Check warning on line 682 in src/mcp/client/auth/oauth2.py
claude / Claude Code Review
registration_endpoint gate drops issuer stamp for legacy same-origin AS, regressing SEP-2352 auto-recovery
The new `registration_endpoint is not None` gate also drops the issuer stamp in the legacy same-origin case (PRM 404 → root ASM discovery succeeds with issuer == resource origin, metadata omits the optional `registration_endpoint`, DCR falls back to that same origin's `/register`) — there the registration genuinely targets the discovered issuer's host, and pre-#2933-PR the record was correctly bound, so a later AS migration triggered discard + re-registration. Since `credentials_match_issuer` re
Check notice on line 485 in src/mcp/client/auth/oauth2.py
claude / Claude Code Review
Refresh response omitting refresh_token wipes the stored refresh token
Pre-existing issue in the block this PR touches: `_handle_refresh_response` now carries an omitted `scope` forward per RFC 6749 §6, but an omitted `refresh_token` (also allowed by §6 and common with non-rotating ASes) is not — the parsed token has `refresh_token=None` and unconditionally overwrites both `context.current_tokens` and persistent storage, so the next expiry forces a full interactive re-authorization (or fails for headless clients). The fix is the same one-line pattern this PR adds f
Loading