docs(07.2-02): complete Wave 1 plan — jti claim + user_nbf NBF check SUMMARY
- 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
This commit is contained in:
+170
@@ -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()),`
|
||||||
Reference in New Issue
Block a user