Skip to content

Conversation

@williamomeara
Copy link

Problem

The MCP SDK's OAuthClientMetadata.validate_redirect_uri() method performs exact string matching on redirect URIs, which violates RFC 8252 Section 7.3 for loopback addresses.

RFC 8252 Section 7.3 states:

"The authorization server MUST allow any port to be specified at the time of the request for loopback IP redirect URIs, to accommodate clients that obtain an available ephemeral port from the operating system at the time of the request."

Impact

This prevents native OAuth clients (like GitHub Copilot CLI) from successfully completing OAuth flows with MCP servers, as these clients typically use ephemeral ports for their loopback redirect listeners.

Solution

This PR updates the validation logic to be RFC 8252-compliant:

  • For loopback addresses (localhost, 127.0.0.1, ::1): Port is ignored during validation; scheme, hostname, and path must match
  • For non-loopback URIs: Exact matching as before (no behavior change)

Changes

  1. Updated src/mcp/shared/auth.py:

    • Added urllib.parse import for URL parsing
    • Enhanced validate_redirect_uri() with RFC 8252 logic
    • Added comprehensive docstring explaining the behavior
  2. Added tests/shared/test_rfc8252_redirect_uri.py:

    • 13 comprehensive tests covering all validation scenarios
    • Tests for loopback addresses (localhost, 127.0.0.1, ::1)
    • Tests for non-loopback addresses (exact matching preserved)
    • Edge cases and error conditions

Testing

All 13 new tests pass:

tests/shared/test_rfc8252_redirect_uri.py::test_exact_match_non_loopback PASSED
tests/shared/test_rfc8252_redirect_uri.py::test_loopback_localhost_port_ignored PASSED
tests/shared/test_rfc8252_redirect_uri.py::test_loopback_ipv4_port_ignored PASSED
tests/shared/test_rfc8252_redirect_uri.py::test_loopback_ipv6_port_ignored PASSED
tests/shared/test_rfc8252_redirect_uri.py::test_loopback_scheme_must_match PASSED
tests/shared/test_rfc8252_redirect_uri.py::test_loopback_path_must_match PASSED
tests/shared/test_rfc8252_redirect_uri.py::test_loopback_hostname_must_match PASSED
tests/shared/test_rfc8252_redirect_uri.py::test_multiple_redirect_uris_loopback PASSED
tests/shared/test_rfc8252_redirect_uri.py::test_mixed_loopback_and_non_loopback PASSED
tests/shared/test_rfc8252_redirect_uri.py::test_no_redirect_uris_registered PASSED
tests/shared/test_rfc8252_redirect_uri.py::test_single_registered_uri_omit_request PASSED
tests/shared/test_rfc8252_redirect_uri.py::test_multiple_registered_uris_omit_request PASSED
tests/shared/test_rfc8252_redirect_uri.py::test_root_path_normalization PASSED

Backward Compatibility

This change is fully backward compatible:

  • Non-loopback URIs continue to use exact matching
  • Existing clients with static ports will continue to work
  • Only adds support for the RFC-mandated ephemeral port behavior

References

The MCP SDK's OAuthClientMetadata.validate_redirect_uri() was performing
exact string matching, which violates RFC 8252 Section 7.3 for loopback
addresses.

RFC 8252 Section 7.3 states:
'The authorization server MUST allow any port to be specified at the time
of the request for loopback IP redirect URIs, to accommodate clients that
obtain an available ephemeral port from the operating system at the time
of the request.'

Changes:
- Added RFC 8252-compliant validation for loopback addresses
- For localhost/127.0.0.1/::1: port is ignored, scheme/hostname/path must match
- For non-loopback URIs: exact matching as before (no behavior change)
- Added comprehensive test coverage (13 tests)

This fix enables native OAuth clients (like GitHub Copilot CLI) to work
properly with MCP servers that use ephemeral loopback redirect ports.
Copilot AI review requested due to automatic review settings January 22, 2026 23:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request implements RFC 8252 Section 7.3 compliance for OAuth redirect URI validation in the MCP SDK. The change allows native OAuth clients to use ephemeral ports for loopback redirect URIs, which is required by the RFC to accommodate clients that dynamically obtain available ports at runtime.

Changes:

  • Enhanced OAuthClientMetadata.validate_redirect_uri() to ignore port numbers when validating loopback addresses (localhost, 127.0.0.1, ::1)
  • Added comprehensive test suite with 13 tests covering loopback and non-loopback scenarios
  • Maintained backward compatibility: non-loopback URIs continue using exact matching

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/mcp/shared/auth.py Added RFC 8252-compliant loopback redirect URI validation with port-flexible matching for localhost/127.0.0.1/::1
tests/shared/test_rfc8252_redirect_uri.py Added comprehensive test suite covering loopback port flexibility, exact non-loopback matching, and edge cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +114 to +119
is_loopback = parsed_requested.hostname in ("localhost", "127.0.0.1", "::1", "[::1]")

if is_loopback:
for registered in self.redirect_uris:
parsed_registered = urlparse(str(registered))
if parsed_registered.hostname not in ("localhost", "127.0.0.1", "::1", "[::1]"):
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IPv6 loopback check includes "[::1]" with brackets in the hostname check, but Python's urlparse().hostname returns IPv6 addresses without brackets (e.g., "::1" not "[::1]"). This means the bracketed form will never match and IPv6 loopback URIs won't work correctly.

The is_loopback check on line 114 includes "[::1]" which will never be returned by parsed_requested.hostname. Similarly, line 119 checks for "[::1]" which will never match parsed_registered.hostname. Remove "[::1]" from both tuples and keep only "::1".

Suggested change
is_loopback = parsed_requested.hostname in ("localhost", "127.0.0.1", "::1", "[::1]")
if is_loopback:
for registered in self.redirect_uris:
parsed_registered = urlparse(str(registered))
if parsed_registered.hostname not in ("localhost", "127.0.0.1", "::1", "[::1]"):
is_loopback = parsed_requested.hostname in ("localhost", "127.0.0.1", "::1")
if is_loopback:
for registered in self.redirect_uris:
parsed_registered = urlparse(str(registered))
if parsed_registered.hostname not in ("localhost", "127.0.0.1", "::1"):

Copilot uses AI. Check for mistakes.
# Without trailing slash - Pydantic normalizes to include slash
result = client.validate_redirect_uri(AnyUrl("http://localhost:9999"))
# AnyUrl normalizes URLs, so both forms match
assert "localhost:9999" in str(result)
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding test coverage for case-insensitive hostname matching (e.g., "LOCALHOST", "LocalHost") to ensure the implementation correctly handles RFC 3986 case-insensitive hostname semantics. Python's urlparse().hostname normalizes to lowercase, so this should work, but explicit testing would confirm this behavior.

Copilot uses AI. Check for mistakes.
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.

1 participant