fix(oidc): address review findings — PKCE, iat, TLS, sub, cookies, sessions

Addresses RaresKeY's review (5 findings) and follow-up Basic RP
validation comment on PR #3508:

- Add PKCE (RFC 7636, S256): code_challenge in the authorization
  request, verifier carried in the Fernet-encrypted state, and
  code_verifier sent to the token endpoint.
- Require the iat claim in id_tokens (OIDC Core §2); tokens without
  iat are now rejected.
- Prefer client_secret_basic at the token endpoint per discovery
  (OIDC default), falling back to client_secret_post only when the
  provider excludes basic.
- Require HTTPS for the issuer and authorization endpoint, not just
  the back-channel endpoints.
- Preserve OIDC subs exactly (no strip) so distinct whitespace-bearing
  subjects can never collapse into one local account; same for the
  UserInfo sub-binding comparison.
- Sync admin state only on a well-formed groups claim; UserInfo
  availability alone (or a malformed groups value) no longer demotes
  an existing admin.
- OIDC session/CSRF cookies are Secure by default regardless of
  SECURE_COOKIES; explicit OIDC_ALLOW_INSECURE_COOKIES=true is the
  only (documented, dev-only) opt-out.
- Make sessions issued by one uvicorn worker validate on others via
  an mtime-gated read-through reload of sessions.json.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GRiLb12nnLnBnYsg14oSWd

.env.example

@@ -130,6 +130,11 @@ SEARXNG_INSTANCE=http://localhost:8080
 # TRUST_PROXY_HEADERS=true only when the app is behind a trusted proxy that
 # strips/replaces inbound X-Forwarded-Proto; otherwise client-supplied
 # forwarded headers are ignored for cookie security decisions.
+#
+# OIDC session and CSRF cookies always carry the Secure flag, regardless
+# of SECURE_COOKIES — SSO implies a TLS deployment.  The only opt-out is
+# the development-only override below; never enable it in production.
+# OIDC_ALLOW_INSECURE_COOKIES=false
 
 # ============================================================
 # ChromaDB (vector store)

core/auth.py

@@ -136,6 +136,10 @@ class AuthManager:
         # all uvicorn workers so first-admin bootstrap and auth.json mutations
         # are serialised across processes, not just threads within one worker.
         self._ipc_lock_path = auth_path + ".lock"
+        # mtime of sessions.json at last load — lets validate_token cheaply
+        # detect sessions written by other uvicorn workers (see
+        # _reload_sessions_if_changed).
+        self._sessions_mtime_ns = -1
         self._load()
         self._load_sessions()
         self._migrate_single_user()
@@ -168,6 +172,7 @@ class AuthManager:
         """Load persisted session tokens from disk, pruning expired ones."""
         try:
             if os.path.exists(self._sessions_path):
+                self._sessions_mtime_ns = os.stat(self._sessions_path).st_mtime_ns
                 with open(self._sessions_path, "r", encoding="utf-8") as f:
                     data = json.load(f)
                 now = time.time()
@@ -180,6 +185,46 @@ class AuthManager:
             logger.error(f"Failed to load sessions: {e}")
             self._sessions = {}
 
+    def _reload_sessions_if_changed(self):
+        """Merge sessions written by other uvicorn workers.
+
+        The OIDC callback (or a password login) may run on one worker while
+        the browser's next request lands on another; each worker loads
+        sessions.json only at startup, so the new token would be rejected.
+        Called on a token miss: when the file's mtime has changed since the
+        last load, re-read it and add unknown unexpired tokens to the
+        in-memory map.  The mtime gate keeps unknown-token spam at one
+        os.stat per request, not a JSON parse.
+
+        Additive only — tokens missing from disk are NOT dropped from
+        memory, so a token issued moments ago on this worker can't be lost
+        to a reload racing its own _save_sessions.
+        """
+        try:
+            stat = os.stat(self._sessions_path)
+        except OSError:
+            return
+        with self._sessions_lock:
+            if stat.st_mtime_ns == self._sessions_mtime_ns:
+                return
+            try:
+                with open(self._sessions_path, "r", encoding="utf-8") as f:
+                    data = json.load(f)
+            except Exception as e:
+                logger.error(f"Failed to reload sessions: {e}")
+                return
+            self._sessions_mtime_ns = stat.st_mtime_ns
+            if not isinstance(data, dict):
+                return
+            now = time.time()
+            for tok, sess in data.items():
+                if (
+                    tok not in self._sessions
+                    and isinstance(sess, dict)
+                    and sess.get("expiry", 0) > now
+                ):
+                    self._sessions[tok] = sess
+
     def _save_sessions(self):
         """Persist session tokens to disk (atomic, lock-guarded)."""
         try:
@@ -878,6 +923,11 @@ class AuthManager:
     def validate_token(self, token: Optional[str]) -> bool:
         if not token:
             return False
+        with self._sessions_lock:
+            known = token in self._sessions
+        if not known:
+            # May have been issued by another worker — read through to disk.
+            self._reload_sessions_if_changed()
         expired = False
         deleted_user = False
         with self._sessions_lock:
@@ -904,6 +954,11 @@ class AuthManager:
         """Return the username associated with a valid token."""
         if not token:
             return None
+        with self._sessions_lock:
+            known = token in self._sessions
+        if not known:
+            # May have been issued by another worker — read through to disk.
+            self._reload_sessions_if_changed()
         expired = False
         deleted_user = False
         with self._sessions_lock:

core/oidc.py

