--- phase: 07.2-security-jti-claim-redis-access-token-revocation-inserted verified: 2026-06-06T00:00:00Z status: gaps_found score: 8/9 must-haves verified overrides_applied: 0 gaps: - truth: "password_reset_confirm writes user_nbf to Redis — pre-reset access tokens are blocked within TTL" status: failed reason: "password_reset_confirm (api/auth.py:700-746) revokes all refresh tokens but never writes user_nbf:{user_id} to Redis. Pre-reset access tokens remain valid for up to 15 minutes after a successful password-reset-link confirm. This is the identical gap the rest of Phase 7.2 was designed to close (T-7.2-01), and the code review (CR-02 in 07.2-REVIEW.md) explicitly flagged it." artifacts: - path: "backend/api/auth.py" issue: "password_reset_confirm (line 700) calls revoke_all_refresh_tokens at line 742 and then session.commit() at line 743 with no user_nbf Redis write in between" missing: - "Add `await request.app.state.redis.set(f'user_nbf:{user.id}', int(time.time()), ex=settings.access_token_expire_minutes * 60)` before session.commit() in password_reset_confirm" - "Add `request: Request` parameter to password_reset_confirm signature (currently absent — the handler has no Request parameter)" - "Add a test parallel to test_change_password_writes_user_nbf_to_redis covering password_reset_confirm" deferred: [] human_verification: - test: "Recovering session after password reset via email link" expected: "After POST /api/auth/password-reset/confirm with a valid token, any previously-issued access token returns HTTP 401 with detail 'Session invalidated' on the next authenticated request" why_human: "password_reset_confirm does not currently write user_nbf so this cannot be verified as passing; it requires manual end-to-end test after the gap is fixed" --- # Phase 07.2: JTI Claim + Redis NBF Revocation — Verification Report **Phase Goal:** JTI claim added to access tokens + Redis-based user_nbf revocation check closes the 15-minute access-token window after refresh-token revocation events **Verified:** 2026-06-06 **Status:** gaps_found **Re-verification:** No — initial verification --- ## Goal Achievement ### Observable Truths | # | Truth | Status | Evidence | |---|-------|--------|----------| | 1 | Every access token issued by `create_access_token` contains a unique `jti` UUID claim | VERIFIED | `services/auth.py:98` — `"jti": str(uuid.uuid4())` present in payload dict after `exp`; `import uuid` already at module top (line 25) | | 2 | `get_current_user` rejects any access token whose `iat` predates `user_nbf:{user_id}` in Redis with HTTP 401 | VERIFIED | `deps/auth.py:68-85` — full NBF check block present; reads `user_nbf:{payload['sub']}`; raises `HTTPException(401, "Session invalidated")` when `payload["iat"] < int(nbf_str)` | | 3 | `get_current_user` accepts tokens whose `iat` post-dates user_nbf (intended recovery path) | VERIFIED | Same block at `deps/auth.py:75` — comparison is strict `<` not `<=`; tokens issued after the event pass through | | 4 | A Redis outage during the NBF check does not block requests — fail-open with warning log | VERIFIED | `deps/auth.py:83-84` — broad `except Exception as exc: _logger.warning("Redis user_nbf check failed (fail-open): %s", exc)` with no re-raise | | 5 | The HTTPException raised by the NBF check is NOT swallowed by the fail-open broad-catch (Pitfall 1 guard) | VERIFIED | `deps/auth.py:81-82` — `except HTTPException: raise` at line 81 precedes `except Exception` at line 83; ordering confirmed by grep line numbers | | 6 | `change_password`, `enable_totp`, `disable_totp` each write `user_nbf:{user_id}` to Redis before `session.commit()` | VERIFIED | `api/auth.py:508-511` (change_password), `api/auth.py:607-611` (enable_totp via `redis_client`), `api/auth.py:653-657` (disable_totp); all use `settings.access_token_expire_minutes * 60` TTL | | 7 | Admin deactivation writes `user_nbf:{user_id}` only when `is_active=False` | VERIFIED | `api/admin.py:353-361` — write is strictly inside `if not body.is_active:` block; activation path at line 366 has no write | | 8 | All TTLs use `settings.access_token_expire_minutes * 60` (not hardcoded 900) | VERIFIED | All 4 write sites confirmed by grep: `api/auth.py:511,610,656` and `api/admin.py:360` — all use the derived expression | | 9 | `password_reset_confirm` writes `user_nbf` to close pre-reset access-token window | FAILED | `api/auth.py:700-746` — endpoint revokes refresh tokens (line 742) and commits (line 743) with no `user_nbf` write; has no `Request` parameter; pre-reset access tokens remain valid for up to 15 minutes | **Score:** 8/9 truths verified --- ### Required Artifacts | Artifact | Expected | Status | Details | |----------|----------|--------|---------| | `backend/services/auth.py` | `create_access_token` with `jti=str(uuid.uuid4())` | VERIFIED | Line 98: `"jti": str(uuid.uuid4()),` | | `backend/deps/auth.py` | `user_nbf` Redis check with fail-open + HTTPException re-raise guard | VERIFIED | Lines 62-85: full NBF check block; `import logging` at line 23; `_logger` at line 34 | | `backend/api/auth.py` | `user_nbf` write in `change_password`, `enable_totp`, `disable_totp` | VERIFIED | Lines 507-512, 606-611, 652-657 | | `backend/api/admin.py` | `user_nbf` write in deactivation handler only | VERIFIED | Lines 356-361; `import time` at line 26; `from config import settings` at line 31 | --- ### Key Link Verification | From | To | Via | Status | Details | |------|----|-----|--------|---------| | `services/auth.py create_access_token` | PyJWT encode payload | `"jti": str(uuid.uuid4())` in payload dict | WIRED | Line 98; uuid imported at module level line 25 | | `deps/auth.py get_current_user` | `request.app.state.redis` | `await redis_client.get(f"user_nbf:{payload['sub']}")` + int comparison | WIRED | Lines 69-75 | | `api/auth.py change_password` | `request.app.state.redis` | `await request.app.state.redis.set(f"user_nbf:{current_user.id}", ...)` | WIRED | Line 508-512 | | `api/auth.py enable_totp` | `request.app.state.redis` | `await redis_client.set(f"user_nbf:{current_user.id}", ...)` (redis_client in scope) | WIRED | Lines 607-611 | | `api/auth.py disable_totp` | `request.app.state.redis` | `await request.app.state.redis.set(f"user_nbf:{current_user.id}", ...)` | WIRED | Lines 653-657 | | `api/admin.py deactivation handler` | `request.app.state.redis` | `await request.app.state.redis.set(f"user_nbf:{user.id}", ...)` inside `if not body.is_active:` | WIRED | Lines 357-361 | | `api/auth.py password_reset_confirm` | `request.app.state.redis` | user_nbf write | NOT WIRED | Handler has no `Request` parameter and no Redis write; refresh-token revocation at line 742 is present but NBF write is absent | --- ### Anti-Patterns Found | File | Line | Pattern | Severity | Impact | |------|------|---------|----------|--------| | `backend/api/auth.py` | 700-746 | Missing `user_nbf` write after password reset confirm | BLOCKER | Pre-reset access tokens valid 15 min after successful reset; exactly the gap this phase closes for all other security events | | `backend/services/auth.py` | 100, 110 | `algorithm="HS256"` (symmetric) instead of `ES256` (asymmetric) | WARNING | CLAUDE.md Security Protocol mandates ES256; this is tracked and addressed in Phase 7.3 | Note on HS256 (CR-01 from code review): Phase 7.3 (`07.3-security-es256-algorithm-upgrade-inserted`) is already planned and has research/plan artifacts present in `.planning/phases/`. The HS256 finding is deferred to Phase 7.3. It is a WARNING here, not a BLOCKER for Phase 7.2's stated goal — but it is noted. --- ### Requirements Coverage | Requirement | Source Plan | Description | Status | Evidence | |-------------|-------------|-------------|--------|----------| | CONCERNS:JTI-CLAIM | 07.2-01, 07.2-02 | Every access token carries a `jti` UUID claim | SATISFIED | `services/auth.py:98` | | CONCERNS:JTI-REVOKE-REDIS | 07.2-01, 07.2-02, 07.2-03 | `user_nbf` Redis check closes 15-min revocation window on security events | PARTIALLY SATISFIED | Check implemented (deps/auth.py); 3 of 4 auth write sites present (change_password, enable_totp, disable_totp); admin deactivation present; `password_reset_confirm` missing | --- ### Human Verification Required #### 1. Post-password-reset access-token invalidation **Test:** After POST /api/auth/password-reset/confirm with a valid reset token, immediately use the previously-issued access token to call GET /api/auth/me. **Expected:** HTTP 401 with `{"detail": "Session invalidated"}` — the pre-reset token should be blocked within the Redis TTL window. **Why human:** The gap (missing `user_nbf` write in `password_reset_confirm`) means this currently fails; human verification needed after the fix is applied to confirm the full round-trip works. --- ### Gaps Summary One gap blocks the phase goal: **`password_reset_confirm` missing `user_nbf` write.** The phase goal is to close the 15-minute access-token window after revocation events. All four handlers named in the plans (change_password, enable_totp, disable_totp, admin deactivation) correctly write `user_nbf`. However, `password_reset_confirm` — a fifth security event that changes a user's password — revokes refresh tokens but does not write `user_nbf`. A user whose email is compromised can receive the reset link, confirm it, and the victim's live access token continues to work for up to 15 minutes. The code review (CR-02 in 07.2-REVIEW.md) identified this gap explicitly. The fix is surgical: add `request: Request` to the function signature and one `await request.app.state.redis.set(...)` call before `session.commit()`, matching the pattern in all other handlers. The gap is not a scope ambiguity: the phase goal is stated as closing the "15-minute access-token window after refresh-token revocation events," and a password reset is unambiguously such an event. The other four handlers demonstrate the project is aware of this pattern. --- _Verified: 2026-06-06_ _Verifier: Claude (gsd-verifier)_