diff --git a/docs/servers/auth/oauth-proxy.mdx b/docs/servers/auth/oauth-proxy.mdx index e7fb1909e..f3fe8071a 100644 --- a/docs/servers/auth/oauth-proxy.mdx +++ b/docs/servers/auth/oauth-proxy.mdx @@ -585,6 +585,25 @@ auth = OAuthProxy( Check your server logs for "Client registered with redirect_uri" messages to identify what URLs your clients use. +### Application Type (Web vs. Native) + +During Dynamic Client Registration, a client may declare an `application_type` (per RFC 7591 and SEP-837) that governs which redirect URIs it is allowed to use. The OAuth proxy honors this field both at registration and when authorizing a redirect. + +`application_type` defaults to `"native"` because MCP clients typically run locally and register loopback callbacks. Clients that omit the field keep the permissive behavior described above. A client that explicitly registers as `"web"` is held to the stricter browser-app rules. + +Loopback covers the whole reserved range in both the address and name forms: every address in `127.0.0.0/8`, `::1`, and — per RFC 6761 — the name `localhost` along with any subdomain of it, such as `app.localhost`. The absolute (trailing-dot) spellings `localhost.` and `127.0.0.1.` are treated identically. A name that merely contains `localhost` as a label of a registrable domain, like `localhost.example.com`, is an ordinary public host and is not treated as loopback. + +| `application_type` | Allowed redirect URIs | +| ------------------ | --------------------- | +| `"native"` (default) | `https` URLs; app and private-use schemes (`vscode://callback`, `com.example.app:/callback`, `myapp://callback`, `urn:ietf:wg:oauth:2.0:oob`); and loopback `http` (`http://127.0.0.1`, any address in `127.0.0.0/8`, `http://localhost`, subdomains such as `http://app.localhost`, `http://[::1]`, any port) | +| `"web"` | `https` on a non-loopback host only | + +Web clients must register a non-loopback `https` callback — that is the restriction SEP-837 asks for, and a web client that registers no redirect URI at all is refused, since it could never complete an authorization. Native clients keep the full range of schemes their platforms use; the only new limit is that cleartext `http` must target a loopback host, per RFC 8252 §7.3. + +Both application types always reject unsafe browser schemes (`javascript:`, `data:`, `file:`, `vbscript:`). FastMCP does not otherwise filter a native client's scheme: there is no reliable way to tell an app-dispatch scheme from a network transport, since the IANA registry lists `vscode:` alongside `coap:` and `smb:`, so any such filter would reject callbacks that real MCP clients depend on. + +A redirect URI that violates the declared type is refused during registration with a `RegistrationError` (`invalid_redirect_uri`). For example, a `"web"` client that registers `http://localhost:12345/callback` is rejected, since web clients must use a non-loopback `https` callback. Configure remote, browser-based clients as `application_type="web"` and give them an `https` callback URL. + ## CIMD Support diff --git a/fastmcp_slim/fastmcp/server/auth/oauth_proxy/models.py b/fastmcp_slim/fastmcp/server/auth/oauth_proxy/models.py index 7668d98fd..6d8770df4 100644 --- a/fastmcp_slim/fastmcp/server/auth/oauth_proxy/models.py +++ b/fastmcp_slim/fastmcp/server/auth/oauth_proxy/models.py @@ -14,6 +14,8 @@ from pydantic import AnyUrl, BaseModel, Field, ValidationError from fastmcp.server.auth.cimd import CIMDDocument from fastmcp.server.auth.redirect_validation import ( + is_loopback_host, + is_redirect_uri_allowed_for_application_type, matches_allowed_pattern, validate_redirect_uri, ) @@ -139,10 +141,6 @@ def _redirect_uri_path(uri_path: str) -> str: return uri_path or "/" -def _is_loopback_host(host: str | None) -> bool: - return host is not None and host.lower() in {"localhost", "127.0.0.1", "::1"} - - def _matches_registered_loopback_redirect_uri( redirect_uri: AnyUrl, registered_uri: AnyUrl, @@ -158,7 +156,7 @@ def _matches_registered_loopback_redirect_uri( requested_host = requested.hostname.lower() if requested.hostname else None registered_host = registered.hostname.lower() if registered.hostname else None - if not _is_loopback_host(registered_host): + if not is_loopback_host(registered_host): return False if requested_host != registered_host: return False @@ -218,6 +216,22 @@ class ProxyDCRClient(OAuthClientInformationFull): cimd_fetched_at: float | None = Field(default=None) allow_unregistered_redirect_uris: bool = Field(default=False, exclude=True) + def _enforce_application_type(self, redirect_uri: AnyUrl) -> None: + """Reject a redirect URI that violates the client's application_type. + + SEP-837: the web/native distinction is enforced at registration, but a + stored web client must not later authorize a loopback or custom-scheme + redirect URI (nor a native client an unsafe scheme), so the same rule is + applied here on the authorization path. + """ + if not is_redirect_uri_allowed_for_application_type( + redirect_uri, self.application_type + ): + raise InvalidRedirectUriError( + f"Redirect URI '{redirect_uri}' is not allowed for " + f"application_type '{self.application_type}'." + ) + def validate_redirect_uri(self, redirect_uri: AnyUrl | None) -> AnyUrl: """Validate redirect URI against proxy patterns and optionally CIMD redirect_uris. @@ -251,6 +265,7 @@ class ProxyDCRClient(OAuthClientInformationFull): f"Redirect URI '{resolved}' does not match allowed patterns." ) + self._enforce_application_type(resolved) return resolved raise InvalidRedirectUriError( @@ -263,6 +278,8 @@ class ProxyDCRClient(OAuthClientInformationFull): f"Redirect URI '{redirect_uri}' uses an unsafe scheme." ) + self._enforce_application_type(redirect_uri) + cimd_redirect_uris = ( self.cimd_document.redirect_uris if self.cimd_document else None ) @@ -316,4 +333,5 @@ class ProxyDCRClient(OAuthClientInformationFull): raise InvalidRedirectUriError( f"Redirect URI '{resolved}' does not match allowed patterns." ) + self._enforce_application_type(resolved) return resolved diff --git a/fastmcp_slim/fastmcp/server/auth/oauth_proxy/proxy.py b/fastmcp_slim/fastmcp/server/auth/oauth_proxy/proxy.py index 1037439bc..3faf72baf 100644 --- a/fastmcp_slim/fastmcp/server/auth/oauth_proxy/proxy.py +++ b/fastmcp_slim/fastmcp/server/auth/oauth_proxy/proxy.py @@ -25,6 +25,7 @@ from base64 import urlsafe_b64encode from collections import OrderedDict from collections.abc import AsyncIterator from contextlib import asynccontextmanager +from contextvars import ContextVar from typing import Any, Literal from urllib.parse import urlencode @@ -42,6 +43,7 @@ from key_value.aio.stores.filetree import ( ) from key_value.aio.wrappers.encryption import FernetEncryptionWrapper from mcp.server.auth.handlers.metadata import MetadataHandler +from mcp.server.auth.handlers.register import RegistrationHandler from mcp.server.auth.middleware.client_auth import ClientAuthenticator from mcp.server.auth.provider import ( AccessToken, @@ -58,10 +60,14 @@ from mcp.server.auth.settings import ( ClientRegistrationOptions, RevocationOptions, ) -from mcp.shared.auth import OAuthClientInformationFull, OAuthToken -from pydantic import AnyHttpUrl, AnyUrl, SecretStr +from mcp.shared.auth import ( + OAuthClientInformationFull, + OAuthClientMetadata, + OAuthToken, +) +from pydantic import AnyHttpUrl, AnyUrl, SecretStr, ValidationError from starlette.requests import Request -from starlette.responses import HTMLResponse, RedirectResponse +from starlette.responses import HTMLResponse, RedirectResponse, Response from starlette.routing import Route from typing_extensions import override @@ -105,6 +111,7 @@ from fastmcp.server.auth.oauth_proxy.ui import create_error_html from fastmcp.server.auth.oauth_proxy.upstream import AsyncOAuth2Client from fastmcp.server.auth.redirect_validation import ( build_client_redirect, + is_redirect_uri_allowed_for_application_type, validate_redirect_uri, ) from fastmcp.utilities.auth import parse_scopes @@ -114,6 +121,52 @@ logger = get_logger(__name__) _REFRESH_LOCK_CACHE_SIZE = 10_000 +#: SEP-837: the client's declared `application_type`, recovered from the raw DCR +#: request body by `_ApplicationTypeRegistrationHandler` before the SDK's +#: `RegistrationHandler` runs. The SDK parses `application_type` into +#: `OAuthClientMetadata` but drops it when it builds the +#: `OAuthClientInformationFull` handed to `register_client`, so this ContextVar +#: is the only place the real value survives into the provider. It is `None` when +#: `register_client` is called directly (outside the HTTP route) or when the body +#: failed to parse, in which case the object's own `application_type` is used. +_pending_application_type: ContextVar[Literal["web", "native"] | None] = ContextVar( + "_pending_application_type", default=None +) + + +class _ApplicationTypeRegistrationHandler: + """Recover the DCR `application_type` the SDK handler drops (SEP-837). + + The SDK's `RegistrationHandler` validates the request body into an + `OAuthClientMetadata` (which carries `application_type`) but omits the field + when constructing the `OAuthClientInformationFull` it passes to + `register_client`. This thin wrapper re-parses `application_type` from the + same request body and publishes it on a ContextVar so `register_client` can + enforce the web/native redirect rules, then delegates to the SDK handler + unchanged. Reading `request.body()` here is safe: Starlette caches the body, + so the SDK handler's own read returns the same bytes. + """ + + def __init__(self, handler: RegistrationHandler) -> None: + self._handler = handler + + async def handle(self, request: Request) -> Response: + application_type: Literal["web", "native"] | None = None + try: + metadata = OAuthClientMetadata.model_validate_json(await request.body()) + except ValidationError: + # Let the SDK handler surface the validation error verbatim. + application_type = None + else: + application_type = metadata.application_type + + token = _pending_application_type.set(application_type) + try: + return await self._handler.handle(request) + finally: + _pending_application_type.reset(token) + + #: Marker claim identifying a FastMCP access token minted from a SEP-990 ID-JAG. #: These tokens are self-contained (they carry the asserted subject directly) and #: are validated without the upstream token-swap that regular proxy tokens use. @@ -907,6 +960,16 @@ class OAuthProxy(OAuthProvider, ConsentMixin): # Create a ProxyDCRClient with configured redirect URI validation if client_info.client_id is None: raise ValueError("client_id is required for client registration") + + # SEP-837: the SDK's RegistrationHandler drops application_type when it + # builds this object, so prefer the value the HTTP route recovered from + # the raw request body. Fall back to the object's own field for direct + # (non-HTTP) callers. Write it back so the DCR response echoes the type. + pending_application_type = _pending_application_type.get() + if pending_application_type is not None: + client_info.application_type = pending_application_type + application_type = client_info.application_type + if client_info.redirect_uris: for redirect_uri in client_info.redirect_uris: if not validate_redirect_uri( @@ -917,6 +980,28 @@ class OAuthProxy(OAuthProvider, ConsentMixin): "invalid_redirect_uri", f"Redirect URI '{redirect_uri}' is not allowed.", ) + # SEP-837: honor the client's declared application_type. "web" + # clients are restricted to non-loopback https redirect URIs. + if not is_redirect_uri_allowed_for_application_type( + redirect_uri, + application_type, + ): + raise RegistrationError( + "invalid_redirect_uri", + f"Redirect URI '{redirect_uri}' is not allowed for " + f"application_type '{application_type}'.", + ) + elif application_type == "web": + # Clients may omit redirect_uris and supply one at authorization, + # which falls back to the `http://localhost` placeholder below. A web + # client can never authorize against that placeholder (loopback http + # fails its own rule), so registering one would only produce a client + # that is guaranteed to fail later. Refuse it now, with a reason. + raise RegistrationError( + "invalid_redirect_uri", + "redirect_uris is required for application_type 'web'; web " + "clients must register a non-loopback https redirect URI.", + ) redirect_uris = client_info.redirect_uris or [AnyUrl("http://localhost")] @@ -945,6 +1030,7 @@ class OAuthProxy(OAuthProvider, ConsentMixin): grant_types=registered_grant_types, scope=client_info.scope or self._default_scope_str, token_endpoint_auth_method="none", + application_type=application_type, allowed_redirect_uri_patterns=self._allowed_client_redirect_uris, client_name=getattr(client_info, "client_name", None), ) @@ -2339,6 +2425,35 @@ class OAuthProxy(OAuthProvider, ConsentMixin): methods=["POST", "OPTIONS"], ) ) + elif ( + isinstance(route, Route) + and route.path == "/register" + and route.methods is not None + and "POST" in route.methods + ): + # SEP-837: wrap the SDK RegistrationHandler so the client's + # declared application_type (which the SDK parses but drops before + # calling register_client) survives into the provider and its + # web/native redirect rules are enforced over HTTP. + registration_options = ( + self.client_registration_options or ClientRegistrationOptions() + ) + sdk_registration_handler = RegistrationHandler( + provider=self, + options=registration_options, + ) + registration_handler = _ApplicationTypeRegistrationHandler( + sdk_registration_handler + ) + custom_routes.append( + Route( + path="/register", + endpoint=cors_middleware( + registration_handler.handle, ["POST", "OPTIONS"] + ), + methods=["POST", "OPTIONS"], + ) + ) elif isinstance(route, Route) and route.path.startswith( "/.well-known/oauth-authorization-server" ): diff --git a/fastmcp_slim/fastmcp/server/auth/redirect_validation.py b/fastmcp_slim/fastmcp/server/auth/redirect_validation.py index fb7cba969..a2519d567 100644 --- a/fastmcp_slim/fastmcp/server/auth/redirect_validation.py +++ b/fastmcp_slim/fastmcp/server/auth/redirect_validation.py @@ -5,6 +5,7 @@ protecting against userinfo-based bypass attacks like http://localhost@evil.com. """ import fnmatch +import ipaddress from urllib.parse import unquote, urlencode, urlparse, urlunparse from pydantic import AnyUrl @@ -170,15 +171,58 @@ def _match_host(uri_host: str | None, pattern_host: str | None) -> bool: return uri_host == pattern_host -def _is_loopback_host(host: str | None) -> bool: +def is_loopback_host(host: str | None) -> bool: """Check if a host is a loopback address. - Per RFC 8252 §7.3, loopback addresses include localhost, 127.0.0.1, and ::1. + Per RFC 8252 §7.3, loopback covers the whole reserved loopback range, not + just the two familiar literals: IPv4 `127.0.0.0/8` (so `127.0.0.2` and + `127.5.5.5` are loopback just as much as `127.0.0.1`) and IPv6 `::1`. IP + hosts are therefore classified with `ipaddress.ip_address().is_loopback` + rather than string equality — checking only `127.0.0.1` would let a web + client register `https://127.0.0.2/callback` and slip past the + non-loopback requirement. + + Names are handled per RFC 6761 §6.3, which reserves the entire `localhost` + namespace for the local machine: the exact name `localhost` *and* any + subdomain of it (`app.localhost`, `api.app.localhost`). `.localhost` is a + reserved TLD that cannot be registered, so a subdomain of it always resolves + to the loopback interface and must count as loopback in both directions — + otherwise a web client could register `https://app.localhost/callback` and + slip past the non-loopback requirement, while a native client using + `http://app.localhost:3000/callback` would be wrongly rejected. + + The suffix test is anchored on a leading dot so it cannot be spoofed by a + registrable domain: `localhost.evil.com` and `notlocalhost` are ordinary + public names and are *not* loopback. + + Hosts are also normalized before classification: bracketed IPv6 literals + (`[::1]`) are unwrapped, and a single trailing dot (the absolute/FQDN form, + e.g. `localhost.` or `127.0.0.1.`) is stripped, since it denotes the same + host. Non-IP hosts fall through to the name check without raising. """ if not host: return False + host = host.lower() - return host in ("localhost", "127.0.0.1", "::1") + + # urlparse().hostname strips brackets, but callers that parse the netloc + # themselves may still pass a bracketed IPv6 literal. + if host.startswith("[") and host.endswith("]"): + host = host[1:-1] + + # Absolute (fully qualified) form: `localhost.` and `127.0.0.1.` name the + # same hosts as their relative spellings. + if host.endswith("."): + host = host[:-1] + + if not host: + return False + + try: + return ipaddress.ip_address(host).is_loopback + except ValueError: + # Not an IP literal — RFC 6761 §6.3 reserved localhost namespace. + return host == "localhost" or host.endswith(".localhost") def _match_port( @@ -314,7 +358,7 @@ def matches_allowed_pattern(uri: str, pattern: str) -> bool: return False # RFC 8252 §7.3: loopback patterns without an explicit port match any port - if not (_is_loopback_host(pattern_host) and pattern_port is None): + if not (is_loopback_host(pattern_host) and pattern_port is None): if not _match_port(uri_port, pattern_port, uri_parsed.scheme.lower()): return False @@ -322,6 +366,62 @@ def matches_allowed_pattern(uri: str, pattern: str) -> bool: return _match_path(uri_parsed.path, pattern_parsed.path) +def is_redirect_uri_allowed_for_application_type( + redirect_uri: str | AnyUrl, + application_type: str, +) -> bool: + """Check a redirect URI against RFC 7591 / SEP-837 `application_type` rules. + + `application_type` governs which redirect URIs a Dynamically Registered + Client may use (RFC 7591 §2, OpenID Connect Dynamic Client Registration §2): + + - `"web"` clients must use `https` redirect URIs on a non-loopback host. + Loopback `http`, `https://localhost`, and app/custom schemes are rejected. + This is the restriction SEP-837 actually asks for. + - `"native"` clients keep every scheme FastMCP already allowed, except that + `http` is restricted to loopback hosts (RFC 8252 §7.3, any port). App and + private-use schemes pass through untouched: `vscode://`, + `com.example.app:/callback`, `myapp://callback`, `urn:ietf:wg:oauth:2.0:oob`. + + Deliberately absent: any attempt to classify a native client's scheme as + "private-use" versus "a network transport". There is no sound test. The IANA + registry cannot separate them — `vscode` is registered *because* it is an + app-dispatch scheme, alongside transports like `coap` and `smb` — and + reverse-domain notation fails too, since `iris.beep` and + `microsoft.windows.camera` are registered while `myapp` is not. Every + formulation either rejects schemes real MCP clients depend on or admits the + ones it meant to exclude, so native scheme filtering is left to the + unsafe-scheme check below. + + Unsafe browser schemes (`javascript:`, `data:`, `file:`, `vbscript:`) are + always rejected regardless of `application_type`. That check predates this + function and is unchanged by it. + + The MCP SDK defaults `application_type` to `"native"` because MCP clients + typically register loopback redirect URIs, so omitting the field preserves + the behavior clients relied on before this check existed. + """ + uri_str = str(redirect_uri) + + if _is_unsafe_redirect_uri(uri_str): + return False + + parsed = urlparse(uri_str) + scheme = parsed.scheme.lower() + + if application_type == "web": + # "web": require an https redirect URI on a non-loopback host. + if scheme != "https": + return False + return not is_loopback_host(parsed.hostname) + + # "native" (and the SDK default): cleartext http only to a loopback host; + # every other non-unsafe scheme is left alone. + if scheme == "http": + return is_loopback_host(parsed.hostname) + return True + + def validate_redirect_uri( redirect_uri: str | AnyUrl | None, allowed_patterns: list[str] | None, diff --git a/tests/server/auth/oauth_proxy/test_client_registration.py b/tests/server/auth/oauth_proxy/test_client_registration.py index e4f2837d1..f90211349 100644 --- a/tests/server/auth/oauth_proxy/test_client_registration.py +++ b/tests/server/auth/oauth_proxy/test_client_registration.py @@ -227,6 +227,214 @@ class TestOAuthProxyClientRegistration: assert client_info.get("client_secret") is None +class TestApplicationTypeRegistration: + """SEP-837: DCR registration honors the client's application_type.""" + + async def test_default_application_type_is_native(self, oauth_proxy): + """Omitting application_type defaults to native (the SDK default), so a + loopback redirect URI registers successfully.""" + client_info = OAuthClientInformationFull( + client_id="default-client", + redirect_uris=[AnyUrl("http://localhost:12345/callback")], + ) + + await oauth_proxy.register_client(client_info) + + stored = await oauth_proxy.get_client("default-client") + assert stored is not None + assert stored.application_type == "native" + + async def test_native_loopback_range_registers_then_authorizes_new_port( + self, oauth_proxy + ): + """A native client on 127.0.0.2 keeps loopback port flexibility. + + Registration accepts the whole 127.0.0.0/8 range, and the stored client + must then authorize a different ephemeral port on that same address. + """ + client_info = OAuthClientInformationFull( + client_id="loopback-range-client", + redirect_uris=[AnyUrl("http://127.0.0.2:3000/callback")], + application_type="native", + ) + + await oauth_proxy.register_client(client_info) + + stored = await oauth_proxy.get_client("loopback-range-client") + assert stored is not None + + uri = stored.validate_redirect_uri(AnyUrl("http://127.0.0.2:54321/callback")) + assert str(uri) == "http://127.0.0.2:54321/callback" + + # A different host is still rejected — flexibility is loopback-only. + with pytest.raises(InvalidRedirectUriError): + stored.validate_redirect_uri( + AnyUrl("http://evil.example.com:54321/callback") + ) + + async def test_native_client_accepts_loopback(self, oauth_proxy): + client_info = OAuthClientInformationFull( + client_id="native-client", + redirect_uris=[AnyUrl("http://127.0.0.1:55555/callback")], + application_type="native", + ) + + await oauth_proxy.register_client(client_info) + + stored = await oauth_proxy.get_client("native-client") + assert stored is not None + assert stored.application_type == "native" + + async def test_web_client_accepts_https(self, oauth_proxy): + client_info = OAuthClientInformationFull( + client_id="web-client", + redirect_uris=[AnyUrl("https://client.example.com/callback")], + application_type="web", + ) + + await oauth_proxy.register_client(client_info) + + stored = await oauth_proxy.get_client("web-client") + assert stored is not None + assert stored.application_type == "web" + + async def test_web_client_rejects_loopback(self, oauth_proxy): + client_info = OAuthClientInformationFull( + client_id="web-loopback-client", + redirect_uris=[AnyUrl("http://localhost:12345/callback")], + application_type="web", + ) + + with pytest.raises(RegistrationError, match="application_type 'web'"): + await oauth_proxy.register_client(client_info) + + async def test_web_client_rejects_custom_scheme(self, oauth_proxy): + client_info = OAuthClientInformationFull( + client_id="web-custom-client", + redirect_uris=[AnyUrl("com.example.app:/oauth/callback")], + application_type="web", + ) + + with pytest.raises(RegistrationError, match="application_type 'web'"): + await oauth_proxy.register_client(client_info) + + async def test_web_client_without_redirect_uris_is_rejected(self, oauth_proxy): + """A web client with no redirect_uris could never authorize. + + Omitted redirect_uris fall back to the `http://localhost` placeholder, + which a web client can never use (loopback http fails its own rule), so + registering one would only create a client guaranteed to fail later. + """ + client_info = OAuthClientInformationFull( + client_id="web-no-uris", + redirect_uris=None, + application_type="web", + ) + + with pytest.raises(RegistrationError, match="required for application_type"): + await oauth_proxy.register_client(client_info) + + async def test_native_client_without_redirect_uris_still_allowed(self, oauth_proxy): + """Native clients may still defer redirect_uris to authorization time.""" + client_info = OAuthClientInformationFull( + client_id="native-no-uris", + redirect_uris=None, + application_type="native", + ) + + await oauth_proxy.register_client(client_info) + + stored = await oauth_proxy.get_client("native-no-uris") + assert stored is not None + + @pytest.mark.parametrize("application_type", ["web", "native"]) + async def test_unsafe_scheme_rejected_regardless_of_type( + self, oauth_proxy, application_type + ): + client_info = OAuthClientInformationFull( + client_id="unsafe-client", + redirect_uris=[AnyUrl("javascript:alert(document.cookie)//")], + application_type=application_type, + ) + + with pytest.raises(RegistrationError, match="invalid_redirect_uri"): + await oauth_proxy.register_client(client_info) + + +class TestApplicationTypeRegistrationOverHTTP: + """SEP-837: application_type is honored on the real POST /register route. + + The SDK's RegistrationHandler parses application_type but drops it before + calling register_client, so these tests exercise the actual ASGI route to + prove FastMCP recovers the value end to end (a direct register_client call + would not catch the SDK dropping the field).""" + + async def _register(self, oauth_proxy, payload: dict): + app = Starlette(routes=oauth_proxy.get_routes()) + transport = httpx2.ASGITransport(app=app) + async with httpx2.AsyncClient( + transport=transport, + base_url="https://myserver.com", + ) as client: + return await client.post("/register", json=payload) + + async def test_web_client_with_loopback_rejected_over_http(self, oauth_proxy): + response = await self._register( + oauth_proxy, + { + "redirect_uris": ["http://localhost:12345/callback"], + "application_type": "web", + }, + ) + + assert response.status_code == 400 + body = response.json() + assert body["error"] == "invalid_redirect_uri" + assert "application_type 'web'" in body["error_description"] + + async def test_web_client_with_https_accepted_over_http(self, oauth_proxy): + response = await self._register( + oauth_proxy, + { + "redirect_uris": ["https://client.example.com/callback"], + "application_type": "web", + }, + ) + + assert response.status_code == 201 + body = response.json() + assert body["application_type"] == "web" + + stored = await oauth_proxy.get_client(body["client_id"]) + assert stored is not None + assert stored.application_type == "web" + + async def test_native_client_with_loopback_accepted_over_http(self, oauth_proxy): + response = await self._register( + oauth_proxy, + { + "redirect_uris": ["http://localhost:12345/callback"], + "application_type": "native", + }, + ) + + assert response.status_code == 201 + body = response.json() + assert body["application_type"] == "native" + + async def test_default_application_type_is_native_over_http(self, oauth_proxy): + """Omitting application_type over HTTP defaults to native, so a loopback + redirect is accepted (preserving pre-SEP-837 behavior).""" + response = await self._register( + oauth_proxy, + {"redirect_uris": ["http://localhost:12345/callback"]}, + ) + + assert response.status_code == 201 + body = response.json() + assert body["application_type"] == "native" + + class TestUpstreamClientIdFallback: """Tests for clients that skip DCR and use the upstream client_id directly.""" diff --git a/tests/server/auth/test_oauth_proxy_redirect_validation.py b/tests/server/auth/test_oauth_proxy_redirect_validation.py index 15f2f8d41..72cf68c2a 100644 --- a/tests/server/auth/test_oauth_proxy_redirect_validation.py +++ b/tests/server/auth/test_oauth_proxy_redirect_validation.py @@ -11,6 +11,10 @@ from fastmcp.server.auth.auth import TokenVerifier from fastmcp.server.auth.cimd import CIMDDocument from fastmcp.server.auth.oauth_proxy import OAuthProxy from fastmcp.server.auth.oauth_proxy.models import ProxyDCRClient +from fastmcp.server.auth.redirect_validation import ( + is_loopback_host, + is_redirect_uri_allowed_for_application_type, +) # Standard public IP used for DNS mocking in tests TEST_PUBLIC_IP = "93.184.216.34" @@ -492,3 +496,316 @@ class TestOAuthProxyCIMDClient: # NOT in CIMD but matches proxy pattern → rejected with pytest.raises(InvalidRedirectUriError): client.validate_redirect_uri(AnyUrl("http://localhost:9999/other")) + + +class TestRegisteredLoopbackPortFlexibility: + """The registered-URI port-flexible match uses the shared loopback classifier. + + `models.py` previously carried its own `_is_loopback_host` that only knew + `127.0.0.1`, so a client registered on another address in `127.0.0.0/8` + silently lost port flexibility and was rejected as unregistered. + """ + + @pytest.mark.parametrize( + "host", + ["127.0.0.1", "127.0.0.2", "127.5.5.5", "localhost", "app.localhost"], + ) + def test_loopback_range_keeps_port_flexibility(self, host: str): + client = ProxyDCRClient( + client_id="native", + client_secret="secret", + redirect_uris=[AnyUrl(f"http://{host}:3000/callback")], + ) + + uri = client.validate_redirect_uri(AnyUrl(f"http://{host}:54321/callback")) + assert str(uri) == f"http://{host}:54321/callback" + + def test_non_loopback_host_still_requires_exact_match(self): + """Port flexibility is loopback-only; other hosts must match exactly.""" + client = ProxyDCRClient( + client_id="external", + client_secret="secret", + redirect_uris=[AnyUrl("https://client.example.com:3000/callback")], + ) + + uri = client.validate_redirect_uri( + AnyUrl("https://client.example.com:3000/callback") + ) + assert str(uri) == "https://client.example.com:3000/callback" + + with pytest.raises(InvalidRedirectUriError): + client.validate_redirect_uri( + AnyUrl("https://client.example.com:54321/callback") + ) + + +class TestStoredApplicationTypeAtAuthorization: + """SEP-837: a stored client's application_type is enforced at authorization.""" + + def test_web_client_rejects_loopback_at_authorization(self): + """A registered web client cannot later authorize a loopback redirect.""" + client = ProxyDCRClient( + client_id="web", + client_secret="secret", + redirect_uris=[AnyUrl("https://client.example.com/callback")], + application_type="web", + ) + + uri = client.validate_redirect_uri( + AnyUrl("https://client.example.com/callback") + ) + assert str(uri) == "https://client.example.com/callback" + + with pytest.raises(InvalidRedirectUriError, match="application_type 'web'"): + client.validate_redirect_uri(AnyUrl("http://localhost:8080/callback")) + + def test_web_client_rejects_loopback_even_when_pattern_allows(self): + """The application_type check applies on top of the global allowlist.""" + client = ProxyDCRClient( + client_id="web", + client_secret="secret", + redirect_uris=[AnyUrl("https://client.example.com/callback")], + application_type="web", + allowed_redirect_uri_patterns=["http://localhost:*", "https://*/*"], + ) + + with pytest.raises(InvalidRedirectUriError, match="application_type 'web'"): + client.validate_redirect_uri(AnyUrl("http://localhost:8080/callback")) + + def test_native_client_accepts_loopback_at_authorization(self): + client = ProxyDCRClient( + client_id="native", + client_secret="secret", + redirect_uris=[AnyUrl("http://localhost:8080/callback")], + application_type="native", + ) + + uri = client.validate_redirect_uri(AnyUrl("http://localhost:55555/callback")) + assert str(uri) == "http://localhost:55555/callback" + + def test_web_client_rejects_localhost_namespace_at_authorization(self): + """The shared classifier means the namespace fix reaches this path too.""" + client = ProxyDCRClient( + client_id="web", + client_secret="secret", + redirect_uris=[AnyUrl("https://client.example.com/callback")], + application_type="web", + allowed_redirect_uri_patterns=["https://*/*"], + ) + + with pytest.raises(InvalidRedirectUriError, match="application_type 'web'"): + client.validate_redirect_uri(AnyUrl("https://app.localhost/callback")) + + +class TestApplicationTypeRedirectRules: + """SEP-837: application_type governs the web vs native redirect rules.""" + + @pytest.mark.parametrize( + "uri", + [ + "https://client.example.com/callback", + "https://app.example.com:8443/oauth/callback", + ], + ) + def test_web_accepts_https(self, uri: str): + assert is_redirect_uri_allowed_for_application_type(uri, "web") is True + + @pytest.mark.parametrize( + "uri", + [ + "http://127.0.0.1:8080/callback", + "http://localhost:12345/callback", + "http://[::1]:9000/callback", + "https://localhost/callback", + "com.example.app:/oauth/callback", + "myapp://callback", + "http://client.example.com/callback", + ], + ) + def test_web_rejects_loopback_and_custom_schemes(self, uri: str): + """Web clients must use https on a non-loopback host.""" + assert is_redirect_uri_allowed_for_application_type(uri, "web") is False + + @pytest.mark.parametrize( + "uri", + [ + "http://127.0.0.1:8080/callback", + "http://localhost:12345/callback", + "http://[::1]:9000/callback", + "com.example.app:/oauth/callback", + "cursor://anysphere.cursor-mcp/oauth/callback", + "myapp://callback", + "https://client.example.com/callback", + ], + ) + def test_native_accepts_loopback_and_custom_schemes(self, uri: str): + assert is_redirect_uri_allowed_for_application_type(uri, "native") is True + + @pytest.mark.parametrize( + "uri", + [ + "http://client.example.com/callback", + "http://example.com:8080/callback", + ], + ) + def test_native_rejects_non_loopback_cleartext_http(self, uri: str): + """Native may use cleartext http only against a loopback host.""" + assert is_redirect_uri_allowed_for_application_type(uri, "native") is False + + @pytest.mark.parametrize( + "uri", + [ + # Real MCP client callbacks — these must keep working. + "vscode://callback", + "vscode-insiders://callback", + "urn:ietf:wg:oauth:2.0:oob", + "cursor://anysphere.cursor-mcp/oauth/callback", + # Reverse-domain and plain app schemes. + "com.example.app://callback", + "com.example.app:/oauth/callback", + "myapp://callback", + ], + ) + def test_native_accepts_app_and_private_use_schemes(self, uri: str): + """Native clients keep every scheme outside the unsafe set. + + FastMCP deliberately does not try to classify a native client's scheme + as "private-use" versus "network transport": the IANA registry lists + `vscode` (an app-dispatch scheme) alongside `coap` and `smb`, so no + membership test separates the two without rejecting schemes that real + MCP clients depend on. + """ + assert is_redirect_uri_allowed_for_application_type(uri, "native") is True + + @pytest.mark.parametrize( + "host", + ["127.0.0.1", "127.0.0.2", "127.5.5.5", "127.255.255.254"], + ) + def test_web_rejects_entire_loopback_range(self, host: str): + """RFC 8252 §7.3 loopback is all of 127.0.0.0/8, not just 127.0.0.1. + + Checking only 127.0.0.1 would let a web client bypass the non-loopback + requirement with any other address in the range. + """ + uri = f"https://{host}/callback" + assert is_redirect_uri_allowed_for_application_type(uri, "web") is False + + @pytest.mark.parametrize( + "uri", + [ + "http://127.0.0.1:1234/cb", + "http://127.0.0.2:1234/cb", + "http://127.5.5.5:1234/cb", + "http://[::1]:1234/cb", + "http://localhost:1234/cb", + ], + ) + def test_native_accepts_entire_loopback_range(self, uri: str): + """The widened loopback range cuts both ways: native gains 127.0.0.0/8.""" + assert is_redirect_uri_allowed_for_application_type(uri, "native") is True + + +class TestLocalhostNamespaceIsLoopback: + """RFC 6761 §6.3 reserves the whole `localhost` namespace for the local machine.""" + + @pytest.mark.parametrize( + "host", + [ + "localhost", + "localhost.", # absolute (FQDN) form + "LOCALHOST", + "app.localhost", # reserved namespace + "api.app.localhost", + "App.LocalHost", + "evil.localhost", # .localhost is a reserved TLD — genuinely local + "127.0.0.1", + "127.0.0.1.", # absolute form of an IP literal + "127.0.0.2.", + "::1", + "[::1]", + ], + ) + def test_loopback_names_and_literals(self, host: str): + assert is_loopback_host(host) is True + + @pytest.mark.parametrize( + "host", + [ + # `localhost` as a *label* of a registrable domain is not local. The + # suffix test is anchored on a leading dot so these cannot spoof it. + "localhost.evil.com", + "localhost.evil.com.", + "notlocalhost", + "mylocalhost", + "localhostx", + "evil.com", + "", + ".", + ], + ) + def test_non_loopback_names_are_not_spoofable(self, host: str): + assert is_loopback_host(host) is False + + @pytest.mark.parametrize( + "uri", + [ + "https://app.localhost/cb", + "https://localhost./cb", + "https://api.app.localhost/cb", + "https://127.0.0.1./cb", + ], + ) + def test_web_rejects_localhost_namespace(self, uri: str): + """Web clients must not reach the local machine by name.""" + assert is_redirect_uri_allowed_for_application_type(uri, "web") is False + + @pytest.mark.parametrize( + "uri", + [ + "https://localhost.evil.com/cb", + "https://notlocalhost/cb", + ], + ) + def test_web_still_accepts_ordinary_public_https(self, uri: str): + """Names that merely contain 'localhost' remain ordinary public hosts.""" + assert is_redirect_uri_allowed_for_application_type(uri, "web") is True + + @pytest.mark.parametrize( + "uri", + [ + "http://app.localhost:3000/cb", + "http://localhost.:3000/cb", + "http://api.app.localhost:3000/cb", + "http://127.0.0.1.:3000/cb", + ], + ) + def test_native_accepts_localhost_namespace(self, uri: str): + """These are legitimate loopback dev callbacks and must not be rejected.""" + assert is_redirect_uri_allowed_for_application_type(uri, "native") is True + + @pytest.mark.parametrize( + "uri", + [ + "http://localhost.evil.com:3000/cb", + "http://notlocalhost:3000/cb", + ], + ) + def test_native_rejects_plain_http_to_non_loopback_lookalikes(self, uri: str): + assert is_redirect_uri_allowed_for_application_type(uri, "native") is False + + @pytest.mark.parametrize("application_type", ["web", "native"]) + @pytest.mark.parametrize( + "uri", + [ + "javascript:alert(document.cookie)//", + "data:text/html,", + "file:///etc/passwd", + "vbscript:msgbox(1)", + ], + ) + def test_unsafe_schemes_rejected_for_all_types( + self, uri: str, application_type: str + ): + assert ( + is_redirect_uri_allowed_for_application_type(uri, application_type) is False + )