From c00c1fbaa7d663593cc0396262f0051c31ce9e39 Mon Sep 17 00:00:00 2001 From: curo1305 Date: Fri, 5 Jun 2026 19:15:01 +0200 Subject: [PATCH 1/3] feat(07.2-02): add jti UUID claim to create_access_token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Added "jti": str(uuid.uuid4()) to the access token payload in services/auth.py - Promoted Wave 0 xfail stub test_create_access_token_includes_jti_claim to PASS - Added test_create_access_token_jti_is_unique_per_call uniqueness guard - uuid module was already imported at module top — no new imports needed --- backend/services/auth.py | 1 + backend/tests/test_task2_auth_service.py | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/backend/services/auth.py b/backend/services/auth.py index 24ed573..a51da1b 100644 --- a/backend/services/auth.py +++ b/backend/services/auth.py @@ -95,6 +95,7 @@ def create_access_token(user_id: str, role: str) -> str: "typ": "access", "iat": now, "exp": now + timedelta(minutes=settings.access_token_expire_minutes), + "jti": str(uuid.uuid4()), } return jwt.encode(payload, settings.secret_key, algorithm="HS256") diff --git a/backend/tests/test_task2_auth_service.py b/backend/tests/test_task2_auth_service.py index d3502eb..2bb6f9e 100644 --- a/backend/tests/test_task2_auth_service.py +++ b/backend/tests/test_task2_auth_service.py @@ -194,15 +194,24 @@ async def test_rotate_revoked_token_raises(db_session): await rotate_refresh_token(db_session, raw) -@pytest.mark.xfail(strict=False, reason="Phase 7.2 Wave 1 — jti claim not yet added to create_access_token") def test_create_access_token_includes_jti_claim(): """create_access_token payload must include a 'jti' key with a UUID-format string value.""" import uuid from services.auth import create_access_token, decode_access_token t = create_access_token("test-uid", "user") - payload = decode_access_token(t) - assert "jti" in payload, "jti claim missing from access token payload" - uuid.UUID(payload["jti"]) # raises ValueError if not a valid UUID + decoded = decode_access_token(t) + assert "jti" in decoded, "jti claim missing from access token payload" + assert isinstance(decoded["jti"], str), "jti claim must be a string" + uuid.UUID(decoded["jti"]) # raises ValueError if not a valid UUID + + +def test_create_access_token_jti_is_unique_per_call(): + """Two consecutive create_access_token calls must produce different jti values.""" + import uuid + from services.auth import create_access_token, decode_access_token + t1 = decode_access_token(create_access_token("test-uid", "user")) + t2 = decode_access_token(create_access_token("test-uid", "user")) + assert t1["jti"] != t2["jti"], "jti values must differ between calls" @pytest.mark.asyncio From 30ad9fd01510ed31f5226885c06d6cf16935bd49 Mon Sep 17 00:00:00 2001 From: curo1305 Date: Fri, 5 Jun 2026 19:19:06 +0200 Subject: [PATCH 2/3] security(07.2-02): add user_nbf Redis NBF check to get_current_user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Added import logging + _logger = logging.getLogger(__name__) to deps/auth.py - Inserted user_nbf Redis check block after decode_access_token in get_current_user - Check reads user_nbf:{payload['sub']} from Redis; rejects if payload['iat'] < stored timestamp - Uses strict < comparison (token issued exactly at event second is allowed — D-02) - Handles both bytes and str returns from Redis for FakeRedis/aioredis compatibility - except HTTPException: raise guard precedes broad except Exception (T-7.2-02 Pitfall 1) - Broad except logs warning and fails open (D-04 — Redis outage must not block requests) - Promoted all 3 Wave 0 NBF xfail stubs to passing assertions in test_auth_deps.py --- backend/deps/auth.py | 28 ++++++++++++++++++++++++++++ backend/tests/test_auth_deps.py | 15 ++++++--------- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/backend/deps/auth.py b/backend/deps/auth.py index 75907d9..ad6a5e2 100644 --- a/backend/deps/auth.py +++ b/backend/deps/auth.py @@ -20,6 +20,7 @@ Usage in route handlers: ): ... """ +import logging import uuid from fastapi import Depends, HTTPException, Request, status @@ -30,6 +31,8 @@ from db.models import User from deps.db import get_db from services import auth as auth_service +_logger = logging.getLogger(__name__) + # HTTPBearer parses the Authorization: Bearer header. # auto_error=True (default) raises 403 if no Authorization header is present. security = HTTPBearer() @@ -56,6 +59,31 @@ async def get_current_user( headers={"WWW-Authenticate": "Bearer"}, ) from exc + # ── user_nbf check (D-02, D-03, D-04) ────────────────────────────────────── + # Reject tokens issued before a security event (password change, TOTP enroll, + # deactivation). The user_nbf:{user_id} key is written by Wave 2 (Plan 03). + # Fail-open on Redis errors (D-04): a Redis outage must never deny service. + # CRITICAL: the `except HTTPException: raise` guard MUST precede the broad + # `except Exception` — otherwise the intentional 401 is swallowed (T-7.2-02). + try: + redis_client = request.app.state.redis + nbf_bytes = await redis_client.get(f"user_nbf:{payload['sub']}") + if nbf_bytes is not None: + # Tolerate both bytes and str returns (FakeRedis in tests may return + # either depending on what was stored; real aioredis returns bytes). + 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 ────────────────────────────────────────────────────── + try: user_uuid = uuid.UUID(payload["sub"]) except (KeyError, ValueError) as exc: diff --git a/backend/tests/test_auth_deps.py b/backend/tests/test_auth_deps.py index d27dc3b..a8fc9e2 100644 --- a/backend/tests/test_auth_deps.py +++ b/backend/tests/test_auth_deps.py @@ -186,7 +186,6 @@ def test_deps_auth_has_http_403(): # by replacing `assert False, "stub"` with the actual assertion logic. -@pytest.mark.xfail(strict=False, reason="Phase 7.2 Wave 1 — NBF check not yet implemented in get_current_user") @pytest.mark.asyncio async def test_get_current_user_rejects_token_when_iat_before_user_nbf(auth_client, db_session): """Token with iat < user_nbf in Redis should return 401 'Session invalidated'.""" @@ -204,11 +203,11 @@ async def test_get_current_user_rejects_token_when_iat_before_user_nbf(auth_clie resp = await auth_client.get( "/test/me", headers={"Authorization": f"Bearer {token}"} ) - # Wave 1 will implement the NBF check; this assertion is the target behaviour - assert False, "stub — Wave 1 will fill in: assert resp.status_code == 401 and 'Session invalidated' in resp.json()['detail']" + assert resp.status_code == 401 + assert resp.json()["detail"] == "Session invalidated" + assert resp.headers.get("WWW-Authenticate") == "Bearer" -@pytest.mark.xfail(strict=False, reason="Phase 7.2 Wave 1 — NBF check not yet implemented in get_current_user") @pytest.mark.asyncio async def test_get_current_user_allows_token_when_iat_after_user_nbf(auth_client, db_session): """Token with iat > user_nbf in Redis should return 200 (token is valid).""" @@ -226,11 +225,9 @@ async def test_get_current_user_allows_token_when_iat_after_user_nbf(auth_client resp = await auth_client.get( "/test/me", headers={"Authorization": f"Bearer {token}"} ) - # Wave 1 will implement the NBF check; this assertion is the target behaviour - assert False, "stub — Wave 1 will fill in: assert resp.status_code == 200" + assert resp.status_code == 200 -@pytest.mark.xfail(strict=False, reason="Phase 7.2 Wave 1 — NBF check not yet implemented in get_current_user") @pytest.mark.asyncio async def test_get_current_user_failopen_on_redis_error(auth_client, db_session): """Redis error during NBF check must fail-open (return 200, not crash).""" @@ -249,5 +246,5 @@ async def test_get_current_user_failopen_on_redis_error(auth_client, db_session) resp = await auth_client.get( "/test/me", headers={"Authorization": f"Bearer {token}"} ) - # Wave 1 will implement fail-open (D-04): assert resp.status_code == 200 - assert False, "stub — Wave 1 will fill in: assert resp.status_code == 200" + # D-04: Redis outage must fail-open; request proceeds normally + assert resp.status_code == 200 From 61199752b505b8a13ac0a1e81704d45939f39b74 Mon Sep 17 00:00:00 2001 From: curo1305 Date: Fri, 5 Jun 2026 19:20:42 +0200 Subject: [PATCH 3/3] =?UTF-8?q?docs(07.2-02):=20complete=20Wave=201=20plan?= =?UTF-8?q?=20=E2=80=94=20jti=20claim=20+=20user=5Fnbf=20NBF=20check=20SUM?= =?UTF-8?q?MARY?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - jti UUID claim in create_access_token (D-01) - user_nbf Redis NBF check in get_current_user with fail-open + re-raise guard (D-02, D-03, D-04, T-7.2-02) - 4 Wave 0 xfail stubs promoted to PASS; 29 tests passing in target suites --- .../07.2-02-SUMMARY.md | 170 ++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 .planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-02-SUMMARY.md diff --git a/.planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-02-SUMMARY.md b/.planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-02-SUMMARY.md new file mode 100644 index 0000000..9dded27 --- /dev/null +++ b/.planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-02-SUMMARY.md @@ -0,0 +1,170 @@ +--- +phase: 07.2-security-jti-claim-redis-access-token-revocation-inserted +plan: "02" +subsystem: auth +tags: + - security + - jwt + - redis + - access-control + - wave-1 + +dependency_graph: + requires: + - "07.2-01: FakeRedis mounted on test_auth_deps + xfail stubs" + provides: + - "jti UUID claim in every access token issued by create_access_token" + - "user_nbf Redis NBF check in get_current_user with fail-open + HTTPException re-raise guard" + - "Wave 2 write sites (Plan 03) can now write user_nbf:{user_id} and the check will fire" + affects: + - "backend/services/auth.py" + - "backend/deps/auth.py" + - "backend/tests/test_task2_auth_service.py" + - "backend/tests/test_auth_deps.py" + +tech_stack: + added: [] + patterns: + - "jti=str(uuid.uuid4()) inserted into access token payload — uuid already imported, no new deps" + - "user_nbf Redis check in get_current_user: read-then-compare with fail-open and HTTPException re-raise guard" + - "Bytes/str tolerance for FakeRedis test fixtures: decode() only if isinstance bytes/bytearray" + - "except HTTPException: raise MUST precede except Exception — T-7.2-02 Pitfall 1 guard" + +key_files: + created: [] + modified: + - backend/services/auth.py + - backend/deps/auth.py + - backend/tests/test_task2_auth_service.py + - backend/tests/test_auth_deps.py + +decisions: + - "jti added as the last key in the payload dict (after exp) for grep-ability; no function signature change" + - "NBF comparison uses strict < not <= (token issued exactly at event second is allowed — edge case per D-02)" + - "Bytes/str tolerance guard added: nbf_bytes.decode() if isinstance(bytes|bytearray) else nbf_bytes — FakeRedis may return str in tests" + - "No get_redis FastAPI dependency added — direct access via request.app.state.redis per D-03" + - "get_current_admin and get_regular_user not touched — they inherit via Depends(get_current_user)" + - "test_extract_docx failure confirmed pre-existing (missing python-docx module in local env) — unrelated to this plan" + +metrics: + duration: "600s (10m)" + completed_date: "2026-06-05T18:00:00Z" + tasks_completed: 2 + tasks_total: 2 + files_changed: 4 +--- + +# Phase 07.2 Plan 02: jti Claim + user_nbf NBF Check Summary + +**One-liner:** jti UUID claim added to all access tokens; user_nbf Redis NBF check with fail-open and HTTPException re-raise guard wired into get_current_user, closing the 15-minute revocation window. + +## What Was Built + +### Task 1: Add jti claim to create_access_token (TDD) + +**services/auth.py** — one-line surgical edit to `create_access_token`: + +```python +"jti": str(uuid.uuid4()), +``` + +Added as the last key after `exp` in the payload dict. The `uuid` module was already imported at module top (line 25) — no new imports. PyJWT encodes the `jti` key verbatim into the signed token and decodes it back as a plain string after verification. + +**Tests promoted from xfail:** + +- `test_create_access_token_includes_jti_claim` — verifies `"jti"` key present, value is a `str`, and parseable as `uuid.UUID` +- `test_create_access_token_jti_is_unique_per_call` — verifies two consecutive calls produce different `jti` values (new test) + +Both promoted to PASS. All existing tests (`test_create_access_token_jwt_format`, `test_decode_access_token_valid`) remain PASSED — no regressions. + +### Task 2: Add user_nbf Redis check to get_current_user (TDD) + +**deps/auth.py** — module-level additions: + +```python +import logging +_logger = logging.getLogger(__name__) +``` + +**get_current_user** — new block inserted after `decode_access_token` try/except and before the `uuid.UUID(payload["sub"])` parse: + +```python +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) +``` + +**Tests promoted from xfail:** + +- `test_get_current_user_rejects_token_when_iat_before_user_nbf` — sets future `user_nbf`, verifies HTTP 401 + `"Session invalidated"` + `WWW-Authenticate: Bearer` +- `test_get_current_user_allows_token_when_iat_after_user_nbf` — sets past `user_nbf`, verifies HTTP 200 +- `test_get_current_user_failopen_on_redis_error` — replaces `app.state.redis` with `_BrokenRedis` that raises `RuntimeError`, verifies HTTP 200 (not 5xx, not 401) + +All 3 promoted to PASS. All existing tests (7 baseline auth-dep tests) still PASSED. Wave 2 write stubs in `test_auth_api.py` (3 XFAIL) and admin stubs in `test_admin_api.py` (2 XFAIL) correctly remain XFAIL — they are Wave 2's job. + +## Verification + +Full suite after this plan: **382 passed, 12 xfailed** (1 pre-existing `test_extract_docx` failure from missing `python-docx` module in local env — confirmed pre-existing before Wave 0 baseline; unrelated to this plan's changes). + +Target file suites: `tests/test_auth_deps.py tests/test_task2_auth_service.py` — **29 passed, 0 xfailed, 0 failures**. + +## Commits + +| Task | Commit | Description | +|------|--------|-------------| +| 1 | c00c1fb | feat(07.2-02): add jti UUID claim to create_access_token | +| 2 | 30ad9fd | security(07.2-02): add user_nbf Redis NBF check to get_current_user | + +## Deviations from Plan + +None — plan executed exactly as written. + +## Known Stubs + +None — this plan contains no stubs. All 4 Wave 1 stubs (1 jti + 3 NBF-check) were promoted to fully-passing tests. The 5 remaining XFAIL tests in `test_auth_api.py` and `test_admin_api.py` are Wave 2 stubs created in Plan 01 — they are out of scope for this plan. + +## Patterns Established + +- **NBF check pattern in get_current_user**: read `user_nbf:{payload['sub']}` from `request.app.state.redis`; compare `payload["iat"] < int(nbf_str)` to reject; strict `<` not `<=` +- **HTTPException re-raise guard before broad catch**: `except HTTPException: raise` MUST precede `except Exception` — verified by line-number ordering grep. This is T-7.2-02 / Pitfall 1 and the single highest-impact correctness invariant in this plan. +- **bytes/str tolerance for FakeRedis**: `nbf_bytes.decode() if isinstance(nbf_bytes, (bytes, bytearray)) else nbf_bytes` before `int(...)` — ensures tests using `FakeRedis.set(..., value.encode())` work identically to production `aioredis` returning raw bytes. +- **Fail-open on Redis errors**: `except Exception as exc: _logger.warning(...)` — no re-raise; mirrors HIBP fail-open pattern in services/auth.py + +## Patterns to Avoid + +- Do NOT use `<=` in the NBF comparison — a token issued at the exact same second as the event is intentionally allowed (strictly-before semantic matches D-02) +- Do NOT add a `get_redis` FastAPI dependency — access must be direct via `request.app.state.redis` per D-03 +- Do NOT touch `get_current_admin` or `get_regular_user` — they inherit the NBF check transparently via `Depends(get_current_user)` +- Do NOT move the broad `except Exception` before `except HTTPException: raise` — this would swallow the intentional 401 and let revoked tokens through (T-7.2-02) + +## Provides (for Wave 2) + +Wave 2 (Plan 03) can now write `user_nbf:{user_id} = int(time.time())` with `ex=900` from the four security-event handlers (`change_password`, `enable_totp`, `disable_totp`, admin deactivation) and the check will fire on the very next request from any pre-event access token. The read side is complete; Wave 2 only needs to add the write sites. + +## Threat Flags + +None — no new network endpoints, auth paths, or schema changes introduced. The changes reduce the attack surface by closing the 15-minute revocation window (T-7.2-01 mitigated). + +## Self-Check: PASSED + +- [x] `backend/services/auth.py` modified with `"jti": str(uuid.uuid4())` — confirmed by grep +- [x] `backend/deps/auth.py` modified with NBF check block — confirmed by grep (user_nbf, logging, Session invalidated, request.app.state.redis) +- [x] `backend/tests/test_task2_auth_service.py` modified — xfail removed, real assertions, new uniqueness test +- [x] `backend/tests/test_auth_deps.py` modified — 3 xfail stubs promoted to passing assertions +- [x] Commit c00c1fb exists: `git log --oneline | grep c00c1fb` +- [x] Commit 30ad9fd exists: `git log --oneline | grep 30ad9fd` +- [x] 29 tests pass in target suites: `pytest tests/test_auth_deps.py tests/test_task2_auth_service.py` +- [x] HTTPException re-raise guard ordering verified: except HTTPException at line 81 < except Exception at line 83 +- [x] jti pattern grep returns 1 match: `"jti": str(uuid.uuid4()),`