fix(auth): validate redirect_uri schemes at DCR registration (RFC 9700 §4.1.1, RFC 7591 §2)#2630
Open
CrypticCortex wants to merge 3 commits into
Conversation
Extracts validate_registered_redirect_uri to mcp.server.auth._redirect_uri to avoid a circular import with handlers.register; re-exports from routes to preserve the public test import path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2629.
Summary
Adds
validate_registered_redirect_uri(url: AnyUrl)and wires it intoRegistrationHandler.handleso each submittedredirect_uriis validated immediately aftermodel_validate_jsonsucceeds. On failure, returns400 invalid_redirect_uriper RFC 7591 §3.2.2.The helper mirrors the existing
validate_issuer_urlpolicy:httpsis allowed.httpis allowed only for loopback hosts (localhost,127.0.0.1,[::1]) per RFC 8252 §7.3.#fragments.The helper lives in a new
mcp.server.auth._redirect_urimodule to avoid a circular import withhandlers.register(the existingmcp.server.auth.routesre-exports it so the public import path stays stable).Why now
OAuthClientMetadata.redirect_urisis typedlist[AnyUrl], which Pydantic accepts for any well-formed URL — includingjavascript:,data:,file:,vbscript:,ftp:, cleartexthttp://to non-loopback hosts, andhttps://...#. The handler stores those values verbatim and authorize-time validation accepts them by exact match. Issue #<ISSUE_NUM> has the full reproduction.Tests
tests/server/auth/test_routes.py: 13 unit tests covering each accept/reject path of the new helper, mirroring the existingtest_validate_issuer_url_*cases (includes an explicit empty-fragment test).tests/server/auth/test_error_handling.py: 5 end-to-end tests posting against/register— three rejection cases (javascript:, cleartext-http://, fragment) and two acceptance cases (HTTPS, loopback HTTP).Local:
./scripts/testgreen; 1190 passed, 100.00% coverage, strict-no-cover clean.Compatibility
No public-API removal. The change tightens what
/registeraccepts; clients submitting non-spec-compliant URIs will now get400 invalid_redirect_uriwhere they previously got201. Existing stored client records are not re-validated.Related
Checks
uv run --frozen ruff format .uv run --frozen ruff check .uv run --frozen pyright./scripts/test— green, 100% coverage, strict-no-cover clean