diff --git a/.planning/STATE.md b/.planning/STATE.md index 1000307..58b69b3 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -36,7 +36,7 @@ progress: | 7.1 | Security: session revocation on privilege change (CR-01..03) | ✓ Complete (2/2 plans, 3 new tests, frontend toasts) | | 7.2 | Security: JTI claim + Redis access-token revocation | ✓ Complete (3/3 plans, 9/9 verified, 84/84 tests) | | 7.3 | Security: ES256 algorithm upgrade | ✓ Complete (3/3 plans, all 9 tests passing, remember_me shipped) | -| 7.4 | Security: Token fingerprinting / token binding | ◆ Not planned yet | +| 7.4 | Security: Token fingerprinting / token binding | ◆ Planned (2/2 plans, ready to execute) | ## Current Position @@ -210,6 +210,6 @@ _Updated at each phase transition._ | Last session | 2026-06-05 — Phase 7.1 complete: CR-01/CR-02/CR-03 implemented; skip_token_hash added to revoke_all_refresh_tokens; change_password, enable_totp, disable_totp now revoke other sessions and return sessions_revoked; 3 new tests passing; frontend toasts in SettingsAccountTab + TotpEnrollment; 373 passed 0 failed; v0.1.1 | | Last session | 2026-06-05 — Phase 7.2 planned: 3 plans (Wave 0 test scaffolding, Wave 1 jti+NBF check, Wave 2 user_nbf writes in 4 handlers); verification passed (0 blockers, 1 warning fixed); ready to execute | | Last session | 2026-06-06 — Phase 7.3 complete: ES256 asymmetric JWT signing live; default session 16h; remember_me opt-in 30d; startup bulk-revocation hook; all 9 ES256/remember-me tests green; v0.1.2 | -| Next action | Plan Phase 7.4: /gsd:plan-phase 7.4 (token fingerprinting) | +| Next action | Execute Phase 7.4: /gsd:execute-phase 7.4 (token fingerprinting) | | Pending decisions | None | | Resume file | None | diff --git a/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-02-PLAN.md b/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-02-PLAN.md index 752e282..58e1847 100644 --- a/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-02-PLAN.md +++ b/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-02-PLAN.md @@ -10,6 +10,8 @@ files_modified: - backend/deps/auth.py - backend/api/auth.py - backend/tests/test_auth_fgp.py + - backend/main.py + - frontend/package.json autonomous: true requirements: - FGP-CONCERN # tracked in .planning/codebase/CONCERNS.md §"No Token Fingerprint / Token Binding" @@ -265,6 +267,10 @@ For FGP-03 (manual token without fgp claim): Edit 3 — backend/tests/test_auth_fgp.py: add required imports and replace stub bodies. + Edit 4 — Version bump (per CLAUDE.md version bump rule: patch increment for plans shipping user-facing auth changes). + In backend/main.py: change `version="0.1.2"` to `version="0.1.3"`. + In frontend/package.json: change `"version": "0.1.2"` to `"version": "0.1.3"`. + Add to the imports block (if not already present from Wave 0): import base64 import uuid as _uuid @@ -299,6 +305,8 @@ For FGP-03 (manual token without fgp claim): - backend/api/auth.py login call site contains `user_agent=request.headers.get("User-Agent", "")` - backend/api/auth.py refresh call site contains `accept_lang=request.headers.get("Accept-Language", "")` - Full suite: cd backend && pytest -v exits 0 with zero failures + - backend/main.py version string incremented to 0.1.3 + - frontend/package.json version field incremented to 0.1.3 @@ -341,6 +349,7 @@ After this plan completes: - Login and refresh call sites in api/auth.py pass User-Agent and Accept-Language headers (per D-05) - All 4 FGP tests pass: FGP-01 (match→200), FGP-02 (mismatch→401), FGP-03 (no claim→200), FGP-04 (missing headers→200) - Full pytest suite exits 0 with zero failures +- backend/main.py and frontend/package.json both show version 0.1.3 diff --git a/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-PATTERNS.md b/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-PATTERNS.md new file mode 100644 index 0000000..347c0d1 --- /dev/null +++ b/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-PATTERNS.md @@ -0,0 +1,386 @@ +# Phase 7.4: Security — Token Fingerprinting / Token Binding - Pattern Map + +**Mapped:** 2026-06-06 +**Files analyzed:** 4 +**Analogs found:** 4 / 4 + +--- + +## File Classification + +| New/Modified File | Role | Data Flow | Closest Analog | Match Quality | +|-------------------|------|-----------|----------------|---------------| +| `backend/services/auth.py` | service | request-response | `backend/services/auth.py` itself (extend existing) | exact — same file, additive change | +| `backend/deps/auth.py` | middleware/dependency | request-response | `backend/deps/auth.py` itself (extend user_nbf block) | exact — same file, additive block | +| `backend/api/auth.py` | controller | request-response | `backend/api/auth.py` itself (update 2 call sites) | exact — same file, call-site tweak | +| `backend/tests/test_auth_fgp.py` | test | request-response | `backend/tests/test_auth_deps.py` (NBF test pattern) | exact — same app harness, same fixture style | + +--- + +## Pattern Assignments + +### `backend/services/auth.py` (service, request-response) + +**Analog:** Same file — additive change. Extend `create_access_token` and add `_compute_fgp` helper before it. + +**Existing imports block** (lines 18-39): +```python +from __future__ import annotations + +import base64 +import hashlib +import hmac +import logging +import re +import secrets +import uuid +from datetime import datetime, timezone, timedelta +from typing import Optional + +import httpx +import jwt +import pyotp +from pwdlib import PasswordHash +from pwdlib.hashers.argon2 import Argon2Hasher +from sqlalchemy import select, update +from sqlalchemy.ext.asyncio import AsyncSession + +from config import settings +from db.models import BackupCode, Quota, RefreshToken, User +``` +Both `import hashlib` (line 21) and `import hmac` (line 22) are already present. No new imports required. + +**Existing `create_access_token` signature** (line 87): +```python +def create_access_token(user_id: str, role: str) -> str: +``` + +**Existing JWT payload dict** (lines 93-100) — `"fgp"` claim inserts here alongside existing claims: +```python + payload = { + "sub": str(user_id), + "role": role, + "typ": "access", + "iat": now, + "exp": now + timedelta(minutes=settings.access_token_expire_minutes), + "jti": str(uuid.uuid4()), + } +``` + +**Existing `hmac.compare_digest` usage pattern** (lines 419-425) — copy constant-time comparison idiom for the fgp check in `deps/auth.py`: +```python + if hmac.compare_digest(candidate_suffix.upper(), suffix): +``` + +**New `_compute_fgp` helper to add** (insert before `create_access_token` at line 87, per D-04): +```python +def _compute_fgp(user_agent: str, accept_lang: str) -> str: + """Return 16-char hex fingerprint binding a token to its client context (D-04).""" + return hmac.new( + settings.secret_key.encode(), + (user_agent + accept_lang).encode(), + hashlib.sha256, + ).hexdigest()[:16] +``` + +**Updated `create_access_token` signature** (D-05 — backward-compatible, empty-string defaults): +```python +def create_access_token( + user_id: str, + role: str, + user_agent: str = "", + accept_lang: str = "", +) -> str: +``` +Add `"fgp": _compute_fgp(user_agent, accept_lang)` to the `payload` dict. + +--- + +### `backend/deps/auth.py` (dependency/middleware, request-response) + +**Analog:** Same file — additive block inserted after the existing `user_nbf` check. + +**Existing imports block** (lines 23-33): +```python +import logging +import uuid + +from fastapi import Depends, HTTPException, Request, status +from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer +from sqlalchemy.ext.asyncio import AsyncSession + +from db.models import User +from deps.db import get_db +from services import auth as auth_service +``` +`hmac` is NOT imported in `deps/auth.py`. The fgp comparison uses `auth_service._compute_fgp` (called via the already-imported `auth_service`) plus `hmac.compare_digest`. Add `import hmac` at the top of this file. + +**`get_current_user` signature** (lines 41-45) — `request: Request` already present: +```python +async def get_current_user( + request: Request, + credentials: HTTPAuthorizationCredentials = Depends(security), + session: AsyncSession = Depends(get_db), +) -> User: +``` + +**Existing `user_nbf` block** (lines 62-85) — this is the exact structural model to follow for placement and exception guard order: +```python + # ── user_nbf check (D-02, D-03, D-04) ────────────────────────────────────── + try: + redis_client = request.app.state.redis + nbf_bytes = await redis_client.get(f"user_nbf:{payload['sub']}") + if nbf_bytes is not None: + nbf_str = nbf_bytes.decode() if isinstance(nbf_bytes, (bytes, bytearray)) else nbf_bytes + if payload["iat"] < int(nbf_str): + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Session invalidated", + headers={"WWW-Authenticate": "Bearer"}, + ) + except HTTPException: + raise # re-raise the 401 we just constructed (T-7.2-02: Pitfall 1 guard) + except Exception as exc: + _logger.warning("Redis user_nbf check failed (fail-open): %s", exc) + # ── end user_nbf check ────────────────────────────────────────────────────── +``` + +**Critical ordering rule** (line 81-84): `except HTTPException: raise` MUST precede `except Exception` — this guard re-raises any intentional HTTPException without it being swallowed. The fgp block raises `HTTPException` directly inside a try block; this guard is what ensures it propagates correctly. + +**fgp validation block to insert after line 85** (per D-06, RESEARCH.md Pattern 3): +```python + # ── fgp check (D-06, Phase 7.4) ──────────────────────────────────────────── + fgp_claim = payload.get("fgp", "") + if fgp_claim: + fgp_actual = auth_service._compute_fgp( + request.headers.get("User-Agent", ""), + request.headers.get("Accept-Language", ""), + ) + if not hmac.compare_digest(fgp_claim, fgp_actual): + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Token fingerprint mismatch", + headers={"WWW-Authenticate": "Bearer"}, + ) + # ── end fgp check ─────────────────────────────────────────────────────────── +``` + +**Key difference from user_nbf block:** No try/except wrapper needed — `_compute_fgp` is pure computation with no I/O, so there is no infrastructure-failure path requiring fail-open. The fgp mismatch must always be enforced (D-03). + +**Insertion point:** After line 85 (end of `user_nbf` block comment), before line 87 (`try: user_uuid = uuid.UUID(payload["sub"])`). + +--- + +### `backend/api/auth.py` (controller, request-response) + +**Analog:** Same file — two targeted call-site updates. + +**Existing imports** (lines 21-42) — no new imports needed; `Request` already imported: +```python +from fastapi import APIRouter, Depends, HTTPException, Request, Response, status +``` + +**Login handler signature** (lines 190-197) — `request: Request` already present: +```python +@router.post("/login") +@limiter.limit("10/minute") +async def login( + request: Request, + body: LoginRequest, + response: Response, + session: AsyncSession = Depends(get_db), +): +``` + +**Login call site** (line 290) — current: +```python +access_token = auth_service.create_access_token(str(user.id), user.role) +``` +Updated form (pass headers through): +```python +access_token = auth_service.create_access_token( + str(user.id), + user.role, + user_agent=request.headers.get("User-Agent", ""), + accept_lang=request.headers.get("Accept-Language", ""), +) +``` + +**Refresh handler signature** (lines 320-326) — `request: Request` already present: +```python +@router.post("/refresh") +@limiter.limit("10/minute") +async def refresh_token( + request: Request, + response: Response, + session: AsyncSession = Depends(get_db), +): +``` + +**Refresh call site** (line 364) — current: +```python +access_token = auth_service.create_access_token(user_id_str, user.role) +``` +Updated form: +```python +access_token = auth_service.create_access_token( + user_id_str, + user.role, + user_agent=request.headers.get("User-Agent", ""), + accept_lang=request.headers.get("Accept-Language", ""), +) +``` + +--- + +### `backend/tests/test_auth_fgp.py` (test, request-response) + +**Analog:** `backend/tests/test_auth_deps.py` — copy the full harness pattern (FakeRedis, `make_test_app`, `auth_client` fixture, `_create_user` helper). + +**Test file module docstring pattern** (lines 1-15 of `test_auth_deps.py`): +```python +""" +Tests for backend/deps/auth.py — FastAPI authentication dependency chain. +... +""" +``` + +**Standard imports block** (lines 17-24 of `test_auth_deps.py`): +```python +import uuid + +import pytest +import pytest_asyncio +from httpx import ASGITransport, AsyncClient +from fastapi import FastAPI, Depends +from sqlalchemy.ext.asyncio import AsyncSession + +from tests.test_auth_api import FakeRedis +``` +New test file also needs: `import hmac`, `import hashlib`, `import base64`, `import time` for crafting tokens without `fgp` claim (FGP-03) and for testing hmac values. + +**`make_test_app()` helper** (lines 29-52 of `test_auth_deps.py`) — copy verbatim; it creates a minimal `/test/me` route wired to `get_current_user`, with `app.state.redis = FakeRedis()`: +```python +def make_test_app(): + from deps.auth import get_current_user + from db.models import User + + test_app = FastAPI() + + @test_app.get("/test/me") + async def get_me(current_user: User = Depends(get_current_user)): + return {"id": str(current_user.id), "role": current_user.role} + + test_app.state.redis = FakeRedis() + return test_app +``` + +**`auth_client` fixture** (lines 55-71 of `test_auth_deps.py`) — copy verbatim; sets `get_db` override: +```python +@pytest_asyncio.fixture +async def auth_client(db_session: AsyncSession): + from deps.db import get_db + + app = make_test_app() + app.dependency_overrides[get_db] = lambda: db_session + + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as c: + yield c + + app.dependency_overrides.clear() +``` + +**`_create_user` helper** (lines 74-89 of `test_auth_deps.py`) — copy verbatim: +```python +async def _create_user(db_session, role: str = "user", is_active: bool = True): + from db.models import User + from services.auth import hash_password + + user = User( + id=uuid.uuid4(), + handle=f"user_{uuid.uuid4().hex[:6]}", + email=f"{uuid.uuid4().hex[:6]}@example.com", + password_hash=hash_password("testpassword"), + role=role, + is_active=is_active, + ) + db_session.add(user) + await db_session.commit() + return user +``` + +**Standard test function pattern** (lines 94-108 of `test_auth_deps.py`) — `@pytest.mark.asyncio`, inline import of `create_access_token`, `auth_client.get` with `Authorization` header: +```python +@pytest.mark.asyncio +async def test_get_current_user_returns_user(auth_client, db_session): + from services.auth import create_access_token + + user = await _create_user(db_session, role="user") + token = create_access_token(str(user.id), "user") + + resp = await auth_client.get( + "/test/me", headers={"Authorization": f"Bearer {token}"} + ) + assert resp.status_code == 200 +``` + +**FGP-03 — crafting a token without `fgp` claim** — pattern from `test_auth_es256.py` (lines 52-75): use PyJWT directly with the ES256 private key extracted from `settings`. In the test file import `import jwt as _jwt`, `import config`, decode `settings.jwt_private_key` via `base64.b64decode`, and call `_jwt.encode(payload_no_fgp, private_pem, algorithm="ES256")`. The `conftest._patch_es256_test_keys` autouse fixture (session-scoped) is already wired to all tests in `tests/` — no extra fixture needed in the new test file. + +**Access to conftest `_patch_es256_test_keys` autouse fixture** — because it is `autouse=True` in `conftest.py`, every test in `backend/tests/` automatically gets it. The new `test_auth_fgp.py` benefits from it without any explicit reference. + +--- + +## Shared Patterns + +### `hmac.compare_digest` — constant-time comparison +**Source:** `backend/services/auth.py` line 419 +**Apply to:** fgp validation block in `backend/deps/auth.py` +```python +if not hmac.compare_digest(fgp_claim, fgp_actual): +``` +Never use `==` for token/fingerprint comparison. `hmac.compare_digest` is mandatory (CLAUDE.md SEC-06). + +### `except HTTPException: raise` guard — before broad `except Exception` +**Source:** `backend/deps/auth.py` lines 81-84 +**Apply to:** Any try block in `get_current_user` that raises an intentional `HTTPException` +```python + except HTTPException: + raise # re-raise the 401 we just constructed (T-7.2-02: Pitfall 1 guard) + except Exception as exc: + _logger.warning("Redis user_nbf check failed (fail-open): %s", exc) +``` +The fgp block does NOT need this try/except (pure computation, no I/O). If placed inside the `user_nbf` try block, the guard order already handles it. If placed as a standalone `if` block outside any try, no wrapper is needed at all. + +### `request.headers.get("X", "")` — header extraction with empty-string fallback +**Source:** `backend/api/auth.py` — idiomatic FastAPI; `request: Request` already injected in all callers +**Apply to:** Both `api/auth.py` call sites and `deps/auth.py` fgp validation block +```python +request.headers.get("User-Agent", "") +request.headers.get("Accept-Language", "") +``` + +### Error response format for 401 +**Source:** `backend/deps/auth.py` lines 75-80 +**Apply to:** fgp mismatch raise +```python +raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Token fingerprint mismatch", + headers={"WWW-Authenticate": "Bearer"}, +) +``` +All 401 raises in `get_current_user` include `headers={"WWW-Authenticate": "Bearer"}`. + +--- + +## No Analog Found + +All four files have close analogs. No gaps. + +--- + +## Metadata + +**Analog search scope:** `backend/services/`, `backend/deps/`, `backend/api/`, `backend/tests/` +**Files scanned:** 5 (services/auth.py, deps/auth.py, api/auth.py, tests/test_auth_deps.py, tests/test_auth_es256.py, tests/conftest.py) +**Pattern extraction date:** 2026-06-06 diff --git a/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-RESEARCH.md b/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-RESEARCH.md new file mode 100644 index 0000000..57dd9e0 --- /dev/null +++ b/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-RESEARCH.md @@ -0,0 +1,469 @@ +# Phase 7.4: Security — Token Fingerprinting / Token Binding - Research + +**Researched:** 2026-06-06 +**Domain:** JWT access token fingerprinting via HMAC-SHA256 claim +**Confidence:** HIGH + +--- + + +## User Constraints (from CONTEXT.md) + +### Locked Decisions + +- **D-01:** Use `settings.secret_key` (`SECRET_KEY` env var) as the HMAC key — no new env var. +- **D-02:** Missing `User-Agent` or `Accept-Language` falls back to `""`. The `fgp` claim is always computed and always validated — no skip path. +- **D-03:** Fingerprint mismatch raises HTTP 401 immediately with `detail="Token fingerprint mismatch"`. No soft mode. +- **D-04:** Helper `_compute_fgp(user_agent: str, accept_lang: str) -> str` defined in `backend/services/auth.py` (module-private). Returns `hmac.new(settings.secret_key.encode(), (user_agent + accept_lang).encode(), sha256).hexdigest()[:16]`. +- **D-05:** `create_access_token(user_id, role)` gains `user_agent: str = ""` and `accept_lang: str = ""` parameters. Default empty strings preserve backward compatibility with existing tests. +- **D-06:** fgp validation in `get_current_user` placed after the `user_nbf` block. If `fgp_claim` is empty (old token without claim) → allow (graceful migration). If non-empty and mismatch → 401. + +### Claude's Discretion + +None stated. + +### Deferred Ideas (OUT OF SCOPE) + +- Key rotation process for `SECRET_KEY` +- Per-request fingerprint rotation / device key pinning + + +--- + +## Summary + +Phase 7.4 is a pure backend code change with no schema migrations, no new endpoints, and no frontend work. The change set is small and surgically scoped to three files: + +1. `backend/services/auth.py` — add `_compute_fgp` helper, extend `create_access_token` signature, embed `fgp` claim in payload. +2. `backend/deps/auth.py` — add fgp validation block after the `user_nbf` check. +3. `backend/api/auth.py` — pass request headers to `create_access_token` at the two call sites (login and refresh handlers). +4. `backend/tests/test_auth_fgp.py` — new test file with four test cases following Phase 7.2/7.3 patterns. + +No new dependencies. No new environment variables. `hashlib` is already imported in `services/auth.py` (line 21). `hmac` is already imported (line 22). `request.headers` is already accessible in both call sites and in `get_current_user`. The conftest `_patch_es256_test_keys` autouse fixture already covers all tests. + +**Primary recommendation:** Three targeted edits to existing files plus one new test file — total diff should be under 60 lines of production code. + +--- + +## Architectural Responsibility Map + +| Capability | Primary Tier | Secondary Tier | Rationale | +|------------|-------------|----------------|-----------| +| fgp claim creation | API / Backend (service layer) | — | `_compute_fgp` in `services/auth.py`; pure Python, no FastAPI coupling | +| fgp claim embedding | API / Backend (service layer) | — | `create_access_token` builds the JWT payload | +| fgp claim validation | API / Backend (dependency layer) | — | `get_current_user` in `deps/auth.py` validates on every authenticated request | +| Header extraction | API / Backend (router/dep layer) | — | `request.headers.get(...)` idiomatic FastAPI pattern; already in both callers | +| HMAC key storage | Config / env | — | `settings.secret_key` — no new field needed | + +--- + +## Standard Stack + +No new packages. All required modules are already installed and imported. + +### Already Available +| Module | Location | Already Imported? | Notes | +|--------|----------|-------------------|-------| +| `hmac` | stdlib | Yes — `services/auth.py:22` [VERIFIED: read file] | Use `hmac.new(...)` (Python stdlib name is `hmac.new`) | +| `hashlib` | stdlib | Yes — `services/auth.py:21` [VERIFIED: read file] | `hashlib.sha256` used as digestmod | +| `hmac.compare_digest` | stdlib | Already used — `services/auth.py:419` [VERIFIED: read file] | Same pattern as backup code comparison | +| `request.headers` | FastAPI `Request` | `request: Request` already in `get_current_user` signature [VERIFIED: read file] | Pattern: `request.headers.get("User-Agent", "")` | + +### Installation + +No new packages to install. + +--- + +## Package Legitimacy Audit + +Not applicable — no new packages. + +--- + +## Architecture Patterns + +### System Architecture Diagram + +``` +POST /api/auth/login POST /api/auth/refresh + | | + | request.headers.get(...) | request.headers.get(...) + v v +services/auth.create_access_token(user_id, role, user_agent, accept_lang) + | + | _compute_fgp(user_agent, accept_lang) -> hexdigest[:16] + | payload["fgp"] = fgp_value + | jwt.encode(payload, private_pem, algorithm="ES256") + v + JWT access token (fgp claim embedded) + +Every authenticated request: + | + v +deps/auth.get_current_user(request, credentials, session) + | + | decode_access_token -> payload + | user_nbf check (Phase 7.2 block, lines 62–85) + | + | [NEW] fgp_claim = payload.get("fgp", "") + | if fgp_claim: + | fgp_actual = _compute_fgp(request.headers.get(...), request.headers.get(...)) + | if not hmac.compare_digest(fgp_claim, fgp_actual): + | raise HTTP 401 "Token fingerprint mismatch" + | + v + uuid.UUID(payload["sub"]) -> User lookup +``` + +### Recommended Project Structure + +No new files in `src/`. One new test file: + +``` +backend/ +├── services/auth.py # add _compute_fgp + extend create_access_token +├── deps/auth.py # add fgp validation block after user_nbf +├── api/auth.py # update 2 call sites: login + refresh +└── tests/ + └── test_auth_fgp.py # new file, 4 test cases +``` + +### Pattern 1: fgp helper (_compute_fgp) + +**What:** Module-private function in `services/auth.py`, placed before `create_access_token`. +**When to use:** Called from `create_access_token` (at issuance) and referenced by `get_current_user` via import. + +```python +# Source: CONTEXT.md D-04 +import hmac as _hmac_mod # avoid shadowing module if 'hmac' name is also used as variable +import hashlib + +def _compute_fgp(user_agent: str, accept_lang: str) -> str: + """Return 16-char hex fingerprint of User-Agent + Accept-Language (D-04).""" + return _hmac_mod.new( + settings.secret_key.encode(), + (user_agent + accept_lang).encode(), + hashlib.sha256, + ).hexdigest()[:16] +``` + +**Note on import name:** `hmac` is already imported at line 22 of `services/auth.py` as `import hmac`. The `hmac` module exposes `.new()` directly. No rename needed — `hmac.new(...)` works. [VERIFIED: stdlib docs, `hmac` module] + +### Pattern 2: create_access_token signature extension + +**What:** Add two keyword parameters with empty-string defaults to `create_access_token`. +**Exact current signature (line 87):** `def create_access_token(user_id: str, role: str) -> str:` [VERIFIED: read file] + +Updated signature: +```python +def create_access_token( + user_id: str, + role: str, + user_agent: str = "", + accept_lang: str = "", +) -> str: +``` + +Inside the function, add `"fgp": _compute_fgp(user_agent, accept_lang)` to the `payload` dict alongside existing claims (`sub`, `role`, `typ`, `iat`, `exp`, `jti`). [VERIFIED: payload dict at lines 93–100 of services/auth.py] + +### Pattern 3: fgp validation block in get_current_user + +**Placement:** After line 85 (end of `user_nbf` block), before line 87 (`uuid.UUID(payload["sub"])`). [VERIFIED: read deps/auth.py] + +**Structural model — copy the user_nbf block pattern** (try / except HTTPException: raise / except Exception: log + fail-open), but with a critical difference from NBF: the fgp mismatch must NOT be swallowed by the broad except. The mismatch raise is an explicit `HTTPException` inside the try block, which the `except HTTPException: raise` guard re-raises correctly. + +```python +# ── fgp check (D-06, Phase 7.4) ──────────────────────────────────────────── +fgp_claim = payload.get("fgp", "") +if fgp_claim: + fgp_actual = auth_service._compute_fgp( + request.headers.get("User-Agent", ""), + request.headers.get("Accept-Language", ""), + ) + if not hmac.compare_digest(fgp_claim, fgp_actual): + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Token fingerprint mismatch", + headers={"WWW-Authenticate": "Bearer"}, + ) +# ── end fgp check ─────────────────────────────────────────────────────────── +``` + +**Note:** `_compute_fgp` is a module-private function in `services/auth.py`. `deps/auth.py` already imports `from services import auth as auth_service` (line 32). Calling `auth_service._compute_fgp(...)` follows the existing import pattern. Alternatively, the function can be made public (no underscore), but keeping it private signals it is not part of the public service API. Either approach is fine — the planner should decide. [ASSUMED: naming convention choice] + +### Pattern 4: Caller updates in api/auth.py + +**Login handler (line 290):** [VERIFIED: read api/auth.py] +```python +# Before: +access_token = auth_service.create_access_token(str(user.id), user.role) + +# After: +access_token = auth_service.create_access_token( + str(user.id), + user.role, + user_agent=request.headers.get("User-Agent", ""), + accept_lang=request.headers.get("Accept-Language", ""), +) +``` + +**Refresh handler (line 364):** [VERIFIED: read api/auth.py] +```python +# Before: +access_token = auth_service.create_access_token(user_id_str, user.role) + +# After: +access_token = auth_service.create_access_token( + user_id_str, + user.role, + user_agent=request.headers.get("User-Agent", ""), + accept_lang=request.headers.get("Accept-Language", ""), +) +``` + +Both handlers already have `request: Request` in their signature. [VERIFIED: read api/auth.py lines 192–195 and 321–325] + +### Anti-Patterns to Avoid + +- **Calling `_compute_fgp` in `deps/auth.py` directly instead of via `auth_service`:** `deps/auth.py` already imports `auth_service` — use it for consistency. Do not add a second import of the function. +- **Wrapping the fgp `HTTPException` raise in a broad except:** The `except HTTPException: raise` guard at line 81 of `deps/auth.py` ensures the intentional 401 is never swallowed. The fgp check must be structured inside the same or a separate try block with the same guard. [VERIFIED: read deps/auth.py lines 81–84] +- **Treating `fgp_claim == ""` as a mismatch:** Empty fgp_claim means the token predates this phase — it must be allowed (graceful migration, D-06). +- **Using `==` instead of `hmac.compare_digest`:** Constant-time comparison is mandatory for all token comparisons (SEC-06, CLAUDE.md). + +--- + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Constant-time string comparison | Custom loop | `hmac.compare_digest` | SEC-06 — timing attack prevention; already established in codebase | +| HMAC computation | Manual SHA-256 + XOR | `hmac.new(..., hashlib.sha256)` | Correct keyed-HMAC semantics; raw SHA-256 is not an HMAC | + +--- + +## Common Pitfalls + +### Pitfall 1: fgp mismatch swallowed by broad except +**What goes wrong:** The fgp `HTTPException` is raised inside the try block but the broad `except Exception` catches it before `except HTTPException: raise` runs — only if the guard order is reversed. +**Why it happens:** Python evaluates except clauses in order; if `except Exception` appears before `except HTTPException`, the HTTPException is matched and swallowed. +**How to avoid:** The existing pattern in `deps/auth.py` already has `except HTTPException: raise` before `except Exception` (lines 81–84). Maintain this order. [VERIFIED: read file] +**Warning signs:** fgp mismatch returns 200 or 500 instead of 401. + +### Pitfall 2: test fixtures call create_access_token without new params — will fgp validation break? +**What goes wrong:** After Phase 7.4, all test-issued tokens will carry `fgp` computed from `("", "")` because `user_agent=""` and `accept_lang=""` are the defaults. Test requests sent without User-Agent/Accept-Language headers will also compute `_compute_fgp("", "")`. These will match — no test breakage. +**Why it happens:** Default parameters make the no-arg call `create_access_token(user_id, role)` emit `fgp = hmac("" + "")[:16]`. An unauthenticated test request with no headers also computes the same value. Match succeeds. +**How to avoid:** No action required for existing tests. The test for "wrong fgp" must explicitly issue a token with headers A then make a request with different headers B. +**Warning signs:** All existing auth tests still pass (expected); they should not require changes. + +### Pitfall 3: hmac.new vs hmac.HMAC +**What goes wrong:** `hmac.new(...)` is the correct stdlib constructor. Attempting `hmac.HMAC(...)` directly is not the intended public API. +**How to avoid:** Use `hmac.new(key, msg, digestmod)` exactly as specified in D-04. +**Warning signs:** `AttributeError: module 'hmac' has no attribute 'HMAC'` (HMAC is the class, new() is the factory). + +### Pitfall 4: _compute_fgp visibility across module boundary +**What goes wrong:** `deps/auth.py` calls `auth_service._compute_fgp(...)`. Python name mangling does NOT apply to module-level `_` names — they are accessible from other modules, just conventionally private. This will work fine. +**How to avoid:** No special handling needed; the underscore is a convention, not an enforcement. [VERIFIED: Python docs] + +### Pitfall 5: conftest fixtures will produce fgp-bearing tokens after Phase 7.4 +**What goes wrong:** After the change, `create_access_token(str(user_id), "user")` in conftest (and all test files) will embed `fgp` = `hmac("", "")[:16]`. Requests made by `auth_client` or `async_client` without `User-Agent`/`Accept-Language` headers compute the same fingerprint → 200. This is the intended behaviour. +**Warning signs:** If a test explicitly sets `User-Agent` in its request headers, the token issued by the fixture (with `user_agent=""`) will mismatch the request headers → 401. This would only affect tests that set custom `User-Agent` headers, which currently none do. [VERIFIED: grep of conftest and test_auth_deps.py] + +--- + +## Code Examples + +### Complete fgp Helper (from CONTEXT.md D-04) +```python +# Source: CONTEXT.md D-04 (canonical) +def _compute_fgp(user_agent: str, accept_lang: str) -> str: + """Return 16-char hex fingerprint binding a token to its client context (D-04).""" + return hmac.new( + settings.secret_key.encode(), + (user_agent + accept_lang).encode(), + hashlib.sha256, + ).hexdigest()[:16] +``` + +### Test Case Structure (4 required by CONTEXT.md) +```python +# Source: CONTEXT.md §Specific Ideas + +# Test 1 — correct fgp → 200 +token = create_access_token(user_id, "user", user_agent="Mozilla/5.0", accept_lang="en") +resp = await auth_client.get( + "/test/me", + headers={ + "Authorization": f"Bearer {token}", + "User-Agent": "Mozilla/5.0", + "Accept-Language": "en", + }, +) +assert resp.status_code == 200 + +# Test 2 — wrong fgp → 401 +token = create_access_token(user_id, "user", user_agent="Mozilla/5.0", accept_lang="en") +resp = await auth_client.get( + "/test/me", + headers={ + "Authorization": f"Bearer {token}", + "User-Agent": "different-agent", # mismatch + "Accept-Language": "en", + }, +) +assert resp.status_code == 401 +assert resp.json()["detail"] == "Token fingerprint mismatch" + +# Test 3 — token without fgp claim → 200 (migration grace) +# Manually craft a token without "fgp" in the payload +payload_no_fgp = { + "sub": str(user_id), "role": "user", "typ": "access", + "iat": ..., "exp": ..., "jti": str(uuid.uuid4()), +} +token_no_fgp = jwt.encode(payload_no_fgp, private_pem, algorithm="ES256") +resp = await auth_client.get("/test/me", headers={"Authorization": f"Bearer {token_no_fgp}"}) +assert resp.status_code == 200 + +# Test 4 — missing User-Agent → empty-string binding works +token = create_access_token(user_id, "user") # user_agent="", accept_lang="" +resp = await auth_client.get( + "/test/me", + headers={"Authorization": f"Bearer {token}"}, + # No User-Agent, no Accept-Language — both default to "" +) +assert resp.status_code == 200 +``` + +--- + +## State of the Art + +| Old Approach | Current Approach | When Changed | Impact | +|--------------|------------------|--------------|--------| +| Access tokens had no device binding | Tokens carry `fgp` HMAC claim | Phase 7.4 | Stolen tokens can only be replayed from same UA/language context | + +--- + +## Detailed File Change Map + +| File | Change | Lines affected (approx) | +|------|--------|-------------------------| +| `backend/services/auth.py` | Add `_compute_fgp` helper (6 lines) before `create_access_token`; extend `create_access_token` signature (+2 params); add `"fgp"` to payload dict (+1 line) | ~10 lines added | +| `backend/deps/auth.py` | Add fgp validation block after line 85 | ~9 lines added | +| `backend/api/auth.py` | Update login call site (line 290) and refresh call site (line 364) to pass headers | ~8 lines changed | +| `backend/tests/test_auth_fgp.py` | New file: 4 test functions + 1 helper fixture | ~100 lines new | + +**Total production code change: ~27 lines.** Well within the ≤50 line bug-fix ceiling for any single plan. + +--- + +## Validation Architecture + +### Test Framework +| Property | Value | +|----------|-------| +| Framework | pytest + pytest-asyncio | +| Config file | `backend/pytest.ini` (or `pyproject.toml`) | +| Quick run command | `pytest tests/test_auth_fgp.py -v` | +| Full suite command | `pytest -v` | + +### Phase Requirements → Test Map + +| Behaviour | Test ID | Test Type | Automated Command | +|-----------|---------|-----------|-------------------| +| Token with correct fgp → 200 | FGP-01 | integration | `pytest tests/test_auth_fgp.py::test_fgp_match_returns_200 -x` | +| Token with wrong fgp → 401 | FGP-02 | integration | `pytest tests/test_auth_fgp.py::test_fgp_mismatch_returns_401 -x` | +| Token without fgp claim → 200 (migration grace) | FGP-03 | integration | `pytest tests/test_auth_fgp.py::test_no_fgp_claim_allowed -x` | +| Missing headers → empty-string binding works | FGP-04 | integration | `pytest tests/test_auth_fgp.py::test_missing_headers_empty_string_binding -x` | + +### Sampling Rate +- **Per task commit:** `pytest tests/test_auth_fgp.py tests/test_auth_deps.py -x` +- **Per wave merge:** `pytest -v` (all 411+ tests) +- **Phase gate:** Full suite green before `/gsd:verify-work` + +### Wave 0 Gaps +- [ ] `tests/test_auth_fgp.py` — new file, covers FGP-01..04. Must be created in Wave 0 as xfail stubs before implementation. + +--- + +## Security Domain + +### Applicable ASVS Categories + +| ASVS Category | Applies | Standard Control | +|---------------|---------|-----------------| +| V2 Authentication | yes | `hmac.compare_digest` for all fingerprint comparison | +| V3 Session Management | yes | fgp binds access token to originating client context | +| V5 Input Validation | yes | Headers extracted via `.get(..., "")` — no injection possible; HMAC treats as opaque bytes | +| V6 Cryptography | yes | `hmac.new(..., hashlib.sha256)` — standard keyed HMAC, not custom | + +### Known Threat Patterns + +| Pattern | STRIDE | Standard Mitigation | +|---------|--------|---------------------| +| Stolen access token replay from different device | Spoofing / Elevation | fgp mismatch → HTTP 401 | +| Timing attack on fingerprint comparison | Information Disclosure | `hmac.compare_digest` (constant-time) | +| Empty-string collision (all CLI clients share one fingerprint) | Spoofing | Accepted by D-02 — deliberate design; equivalent to pre-Phase-7.4 security for CLI clients | + +--- + +## Open Questions (RESOLVED) + +1. **`_compute_fgp` visibility: private (`_compute_fgp`) vs public (`compute_fgp`)?** + - What we know: `deps/auth.py` needs to call it; it is currently planned as `_compute_fgp` (module-private by convention). + - What's unclear: The CONTEXT.md specifies `_compute_fgp`. Calling `auth_service._compute_fgp` from `deps/auth.py` works but is unconventional. + - Recommendation: Keep the underscore per D-04; `auth_service._compute_fgp` is acceptable in this tightly coupled internal boundary. Alternatively, expose as `compute_fgp` (no underscore) for cleaner inter-module use — either is consistent with the codebase. The planner should pick one and document it. [ASSUMED: naming decision] + - **RESOLVED:** Keep private (`_compute_fgp`) per D-04. The underscore convention signals it is not part of the public service API. `auth_service._compute_fgp(...)` in `deps/auth.py` is acceptable at this tightly coupled internal boundary. + +2. **Should the fgp check be inside the existing user_nbf try block or in its own try block?** + - What we know: The user_nbf block is a try/except with fail-open on Redis errors (D-04 of Phase 7.2). The fgp check has no external I/O — it cannot fail due to infrastructure. + - What's unclear: Structurally, putting fgp inside the same try block would work, but it conflates two different concerns. A separate, unconditional if-block (no try/except needed) is cleaner. + - Recommendation: Place fgp as a plain `if fgp_claim:` block AFTER the user_nbf try/except ends (after line 85). No try/except wrapper needed for fgp — it is pure computation, not I/O. [ASSUMED] + - **RESOLVED:** Place as a standalone `if fgp_claim:` block AFTER the user_nbf try/except ends (after line 85). No try/except wrapper — pure computation, no I/O. The existing `except HTTPException: raise` guard in any wrapping try block would still catch the mismatch 401 correctly, but keeping it outside is cleaner. + +--- + +## Environment Availability + +Step 2.6: SKIPPED — this phase is a pure backend code change with no external dependencies beyond the existing Python stdlib and already-installed PyJWT. No new tools, services, or runtimes required. + +--- + +## Assumptions Log + +| # | Claim | Section | Risk if Wrong | +|---|-------|---------|---------------| +| A1 | `_compute_fgp` naming convention (underscore vs public) | Open Questions #1 | Cosmetic only — either works at runtime | +| A2 | fgp check is a plain if-block, not wrapped in try/except | Open Questions #2 | Cosmetic only — try/except around pure computation is harmless but unnecessary | + +--- + +## Sources + +### Primary (HIGH confidence) +- `backend/services/auth.py` — verified exact current signatures, imports, payload structure, existing `hmac.compare_digest` usage [VERIFIED: read file] +- `backend/deps/auth.py` — verified `get_current_user` structure, user_nbf block lines 62–85, `request: Request` in signature, `except HTTPException: raise` guard [VERIFIED: read file] +- `backend/api/auth.py` — verified login call site (line 290), refresh call site (line 364), both have `request: Request` [VERIFIED: read file] +- `backend/config.py` — verified `secret_key: str = "CHANGEME"` at line 31 [VERIFIED: read file] +- `backend/tests/conftest.py` — verified autouse `_patch_es256_test_keys` fixture, `auth_user`/`admin_user` fixtures call `create_access_token(str(user_id), role)` [VERIFIED: read file] +- `backend/tests/test_auth_deps.py` — verified Phase 7.2 NBF test pattern (FakeRedis, make_test_app, `_create_user` helper) [VERIFIED: read file] +- `backend/tests/test_auth_es256.py` — verified Phase 7.3 test pattern (autouse `es256_keys` fixture, test naming convention) [VERIFIED: read file] +- CONTEXT.md — locked decisions D-01 through D-06 [VERIFIED: read file] + +### Secondary (MEDIUM confidence) +- `grep -rn "create_access_token"` output — confirmed only two production call sites exist (`api/auth.py:290` login and `api/auth.py:364` refresh); all other hits are test files [VERIFIED: bash grep] +- Python stdlib `hmac` module — `hmac.new(key, msg, digestmod)` is the correct constructor [CITED: docs.python.org/3/library/hmac.html] + +--- + +## Metadata + +**Confidence breakdown:** +- File change map: HIGH — read all target files directly +- Standard stack: HIGH — no new packages; all modules verified present +- Test patterns: HIGH — read Phase 7.2/7.3 test files directly +- Architecture: HIGH — CONTEXT.md fully specifies implementation +- Pitfalls: HIGH — derived from direct code inspection + +**Research date:** 2026-06-06 +**Valid until:** N/A — this is a single-session phase with no external dependencies diff --git a/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-VALIDATION.md b/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-VALIDATION.md new file mode 100644 index 0000000..ada1030 --- /dev/null +++ b/.planning/phases/07.4-security-token-fingerprinting-token-binding-inserted/07.4-VALIDATION.md @@ -0,0 +1,65 @@ +--- +phase: 7.4 +slug: 07.4-security-token-fingerprinting-token-binding-inserted +status: draft +nyquist_compliant: false +wave_0_complete: false +created: 2026-06-06 +--- + +# Phase 7.4 — Validation Strategy + +> Per-phase validation contract for feedback sampling during execution. + +--- + +## Test Infrastructure + +| Property | Value | +|----------|-------| +| **Framework** | pytest + pytest-asyncio | +| **Config file** | `backend/pytest.ini` | +| **Quick run command** | `pytest tests/test_auth_fgp.py -v` | +| **Full suite command** | `pytest -v` | +| **Estimated runtime** | ~30 seconds | + +--- + +## Sampling Rate + +- **After every task commit:** Run `pytest tests/test_auth_fgp.py tests/test_auth_deps.py -x` +- **After every plan wave:** Run `pytest -v` +- **Before `/gsd:verify-work`:** Full suite must be green +- **Max feedback latency:** 30 seconds + +--- + +## Per-Task Verification Map + +| Task ID | Plan | Wave | Requirement | Threat Ref | Secure Behavior | Test Type | Automated Command | File Exists | Status | +|---------|------|------|-------------|------------|-----------------|-----------|-------------------|-------------|--------| +| FGP-01 | 01 | 0 | D-04/D-05 | T-7.4-01 | xfail stubs created for all 4 test cases | unit | `pytest tests/test_auth_fgp.py -v` | ❌ W0 | ⬜ pending | +| FGP-02 | 01 | 1 | D-04 | T-7.4-01 | `_compute_fgp` helper returns 16-char hex HMAC | unit | `pytest tests/test_auth_fgp.py::test_fgp_match_returns_200 -x` | ✅ | ⬜ pending | +| FGP-03 | 01 | 1 | D-05 | — | `create_access_token` embeds `fgp` claim | unit | `pytest tests/test_auth_fgp.py::test_fgp_match_returns_200 -x` | ✅ | ⬜ pending | +| FGP-04 | 01 | 1 | D-06 | T-7.4-01 | Correct fgp → 200 | integration | `pytest tests/test_auth_fgp.py::test_fgp_match_returns_200 -x` | ✅ | ⬜ pending | +| FGP-05 | 01 | 1 | D-03/D-06 | T-7.4-01 | Wrong fgp → 401 "Token fingerprint mismatch" | integration | `pytest tests/test_auth_fgp.py::test_fgp_mismatch_returns_401 -x` | ✅ | ⬜ pending | +| FGP-06 | 01 | 1 | D-06 | — | Token without fgp claim → 200 (migration grace) | integration | `pytest tests/test_auth_fgp.py::test_no_fgp_claim_allowed -x` | ✅ | ⬜ pending | +| FGP-07 | 01 | 1 | D-02 | — | Missing headers → empty-string binding works | integration | `pytest tests/test_auth_fgp.py::test_missing_headers_empty_string_binding -x` | ✅ | ⬜ pending | + +*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky* + +--- + +## Wave 0 Requirements + +- [ ] `tests/test_auth_fgp.py` — new file with 4 xfail stubs covering FGP-01..04 (correct match, wrong fgp, no fgp claim, missing headers) + +*Existing infrastructure covers all other requirements (conftest autouse fixtures, pytest-asyncio, FakeRedis).* + +--- + +## Manual-Only Verifications + +| Behavior | Requirement | Why Manual | Test Instructions | +|----------|-------------|------------|-------------------| +| Full suite regression check | All prior phases | Confirm 0 regressions from signature changes | Run `pytest -v` and verify count ≥ prior passing total |