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>
19 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 | 03 | execute | 2 |
|
|
true |
|
|
|
Purpose: Plan 02 established the check (reader); this plan adds the writers. Until these four writes are in place, the user_nbf:{user_id} key is never set in production (it only exists in tests via direct FakeRedis manipulation), so the 15-minute revocation window remains open.
This plan is the final implementation step for Phase 7.2. After it completes:
- Any password change, TOTP enroll, TOTP revoke, or admin deactivation writes user_nbf:{user_id} = now() to Redis with TTL = access token lifetime.
- get_current_user (Plan 02) reads the key and blocks pre-event tokens.
- All Wave 0 xfail stubs from Plan 01 that weren't promoted in Plan 02 flip to PASSED.
Output: Two files edited (api/auth.py, api/admin.py), four test stubs promoted.
<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/api/auth.py @backend/api/admin.py @backend/tests/test_auth_api.py @backend/tests/test_admin_api.py @backend/config.pyHandler already calls:
- auth_service.verify_password
- auth_service.check_hibp
- auth_service.validate_password_strength
- auth_service.revoke_all_refresh_tokens(session, current_user.id, skip_token_hash=skip_hash)
- write_audit_log(...)
- await session.commit() ← insert user_nbf write before this line
The redis_client is NOT yet assigned in change_password — use request.app.state.redis inline.
Handler already has at the top: redis_client = request.app.state.redis ← reuse this variable Then calls:
- auth_service.verify_totp(..., redis_client)
- user.totp_enabled = True
- auth_service.generate_backup_codes / store_backup_codes
- auth_service.revoke_all_refresh_tokens(session, current_user.id, skip_token_hash=skip_hash)
- write_audit_log(...)
- await session.commit() ← insert user_nbf write before this line (redis_client already in scope — use it directly)
Handler does NOT yet have a redis_client assignment. Calls:
- user.totp_enabled = False; user.totp_secret = None
- delete(BackupCode) for user
- auth_service.revoke_all_refresh_tokens(session, current_user.id, skip_token_hash=skip_hash)
- write_audit_log(...)
- await session.commit() ← insert user_nbf write before this line
Handler at line ~351: if not body.is_active: # Revoke all refresh tokens on deactivation await revoke_all_refresh_tokens(session, user.id) ← INSERT user_nbf write HERE (inside the if not body.is_active block, after revoke_all_refresh_tokens) ← Do NOT write if body.is_active is True (anti-pattern from RESEARCH.md)
admin.py does NOT currently import time (grep confirms no import time at module level).
auth.py does NOT currently import time (confirmed by codebase read).
settings.access_token_expire_minutes = 15 (from backend/config.py)
TTL = settings.access_token_expire_minutes * 60 (= 900 at default config)
Use this derived form — do NOT hardcode 900. Import settings is already present in api/auth.py.
In api/admin.py, verify settings is importable (it uses from config import settings per existing imports).
await request.app.state.redis.set( f"user_nbf:{current_user.id}", int(time.time()), ex=settings.access_token_expire_minutes * 60, )
In enable_totp where redis_client is already assigned: await redis_client.set( f"user_nbf:{current_user.id}", int(time.time()), ex=settings.access_token_expire_minutes * 60, )
In admin deactivation, the user variable is the target (not current_user): await request.app.state.redis.set( f"user_nbf:{user.id}", int(time.time()), ex=settings.access_token_expire_minutes * 60, )
Task 1: Add import time + user_nbf writes to api/auth.py (change_password, enable_totp, disable_totp) backend/api/auth.py - backend/api/auth.py lines 24–35 (module-level imports — add `import time` here, after existing stdlib imports) - backend/api/auth.py lines 455–510 (change_password handler — find the `await session.commit()` line, insert write above it) - backend/api/auth.py lines 552–605 (enable_totp handler — redis_client already in scope; find `await session.commit()`, insert write above it) - backend/api/auth.py lines 607–650 (disable_totp handler — find `await session.commit()`, insert write above it) - backend/config.py (confirm access_token_expire_minutes field name — e.g., `access_token_expire_minutes: int = 15`) - .planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-RESEARCH.md "Pattern 2" section (lines 167–184) and "Anti-Patterns" (do NOT write on activation) - change_password: after revoke_all_refresh_tokens + write_audit_log and before session.commit(), Redis key user_nbf:{current_user.id} is set with TTL=access_token_expire_minutes*60. - enable_totp: same write, using the already-assigned redis_client variable, before session.commit(). - disable_totp: same write via request.app.state.redis, before session.commit(). - No write on any other handler (login, logout, register, refresh, etc.). - `import time` is present at module level of api/auth.py. Add `import time` to api/auth.py after the existing `import uuid` line (line 26) — alphabetical placement in stdlib imports.In change_password (line 455): locate the `await session.commit()` line at end of handler. Insert immediately above it:
`await request.app.state.redis.set(f"user_nbf:{current_user.id}", int(time.time()), ex=settings.access_token_expire_minutes * 60)`
In enable_totp (line 552): locate `await session.commit()` at end of handler. `redis_client` is already assigned near the top of this function (`redis_client = request.app.state.redis`). Insert immediately above session.commit():
`await redis_client.set(f"user_nbf:{current_user.id}", int(time.time()), ex=settings.access_token_expire_minutes * 60)`
In disable_totp (line 607): locate `await session.commit()` at end of handler. Insert immediately above it:
`await request.app.state.redis.set(f"user_nbf:{current_user.id}", int(time.time()), ex=settings.access_token_expire_minutes * 60)`
Do NOT add `settings` import (already present). Do NOT extract a TTL constant — the derived form is readable and stays synchronized with config automatically.
Locate the deactivation block in the status handler (around line 351):
```
if not body.is_active:
# Revoke all refresh tokens on deactivation
await revoke_all_refresh_tokens(session, user.id)
```
Add one line immediately after `await revoke_all_refresh_tokens(session, user.id)`, still inside the `if not body.is_active:` block:
`await request.app.state.redis.set(f"user_nbf:{user.id}", int(time.time()), ex=settings.access_token_expire_minutes * 60)`
Verify `settings` is already imported in admin.py (it should be — grep for `from config import settings`). If not present, add `from config import settings` to the imports section.
Promote the two Wave 0 stubs in backend/tests/test_admin_api.py:
- test_deactivate_user_writes_user_nbf_to_redis: remove xfail decorator; replace placeholder body with real assertions: PATCH is_active=false → response 200; then `await client._transport.app.state.redis.get(f"user_nbf:{user.id}")` returns non-None bytes whose `int(decode())` > 0.
- test_activate_user_does_not_write_user_nbf: remove xfail decorator; replace body: PATCH is_active=true (on a deactivated user); then assert `await app.state.redis.get(f"user_nbf:{user.id}")` is None (the key should not exist for a reactivation event).
<threat_model>
Trust Boundaries
| Boundary | Description |
|---|---|
| client → API (security events) | Authenticated user or admin triggers password change / TOTP / deactivation; write must fire unconditionally before commit |
| API → Redis (app.state.redis) | Trusted local network; write is best-effort — if Redis is down, the session.commit() still completes (no rollback) |
STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|---|---|---|---|---|
| T-7.2-01 | Elevation of Privilege | All four security-event handlers | mitigate | user_nbf:{user_id} written with TTL=access_token_expire_minutes*60 before session.commit() on change_password, enable_totp, disable_totp, admin deactivation. Plan 02's get_current_user check fires on the next request from any pre-event token. |
| T-7.2-ACT | Elevation of Privilege | Admin activation path (is_active=True) | mitigate | Write is strictly inside if not body.is_active: block. Reactivation does NOT write user_nbf — a freshly reactivated user's first token is issued after reactivation and will have iat > any prior nbf. Verified by test_activate_user_does_not_write_user_nbf. |
| T-7.2-TTL | Elevation of Privilege | TTL mismatch between write sites | mitigate | All four sites use settings.access_token_expire_minutes * 60 (not hardcoded 900) — stays synchronized if TTL changes in config. |
| T-7.2-W2-SC | Tampering | npm/pip/cargo installs | n/a | No new packages — only import time (stdlib) added. RESEARCH.md "Package Legitimacy Audit" is intentionally blank for this phase. |
| </threat_model> |
<success_criteria>
- All four security-event handlers write user_nbf:{user_id} to Redis with correct TTL
- Reactivation path does NOT write user_nbf
- TTL expression uses settings.access_token_expire_minutes * 60 in all four write sites
- Complete Phase 7.2 test matrix (9 behaviors from VALIDATION.md) reports PASSED
- Zero regressions: Phase 7.1's 373-test baseline unchanged </success_criteria>
Required fields in SUMMARY: artifacts (api/auth.py + api/admin.py), patterns_established (user_nbf write pattern; conditional deactivation-only write; TTL derived from settings), patterns_to_avoid (do NOT write on activation; do NOT hardcode 900), provides (full Phase 7.2 revocation pipeline complete — jti issued + NBF checked + user_nbf written on all four security events).