Skip to content

Fix OAuth refresh resource handling - #2646

Closed
FU-max-boop wants to merge 1 commit into
modelcontextprotocol:mainfrom
FU-max-boop:fix/oauth-refresh-resource-param
Closed

Fix OAuth refresh resource handling#2646
FU-max-boop wants to merge 1 commit into
modelcontextprotocol:mainfrom
FU-max-boop:fix/oauth-refresh-resource-param

Conversation

@FU-max-boop

Copy link
Copy Markdown

Summary

  • Strip the trailing slash that Pydantic AnyHttpUrl adds to bare-domain protected resource metadata before choosing the RFC 8707 resource URL.
  • Stop sending resource on refresh_token grants while keeping it on authorization-code token exchanges.
  • Add regression coverage for recent protocol versions, PRM-backed requests, and bare-domain PRM resources.

Fixes #2578

Validation

  • uv run pytest tests/client/test_auth.py -q
  • uv run ruff check src/mcp/client/auth/oauth2.py tests/client/test_auth.py
  • uv run ruff format --check src/mcp/client/auth/oauth2.py tests/client/test_auth.py
  • git diff --check

@maxisbey

Copy link
Copy Markdown
Contributor

Thanks for the PR. Closing in favor of #2590, which was opened first (May 13) and covers the same two fixes — dropping resource from the refresh_token grant and undoing Pydantic's trailing-slash normalization on the PRM resource URL — with equivalent test coverage.

One substantive difference worth noting: this PR strips the slash with .rstrip("/"), which also removes intentional trailing slashes from non-root paths (e.g. https://api.example.com/mcp/…/mcp). RFC 9728 requires exact-string identity on the resource identifier, so #2590's narrower approach (only strip when the path is exactly / with no query or fragment) is the safer one.

If you spot anything #2590 misses, a review there would be welcome.

AI Disclaimer

@maxisbey maxisbey closed this May 27, 2026
fede-kamel added a commit to fede-kamel/python-sdk that referenced this pull request Aug 19, 2026
…railing slash from PRM resource

Two compounding bugs broke silent token refresh against Microsoft Entra
ID v2.0 (AADSTS9010010), causing MCP servers using Entra OAuth to lose
authentication after ~1 hour:

- _refresh_token() sent the RFC 8707 resource parameter on
  refresh_token grants, which Entra v2.0 strictly rejects since
  March 2026. The parameter is now omitted from refresh requests.
- Pydantic's AnyHttpUrl normalizes bare-domain PRM resource URLs to
  include a trailing slash, so the resource audience never matched the
  IdP app registration. get_resource_url() now strips the slash, but
  only when the path is exactly "/" with no query or fragment - RFC
  9728 requires exact-string identity, so intentional trailing slashes
  on deeper paths are preserved.

Implements the approach maintainers endorsed when consolidating earlier
attempts (modelcontextprotocol#2590, since closed unmerged; see discussion on modelcontextprotocol#2645/modelcontextprotocol#2646).

Fixes modelcontextprotocol#2578
fede-kamel added a commit to fede-kamel/python-sdk that referenced this pull request Aug 20, 2026
…railing slash from PRM resource

Two compounding bugs broke silent token refresh against Microsoft Entra
ID v2.0 (AADSTS9010010), causing MCP servers using Entra OAuth to lose
authentication after ~1 hour:

- _refresh_token() sent the RFC 8707 resource parameter on
  refresh_token grants, which Entra v2.0 strictly rejects since
  March 2026. The parameter is now omitted from refresh requests.
- Pydantic's AnyHttpUrl normalizes bare-domain PRM resource URLs to
  include a trailing slash, so the resource audience never matched the
  IdP app registration. get_resource_url() now strips the slash, but
  only when the path is exactly "/" with no query or fragment - RFC
  9728 requires exact-string identity, so intentional trailing slashes
  on deeper paths are preserved.

Implements the approach maintainers endorsed when consolidating earlier
attempts (modelcontextprotocol#2590, since closed unmerged; see discussion on modelcontextprotocol#2645/modelcontextprotocol#2646).

Fixes modelcontextprotocol#2578
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OAuth token refresh sends RFC 8707 resource parameter that Entra ID v2.0 rejects (AADSTS9010010)

2 participants