fix(auth): preserve root protected resource URI - #3259
Conversation
This comment was marked as spam.
This comment was marked as spam.
|
This PR has been closed automatically. It's still a draft, but we close those early so you don't put in more time only to have it closed the moment you mark it ready. This repo only keeps pull requests open when they come from a maintainer, or from a contributor a maintainer has assigned to the linked issue, and this PR doesn't link an open issue yet.
You're welcome to keep pushing commits here (just avoid force-pushing, since GitHub can't reopen a rewritten branch), but that on its own won't get the PR reviewed or the issue assigned, and realistically most auto-closed PRs stay closed. There's no need to open a new PR either way. CONTRIBUTING.md has the full reasoning, but in short:
Maintainers: reopen, remove |
Problem
ProtectedResourceMetadata.resourcecan be serialized with a synthetic trailing slash when a root resource URL is supplied as an already-validatedAnyHttpUrl. This changeshttps://example.comtohttps://example.com/and can break strict RFC 9728 resource matching.Root Cause
AnyHttpUrlnormalizes an empty URL path to/beforeProtectedResourceMetadata'surl_preserve_empty_pathmodel setting can preserve the original spelling. The server auth settings pass an already-validated URL object into the metadata model.Solution
Normalize only the root path in a
ProtectedResourceMetadatapre-validator, removing the synthetic slash beforeAnyHttpUrlvalidation. Non-root paths and URL validation remain unchanged.Changes
ProtectedResourceMetadata.resource.Testing
py -m pytest tests/shared/test_auth.py -q— 43 passed.py -m pytest tests/shared/test_auth_utils.py tests/shared/test_auth.py -q— 58 passed.py -m pytest tests/client/test_auth.py -q— 140 passed, 1 xfailed.py -m ruff format --check src/mcp/shared/auth.py tests/shared/test_auth.py— passed.py -m ruff check src/mcp/shared/auth.py tests/shared/test_auth.py— passed.git diff --check— passed.Compatibility/Risk
The public field type remains
AnyHttpUrl. Only a root-path slash synthesized by URL normalization is removed; non-root paths, query strings, and validation behavior are unchanged.Notes for Reviewer
Pyright could not be run with the repository's configured
.venvbecause this incremental worktree does not contain that environment. A separate available environment reported no source-file diagnostics but reported unrelated baseline test typing diagnostics; this is explicitly unverified locally.Linked Issue
Closes #2883