Don't broaden initial identity-assertion scope; normalize origin ports

Addresses the cubic review:

- Scope no longer broadens on the initial exchange (P1). The previous union pulled the
  server-advertised scopes (which the base 401 scope-selection step writes onto
  client_metadata.scope) into the request, widening beyond what the caller asked for. The
  initial exchange now sends exactly the configured scope; only a 403 insufficient_scope
  step-up - distinguished by an already-issued token - unions in the challenged scope, so
  SEP-2350 escalation still works.
- _origin normalizes the scheme's default port (P2), so an expected_issuer written with an
  explicit :443/:80 compares equal to the port-less token endpoint and a valid same-origin
  exchange is not rejected.

Tests: initial-exchange-does-not-broaden, step-up-unions (now with a prior token present),
and _origin default-port normalization.

Author: Kludex

src/mcp/client/auth/extensions/identity_assertion.py

@@ -32,10 +32,18 @@
 JWT_BEARER_GRANT_TYPE = "urn:ietf:params:oauth:grant-type:jwt-bearer"
 
 
+_DEFAULT_PORTS = {"https": 443, "http": 80}
+
+
 def _origin(url: str) -> tuple[str, str, int | None]:
-    """Return the (scheme, host, port) origin of a URL for same-origin comparison."""
+    """Return the (scheme, host, port) origin of a URL for same-origin comparison.
+
+    The port is normalized to the scheme's default so an explicit `:443`/`:80` compares equal to the
+    same origin written without a port.
+    """
     parsed = urlsplit(url)
-    return (parsed.scheme, parsed.hostname or "", parsed.port)
+    port = parsed.port if parsed.port is not None else _DEFAULT_PORTS.get(parsed.scheme)
+    return (parsed.scheme, parsed.hostname or "", port)
 
 
 class IdentityAssertionOAuthProvider(OAuthClientProvider):
@@ -196,13 +204,20 @@ async def _exchange_assertion(self) -> httpx.Request:
         if self.context.should_include_resource_param(self.context.protocol_version):
             token_data["resource"] = resource
 
-        # Honour the caller's requested scope, unioned with any scope the base flow selected or a
-        # 403 step-up challenged in (SEP-2350). Write it back to client_metadata.scope so the base
-        # _handle_token_response RFC 6749 §5.1 backfill records the same value on the stored token.
-        scope = union_scopes(self._scopes, self.context.client_metadata.scope)
+        # Scope handling. On the initial exchange send exactly the caller's requested scope: the
+        # base 401 scope-selection step overwrites client_metadata.scope with the server-advertised
+        # set, which must NOT silently broaden the request. On a 403 insufficient_scope step-up the
+        # base flow re-runs this with client_metadata.scope already holding the SEP-2350 union of the
+        # prior and challenged scopes; honour that so the retry actually escalates. The two are told
+        # apart by whether a token already exists (a step-up only happens after one was issued).
+        if self.context.current_tokens is None:
+            scope = self._scopes
+        else:
+            scope = union_scopes(self._scopes, self.context.client_metadata.scope)
+        # Write it back so the base _handle_token_response RFC 6749 §5.1 backfill records the same
+        # value on the stored token rather than the server-advertised set.
         self.context.client_metadata.scope = scope
         if scope:
             token_data["scope"] = scope
 
-        token_url = self._get_token_endpoint()
         return httpx.Request("POST", token_url, data=token_data, headers=headers)

tests/client/auth/extensions/test_identity_assertion.py

@@ -7,6 +7,7 @@
 from mcp.client.auth.extensions.identity_assertion import (
     JWT_BEARER_GRANT_TYPE,
     IdentityAssertionOAuthProvider,
+    _origin,
 )
 from mcp.shared.auth import (
     OAuthClientInformationFull,
@@ -192,12 +193,18 @@ async def test_missing_metadata_raises(mock_storage: MockTokenStorage) -> None:
 
 @pytest.mark.anyio
 async def test_step_up_scope_is_unioned_with_configured_scope(mock_storage: MockTokenStorage) -> None:
-    """A 403 step-up challenge (written to client_metadata.scope) is unioned with the configured scope."""
+    """A 403 step-up challenge (written to client_metadata.scope) is unioned with the configured scope.
+
+    A step-up only happens after a token was issued, so the union branch is keyed on current_tokens
+    being set; the test simulates both the existing token and the base flow's challenge write-back.
+    """
     provider = make_provider(mock_storage)  # configured scopes="mcp"
     await provider._initialize()
     provider.context.oauth_metadata = oauth_metadata()
     provider.context.protocol_version = "2025-06-18"
-    # Simulate the base 403 step-up writing the challenged scope onto client_metadata.scope.
+    # A token exists (the one that drew the 403); the base step-up wrote the challenged union onto
+    # client_metadata.scope.
+    provider.context.current_tokens = OAuthToken(access_token="prior", scope="mcp")
     provider.context.client_metadata.scope = "files:write"
 
     request = await provider._perform_authorization()
@@ -206,6 +213,29 @@ async def test_step_up_scope_is_unioned_with_configured_scope(mock_storage: Mock
     assert "scope=mcp files:write" in content
 
 
+@pytest.mark.anyio
+async def test_initial_exchange_does_not_broaden_to_server_scopes(mock_storage: MockTokenStorage) -> None:
+    """On the initial 401 exchange the request carries only the configured scope, not the server set.
+
+    The base 401 scope-selection step overwrites client_metadata.scope with server-advertised
+    scopes; the provider must not silently broaden the caller's request with them.
+    """
+    provider = make_provider(mock_storage)  # configured scopes="mcp"
+    await provider._initialize()
+    provider.context.oauth_metadata = oauth_metadata()
+    provider.context.protocol_version = "2025-06-18"
+    # No token yet (initial exchange). The base flow advertised a broader set.
+    assert provider.context.current_tokens is None
+    provider.context.client_metadata.scope = "mcp extra admin"
+
+    request = await provider._perform_authorization()
+
+    content = urllib.parse.unquote_plus(request.content.decode())
+    assert "scope=mcp&" in content or content.endswith("scope=mcp")
+    assert "extra" not in content
+    assert "admin" not in content
+
+
 def test_empty_client_secret_is_rejected(mock_storage: MockTokenStorage) -> None:
     """SEP-990 mandates a confidential client, so an empty client_secret is refused at construction."""
 
@@ -277,3 +307,15 @@ async def test_resource_match_accepts_prm_naming_the_expected_as(mock_storage: M
     )
 
     await provider._validate_resource_match(prm)  # does not raise
+
+
+def test_origin_normalizes_default_ports() -> None:
+    """`_origin` treats an explicit scheme-default port as equal to the port-less form.
+
+    The token_endpoint (an AnyHttpUrl) has its default port normalized away, but a caller may
+    configure expected_issuer with the port spelled out; the origin check must not reject that.
+    """
+    assert _origin("https://host") == _origin("https://host:443")
+    assert _origin("http://host") == _origin("http://host:80")
+    assert _origin("https://host") != _origin("https://host:8443")
+    assert _origin("https://host") != _origin("https://other")