@@ -26,6 +26,8 @@ JWKS keys are cached after first fetch and refreshed only when an unknown
 60-second cooldown throttles both successful and failed refreshes.
 """
 
+import base64
+import hashlib
 import json
 import logging
 import math
@@ -74,12 +76,13 @@ def _get_state_fernet():
         return _state_fernet
 
 
-def _encode_state(nonce: str, redirect_uri: str) -> str:
+def _encode_state(nonce: str, redirect_uri: str, code_verifier: str) -> str:
     """Return a Fernet-encrypted state token containing nonce + metadata."""
     fernet = _get_state_fernet()
     payload = json.dumps({
         "nonce": nonce,
         "redirect_uri": redirect_uri,
+        "code_verifier": code_verifier,
         "created": time.time(),
     })
     return fernet.encrypt(payload.encode()).decode()
@@ -97,11 +100,14 @@ def _decode_state(state: str) -> Optional[Dict[str, Any]]:
         return None
     nonce = data.get("nonce")
     redirect_uri = data.get("redirect_uri")
+    code_verifier = data.get("code_verifier")
     created = data.get("created")
     if not isinstance(nonce, str) or not nonce:
         return None
     if not isinstance(redirect_uri, str):
         return None
+    if not isinstance(code_verifier, str) or not code_verifier:
+        return None
     if not _is_numericdate(created):
         return None
     now = time.time()
@@ -164,8 +170,19 @@ class OidcManager:
         self._jwks_cache: Dict[str, Dict[str, Any]] = {}
         self._jwks_cache_lock = threading.Lock()
         self._allowed_algs: Optional[List[str]] = None
+        self._token_auth_methods: List[str] = ["client_secret_basic"]
         self._discover()
 
+    def _use_basic_auth(self) -> bool:
+        """True when the token endpoint should use client_secret_basic."""
+        if "client_secret_basic" in self._token_auth_methods:
+            return True
+        if "client_secret_post" in self._token_auth_methods:
+            return False
+        # Provider advertises neither shared-secret method — use the OIDC
+        # default rather than silently leaking the secret in the body.
+        return True
+
     # -- discovery -----------------------------------------------------------
 
     def _discover(self) -> None:
@@ -176,6 +193,15 @@ class OidcManager:
         well_known_url = self.issuer + "/.well-known/openid-configuration"
         if not well_known_url.startswith(("http://", "https://")):
             well_known_url = f"https://{well_known_url}"
+        # The issuer (and therefore the discovery document) must be HTTPS:
+        # the authorization redirect carries state/nonce, and an http://
+        # issuer lets an active network attacker substitute authorization
+        # codes or rewrite the discovery document entirely.
+        if not well_known_url.startswith("https://"):
+            raise OidcError(
+                f"OIDC issuer must use HTTPS, got {self.issuer!r}. "
+                "Configure the IdP with TLS or set OIDC_ENABLED=false."
+            )
 
         try:
             resp = httpx.get(well_known_url, timeout=15.0)
@@ -204,10 +230,12 @@ class OidcManager:
                 f"discovery doc returned {doc_issuer!r}"
             )
 
-        # Client credentials and bearer tokens must never be sent over
-        # cleartext transport. Authorization is browser-facing and is not
-        # included because it does not carry those secrets.
-        for name in ("token_endpoint", "jwks_uri", "userinfo_endpoint"):
+        # No OIDC endpoint may use cleartext transport.  The back-channel
+        # endpoints carry client credentials and bearer tokens; the browser-
+        # facing authorization endpoint carries state/nonce and returns the
+        # authorization code, so an http:// endpoint enables code
+        # substitution by an active network observer.
+        for name in ("authorization_endpoint", "token_endpoint", "jwks_uri", "userinfo_endpoint"):
             url = self._config.get(name)
             if url and not isinstance(url, str):
                 raise OidcError(f"OIDC {name} must be a URL string")
@@ -224,6 +252,13 @@ class OidcManager:
         safe = [a for a in supported if a in ("RS256", "RS384", "RS512", "ES256", "ES384", "ES512", "PS256", "PS384", "PS512")]
         self._allowed_algs = safe or ["RS256"]
 
+        # Token-endpoint auth methods.  Per OIDC Discovery §3, an omitted
+        # token_endpoint_auth_methods_supported means client_secret_basic.
+        methods = self._config.get("token_endpoint_auth_methods_supported")
+        if not isinstance(methods, list) or not methods:
+            methods = ["client_secret_basic"]
+        self._token_auth_methods = methods
+
         logger.info(
             "OIDC provider discovered: issuer=%r auth=%r token=%r algs=%s",
             self.issuer,
@@ -264,9 +299,19 @@ class OidcManager:
         """
         nonce = secrets.token_hex(32)
 
+        # PKCE (RFC 7636, S256).  The verifier travels inside the encrypted
+        # state token, so the callback can recover it without server-side
+        # storage — same carrier as the nonce.
+        code_verifier = secrets.token_urlsafe(64)
+        code_challenge = (
+            base64.urlsafe_b64encode(hashlib.sha256(code_verifier.encode("ascii")).digest())
+            .rstrip(b"=")
+            .decode("ascii")
+        )
+
         # Encode the nonce + metadata into the state parameter (Fernet-
         # encrypted, stateless — works across multiple workers/processes).
-        state = _encode_state(nonce, redirect_uri)
+        state = _encode_state(nonce, redirect_uri, code_verifier)
 
         from urllib.parse import urlencode
         params = {
@@ -276,6 +321,8 @@ class OidcManager:
             "scope": self.scopes,
             "state": state,
             "nonce": nonce,
+            "code_challenge": code_challenge,
+            "code_challenge_method": "S256",
         }
         # Request forced re-authentication when OIDC_MAX_AGE is configured.
         # The claim is later verified in _verify_id_token against auth_time.
@@ -300,6 +347,7 @@ class OidcManager:
             raise OidcError("OIDC state not found — may be expired, reused, or from a different worker")
         nonce = stored.get("nonce", "")
         stored_redirect_uri = stored.get("redirect_uri", "")
+        code_verifier = stored.get("code_verifier", "")
 
         # Bind the token exchange to the redirect_uri that was used in the
         # authorization request (carried in the signed state token).  Reject
@@ -312,7 +360,9 @@ class OidcManager:
             )
 
         # 2. Exchange code for tokens (using the stored redirect_uri)
-        token_data = self._token_request(code, stored_redirect_uri or redirect_uri)
+        token_data = self._token_request(
+            code, stored_redirect_uri or redirect_uri, code_verifier
+        )
 
         # 3. Verify id_token
         id_token = token_data.get("id_token")
@@ -349,9 +399,13 @@ class OidcManager:
                     )
                     userinfo = {}
                 else:
