3 plans (Wave 0 test scaffolding, Wave 1 jti+NBF check, Wave 2 user_nbf writes), VALIDATION.md, RESEARCH.md with resolved open questions. Checker: 0 blockers, 1 warning resolved (RESEARCH.md open questions marked). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
21 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, tags, must_haves
| phase | plan | type | wave | depends_on | files_modified | autonomous | requirements | tags | must_haves | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 07.2-security-jti-claim-redis-access-token-revocation-inserted | 02 | execute | 1 |
|
|
true |
|
|
|
Purpose: Closes the 15-minute window between Phase 7.1's refresh-token revocation and natural access-token expiry. Implements D-01 (jti claim) and D-02/D-03/D-04 (user-level NBF check with fail-open) verbatim.
This plan is single-concern (token issuance + validation) and self-contained:
no API handler or admin code is touched. Wave 2 (Plan 03) wires the actual
user_nbf writes into the four security-event handlers.
Output: Two surgical edits — one to services/auth.py (add jti), one to deps/auth.py (add NBF check block). Wave 0's XFAIL stubs for jti-claim and NBF-check flip to XPASS. Existing 373-test baseline stays green.
<execution_context> @$HOME/.claude/get-shit-done/workflows/execute-plan.md @$HOME/.claude/get-shit-done/templates/summary.md </execution_context>
@.planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-CONTEXT.md @.planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-RESEARCH.md @backend/services/auth.py @backend/deps/auth.py @.planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-01-PLAN.mddef create_access_token(user_id: str, role: str) -> str:
- Returns jwt.encode(payload, settings.secret_key, algorithm="HS256")
- Current payload keys: sub, role, typ, iat, exp
- Wave 1 adds: jti=str(uuid.uuid4())
- uuid is ALREADY imported at module top (services/auth.py:25)
- Settings used: settings.access_token_expire_minutes (already in scope)
async def get_current_user( request: Request, # already present, no change credentials: HTTPAuthorizationCredentials = Depends(security), session: AsyncSession = Depends(get_db), ) -> User:
- Step 1: payload = auth_service.decode_access_token(credentials.credentials) — raises ValueError → 401
- Step 2 (NEW Wave 1): user_nbf check via request.app.state.redis
- Step 3 (existing): uuid.UUID(payload["sub"]) → 401 on KeyError/ValueError
- Step 4 (existing): session.get(User, user_uuid); reject if None or not is_active → 401
- Step 5 (existing): request.state.current_user = user; return user
try: # ... call ... except Exception as exc: logger.warning("HIBP check failed (fail-open): %s", exc) return False
settings.access_token_expire_minutes: int = 15 (confirmed by RESEARCH.md L182) TTL for user_nbf key = settings.access_token_expire_minutes * 60 = 900 NOTE: this Wave does NOT write user_nbf — only reads. TTL constant is used by Wave 2 writers.
redis_client = request.app.state.redis nbf_bytes = await redis_client.get(f"user_nbf:{payload['sub']}") # returns bytes | None
Compare:
if nbf_bytes is not None and payload["iat"] < int(nbf_bytes.decode()): raise HTTPException(401, "Session invalidated", headers={"WWW-Authenticate": "Bearer"})
After jwt.decode():
- payload["iat"] is type int (Unix timestamp)
- payload["jti"] is verbatim the string we passed in (PyJWT does not validate jti format)
- Direct integer comparison: payload["iat"] < int(nbf_bytes.decode()) works correctly
Do NOT change the function signature. Do NOT change algorithm. Do NOT add a TTL constant here (Wave 2 handles user_nbf TTL).
Promote the Wave 0 stub `test_create_access_token_includes_jti_claim` in `backend/tests/test_task2_auth_service.py`: remove the `@pytest.mark.xfail(...)` decorator and replace any placeholder body with the real assertions:
- decoded = decode_access_token(token)
- `assert "jti" in decoded`
- `assert isinstance(decoded["jti"], str)`
- `uuid.UUID(decoded["jti"]) # raises ValueError if not a valid UUID`
Append one new test alongside it: `def test_create_access_token_jti_is_unique_per_call():` — mint two tokens for the same user, decode both, assert `t1["jti"] != t2["jti"]`. Imports per existing in-function-scope convention in this test file.
Do NOT modify decode_access_token — PyJWT decodes the jti claim automatically as part of the payload dict.
Implementation requirements (concrete; do NOT inline code in this plan — read RESEARCH.md Pattern 3 for the exact shape):
1. Add module-level imports at top of deps/auth.py:
- `import logging`
- `_logger = logging.getLogger(__name__)` (module-level, after the imports)
2. Inside get_current_user, after the existing decode try/except, add a new try/except block:
- Inner work: `redis_client = request.app.state.redis`; `nbf_bytes = await redis_client.get(f"user_nbf:{payload['sub']}")`; if `nbf_bytes is not None and payload["iat"] < int(nbf_bytes.decode())` → raise `HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Session invalidated", headers={"WWW-Authenticate": "Bearer"})`.
- First except: `except HTTPException: raise` — MUST be present and MUST come BEFORE the broad except. This is Pitfall 1 from RESEARCH.md and is also documented in the threat model below (T-7.2-02).
- Second except: `except Exception as exc: _logger.warning("Redis user_nbf check failed (fail-open): %s", exc)` — mirrors the HIBP fail-open template at services/auth.py:~394.
3. Comparison direction MUST be `payload["iat"] < int(nbf_bytes.decode())` → reject (Pitfall 2). Do NOT use `<=` (a token issued at the exact same second as the event is allowed — edge case, but matches the "issued strictly before" semantic).
4. The `nbf_bytes` value handling: be tolerant of both `bytes` and `str` returns from Redis (FakeRedis in tests may return either depending on what was stored). Use `nbf_bytes.decode() if isinstance(nbf_bytes, (bytes, bytearray)) else nbf_bytes` before `int(...)`. Comment this with a reference to FakeRedis in test fixtures.
5. Do NOT add a `get_redis` FastAPI dependency. Per D-03 the access is direct via `request.app.state.redis`.
6. Do NOT change the function signature (request: Request is already present).
7. Do NOT touch `get_current_admin` or `get_regular_user` — they wrap get_current_user via Depends and inherit the check transparently.
Promote the three Wave 0 stubs in backend/tests/test_auth_deps.py:
- Remove the `@pytest.mark.xfail(...)` decorators from all three NBF-check tests created in Plan 01.
- Fill in the placeholder bodies with concrete assertions:
* Reject test: set `auth_client._transport.app.state.redis._store[f"user_nbf:{user.id}"] = (b"9999999999", None)` (FakeRedis tuple format: (value, expiry)) or use `await app.state.redis.set(f"user_nbf:{user.id}", b"9999999999")`. Mint token with create_access_token. GET /test/me with Bearer. Assert resp.status_code == 401 and resp.json()["detail"] == "Session invalidated".
* Allow test: set the key to `b"1"` (Unix ts way in the past). Mint token. Assert resp.status_code == 200.
* Fail-open test: replace app.state.redis with a small `class _BrokenRedis` whose async get/set raise RuntimeError. Mint a token. Assert resp.status_code == 200 (NOT 5xx, NOT 401).
Verify by direct grep that the HTTPException re-raise guard is in place — Pitfall 1 is the single highest-impact correctness risk in this plan.
<threat_model>
Trust Boundaries
| Boundary | Description |
|---|---|
| client → API (Authorization: Bearer) | Untrusted JWT crosses here; signature + typ + NBF all validated in get_current_user |
| API → Redis (app.state.redis) | Trusted local network within Docker Compose; failures must not deny service (D-04 fail-open) |
STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|---|---|---|---|---|
| T-7.2-01 | Elevation of Privilege | get_current_user (NBF check) | mitigate | After successful decode, read user_nbf:{payload['sub']} from Redis; if present and payload["iat"] < int(nbf_bytes.decode()), raise HTTP 401 "Session invalidated" with WWW-Authenticate header. Closes the 15-minute window where a revoked session's live access token remains valid. |
| T-7.2-02 | Elevation of Privilege | fail-open except block in get_current_user | mitigate | Explicit except HTTPException: raise MUST precede the broad except Exception — otherwise the intentional 401 from the NBF check is swallowed and a revoked token leaks through. Verified by source assertion (grep ordering) in Task 2 acceptance criteria. |
| T-7.2-03 | Elevation of Privilege | NBF comparison direction | mitigate | Comparison is payload["iat"] < int(nbf_bytes.decode()) — token issued BEFORE the event is rejected. Pitfall 2 from RESEARCH.md. Verified by test_get_current_user_rejects_token_when_iat_before_user_nbf. |
| T-7.2-04 | Denial of Service / EoP | Redis outage during NBF check | accept | D-04 mandates fail-open: if Redis raises Exception, log a warning and allow the request to proceed. Mirrors the HIBP fail-open pattern (services/auth.py:~394). Availability is prioritized over blocking-during-outage; the surface area is the 15-minute access-token lifetime. |
| T-7.2-05 | Information Disclosure | jti claim in JWT body | accept | jti is a UUIDv4 (no PII, no user identifier). RFC 7519 standard. Visible in any base64-decoded JWT — acceptable. |
| T-7.2-W1-SC | Tampering | npm/pip/cargo installs | n/a | No packages installed in this plan — uses only stdlib (uuid, logging) already imported in scope (RESEARCH.md "Package Legitimacy Audit" intentionally blank) |
| </threat_model> |
<success_criteria>
- Every access token issued after this plan deploys contains a
jtiUUID claim (verifiable: decode any new token) - get_current_user rejects tokens issued before any user_nbf timestamp written for that user (verifiable: pytest tests/test_auth_deps.py -k nbf)
- Redis outages produce log warnings but do not block requests (verifiable: test_get_current_user_failopen_on_redis_error)
- HTTPException raised by the NBF check is NEVER caught by the fail-open broad-except (verifiable: source ordering grep + test behavior)
- Wave 2 (Plan 03) can write
user_nbf:{user_id}from the four security-event handlers and the check will fire on the next request from any pre-event token </success_criteria>
Required fields in SUMMARY: artifacts (services/auth.py + deps/auth.py), patterns_established (NBF check pattern in get_current_user; HTTPException re-raise guard before broad catch; bytes/str tolerance in Redis return values), patterns_to_avoid (do NOT use <= in NBF comparison; do NOT add a get_redis dependency; do NOT touch get_current_admin/get_regular_user), provides (jti claim issuance + NBF rejection enforcement — ready for Wave 2 write sites).