refactor(cookies): replace the cookie jar with the store gRPC already uses

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

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:

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":

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)