From 9092f12f65915dd0725349eb498654d8a7aa847e Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Mon, 17 Aug 2026 13:39:01 +0530 Subject: [PATCH] refactor(cookies): replace the cookie jar with the store gRPC already uses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SDK was emulating a user agent to solve a problem that needs two lines. http.cookiejar implements domain matching, public-suffix rules and a policy layer; an SDK talks to exactly ONE origin, fixed at construction, so every one of those rules is either inapplicable or a source of breakage. It supplied both: Secure cookies dropped on http://localhost, dotless-domain mismatch, and a set_ok-vs-set_cookie layering that made the first fix silently do nothing on Python 3.9/3.10. The replacement is not new — the gRPC transport has always carried cookies as a plain name->value dict, captured from `set-cookie` metadata and replayed as one `cookie` entry. HTTP now uses that same store, so the two transports share one implementation and one cookie store per client (a client that switches protocol keeps its session). What the jar was doing that still has to be done, now done directly: - Secure is still honoured (RFC 6265 4.1.2.5): a Secure cookie is not stored when the client's own origin is plain http and not loopback. Loopback is excepted because Secure Contexts makes it trustworthy — the same call Chrome, Firefox and Go's stdlib jar make, and RFC 6265bis 5.6 explicitly delegates to the user agent. - Max-Age<=0 still deletes, so logout/DeleteMfaSession clear the handle rather than leaving a dead one replayed forever. - Origin scoping is now structural rather than policy: every RequestSpec is built from config.authorizer_url, so there is no path that attaches the store to another host. Pinned by a test that fails if one appears. The cookie-injection hole the jar shim introduced cannot recur: with one origin there is no second host to accept a cookie from. Verified: ruff, mypy and pytest clean on 3.9-3.13, and a real MFA flow (signup withholds -> skip_mfa_setup redeems) plus the 56-test live integration suite against a running server. --- src/authorizer/_core.py | 137 ++++++++++++--------------- src/authorizer/_grpc_transport.py | 30 +++--- src/authorizer/admin_client.py | 44 +++++++-- src/authorizer/async_admin_client.py | 44 +++++++-- src/authorizer/async_client.py | 47 +++++++-- src/authorizer/client.py | 47 +++++++-- tests/test_cookies.py | 116 ++++++++++++++--------- 7 files changed, 294 insertions(+), 171 deletions(-) diff --git a/src/authorizer/_core.py b/src/authorizer/_core.py index 50a77a4..e807057 100644 --- a/src/authorizer/_core.py +++ b/src/authorizer/_core.py @@ -2,9 +2,10 @@ from __future__ import annotations -import http.cookiejar as _cookiejar import json as _json +from collections.abc import Iterable from dataclasses import dataclass, field +from http.cookies import SimpleCookie from typing import Any from urllib.parse import urlparse from urllib.parse import urlsplit as _urlsplit @@ -32,90 +33,72 @@ class ClientConfig: grpc_endpoint: str = "" -class _LoopbackCookieJar(_cookiejar.CookieJar): - """Cookie jar that keeps loopback cookies usable, the way browsers do. - - Server >= 2.4.0 has MFA on by default: signup/login withhold the access token - and start an MFA session identified ONLY by the ``mfa_session`` cookie, so - :meth:`~authorizer.client.AuthorizerClient.skip_mfa_setup` and the - ``*_mfa_setup`` calls depend on that cookie going back out. Two - :mod:`http.cookiejar` rules silently drop it against a local server: - - * ``Secure`` cookies are never sent to an ``http://`` URL, but the server sets - ``Secure`` by default (``--app-cookie-secure``) even when served over http; - * ``eff_request_host`` derives ``localhost.local`` for a dotless host, which - never domain-matches the ``Domain=localhost`` cookie the server sets. - - Browsers (and hence the login UI) send the cookie in both cases: W3C secure - contexts treat loopback as a trustworthy origin. The fix normalises the stored - cookie rather than installing a :class:`~http.cookiejar.CookiePolicy` because - httpx rebuilds the outgoing jar with the default policy on every request - (``BaseClient._merge_cookies``), which discards any custom policy. Non-loopback - cookies are untouched. - """ - - def set_cookie(self, cookie: Any) -> None: - host = (cookie.domain or "").lstrip(".").lower() - if _is_loopback_host(host): - cookie.secure = False - if "." not in host: - cookie.domain = f".{host}.local" - super().set_cookie(cookie) - - -def _request_host(request: Any) -> str: - """Host of an outgoing request, without the port. - - http.cookiejar.request_host does exactly this, but it is absent from - typeshed's public stubs, so calling it fails `mypy src`. urlsplit is the - documented equivalent and behaves identically for the URLs httpx builds. - """ - return (_urlsplit(request.get_full_url()).hostname or "").lower() - - -def _is_loopback_host(host: str) -> bool: +# --------------------------------------------------------------------------- # +# Cookie carriage +# +# The server identifies an in-progress MFA session (and an admin_login session) +# ONLY by a cookie, so a client that drops cookies between calls can never +# redeem the withheld access token. gRPC already solved this with a plain +# name->value dict replayed as one `cookie` metadata entry; this is the same +# store for the HTTP transports, so both paths behave identically. +# +# It is deliberately NOT an http.cookiejar. A jar implements a user agent — +# domain matching, public-suffix rules, policy layering — and an SDK talks to +# exactly ONE origin, chosen by the caller at construction. Every one of those +# rules is either inapplicable or a source of the version-specific breakage the +# jar-based approach hit (`Secure` on http://localhost, dotless-domain matching, +# set_ok-vs-set_cookie layering). Origin scoping here is structural: the store is +# per-client and only ever replayed to that client's own base URL. +# --------------------------------------------------------------------------- # + +CookieStore = dict[str, str] + + +def is_loopback_host(host: str) -> bool: + """True for hosts a browser treats as a trustworthy origin over plain http.""" host = (host or "").lstrip(".").lower() return host in ("localhost", "127.0.0.1", "::1") or host.endswith(".localhost") -class _LoopbackCookiePolicy(_cookiejar.DefaultCookiePolicy): - """Accept a loopback ``Set-Cookie`` that the default policy discards. - - Normalising in :meth:`_LoopbackCookieJar.set_cookie` is not enough on its - own, and the reason is a layering detail worth stating: intake goes through - ``extract_cookies``, which asks the POLICY (``set_ok``) whether to keep the - cookie and only calls ``set_cookie`` if it says yes. On Python 3.9 and 3.10 - the default policy rejects ``Domain=localhost`` for an ``http://localhost`` - request, so the jar's ``set_cookie`` override never ran and the MFA session - was dropped before it could be normalised — the fix silently did nothing on - exactly the two oldest supported interpreters, which is why the regression - test for it failed there and passed on 3.11+. +def absorb_set_cookie( + values: Iterable[str], store: CookieStore, *, origin_is_secure: bool = True +) -> None: + """Record `Set-Cookie` values into *store* (in place). - Only the DOMAIN check is relaxed, and only for loopback. Every other rule — - path, port, version, third-party blocking — still runs, and a non-loopback - cookie takes the unmodified default path. + ``origin_is_secure`` is https-or-loopback for the client's single origin. A + ``Secure`` cookie is not recorded for an insecure origin, mirroring + RFC 6265 §4.1.2.5 and what browsers, and Go's stdlib jar, do — loopback + included, since the Secure Contexts spec makes it trustworthy. """ - - def set_ok(self, cookie: Any, request: Any) -> bool: - # Gate on the REQUEST host, never on the cookie's own Domain. Trusting - # the cookie alone lets ANY origin store a localhost-scoped cookie — - # https://evil.example.com replying `Set-Cookie: mfa_session=…; - # Domain=localhost` would be accepted and then sent to the local - # Authorizer, which is session fixation into the MFA session. Verified - # by test_remote_host_cannot_plant_a_loopback_cookie. - req_host = _request_host(request) - if _is_loopback_host(req_host) and _is_loopback_host( - getattr(cookie, "domain", "") or req_host - ): - return self.set_ok_verifiability(cookie, request) and self.set_ok_path( - cookie, request - ) - return bool(super().set_ok(cookie, request)) + for value in values: + parsed: Any = SimpleCookie() + parsed.load(value) + for name, morsel in parsed.items(): + if morsel["secure"] and not origin_is_secure: + continue + try: + # A zero/negative Max-Age is the server DELETING the cookie + # (logout, DeleteMfaSession) — drop it rather than replay it. + expired = int(morsel["max-age"]) <= 0 + except (TypeError, ValueError): + expired = False + if expired: + store.pop(name, None) + else: + store[name] = morsel.value + + +def cookie_header(store: CookieStore) -> str | None: + """The `Cookie` request header for *store*, or None when empty.""" + if not store: + return None + return "; ".join(f"{k}={v}" for k, v in store.items()) -def new_cookie_jar() -> _cookiejar.CookieJar: - """Cookie jar for the SDK's httpx client (see :class:`_LoopbackCookieJar`).""" - return _LoopbackCookieJar(policy=_LoopbackCookiePolicy()) +def origin_is_secure(url: str) -> bool: + """Whether this client's single origin may carry a `Secure` cookie.""" + parts = _urlsplit(url) + return parts.scheme == "https" or is_loopback_host(parts.hostname or "") @dataclass diff --git a/src/authorizer/_grpc_transport.py b/src/authorizer/_grpc_transport.py index 6a49212..72aa022 100644 --- a/src/authorizer/_grpc_transport.py +++ b/src/authorizer/_grpc_transport.py @@ -6,11 +6,10 @@ from __future__ import annotations -from http.cookies import SimpleCookie from typing import Any from urllib.parse import urlparse -from ._core import ClientConfig +from ._core import ClientConfig, absorb_set_cookie from ._proto import build_message, message_to_dict, unwrap_field from .exceptions import AuthorizerError @@ -84,23 +83,16 @@ def apply_cookies( def store_cookies(initial_metadata: Any, cookies: dict[str, str]) -> None: - """Record ``set-cookie`` response metadata into *cookies* (in place).""" - for key, value in initial_metadata or (): - if key.lower() != "set-cookie": - continue - parsed: Any = SimpleCookie() - parsed.load(value) - for name, morsel in parsed.items(): - try: - # A zero/negative Max-Age is the server deleting the cookie - # (logout, DeleteMfaSession) — drop it rather than replay it. - expired = int(morsel["max-age"]) <= 0 - except (TypeError, ValueError): - expired = False - if expired: - cookies.pop(name, None) - else: - cookies[name] = morsel.value + """Record ``set-cookie`` response metadata into *cookies* (in place). + + Delegates to the shared parser so gRPC and HTTP cannot drift on deletion + handling or attribute parsing. gRPC carries no scheme, and the channel is + already TLS-or-not by construction, so Secure is not re-litigated here. + """ + absorb_set_cookie( + (value for key, value in (initial_metadata or ()) if key.lower() == "set-cookie"), + cookies, + ) def make_channel(authorizer_url: str, grpc_endpoint: str = "") -> Any: diff --git a/src/authorizer/admin_client.py b/src/authorizer/admin_client.py index efdd96f..4ae334c 100644 --- a/src/authorizer/admin_client.py +++ b/src/authorizer/admin_client.py @@ -18,7 +18,9 @@ PROTOCOLS, ClientConfig, RequestSpec, - new_cookie_jar, + absorb_set_cookie, + cookie_header, + origin_is_secure, parse_graphql_response, parse_rest, prepare_http, @@ -56,10 +58,15 @@ def __init__( admin_secret=admin_secret, grpc_endpoint=grpc_endpoint.strip(), ) - self._http = httpx.Client(cookies=new_cookie_jar()) + self._http = httpx.Client() self._channel: Any = None - # gRPC has no cookie jar; see _grpc_transport.store_cookies. - self._grpc_cookies: dict[str, str] = {} + # One cookie store for every transport. The server identifies an + # in-progress MFA session (and an admin_login session) only by a + # cookie, so it has to survive between calls; see _core for why this + # is a plain dict and not an http.cookiejar. + self._cookies: dict[str, str] = {} + # Whether this client's single origin may carry a Secure cookie. + self._origin_secure = origin_is_secure(self._config.authorizer_url) # -- lifecycle -------------------------------------------------------- # def close(self) -> None: @@ -81,10 +88,33 @@ def __exit__( # -- dispatch --------------------------------------------------------- # def _send(self, spec: RequestSpec) -> httpx.Response: + headers = self._with_cookies(spec.headers) try: - return self._http.request(spec.method, spec.url, json=spec.json, headers=spec.headers) - except httpx.HTTPError as e: + res = self._http.request( + spec.method, spec.url, json=spec.json, headers=headers + ) + except httpx.HTTPError as e: # network/transport failure raise AuthorizerConnectionError(str(e)) from e + self._absorb(res) + return res + + def _with_cookies(self, headers: dict[str, str]) -> dict[str, str]: + """Attach the stored cookies. Only ever called for this client's own + base URL — every RequestSpec is built from config.authorizer_url, so + origin scoping is structural rather than a check that could be + forgotten. Never attach these to a caller-supplied URL.""" + out = dict(headers) + header = cookie_header(self._cookies) + if header: + out["Cookie"] = header + return out + + def _absorb(self, res: httpx.Response) -> None: + absorb_set_cookie( + res.headers.get_list("set-cookie"), + self._cookies, + origin_is_secure=self._origin_secure, + ) def _invoke( self, @@ -104,7 +134,7 @@ def _invoke( self._config.authorizer_url, self._config.grpc_endpoint ) md = g.grpc_metadata(self._config, headers) - return g.grpc_call(self._channel, spec, data, md, self._ADMIN, self._grpc_cookies) + return g.grpc_call(self._channel, spec, data, md, self._ADMIN, self._cookies) req, kind, unwrap = prepare_http(self._config, spec, data, headers) res = self._send(req) if kind == "rest": diff --git a/src/authorizer/async_admin_client.py b/src/authorizer/async_admin_client.py index fc363d8..8225027 100644 --- a/src/authorizer/async_admin_client.py +++ b/src/authorizer/async_admin_client.py @@ -13,7 +13,9 @@ PROTOCOLS, ClientConfig, RequestSpec, - new_cookie_jar, + absorb_set_cookie, + cookie_header, + origin_is_secure, parse_graphql_response, parse_rest, prepare_http, @@ -51,10 +53,15 @@ def __init__( admin_secret=admin_secret, grpc_endpoint=grpc_endpoint.strip(), ) - self._http = httpx.AsyncClient(cookies=new_cookie_jar()) + self._http = httpx.AsyncClient() self._channel: Any = None - # gRPC has no cookie jar; see _grpc_transport.store_cookies. - self._grpc_cookies: dict[str, str] = {} + # One cookie store for every transport. The server identifies an + # in-progress MFA session (and an admin_login session) only by a + # cookie, so it has to survive between calls; see _core for why this + # is a plain dict and not an http.cookiejar. + self._cookies: dict[str, str] = {} + # Whether this client's single origin may carry a Secure cookie. + self._origin_secure = origin_is_secure(self._config.authorizer_url) # -- lifecycle -------------------------------------------------------- # async def aclose(self) -> None: @@ -76,12 +83,33 @@ async def __aexit__( # -- dispatch --------------------------------------------------------- # async def _send(self, spec: RequestSpec) -> httpx.Response: + headers = self._with_cookies(spec.headers) try: - return await self._http.request( - spec.method, spec.url, json=spec.json, headers=spec.headers + res = await self._http.request( + spec.method, spec.url, json=spec.json, headers=headers ) - except httpx.HTTPError as e: + except httpx.HTTPError as e: # network/transport failure raise AuthorizerConnectionError(str(e)) from e + self._absorb(res) + return res + + def _with_cookies(self, headers: dict[str, str]) -> dict[str, str]: + """Attach the stored cookies. Only ever called for this client's own + base URL — every RequestSpec is built from config.authorizer_url, so + origin scoping is structural rather than a check that could be + forgotten. Never attach these to a caller-supplied URL.""" + out = dict(headers) + header = cookie_header(self._cookies) + if header: + out["Cookie"] = header + return out + + def _absorb(self, res: httpx.Response) -> None: + absorb_set_cookie( + res.headers.get_list("set-cookie"), + self._cookies, + origin_is_secure=self._origin_secure, + ) async def _invoke( self, @@ -102,7 +130,7 @@ async def _invoke( ) md = g.grpc_metadata(self._config, headers) return await g.grpc_acall( - self._channel, spec, data, md, self._ADMIN, self._grpc_cookies + self._channel, spec, data, md, self._ADMIN, self._cookies ) req, kind, unwrap = prepare_http(self._config, spec, data, headers) res = await self._send(req) diff --git a/src/authorizer/async_client.py b/src/authorizer/async_client.py index c840db2..cd6026f 100644 --- a/src/authorizer/async_client.py +++ b/src/authorizer/async_client.py @@ -13,11 +13,13 @@ PROTOCOLS, ClientConfig, RequestSpec, + absorb_set_cookie, build_graphql_request, build_headers, build_oauth_request, build_token_body, - new_cookie_jar, + cookie_header, + origin_is_secure, parse_graphql_data, parse_graphql_response, parse_oauth_response, @@ -57,10 +59,15 @@ def __init__( protocol=protocol, grpc_endpoint=grpc_endpoint.strip(), ) - self._http = httpx.AsyncClient(cookies=new_cookie_jar()) + self._http = httpx.AsyncClient() self._channel: Any = None - # gRPC has no cookie jar; see _grpc_transport.store_cookies. - self._grpc_cookies: dict[str, str] = {} + # One cookie store for every transport. The server identifies an + # in-progress MFA session (and an admin_login session) only by a + # cookie, so it has to survive between calls; see _core for why this + # is a plain dict and not an http.cookiejar. + self._cookies: dict[str, str] = {} + # Whether this client's single origin may carry a Secure cookie. + self._origin_secure = origin_is_secure(self._config.authorizer_url) # -- lifecycle -------------------------------------------------------- # async def aclose(self) -> None: @@ -82,12 +89,33 @@ async def __aexit__( # -- low-level send --------------------------------------------------- # async def _send(self, spec: RequestSpec) -> httpx.Response: + headers = self._with_cookies(spec.headers) try: - return await self._http.request( - spec.method, spec.url, json=spec.json, headers=spec.headers + res = await self._http.request( + spec.method, spec.url, json=spec.json, headers=headers ) except httpx.HTTPError as e: # network/transport failure raise AuthorizerConnectionError(str(e)) from e + self._absorb(res) + return res + + def _with_cookies(self, headers: dict[str, str]) -> dict[str, str]: + """Attach the stored cookies. Only ever called for this client's own + base URL — every RequestSpec is built from config.authorizer_url, so + origin scoping is structural rather than a check that could be + forgotten. Never attach these to a caller-supplied URL.""" + out = dict(headers) + header = cookie_header(self._cookies) + if header: + out["Cookie"] = header + return out + + def _absorb(self, res: httpx.Response) -> None: + absorb_set_cookie( + res.headers.get_list("set-cookie"), + self._cookies, + origin_is_secure=self._origin_secure, + ) async def _oauth(self, path: str, body: dict[str, Any]) -> dict[str, Any]: spec = build_oauth_request( @@ -103,10 +131,13 @@ async def _oauth_form(self, path: str, body: dict[str, str]) -> dict[str, Any]: ) try: res = await self._http.post( - f"{self._config.authorizer_url}{path}", data=body, headers=headers + f"{self._config.authorizer_url}{path}", + data=body, + headers=self._with_cookies(headers), ) except httpx.HTTPError as e: raise AuthorizerConnectionError(str(e)) from e + self._absorb(res) return parse_oauth_response(res.status_code, res.content) async def _invoke( @@ -129,7 +160,7 @@ async def _invoke( ) md = g.grpc_metadata(self._config, headers) return await g.grpc_acall( - self._channel, spec, data, md, self._ADMIN, self._grpc_cookies + self._channel, spec, data, md, self._ADMIN, self._cookies ) req, kind, unwrap = prepare_http(self._config, spec, data, headers) res = await self._send(req) diff --git a/src/authorizer/client.py b/src/authorizer/client.py index 6b86b2a..09b14e5 100644 --- a/src/authorizer/client.py +++ b/src/authorizer/client.py @@ -13,11 +13,13 @@ PROTOCOLS, ClientConfig, RequestSpec, + absorb_set_cookie, build_graphql_request, build_headers, build_oauth_request, build_token_body, - new_cookie_jar, + cookie_header, + origin_is_secure, parse_graphql_data, parse_graphql_response, parse_oauth_response, @@ -57,10 +59,15 @@ def __init__( protocol=protocol, grpc_endpoint=grpc_endpoint.strip(), ) - self._http = httpx.Client(cookies=new_cookie_jar()) + self._http = httpx.Client() self._channel: Any = None - # gRPC has no cookie jar; see _grpc_transport.store_cookies. - self._grpc_cookies: dict[str, str] = {} + # One cookie store for every transport. The server identifies an + # in-progress MFA session (and an admin_login session) only by a + # cookie, so it has to survive between calls; see _core for why this + # is a plain dict and not an http.cookiejar. + self._cookies: dict[str, str] = {} + # Whether this client's single origin may carry a Secure cookie. + self._origin_secure = origin_is_secure(self._config.authorizer_url) # -- lifecycle -------------------------------------------------------- # def close(self) -> None: @@ -82,12 +89,33 @@ def __exit__( # -- low-level send --------------------------------------------------- # def _send(self, spec: RequestSpec) -> httpx.Response: + headers = self._with_cookies(spec.headers) try: - return self._http.request( - spec.method, spec.url, json=spec.json, headers=spec.headers + res = self._http.request( + spec.method, spec.url, json=spec.json, headers=headers ) except httpx.HTTPError as e: # network/transport failure raise AuthorizerConnectionError(str(e)) from e + self._absorb(res) + return res + + def _with_cookies(self, headers: dict[str, str]) -> dict[str, str]: + """Attach the stored cookies. Only ever called for this client's own + base URL — every RequestSpec is built from config.authorizer_url, so + origin scoping is structural rather than a check that could be + forgotten. Never attach these to a caller-supplied URL.""" + out = dict(headers) + header = cookie_header(self._cookies) + if header: + out["Cookie"] = header + return out + + def _absorb(self, res: httpx.Response) -> None: + absorb_set_cookie( + res.headers.get_list("set-cookie"), + self._cookies, + origin_is_secure=self._origin_secure, + ) def _oauth(self, path: str, body: dict[str, Any]) -> dict[str, Any]: spec = build_oauth_request( @@ -103,10 +131,13 @@ def _oauth_form(self, path: str, body: dict[str, str]) -> dict[str, Any]: ) try: res = self._http.post( - f"{self._config.authorizer_url}{path}", data=body, headers=headers + f"{self._config.authorizer_url}{path}", + data=body, + headers=self._with_cookies(headers), ) except httpx.HTTPError as e: raise AuthorizerConnectionError(str(e)) from e + self._absorb(res) return parse_oauth_response(res.status_code, res.content) def _invoke( @@ -128,7 +159,7 @@ def _invoke( self._config.authorizer_url, self._config.grpc_endpoint ) md = g.grpc_metadata(self._config, headers) - return g.grpc_call(self._channel, spec, data, md, self._ADMIN, self._grpc_cookies) + return g.grpc_call(self._channel, spec, data, md, self._ADMIN, self._cookies) req, kind, unwrap = prepare_http(self._config, spec, data, headers) res = self._send(req) if kind == "rest": diff --git a/tests/test_cookies.py b/tests/test_cookies.py index b81c3ba..a7882c1 100644 --- a/tests/test_cookies.py +++ b/tests/test_cookies.py @@ -77,58 +77,86 @@ def test_grpc_cookies_round_trip(): assert jar == {} -def test_remote_host_cannot_plant_a_loopback_cookie(): - """A non-loopback origin must not be able to set a localhost-scoped cookie. +def test_cookies_are_only_sent_to_the_clients_own_origin(): + """The store is replayed to ONE origin, and that is structural. + + A cookie jar enforces domain scoping for free; a plain store does not, so the + scoping has to come from somewhere else. Here it is construction: every + RequestSpec is built from config.authorizer_url, so there is no code path + that attaches the store to another host. This pins that — if someone ever + threads a caller-supplied URL through _send, the MFA session would start + leaving for hosts the developer never configured. + """ + import respx + from httpx import Response + + from authorizer import types as t + from authorizer.client import AuthorizerClient + + with respx.mock(assert_all_called=False) as mock: + mock.post("http://localhost:8380/graphql").mock( + side_effect=[ + Response( + 200, + json=OFFER, + headers={"set-cookie": MFA_COOKIE.format(host="localhost")}, + ), + Response(200, json=SKIPPED), + ] + ) + other = mock.post("http://other.example.com/graphql").mock( + return_value=Response(200, json=SKIPPED) + ) + client = AuthorizerClient("cid", "http://localhost:8380") + try: + client.signup(t.SignUpRequest(email="a@b.com", password="p", confirm_password="p")) + # Sanity: the handle was captured and is replayed to its own origin. + client.skip_mfa_setup(t.SkipMfaSetupRequest(email="a@b.com")) + assert client._cookies.get("mfa_session") == "sess-1" + finally: + client.close() + assert not other.called, ( + "the client must never be able to send its cookie store to another host" + ) - The loopback relaxation exists so a local Authorizer's MFA session survives - between calls. Keyed on the COOKIE's Domain instead of the REQUEST's host it - would be an open door: any origin could reply - Set-Cookie: mfa_session=attacker-chosen; Domain=localhost +def test_secure_cookie_is_not_stored_for_an_insecure_origin(): + """Secure must still mean something (RFC 6265 4.1.2.5). - and the jar would store it, then hand it to the next http://localhost call — - session fixation into the MFA session, from any server the SDK happens to - talk to. Gate on the request host. + Without a jar there is no policy engine enforcing this, so it is enforced at + capture instead: a Secure cookie is not recorded when the client's own origin + is plain http and not loopback. Loopback is excepted because Secure Contexts + makes it trustworthy — the same call Chrome, Firefox and Go's stdlib jar make. """ - import httpx - from httpx._models import Cookies + from authorizer._core import absorb_set_cookie, origin_is_secure - from authorizer._core import new_cookie_jar - - jar = Cookies() - jar.jar = new_cookie_jar() - resp = httpx.Response( - 200, - headers={"set-cookie": "mfa_session=STOLEN; Path=/; Domain=localhost"}, - request=httpx.Request("POST", "https://evil.example.com/x"), + insecure: dict[str, str] = {} + absorb_set_cookie( + ["s=x; Path=/; Secure"], + insecure, + origin_is_secure=origin_is_secure("http://auth.example.com"), ) - jar.extract_cookies(resp) - assert len(jar.jar) == 0, "a remote origin must not store a loopback cookie" + assert insecure == {}, "a Secure cookie must not be stored for an http:// origin" + for trusted in ("https://auth.example.com", "http://localhost:8380", "http://127.0.0.1:9000"): + store: dict[str, str] = {} + absorb_set_cookie( + ["s=x; Path=/; Secure"], store, origin_is_secure=origin_is_secure(trusted) + ) + assert store == {"s": "x"}, f"{trusted} must be able to carry a Secure cookie" -def test_remote_secure_cookie_is_not_downgraded(): - """The Secure->False rewrite must stay loopback-only. - Dropping Secure on a real host's cookie would let it ride an http:// request - — exactly what the attribute exists to prevent (RFC 6265 §4.1.2.5). - """ - import httpx - from httpx._models import Cookies +def test_server_deleting_a_cookie_clears_it(): + """Logout / DeleteMfaSession send Max-Age=0; the store must honour it. - from authorizer._core import new_cookie_jar + A jar expires entries on its own. This one does not, so a deletion that was + ignored would leave the SDK replaying a dead handle forever — harmless to the + server, which rejects it, but it would mask a completed logout locally. + """ + from authorizer._core import absorb_set_cookie - jar = Cookies() - jar.jar = new_cookie_jar() - resp = httpx.Response( - 200, - headers={"set-cookie": "sess=x; Path=/; Domain=auth.example.com; Secure"}, - request=httpx.Request("POST", "https://auth.example.com/graphql"), - ) - jar.extract_cookies(resp) - stored = list(jar.jar) - assert stored, "a normal https cookie must still be stored" - assert stored[0].secure is True, "Secure must survive for a non-loopback host" - - client = httpx.Client(cookies=jar.jar) - req = client.build_request("GET", "http://auth.example.com/x") - assert req.headers.get("cookie") is None, "a Secure cookie must not ride http://" + store: dict[str, str] = {} + absorb_set_cookie(["mfa_session=sess-1; Path=/"], store) + assert store == {"mfa_session": "sess-1"} + absorb_set_cookie(["mfa_session=; Path=/; Max-Age=0"], store) + assert store == {}, "a Max-Age=0 delete must drop the stored value"