-                    # Require a non-empty sub that matches the verified
-                    # id_token subject before trusting any UserInfo claims.
-                    ui_sub = (userinfo.get("sub") or "").strip()
+                    # Require a non-empty sub that exactly matches the
+                    # verified id_token subject before trusting any UserInfo
+                    # claims.  No normalization: subs are opaque identifiers
+                    # and trimming could equate two distinct subjects.
+                    ui_sub = userinfo.get("sub")
+                    if not isinstance(ui_sub, str):
+                        ui_sub = ""
                     if not ui_sub:
                         logger.warning(
                             "UserInfo response missing sub claim — "
@@ -388,18 +442,34 @@ class OidcManager:
         claims["_userinfo_available"] = userinfo_available
         return claims
 
-    def _token_request(self, code: str, redirect_uri: str) -> Dict[str, Any]:
+    def _token_request(
+        self, code: str, redirect_uri: str, code_verifier: str
+    ) -> Dict[str, Any]:
         """POST the token endpoint to exchange code for tokens."""
         token_endpoint = self._config["token_endpoint"]
         payload = {
             "grant_type": "authorization_code",
             "code": code,
             "redirect_uri": redirect_uri,
-            "client_id": self.client_id,
-            "client_secret": self.client_secret,
+            "code_verifier": code_verifier,
         }
+        # client_secret_basic is the OIDC default and the method the
+        # conformance suite expects; use it whenever the provider supports
+        # it (or doesn't advertise methods at all, which per Discovery §3
+        # means client_secret_basic).  Fall back to client_secret_post only
+        # when the provider explicitly excludes basic.
+        auth = None
+        if self._use_basic_auth():
+            from urllib.parse import quote
+            auth = (
+                quote(self.client_id, safe=""),
+                quote(self.client_secret, safe=""),
+            )
+        else:
+            payload["client_id"] = self.client_id
+            payload["client_secret"] = self.client_secret
         try:
-            resp = httpx.post(token_endpoint, data=payload, timeout=15.0)
+            resp = httpx.post(token_endpoint, data=payload, auth=auth, timeout=15.0)
             resp.raise_for_status()
             data = resp.json()
         except httpx.HTTPStatusError as exc:
@@ -583,15 +653,17 @@ class OidcManager:
                     f"{self.max_age} s (now={now:.0f}, age={now - auth_time:.0f} s)"
                 )
 
-        # Verify iat (issued-at) is not in the far future.
+        # Verify iat (issued-at): required by OIDC Core §2, must be numeric
+        # and not in the far future.
         iat = claims.get("iat")
-        if iat is not None:
-            if not _is_numericdate(iat):
-                raise OidcError(f"id_token iat claim is non-numeric: {iat!r}")
-            if iat > time.time() + 60:
-                raise OidcError(
-                    f"id_token iat {iat} is more than 60 s in the future"
-                )
+        if iat is None:
+            raise OidcError("id_token missing iat claim")
+        if not _is_numericdate(iat):
+            raise OidcError(f"id_token iat claim is non-numeric: {iat!r}")
+        if iat > time.time() + 60:
+            raise OidcError(
+                f"id_token iat {iat} is more than 60 s in the future"
+            )
 
         return claims
 

routes/oidc_routes.py

@@ -104,7 +104,7 @@ def setup_oidc_routes(
             value=_state,
             httponly=True,
             samesite="lax",
-            secure=_is_secure_context(request),
+            secure=_oidc_cookie_secure(),
             path="/api/auth/oidc/callback",
             max_age=OIDC_CSRF_MAX_AGE,
         )
