@@ -12,6 +12,7 @@
 
 import base64
 import hashlib
+import hmac
 import json
 import logging
 import secrets
@@ -92,7 +93,7 @@ def _cleanup_pending() -> None:
         _pending.pop(state, None)
 
 
-async def begin_login(source_ip: str = "") -> str:
+async def begin_login(source_ip: str = "", browser: str = "") -> str:
     """Start a login and give back the URL to send the browser to.
 
     Two things get made here and both matter.
@@ -113,7 +114,7 @@ async def begin_login(source_ip: str = "") -> str:
 
     _cleanup_pending()
     _pending[state] = {"verifier": verifier, "created": time.time(),
-                       "source_ip": source_ip}
+                       "source_ip": source_ip, "browser": browser}
 
     params = {
         "response_type": "code",
@@ -127,14 +128,24 @@ async def begin_login(source_ip: str = "") -> str:
     return f"{doc['authorization_endpoint']}?{urlencode(params)}"
 
 
-async def complete_login(code: str, state: str, source_ip: str = "") -> dict:
+async def complete_login(code: str, state: str, source_ip: str = "",
+                         browser: str = "") -> dict:
     """Finish the login.  Swap the code for tokens and check the id_token."""
     pending = _pending.pop(state, None)
     if not pending:
         raise OIDCError("that sign in is not one we started, or it took too "
                         "long. Try again.")
     if time.time() - pending["created"] > _PENDING_TTL:
         raise OIDCError("that sign in took too long, start again")
+    # Bind the flow to the browser that started it.  The state alone travels in
+    # the URL, so a stolen or planted callback could sign a victim into the
+    # attacker's account (login CSRF).  The browser nonce was set as a cookie at
+    # the start and is not in the URL, so a callback opened in another browser,
+    # even with a valid state and code, is refused.
+    if pending.get("browser") and not hmac.compare_digest(
+            str(pending.get("browser")), str(browser or "")):
+        raise OIDCError("that sign in was started in a different browser. "
+                        "Start again here.")
 
     doc = await get_discovery()
 
@@ -301,8 +312,22 @@ def sync_user(session, profile: dict) -> User:
     username = (profile.get("username") or "").strip().lower()
     if not username:
         raise OIDCError("the provider did not tell us who you are")
-
-    user = session.query(User).filter(User.username == username).one_or_none()
+    subject = (profile.get("subject") or "").strip()
+
+    # Who this is comes from the provider's immutable subject, not the name,
+    # which the provider can rename or reuse.  Match on the subject first; the
+    # account follows it even if the name changed.  Fall back to the name only
+    # to adopt an account that has no subject recorded yet (the first sign in
+    # after this landed), or when no subject was sent at all.
+    user = None
+    if subject:
+        user = (session.query(User)
+                .filter(User.oidc_subject == subject,
+                        User.source == AuthSource.OIDC).one_or_none())
+    by_name = session.query(User).filter(User.username == username).one_or_none()
+    matched_by_name = user is None
+    if user is None:
+        user = by_name
 
     # Turned off here stays off, whatever the provider says.  Signing in used
     # to clear the flag, so disabling an SSO account lasted until that person
@@ -319,6 +344,15 @@ def sync_user(session, profile: dict) -> User:
             f"there is already a local account called {username}. "
             "An administrator needs to sort that out.")
 
+    # The name matched an SSO account already bound to a different subject: a
+    # different identity wearing the same name.  Refuse, so one person's name
+    # can never inherit another's account, access or pinned role.
+    if (subject and matched_by_name and user is not None
+            and (user.oidc_subject or "") and user.oidc_subject != subject):
+        raise OIDCError(
+            "that name belongs to a different single sign on identity. "
+            "An administrator needs to sort that out.")
+
     # Whose account this is has to be known before the role is decided, because
     # a pinned account keeps the role it was given and the groups do not come
     # into it.  That is the whole point of pinning one: it is the way back in
@@ -339,6 +373,11 @@ def sync_user(session, profile: dict) -> User:
         user = User(username=username, source=AuthSource.OIDC)
         session.add(user)
 
+    # Record the subject the first time we see it (trust on first use), so every
+    # later sign in is matched on it and a colliding name is refused above.
+    if subject and not (user.oidc_subject or ""):
+        user.oidc_subject = subject
+
     user.email = (profile.get("email") or "")[:255]
     user.display_name = (profile.get("full_name") or "")[:255]
     user.role = role
 
@@ -162,10 +162,19 @@ def make_recovery_codes() -> tuple:
     for _ in range(RECOVERY_CODE_COUNT):
         raw = secrets.token_hex(5)
         plain.append(f"{raw[:5]}-{raw[5:]}")
-    hashes = [hashlib.sha256(c.encode()).hexdigest() for c in plain]
+    # Hash the code the way check_totp normalises what is typed (spaces and the
+    # hyphen removed), so a code entered with or without its hyphen verifies.
+    # Hashing the displayed form here while check_totp strips the hyphen before
+    # checking meant no recovery code ever matched.
+    hashes = [hashlib.sha256(_canon_recovery(c).encode()).hexdigest() for c in plain]
     return plain, hashes
 
 
+def _canon_recovery(code: str) -> str:
+    """The one form a recovery code is hashed and checked in."""
+    return (code or "").strip().replace(" ", "").replace("-", "")
+
+
 def _consume_recovery_code(user: User, code: str) -> bool:
     """Burn a recovery code if it matches.  Each one works exactly once."""
     try:
@@ -175,9 +184,15 @@ def _consume_recovery_code(user: User, code: str) -> bool:
     if not stored:
         return False
 
-    candidate = hashlib.sha256(code.encode()).hexdigest()
+    # The code arrives already normalised (check_totp strips spaces and the
+    # hyphen).  Match the canonical hash, and also the older form that hashed
+    # the hyphenated code, so codes issued before the fix still work.
+    candidates = {hashlib.sha256(code.encode()).hexdigest()}
+    if len(code) == 10 and code.isalnum():
+        legacy = f"{code[:5]}-{code[5:]}"
+        candidates.add(hashlib.sha256(legacy.encode()).hexdigest())
     for held in list(stored):
-        if hmac.compare_digest(held, candidate):
+        if any(hmac.compare_digest(held, c) for c in candidates):
             stored.remove(held)
             user.recovery_hashes = json.dumps(stored)
             return True
 
@@ -145,6 +145,13 @@ class User(Base):
     role = Column(Enum(UserRole), nullable=False, default=UserRole.READER)
     source = Column(Enum(AuthSource), nullable=False, default=AuthSource.LOCAL)
 
+    # The immutable identity of an SSO account: the provider's subject claim.
+    # The account is matched on this, not on the user name, which the provider
+    # can rename or reuse.  Recorded the first time an account signs in, and
+    # from then on a different subject carrying the same name is refused rather
+    # than inheriting the account and its role.  Empty on local accounts.
+    oidc_subject = Column(String(255), default="", index=True)
+
     # Hold the role against the provider.
     #
     # The provider owns the role, which is right while it sends group names.
 
@@ -21,6 +21,7 @@
 
 from __future__ import annotations
 
+import functools
 import hashlib
 import json
 import logging
@@ -343,10 +344,16 @@ async def review(request: Request,
             target.parent.mkdir(parents=True, exist_ok=True)
             target.write_bytes(raw)
 
-        completed = subprocess.run(
-            [str(GHSCAN), "review", workdir, "--only", only, "--fail-on", "none"],
-            capture_output=True, text=True, timeout=REVIEW_TIMEOUT,
-            env={**os.environ, "GHSCAN_LOG_LEVEL": "warn"})
+        # Run the scanner off the event loop.  It is a blocking subprocess, and
+        # this is the one web worker: calling it inline would freeze every other
+        # request, including sign in and the health check, for the whole scan.
+        # run_in_threadpool also bounds how many run at once to the thread pool.
+        completed = await run_in_threadpool(
+            functools.partial(
+                subprocess.run,
+                [str(GHSCAN), "review", workdir, "--only", only, "--fail-on", "none"],
+                capture_output=True, text=True, timeout=REVIEW_TIMEOUT,
+                env={**os.environ, "GHSCAN_LOG_LEVEL": "warn"}))
 
         if completed.returncode not in (0, 3) or not completed.stdout.strip():
             tail = (completed.stderr or "").strip().splitlines()[-3:]
 
@@ -8,6 +8,7 @@
 
 import json
 import logging
+import secrets
 
 from fastapi import APIRouter, Form, Request
 from fastapi.responses import RedirectResponse
@@ -27,6 +28,9 @@
 log = logging.getLogger("gcr.auth.routes")
 router = APIRouter()
 
+# The cookie that binds an OIDC flow to the browser that began it.
+OIDC_FLOW_COOKIE = "gcr_oidc_flow"
+
 
 def _set_session_cookie(response, token: str) -> None:
     response.set_cookie(
@@ -265,14 +269,22 @@ async def oidc_start(request: Request):
         return page(request, "login.html", stage="password", auth=live.auth_config(),
                     error="Single sign on is not set up here.")
 
+    # A nonce that stays in a cookie on this browser and never travels in the
+    # URL, so the callback can tell the browser that started the flow from one
+    # a victim was handed (login CSRF).
+    browser = secrets.token_urlsafe(24)
     try:
-        url = await oidc.begin_login(client_ip(request))
+        url = await oidc.begin_login(client_ip(request), browser)
     except oidc.OIDCError as exc:
         log.warning("could not start an oidc login: %s", exc)
         return page(request, "login.html", stage="password", auth=live.auth_config(),
                     error=str(exc))
 
-    return RedirectResponse(url, status_code=303)
+    response = RedirectResponse(url, status_code=303)
+    response.set_cookie(OIDC_FLOW_COOKIE, browser, max_age=600, httponly=True,
+                        secure=live.force_https(), samesite="lax",
+                        path=settings.base_path or "/")
+    return response
 
 
 @router.get("/auth/callback")
@@ -297,8 +309,9 @@ async def oidc_callback(request: Request):
                     error="That callback was missing something. Try again.")
 
     ip = client_ip(request)
+    browser = request.cookies.get(OIDC_FLOW_COOKIE, "")
     try:
-        profile = await oidc.complete_login(code, state, ip)
+        profile = await oidc.complete_login(code, state, ip, browser)
     except oidc.OIDCError as exc:
         log.warning("oidc sign in failed from %s: %s", ip, exc)
         with session_scope() as db:
@@ -327,4 +340,5 @@ async def oidc_callback(request: Request):
     response = landing(request)
     _set_session_cookie(response, token)
     issue_csrf(response, new_csrf_token())
+    response.delete_cookie(OIDC_FLOW_COOKIE, path=settings.base_path or "/")
     return response