Title: OAuth client: harden SEP-2352/SEP-2350 edge cases; fix conformance comment by maxisbey · Pull Request #2936 · modelcontextprotocol/python-sdk · GitHub
Open Graph Title: OAuth client: harden SEP-2352/SEP-2350 edge cases; fix conformance comment by maxisbey · Pull Request #2936 · modelcontextprotocol/python-sdk
X Title: OAuth client: harden SEP-2352/SEP-2350 edge cases; fix conformance comment by maxisbey · Pull Request #2936 · modelcontextprotocol/python-sdk
Description: Three small follow-ups to #2933 / #2931 plus a comment correction from #2911. Motivation and Context Review of the merged SEP-2352 and SEP-2350 implementations turned up a few narrow paths where the new guarantees don't hold: SEP-2352, no-PRM path: the issuer-binding check runs before authorization-server metadata discovery and is gated on auth_server_url. When PRM discovery fails and the AS is found via the root /.well-known/oauth-authorization-server fallback, the recorded issuer is never compared, so credentials bound to a previous AS are reused. The stamping side already falls back to oauth_metadata.issuer in this case; this adds the matching comparison. SEP-2352, stale metadata: when the issuer-mismatch block discards bound credentials, cached oauth_metadata from the old AS is left in place. If discovery for the new AS then fails, dynamic client registration runs against the old server's registration_endpoint while stamping the new issuer, persisting a mis-bound record. SEP-2350, omitted scope: RFC 6749 §5.1 lets the AS omit scope from the token response when granted scope equals requested scope. After a process restart the reloaded token then has scope=None and client_metadata.scope has reverted to its constructor value, so the step-up union collapses to just the challenged scope. Backfilling the requested scope before persisting (and carrying the prior scope forward on refresh, per §6) keeps the stored token self-describing. The conformance-client comment correction is doc-only: the harness picks LATEST_SPEC_VERSION vs DRAFT_PROTOCOL_VERSION per-scenario when --spec-version is omitted, not a flat LATEST_SPEC_VERSION. The value is still log-only today. How Has This Been Tested? Three new tests in tests/client/test_auth.py: test_handle_token_response_backfills_omitted_scope_from_request test_handle_refresh_response_carries_prior_scope_when_response_omits_it test_issuer_binding_re_evaluated_after_asm_when_prm_discovery_failed — drives the auth-flow generator through PRM 404×2 → root ASM 200 with a mismatched issuer and asserts the next yield is a DCR request, not the stale client's authorize redirect The existing tests/interaction/auth/test_lifecycle.py SEP-2352 migration test exercises the oauth_metadata = None clear on the PRM-success path. Breaking Changes None. Types of changes Bug fix (non-breaking change which fixes an issue) New feature (non-breaking change which adds functionality) Breaking change (fix or feature that would cause existing functionality to change) Documentation update Checklist I have read the MCP Documentation My code follows the repository's style guidelines New and existing tests pass locally I have added appropriate error handling I have added or updated documentation as needed Additional context The 403 step-up path also skips PRM/ASM discovery entirely, so after a restart it can fall back to /authorize and /token on the resource server's origin rather than the AS. That fix needs the discovery sequence factored into a sub-generator both the 401 and 403 branches can drive — out of scope for this PR. AI Disclaimer
Open Graph Description: Three small follow-ups to #2933 / #2931 plus a comment correction from #2911. Motivation and Context Review of the merged SEP-2352 and SEP-2350 implementations turned up a few narrow paths where th...
X Description: Three small follow-ups to #2933 / #2931 plus a comment correction from #2911. Motivation and Context Review of the merged SEP-2352 and SEP-2350 implementations turned up a few narrow paths where th...
Opengraph URL: https://github.com/modelcontextprotocol/python-sdk/pull/2936
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/files(.:format) |
| route-controller | pull_requests |
| route-action | files |
| fetch-nonce | v2:f8afef5c-3856-3643-6c34-a1ab3c28118e |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | 8D5E:168706:C945D9:1206D26:6A5A01F9 |
| html-safe-nonce | 4c889a5756da85a067e7f0ac9c2a54a2429aa45467e6b0bc7425ffcd9a21db98 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiI4RDVFOjE2ODcwNjpDOTQ1RDk6MTIwNkQyNjo2QTVBMDFGOSIsInZpc2l0b3JfaWQiOiI2Mzc5NjQ5Nzk2NTQzMjE4MTY5IiwicmVnaW9uX2VkZ2UiOiJpYWQiLCJyZWdpb25fcmVuZGVyIjoiaWFkIn0= |
| visitor-hmac | 3d0810de28b86f197edadf76195ba6734dbb9e412307512e2761d3253faab5b2 |
| hovercard-subject-tag | pull_request:3905112846 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/modelcontextprotocol/python-sdk/pull/2936/files |
| twitter:image | https://avatars.githubusercontent.com/u/224885523?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/224885523?s=400&v=4 |
| og:image:alt | Three small follow-ups to #2933 / #2931 plus a comment correction from #2911. Motivation and Context Review of the merged SEP-2352 and SEP-2350 implementations turned up a few narrow paths where th... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 4d3f961cfc83a4348ee5be7691cf4ae3515aab3ef5200c13a7e41b048125fb61 |
| turbo-cache-control | no-preview |
| diff-view | unified |
| go-import | github.com/modelcontextprotocol/python-sdk git https://github.com/modelcontextprotocol/python-sdk.git |
| octolytics-dimension-user_id | 182288589 |
| octolytics-dimension-user_login | modelcontextprotocol |
| octolytics-dimension-repository_id | 862584018 |
| octolytics-dimension-repository_nwo | modelcontextprotocol/python-sdk |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 862584018 |
| octolytics-dimension-repository_network_root_nwo | modelcontextprotocol/python-sdk |
| turbo-body-classes | logged-out env-production page-responsive full-width |
| disable-turbo | true |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | ac4d28603af300ac851b62521abb878ab5d3319a |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width