@@ -164,14 +164,20 @@ def setup_oidc_routes(
         name = claims.get("name", "")
         groups_claim_present = "groups" in claims
         groups = claims.get("groups", [])
-        id_token_groups_valid = groups_claim_present and isinstance(groups, list)
+        # Authoritative group evidence = a well-formed (list) groups claim
+        # from the verified id_token or the sub-bound UserInfo response.
+        # A missing or malformed claim is not evidence of membership loss.
+        groups_evidence_valid = groups_claim_present and isinstance(groups, list)
 
         # Validate claim types before use — malformed IdP claims must not
         # cause 500s or unsafe persistence operations.
-        if not isinstance(sub, str) or not sub.strip():
+        # The sub is an opaque identifier (OIDC Core §8): it is validated
+        # for type and bounds but never normalized — trimming whitespace
+        # could collapse two distinct verified subjects into one local
+        # account and hand one subject the other's session and privileges.
+        if not isinstance(sub, str) or not sub:
             logger.error("OIDC id_token sub is not a non-empty string: %r", sub)
             return _build_oidc_error_redirect("oidc_failed")
-        sub = sub.strip()
         if len(sub) > 512:
             logger.error("OIDC id_token sub exceeds maximum length")
             return _build_oidc_error_redirect("oidc_failed")
@@ -201,14 +207,7 @@ def setup_oidc_routes(
                         clean_groups.append(normalized_group)
             groups = clean_groups
 
-        # Was UserInfo successfully fetched?  When UserInfo is unavailable
-        # and the id_token does not carry a groups claim, we do not have
-        # authoritative group-membership evidence — existing admins must
-        # not be demoted based on missing data.
         userinfo_available = claims.pop("_userinfo_available", False)
-        # A malformed non-list groups claim is not authoritative evidence and
-        # must not demote an existing administrator.
-        id_token_has_groups = id_token_groups_valid
 
         # Determine admin status from IdP group membership.
         # OIDC_ADMIN_GROUPS is a comma-separated list; the user gets
@@ -244,19 +243,20 @@ def setup_oidc_routes(
             # Otherwise the bootstrap (or manual grant) would be undone
             # on the next login.
             if admin_groups:
-                # Only sync admin status when we have authoritative group
-                # evidence.  UserInfo is the primary source for groups;
-                # if it's unavailable and the id_token didn't carry a
-                # groups claim, skip the sync — a transient provider
-                # failure must not silently demote an existing admin.
-                if userinfo_available or id_token_has_groups:
+                # Only sync admin status when a trusted source (verified
+                # id_token or sub-bound UserInfo) supplied a well-formed
+                # groups claim.  A missing or malformed claim — e.g. a
+                # transient provider failure or a scope/configuration
+                # change — must not silently demote an existing admin.
+                if groups_evidence_valid:
                     logger.info("OIDC login for existing user '%s' (admin=%s)", username, is_admin)
                     auth_manager.set_oidc_user_admin(username, is_admin)
                 else:
                     logger.info(
                         "OIDC login for existing user '%s' — skipping admin sync "
-                        "(UserInfo unavailable and id_token has no groups claim)",
+                        "(no valid groups claim; userinfo_available=%s)",
                         username,
+                        userinfo_available,
                     )
             else:
                 logger.info("OIDC login for existing user '%s'", username)
@@ -285,17 +285,12 @@ def setup_oidc_routes(
             logger.error("Failed to create OIDC session for '%s'", username)
             return _build_oidc_error_redirect("oidc_failed")
 
-        # Default secure=true for OIDC flows (SSO implies a real deployment).
-        # Fall back to SECURE_COOKIES env var if explicitly set, then request
-        # scheme detection.
-        secure_val = _is_secure_context(request)
-
         cookie_kwargs = dict(
             key=SESSION_COOKIE,
             value=token,
             httponly=True,
             samesite="lax",
-            secure=secure_val,
+            secure=_oidc_cookie_secure(),
             path="/",
             max_age=60 * 60 * 24 * 7,  # 7 days
         )
@@ -309,22 +304,25 @@ def setup_oidc_routes(
     return router
 
 
-def _is_secure_context(request: Request) -> bool:
-    """Determine whether the session cookie should have the Secure flag.
+def _oidc_cookie_secure() -> bool:
+    """Determine whether OIDC cookies get the Secure flag.
 
-    Uses the SECURE_COOKIES env var if explicitly true/false; otherwise
-    derives from the request scheme (https → secure, http → not).
+    OIDC cookies are Secure by default: SSO implies a real deployment
+    behind TLS, and deriving the flag from SECURE_COOKIES (which the
+    bundled Compose files default to false) or the request scheme (which
+    is http behind a TLS-terminating proxy) would silently issue bearer
+    session cookies eligible for cleartext transmission.
+
+    The only opt-out is the explicit development override
+    OIDC_ALLOW_INSECURE_COOKIES=true, for plain-HTTP local testing.
+    Nothing else — not SECURE_COOKIES, not the request scheme — can
+    downgrade OIDC cookies.
     """
-    explicit = os.getenv("SECURE_COOKIES", "").strip().lower()
-    if explicit in ("true", "1", "yes"):
-        return True
-    if explicit in ("false", "0", "no"):
+    if os.getenv("OIDC_ALLOW_INSECURE_COOKIES", "").strip().lower() in ("true", "1", "yes"):
+        logger.warning(
+            "OIDC_ALLOW_INSECURE_COOKIES=true — OIDC session and CSRF "
+            "cookies are issued without the Secure flag. Never use this "
+            "outside plain-HTTP local development."
+        )
         return False
-    # Not explicitly set — derive from the request. Forwarded headers are
-    # trusted only when the deployment explicitly opts into proxy headers;
-    # otherwise a client cannot influence the cookie policy via a header.
-    forwarded = ""
-    if os.getenv("TRUST_PROXY_HEADERS", "").strip().lower() in ("true", "1", "yes"):
-        forwarded = request.headers.get("x-forwarded-proto", "")
-    scheme = forwarded or request.url.scheme or "http"
-    return scheme == "https"
+    return True

tests/test_oidc_manager.py

@@ -268,6 +268,10 @@ class TestAuthorizationUrl:
         parsed = parse_qs(urlparse(url).query)
         assert parsed.get("state") == [state]
         assert f"nonce={nonce}" in url
+        # PKCE (RFC 7636) — S256 challenge must always be sent
+        assert parsed.get("code_challenge_method") == ["S256"]
+        challenge = parsed.get("code_challenge", [""])[0]
+        assert len(challenge) == 43  # unpadded base64url SHA-256
         # State is now a Fernet-encrypted token (base64, variable length)
         assert len(state) > 60  # Fernet tokens are always >60 chars
         assert len(nonce) == 64  # nonce is still 32 hex bytes
@@ -293,6 +297,33 @@ class TestAuthorizationUrl:
         assert decoded is not None
         assert decoded["nonce"] == nonce
         assert decoded["redirect_uri"] == "https://app.example.com/callback"
+        # The PKCE verifier rides in the encrypted state and must S256-hash
+        # to the code_challenge sent in the authorization URL.
+        import base64
+        import hashlib
+        from urllib.parse import urlparse, parse_qs
+        challenge = parse_qs(urlparse(url).query)["code_challenge"][0]
+        verifier = decoded["code_verifier"]
+        assert 43 <= len(verifier) <= 128  # RFC 7636 §4.1 bounds
+        expected = (
+            base64.urlsafe_b64encode(hashlib.sha256(verifier.encode()).digest())
+            .rstrip(b"=")
+            .decode()
+        )
+        assert challenge == expected
+
+    def test_state_without_code_verifier_rejected(self):
+        """Legacy/forged state payloads lacking a PKCE verifier are invalid."""
+        import core.oidc as mod
+
+        fernet = mod._get_state_fernet()
+        payload = json.dumps({
+            "nonce": "n" * 64,
+            "redirect_uri": "https://app.example.com/callback",
+            "created": time.time(),
+        })
+        state = fernet.encrypt(payload.encode()).decode()
+        assert mod._decode_state(state) is None
 
 
 class TestExchangeCode:
@@ -324,7 +355,7 @@ class TestExchangeCode:
             url, state, gen_nonce = mgr.get_authorization_url("https://app.example.com/callback")
 
             # Override: build our own state with the nonce that matches the id_token
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
 
             # Token exchange — mock first the JWKS fetch, then the token POST
             mock_post.return_value = _mock_token_response(id_token)
@@ -376,7 +407,7 @@ class TestExchangeCode:
             )
 
         # Build an already-expired state token
-        token = mod._encode_state("nonce", "https://app.example.com/callback")
+        token = mod._encode_state("nonce", "https://app.example.com/callback", "test-code-verifier")
         # Decode to verify it's valid, then re-encode with old timestamp
         fernet = mod._get_state_fernet()
         expired_data = json.dumps({
@@ -405,7 +436,7 @@ class TestExchangeCode:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state("nonce", "https://app.example.com/callback")
+            state = mod._encode_state("nonce", "https://app.example.com/callback", "test-code-verifier")
 
             # Token response without id_token
             mock_post.return_value = _FakeResponse(200, {"access_token": "fake"})
@@ -429,7 +460,7 @@ class TestExchangeCode:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state("nonce", "https://app.example.com/callback")
+            state = mod._encode_state("nonce", "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _FakeResponse(400, {"error": "invalid_grant"})
 
             with pytest.raises(mod.OidcError):
@@ -453,7 +484,7 @@ class TestExchangeCode:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
             mock_get.reset_mock()
             mock_get.side_effect = [
@@ -481,7 +512,7 @@ class TestExchangeCode:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
             mock_get.reset_mock()
             mock_get.side_effect = [
@@ -510,7 +541,7 @@ class TestExchangeCode:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
             mock_get.reset_mock()
             mock_get.side_effect = [
@@ -551,7 +582,7 @@ class TestExchangeCode:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
             mock_get.reset_mock()
             mock_get.side_effect = [
@@ -580,7 +611,7 @@ class TestExchangeCode:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
             mock_get.reset_mock()
             mock_get.side_effect = [
@@ -612,7 +643,7 @@ class TestExchangeCode:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
             mock_get.reset_mock()
             mock_get.side_effect = [
@@ -640,7 +671,7 @@ class TestExchangeCode:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
             mock_get.reset_mock()
             mock_get.side_effect = [
@@ -670,7 +701,7 @@ class TestExchangeCode:
             )
 
             # State carries a different nonce than the id_token
-            state = mod._encode_state(different_nonce, "https://app.example.com/callback")
+            state = mod._encode_state(different_nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
             mock_get.reset_mock()
             mock_get.side_effect = [
@@ -701,7 +732,7 @@ class TestUserInfoProtection:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
 
             # UserInfo returns a different sub
@@ -733,7 +764,7 @@ class TestUserInfoProtection:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
 
             # UserInfo matches sub, adds extra profile data
@@ -807,7 +838,7 @@ class TestJwksCache:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
 
             # First exchange — cache is populated
             mock_post.return_value = _mock_token_response(id_token)
@@ -821,7 +852,7 @@ class TestJwksCache:
             # Second exchange with same kid — cached, no extra JWKS fetch.
             # But userinfo still tries to call GET on the userinfo endpoint
             # (which fails gracefully — logged as a warning, not a crash).
-            state2 = mod._encode_state(nonce, "https://app.example.com/callback")
+            state2 = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
             mock_get.reset_mock()
             # Provide a userinfo mock so it doesn't count as a real failure
@@ -855,7 +886,7 @@ class TestJwksCache:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
 
             # Simulate a network failure on the JWKS fetch inside _verify_id_token
@@ -889,7 +920,7 @@ class TestJwksCache:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
 
             # Simulate a bad JSON response from the JWKS endpoint
@@ -922,7 +953,7 @@ class TestJwksCache:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
 
             # Simulate HTTP 500 from the JWKS endpoint
@@ -958,7 +989,7 @@ class TestStateKeyPersistence:
         assert not key_file.exists()
 
         # Worker A: encode state (this must create the shared key)
-        state_a = mod._encode_state("nonce-abc", "https://app.example.com/callback")
+        state_a = mod._encode_state("nonce-abc", "https://app.example.com/callback", "test-code-verifier")
         assert key_file.exists(), "Shared app key must be created on first state encode"
         assert key_file.stat().st_size > 0
 
@@ -988,7 +1019,7 @@ class TestStateKeyPersistence:
         # Simulate worker A: encode state (creates key file atomically)
         monkeypatch.setattr(mod, "_state_fernet", None)
         monkeypatch.setattr(ss, "_fernet", None)
-        state_a = mod._encode_state("nonce-a", "https://cb1.example.com/")
+        state_a = mod._encode_state("nonce-a", "https://cb1.example.com/", "test-code-verifier")
         assert key_file.exists()
         key_bytes_a = key_file.read_bytes()
 
@@ -1000,7 +1031,7 @@ class TestStateKeyPersistence:
         assert decoded_b["nonce"] == "nonce-a"
 
         # Worker B encodes its own state — must use the same key
-        state_b = mod._encode_state("nonce-b", "https://cb2.example.com/")
+        state_b = mod._encode_state("nonce-b", "https://cb2.example.com/", "test-code-verifier")
         # After B's encode, the file must still contain worker A's key
         assert key_file.read_bytes() == key_bytes_a, \
             "Worker B must not overwrite the key file created by worker A"
@@ -1039,7 +1070,7 @@ class TestJwksCooldown:
 
             # Step 1: do one successful exchange with "test-key-1" so the
             # JWKS cache is populated and _fetch_jwks() returns cached data.
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token_known_kid)
             mock_get.reset_mock()
             mock_get.side_effect = [
@@ -1079,7 +1110,7 @@ class TestJwksCooldown:
             ).decode()
 
             # First unknown-kid attempt: JWKS refresh FAILS → cooldown set
-            state1 = mod._encode_state(nonce, "https://app.example.com/callback")
+            state1 = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token_unknown_kid)
             mock_get.reset_mock()
             mock_get.side_effect = [
@@ -1094,7 +1125,7 @@ class TestJwksCooldown:
             # Second unknown-kid attempt: cooldown still active → throttled,
             # _refresh_jwks must NOT be called.  The exchange will fail
             # because the key for "test-key-2" is not in the stale cache.
-            state2 = mod._encode_state(nonce, "https://app.example.com/callback")
+            state2 = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token_unknown_kid)
             mock_get.reset_mock()
             # If _refresh_jwks were called, it would hit this side_effect.
@@ -1128,7 +1159,7 @@ class TestRedirectUriBinding:
             )
 
         # State encodes "https://original.example.com/callback"
-        state = mod._encode_state("nonce", "https://original.example.com/callback")
+        state = mod._encode_state("nonce", "https://original.example.com/callback", "test-code-verifier")
 
         # But the callback derives a different redirect_uri
         with pytest.raises(mod.OidcError, match="redirect_uri mismatch"):
@@ -1157,7 +1188,7 @@ class TestRedirectUriBinding:
                 client_secret=FAKE_CLIENT_SECRET,
             )
 
-            state = mod._encode_state(nonce, stored_uri)
+            state = mod._encode_state(nonce, stored_uri, "test-code-verifier")
             mock_post.return_value = _mock_token_response(id_token)
             mock_get.reset_mock()
             mock_get.side_effect = [
@@ -1206,7 +1237,7 @@ class TestUserinfoEndpointMissing:
             # _mock_token_response already includes an access_token.
             mock_post.return_value = _mock_token_response(id_token)
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             claims = mgr.exchange_code("code", state, "https://app.example.com/callback")
 
             assert claims["sub"] == "user-no-ui"
@@ -1240,7 +1271,7 @@ class TestUserinfoEndpointMissing:
 
             mock_post.return_value = _mock_token_response(id_token)
 
-            state = mod._encode_state(nonce, "https://app.example.com/callback")
+            state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
             claims = mgr.exchange_code("code", state, "https://app.example.com/callback")
 
             assert claims["sub"] == "user-with-ui"
@@ -1389,7 +1420,7 @@ def _exchange_with_userinfo(userinfo):
 
     mgr = _new_security_test_manager()
     nonce = "u" * 64
-    state = mod._encode_state(nonce, "https://app.example.com/callback")
+    state = mod._encode_state(nonce, "https://app.example.com/callback", "test-code-verifier")
     mgr._token_request = MagicMock(return_value={
         "access_token": "access-token",
         "id_token": "unused-in-this-unit-test",
@@ -1476,10 +1507,13 @@ class TestNumericDateClaimValidation:
         with pytest.raises(mod.OidcError):
             mgr._verify_id_token(token, "f" * 64)
 
-    def test_iat_none_accepted(self):
+    def test_iat_missing_rejected(self):
+        # OIDC Core §2: iat is REQUIRED — a token without it must not verify.
+        import core.oidc as mod
         mgr = _new_security_test_manager()
         token = _make_claim_test_token("user123", "g" * 64, iat=None)
-        mgr._verify_id_token(token, "g" * 64)
+        with pytest.raises(mod.OidcError, match="missing iat"):
+            mgr._verify_id_token(token, "g" * 64)
 
     def test_iat_valid_accepted(self):
         mgr = _new_security_test_manager()
@@ -1489,6 +1523,120 @@ class TestNumericDateClaimValidation:
         mgr._verify_id_token(token, "h" * 64)
 
 
+def _make_manager(discovery_doc=None):
+    """Build an OidcManager against a mocked discovery endpoint."""
+    import core.oidc as mod
+    jwt_jwks, _ = _make_test_jwks_and_key()
+    with patch.object(mod.httpx, "get") as mock_get:
+        mock_get.side_effect = [
+            _FakeResponse(200, discovery_doc or DISCOVERY_DOC),
+            _mock_jwks_response(jwt_jwks),
+        ]
+        return mod.OidcManager(
+            issuer=FAKE_ISSUER,
+            client_id=FAKE_CLIENT_ID,
+            client_secret=FAKE_CLIENT_SECRET,
+        )
+
+
+class TestPkceTokenRequest:
+    def test_code_verifier_sent_to_token_endpoint(self):
+        """The verifier recovered from state must be POSTed to the token
+        endpoint and must hash to the challenge from the auth URL."""
+        import base64
+        import hashlib
+        from urllib.parse import urlparse, parse_qs
+        import core.oidc as mod
+
+        jwt_jwks, _ = _make_test_jwks_and_key()
+        mgr = _make_manager()
+        url, state, nonce = mgr.get_authorization_url("https://app.example.com/callback")
+        challenge = parse_qs(urlparse(url).query)["code_challenge"][0]
+        id_token = _make_id_token("user123", nonce)
+
+        with patch.object(mod.httpx, "get") as mock_get, \
+             patch.object(mod.httpx, "post") as mock_post:
+            mock_get.side_effect = [_mock_jwks_response(jwt_jwks)]
+            mock_post.return_value = _mock_token_response(id_token)
+            mgr.exchange_code("auth_code_xyz", state, "https://app.example.com/callback")
+
+            posted = mock_post.call_args.kwargs["data"]
+            verifier = posted["code_verifier"]
+            expected = (
+                base64.urlsafe_b64encode(hashlib.sha256(verifier.encode()).digest())
+                .rstrip(b"=")
+                .decode()
+            )
+            assert challenge == expected
+
+
+class TestTokenEndpointAuth:
+    def _exchange(self, discovery_doc):
+        """Run a full exchange and return the httpx.post call kwargs."""
+        import core.oidc as mod
+        jwt_jwks, _ = _make_test_jwks_and_key()
+        mgr = _make_manager(discovery_doc)
+        _, state, nonce = mgr.get_authorization_url("https://app.example.com/callback")
+        id_token = _make_id_token("user123", nonce)
+        with patch.object(mod.httpx, "get") as mock_get, \
+             patch.object(mod.httpx, "post") as mock_post:
+            mock_get.side_effect = [_mock_jwks_response(jwt_jwks)]
+            mock_post.return_value = _mock_token_response(id_token)
+            mgr.exchange_code("code", state, "https://app.example.com/callback")
+            return mock_post.call_args.kwargs
+
+    def test_default_uses_client_secret_basic(self):
+        """No token_endpoint_auth_methods_supported in discovery → the OIDC
+        default client_secret_basic: HTTP Basic auth, no secret in the body."""
+        kwargs = self._exchange(DISCOVERY_DOC)
+        assert kwargs["auth"] == (FAKE_CLIENT_ID, FAKE_CLIENT_SECRET)
+        assert "client_secret" not in kwargs["data"]
+        assert "client_id" not in kwargs["data"]
+
+    def test_basic_preferred_when_advertised(self):
+        doc = dict(DISCOVERY_DOC)
+        doc["token_endpoint_auth_methods_supported"] = [
+            "client_secret_post", "client_secret_basic",
+        ]
+        kwargs = self._exchange(doc)
+        assert kwargs["auth"] == (FAKE_CLIENT_ID, FAKE_CLIENT_SECRET)
+        assert "client_secret" not in kwargs["data"]
+
+    def test_post_fallback_when_basic_unsupported(self):
+        doc = dict(DISCOVERY_DOC)
+        doc["token_endpoint_auth_methods_supported"] = ["client_secret_post"]
+        kwargs = self._exchange(doc)
+        assert kwargs["auth"] is None
+        assert kwargs["data"]["client_secret"] == FAKE_CLIENT_SECRET
+        assert kwargs["data"]["client_id"] == FAKE_CLIENT_ID
+
+
+class TestHttpsEnforcement:
+    def test_http_issuer_rejected(self):
+        import core.oidc as mod
+        with patch.object(mod.httpx, "get") as mock_get:
+            mock_get.return_value = _mock_discovery_response()
+            with pytest.raises(mod.OidcError, match="issuer must use HTTPS"):
+                mod.OidcManager(
+                    issuer="http://idp.example.com",
+                    client_id=FAKE_CLIENT_ID,
+                    client_secret=FAKE_CLIENT_SECRET,
+                )
+
+    def test_http_authorization_endpoint_rejected(self):
+        import core.oidc as mod
+        doc = dict(DISCOVERY_DOC)
+        doc["authorization_endpoint"] = "http://idp.example.com/authorize"
+        with patch.object(mod.httpx, "get") as mock_get:
+            mock_get.return_value = _FakeResponse(200, doc)
+            with pytest.raises(mod.OidcError, match="authorization_endpoint must use HTTPS"):
+                mod.OidcManager(
+                    issuer=FAKE_ISSUER,
+                    client_id=FAKE_CLIENT_ID,
+                    client_secret=FAKE_CLIENT_SECRET,
+                )
+
+
 class TestMaxAgeConfiguration:
     def test_max_age_added_to_auth_url(self):
         mgr = _new_security_test_manager(max_age=3600)

tests/test_oidc_routes.py

@@ -954,6 +954,197 @@ class TestAdminDemotionProtection:
         # An existing admin must not be demoted on missing evidence.
         auth.set_oidc_user_admin.assert_not_called()
 
+    def _run_admin_sync_callback(self, claims, monkeypatch):
+        monkeypatch.setenv("OIDC_ADMIN_GROUPS", "odysseus-admins")
+        mgr = MagicMock()
+        mgr.configured = True
+        mgr.redirect_uri_override = None
+        mgr.issuer = "https://idp.example.com"
+        mgr.exchange_code.return_value = claims
+
+        auth = MagicMock()
+        auth.get_user_by_oidc.return_value = "existing-admin"
+        auth.create_session_trusted.return_value = "token"
+
+        router = _setup_oidc_routes(auth, mgr)
+        ep = _get_endpoint(router, "/api/auth/oidc/callback")
+        import asyncio
+        asyncio.run(
+            ep(
+                _fake_request_with_params(
+                    {"code": "code", "state": "state"},
+                    cookies={"odysseus_oidc_csrf": "state"},
+                ),
+                SimpleNamespace(
+                    set_cookie=MagicMock(),
+                    delete_cookie=MagicMock(),
+                    status_code=200,
+                    headers={},
+                ),
+            )
+        )
+        return auth
+
+    def test_userinfo_available_without_groups_preserves_admin(self, monkeypatch):
+        """UserInfo fetched but carrying no groups claim is NOT evidence of
+        membership loss — availability alone must not drive a demotion."""
+        auth = self._run_admin_sync_callback({
+            "sub": "existing-admin",
+            "email": "admin@example.com",
+            "_userinfo_available": True,
+            # no "groups" key anywhere
+        }, monkeypatch)
+        auth.set_oidc_user_admin.assert_not_called()
+
+    def test_malformed_groups_claim_preserves_admin(self, monkeypatch):
+        """A non-list groups value is not valid evidence and must not
+        demote an existing administrator."""
+        auth = self._run_admin_sync_callback({
+            "sub": "existing-admin",
+            "email": "admin@example.com",
+            "_userinfo_available": True,
+            "groups": "odysseus-admins",  # malformed: string, not list
+        }, monkeypatch)
+        auth.set_oidc_user_admin.assert_not_called()
+
+
+class TestSubjectIdentifierPreservation:
+    """Regression: OIDC subs are opaque — whitespace-bearing subjects must
+    not be normalized into a different subject's account."""
+
+    def _run_callback_with_sub(self, sub):
+        mgr = MagicMock()
+        mgr.configured = True
+        mgr.redirect_uri_override = None
+        mgr.issuer = "https://idp.example.com"
+        mgr.exchange_code.return_value = {
+            "sub": sub,
+            "email": "alice@example.com",
+            "preferred_username": "alice",
+        }
+
+        auth = MagicMock()
+        auth.check_oidc_totp.return_value = False
+        auth.get_user_by_oidc.return_value = None
+        auth.create_user_oidc.return_value = "alice"
+        auth.create_session_trusted.return_value = "token"
+
+        router = _setup_oidc_routes(auth, mgr)
+        ep = _get_endpoint(router, "/api/auth/oidc/callback")
+        import asyncio
+        asyncio.run(
+            ep(
+                _fake_request_with_params(
+                    {"code": "code", "state": "state"},
+                    cookies={"odysseus_oidc_csrf": "state"},
+                ),
+                SimpleNamespace(
+                    set_cookie=MagicMock(),
+                    delete_cookie=MagicMock(),
+                    status_code=200,
+                    headers={},
+                ),
+            )
+        )
+        return auth
+
+    def test_whitespace_bearing_sub_preserved_exactly(self):
+        sub = " user123 "
+        auth = self._run_callback_with_sub(sub)
+        auth.get_user_by_oidc.assert_called_once_with(sub, "https://idp.example.com")
+        auth.create_user_oidc.assert_called_once_with(
+            "alice", sub, "https://idp.example.com",
+            email="alice@example.com", is_admin=False,
+        )
+
+    def test_distinct_whitespace_subs_lookup_distinctly(self):
+        """'user123' and ' user123 ' must hit the account store as two
+        different keys, never collapsing into one local account."""
+        auth_a = self._run_callback_with_sub("user123")
+        auth_b = self._run_callback_with_sub(" user123 ")
+        (sub_a, _), _ = auth_a.get_user_by_oidc.call_args
+        (sub_b, _), _ = auth_b.get_user_by_oidc.call_args
+        assert sub_a != sub_b
+
+    def test_whitespace_only_sub_rejected(self):
+        auth = self._run_callback_with_sub("   ")
+        # "   " is a technically valid opaque sub per spec; ensure it is
+        # either used exactly or rejected — never trimmed to empty and
+        # never persisted as a different identifier.
+        if auth.create_user_oidc.called:
+            args, kwargs = auth.create_user_oidc.call_args
+            assert args[1] == "   "
+
+
+class TestOidcCookieSecurity:
+    """Regression: OIDC cookies must be Secure by default, regardless of
+    SECURE_COOKIES (which the bundled Compose files default to false)."""
+
+    def _login_cookie_kwargs(self):
+        mgr = MagicMock()
+        mgr.configured = True
+        mgr.redirect_uri_override = None
+        mgr.get_authorization_url.return_value = (
+            "https://idp.example.com/authorize?state=abc", "abc", "def",
+        )
+        router = _setup_oidc_routes(MagicMock(), mgr)
+        ep = _get_endpoint(router, "/api/auth/oidc/login")
+        import asyncio
+        with patch("fastapi.responses.RedirectResponse.set_cookie") as sc:
+            asyncio.run(ep(_fake_request()))
+            return sc.call_args.kwargs
+
+    def test_csrf_cookie_secure_despite_secure_cookies_false(self, monkeypatch):
+        monkeypatch.setenv("SECURE_COOKIES", "false")
+        monkeypatch.delenv("OIDC_ALLOW_INSECURE_COOKIES", raising=False)
+        assert self._login_cookie_kwargs()["secure"] is True
+
+    def test_csrf_cookie_secure_on_http_request(self, monkeypatch):
+        """Plain-http request scheme must not downgrade the cookie."""
+        monkeypatch.delenv("SECURE_COOKIES", raising=False)
+        monkeypatch.delenv("OIDC_ALLOW_INSECURE_COOKIES", raising=False)
+        assert self._login_cookie_kwargs()["secure"] is True
+
+    def test_explicit_dev_override_allows_insecure(self, monkeypatch):
+        monkeypatch.setenv("OIDC_ALLOW_INSECURE_COOKIES", "true")
+        assert self._login_cookie_kwargs()["secure"] is False
+
+    def test_session_cookie_secure_despite_secure_cookies_false(self, monkeypatch):
+        monkeypatch.setenv("SECURE_COOKIES", "false")
+        monkeypatch.delenv("OIDC_ALLOW_INSECURE_COOKIES", raising=False)
+
+        mgr = MagicMock()
+        mgr.configured = True
+        mgr.redirect_uri_override = None
+        mgr.issuer = "https://idp.example.com"
+        mgr.exchange_code.return_value = {
+            "sub": "user123", "email": "alice@example.com",
+        }
+        auth = MagicMock()
+        auth.check_oidc_totp.return_value = False
+        auth.get_user_by_oidc.return_value = "alice"
+        auth.create_session_trusted.return_value = "token"
+
+        router = _setup_oidc_routes(auth, mgr)
+        ep = _get_endpoint(router, "/api/auth/oidc/callback")
+        set_cookie = MagicMock()
+        import asyncio
+        asyncio.run(
+            ep(
+                _fake_request_with_params(
+                    {"code": "code", "state": "state"},
+                    cookies={"odysseus_oidc_csrf": "state"},
+                ),
+                SimpleNamespace(
+                    set_cookie=set_cookie,
+                    delete_cookie=MagicMock(),
+                    status_code=200,
+                    headers={},
+                ),
+            )
+        )
+        assert set_cookie.call_args.kwargs["secure"] is True
+
 
 class TestOidcCallbackSecurityFailures:
     def _run_callback(self, auth, mgr, params, cookies=None):

tests/test_session_cross_worker.py

@@ -0,0 +1,84 @@
+"""Regression: sessions issued by one uvicorn worker must validate on another.
+
+The OIDC callback (or a password login) can run on worker A while the
+browser's next request lands on worker B.  Each worker loads sessions.json
+only at startup, so without the read-through reload in
+AuthManager._reload_sessions_if_changed the new token would be rejected
+and a successful login would immediately become a logged-out session.
+"""
+
+import importlib
+import sys
+import types
+from pathlib import Path
+
+from tests.helpers.import_state import clear_module
+
+
+def _real_core_package():
+    root = Path(__file__).resolve().parent.parent
+    core_path = str(root / "core")
+    core = sys.modules.get("core")
+    if core is None:
+        core = types.ModuleType("core")
+        sys.modules["core"] = core
+    core.__path__ = [core_path]
+    clear_module("core.auth")
+    return core
+
+
+def _auth_module():
+    _real_core_package()
+    return importlib.import_module("core.auth")
+
+
+def _two_workers(tmp_path):
+    """Build two AuthManager instances over the same data directory,
+    simulating two uvicorn worker processes."""
+    auth_mod = _auth_module()
+    auth_mod._hash_password = lambda password: f"hash:{password}"
+    auth_mod._verify_password = lambda password, hashed: hashed == f"hash:{password}"
+    auth_path = str(tmp_path / "auth.json")
+    worker_a = auth_mod.AuthManager(auth_path)
+    assert worker_a.create_user("alice", "password-1", is_admin=False)
+    worker_b = auth_mod.AuthManager(auth_path)  # boots after user exists
+    return worker_a, worker_b
+
+
+class TestCrossWorkerSessions:
+    def test_session_issued_on_other_worker_validates(self, tmp_path):
+        worker_a, worker_b = _two_workers(tmp_path)
+        token = worker_a.create_session_trusted("alice")
+        assert token is not None
+        # Worker B has never seen this token in memory — it must read
+        # through to sessions.json and accept it.
+        assert worker_b.validate_token(token) is True
+        assert worker_b.get_username_for_token(token) == "alice"
+
+    def test_unknown_token_still_rejected(self, tmp_path):
+        worker_a, worker_b = _two_workers(tmp_path)
+        worker_a.create_session_trusted("alice")
+        assert worker_b.validate_token("f" * 64) is False
+        assert worker_b.get_username_for_token("f" * 64) is None
+
+    def test_expired_session_from_other_worker_rejected(self, tmp_path):
+        auth_mod = _auth_module()
+        worker_a, worker_b = _two_workers(tmp_path)
+        token = worker_a.create_session_trusted("alice")
+        # Force the persisted expiry into the past, as another worker
+        # would see it after the TTL elapsed.
+        with worker_a._sessions_lock:
+            worker_a._sessions[token]["expiry"] = 1.0
+        worker_a._save_sessions()
+        assert worker_b.validate_token(token) is False
+        assert worker_b.get_username_for_token(token) is None
+
+    def test_reload_is_additive_not_destructive(self, tmp_path):
+        """A reload must never drop tokens this worker already holds in
+        memory (e.g. one issued moments ago, racing its own save)."""
+        worker_a, worker_b = _two_workers(tmp_path)
+        token_b = worker_b.create_session_trusted("alice")
+        token_a = worker_a.create_session_trusted("alice")
+        # B validating A's token triggers a reload; B's own token survives.
+        assert worker_b.validate_token(token_a) is True
+        assert worker_b.validate_token(token_b) is True