kite/.planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-03-PLAN.md

---
phase: 07.2-security-jti-claim-redis-access-token-revocation-inserted
plan: 03
type: execute
wave: 2
depends_on:
  - 07.2-02
files_modified:
  - backend/api/auth.py
  - backend/api/admin.py
  - backend/tests/test_auth_api.py
  - backend/tests/test_admin_api.py
autonomous: true
requirements:
  - CONCERNS:JTI-CLAIM
  - CONCERNS:JTI-REVOKE-REDIS
tags:
  - security
  - jwt
  - redis
  - access-control

must_haves:
  truths:
    - "change_password writes user_nbf:{user_id} to Redis before session.commit() (D-02)"
    - "enable_totp writes user_nbf:{user_id} to Redis before session.commit() (D-02)"
    - "disable_totp writes user_nbf:{user_id} to Redis before session.commit() (D-02)"
    - "admin deactivation writes user_nbf:{user_id} to Redis only when is_active=False (D-05)"
    - "admin activation (is_active=True) does NOT write user_nbf (anti-pattern guard)"
    - "TTL for all writes is settings.access_token_expire_minutes * 60 (not hardcoded 900)"
    - "import time added at module level to api/auth.py and api/admin.py"
  artifacts:
    - path: "backend/api/auth.py"
      provides: "user_nbf Redis write in change_password, enable_totp, disable_totp"
      contains: "user_nbf"
    - path: "backend/api/admin.py"
      provides: "user_nbf Redis write in deactivation handler"
      contains: "user_nbf"
  key_links:
    - from: "backend/api/auth.py change_password"
      to: "request.app.state.redis"
      via: "await redis.set(f'user_nbf:{current_user.id}', int(time.time()), ex=settings.access_token_expire_minutes * 60)"
      pattern: "user_nbf.*current_user\\.id"
    - from: "backend/api/admin.py deactivation handler (line ~351)"
      to: "request.app.state.redis"
      via: "conditional write only when not body.is_active"
      pattern: "user_nbf.*user\\.id"
    - from: "Wave 2 implementation"
      to: "Wave 0 xfail stubs (test_auth_api NBF-write × 3, test_admin_api NBF-write × 1)"
      via: "stubs promoted from XFAIL to PASSED"
      pattern: "XPASS|xfail.*strict=False"
---

<objective>
Wire the user_nbf Redis writes into the four security-event handlers that trigger
access-token revocation.

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.
</objective>

<execution_context>
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
@$HOME/.claude/get-shit-done/templates/summary.md
</execution_context>

<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.py

<interfaces>
<!-- change_password handler — backend/api/auth.py:455 -->

Handler already calls:
  1. auth_service.verify_password
  2. auth_service.check_hibp
  3. auth_service.validate_password_strength
  4. auth_service.revoke_all_refresh_tokens(session, current_user.id, skip_token_hash=skip_hash)
  5. write_audit_log(...)
  6. 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.

<!-- enable_totp handler — backend/api/auth.py:552 -->

Handler already has at the top:
  redis_client = request.app.state.redis  ← reuse this variable
Then calls:
  1. auth_service.verify_totp(..., redis_client)
  2. user.totp_enabled = True
  3. auth_service.generate_backup_codes / store_backup_codes
  4. auth_service.revoke_all_refresh_tokens(session, current_user.id, skip_token_hash=skip_hash)
  5. write_audit_log(...)
  6. await session.commit()  ← insert user_nbf write before this line
  (redis_client already in scope — use it directly)

<!-- disable_totp handler — backend/api/auth.py:607 -->

Handler does NOT yet have a redis_client assignment.
Calls:
  1. user.totp_enabled = False; user.totp_secret = None
  2. delete(BackupCode) for user
  3. auth_service.revoke_all_refresh_tokens(session, current_user.id, skip_token_hash=skip_hash)
  4. write_audit_log(...)
  5. await session.commit()  ← insert user_nbf write before this line

<!-- admin deactivation handler — backend/api/admin.py:326 (PATCH /api/admin/users/{id}/status) -->

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).

<!-- TTL constant -->

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).

<!-- Redis write pattern (from RESEARCH.md Pattern 2, line 176) -->

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,
)
</interfaces>
</context>

<tasks>

<task type="auto">
  <name>Task 1: Add import time + user_nbf writes to api/auth.py (change_password, enable_totp, disable_totp)</name>
  <files>backend/api/auth.py</files>
  <read_first>
    - 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)
  </read_first>
  <behavior>
    - 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.
  </behavior>
  <action>
    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.
  </action>
  <verify>
    <automated>cd backend &amp;&amp; pytest tests/test_auth_api.py -v -k "nbf or user_nbf"</automated>
  </verify>
  <acceptance_criteria>
    - `grep -E "^import time" backend/api/auth.py` returns 1 match
    - `grep -c "user_nbf" backend/api/auth.py` returns >= 3 (one per handler)
    - `grep -c "access_token_expire_minutes \* 60" backend/api/auth.py` returns >= 3
    - `cd backend &amp;&amp; pytest tests/test_auth_api.py -v -k "nbf or user_nbf"` exits 0; at least 3 tests report PASSED (was XFAIL)
    - `cd backend &amp;&amp; pytest tests/test_auth_api.py -v` exits 0 with zero failures (zero regressions to existing change_password + totp tests)
    - The three NBF-write stubs (test_change_password_writes_user_nbf_to_redis, test_enable_totp_writes_user_nbf_to_redis, test_disable_totp_writes_user_nbf_to_redis) are promoted from XFAIL to PASSED in this task — update the test bodies, removing the xfail decorators and replacing placeholder bodies with real assertions (see Task 2 of Plan 02 for the assertion pattern: `await app.state.redis.get(f"user_nbf:{user_id}")` returns a non-None bytes-like value whose `int(decode())` > 0)
  </acceptance_criteria>
  <done>import time added; user_nbf written in all three auth security-event handlers; stubs promoted to PASSED.</done>
</task>

<task type="auto">
  <name>Task 2: Add import time + user_nbf write to api/admin.py deactivation handler</name>
  <files>backend/api/admin.py</files>
  <read_first>
    - backend/api/admin.py lines 24–50 (module-level imports — add `import time` here)
    - backend/api/admin.py lines 326–380 (PATCH /api/admin/users/{id}/status handler — locate the `if not body.is_active:` block at ~line 351; the write goes inside this block, after revoke_all_refresh_tokens)
    - backend/config.py (confirm access_token_expire_minutes field name)
    - .planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-RESEARCH.md "Pattern 4" (lines 239–252) and "Anti-Patterns" ("Do not write user_nbf for successful activation")
    - backend/tests/test_admin_api.py (two Wave 0 stubs: test_deactivate_user_writes_user_nbf_to_redis + test_activate_user_does_not_write_user_nbf — promote both in this task)
  </read_first>
  <behavior>
    - When PATCH /api/admin/users/{id}/status is called with `is_active=false`: after revoke_all_refresh_tokens, user_nbf:{user.id} is written to Redis with TTL=access_token_expire_minutes*60.
    - When called with `is_active=true` (reactivation): user_nbf is NOT written (the negative guard test must pass).
    - `import time` is present at module level of api/admin.py.
    - `settings` must be in scope for the TTL expression — confirm it is already imported in admin.py.
  </behavior>
  <action>
    Add `import time` to api/admin.py after existing stdlib imports — alphabetical placement alongside `import uuid` (line 26).

    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).
  </action>
  <verify>
    <automated>cd backend &amp;&amp; pytest tests/test_admin_api.py -v</automated>
  </verify>
  <acceptance_criteria>
    - `grep -E "^import time" backend/api/admin.py` returns 1 match
    - `grep -c "user_nbf" backend/api/admin.py` returns >= 1
    - `grep -c "access_token_expire_minutes \* 60" backend/api/admin.py` returns >= 1
    - The user_nbf write line is inside the `if not body.is_active:` block (not at module scope or outside the conditional) — verify with grep -n and line-number inspection
    - `cd backend &amp;&amp; pytest tests/test_admin_api.py -v` exits 0 with zero failures
    - test_deactivate_user_writes_user_nbf_to_redis reports PASSED (promoted from XFAIL)
    - test_activate_user_does_not_write_user_nbf reports PASSED (promoted from XFAIL; Redis key must be None for activation path)
    - All existing admin tests (test_deactivate_user, test_reactivate_user, etc.) still PASSED
  </acceptance_criteria>
  <done>import time added; user_nbf written in admin deactivation handler (conditional on is_active=False only); both stubs promoted to PASSED; zero admin test regressions.</done>
</task>

<task type="auto">
  <name>Task 3: Full regression run — verify zero new failures across entire test suite</name>
  <files></files>
  <read_first>
    - .planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-VALIDATION.md (Validation Sign-Off checklist)
  </read_first>
  <behavior>
    - Full pytest suite passes with zero new failures vs the Phase 7.1 baseline.
    - All 9 Phase 7.2 behaviors from VALIDATION.md report green: jti-claim PASSED, test-fakeredis PASSED, nbf-write × 4 PASSED, nbf-check × 2 PASSED, nbf-fail-open PASSED.
    - No previously-XFAIL stubs remain (all were promoted in Plans 02 and 03).
  </behavior>
  <action>
    Run the full backend test suite. Review output for any unexpected failures. If any test introduced by this plan fails, investigate and fix before marking this task done.
  </action>
  <verify>
    <automated>cd backend &amp;&amp; pytest -v 2>&amp;1 | tail -20</automated>
  </verify>
  <acceptance_criteria>
    - `cd backend &amp;&amp; pytest -v` exits 0 with zero failures
    - Output contains no FAILED lines
    - `cd backend &amp;&amp; pytest tests/test_auth_deps.py tests/test_auth_api.py tests/test_admin_api.py tests/test_task2_auth_service.py -v -k "jti or nbf or user_nbf or failopen"` exits 0 with at least 9 PASSED results covering all 9 VALIDATION.md behaviors
    - `grep -rn "user_nbf" backend/api/auth.py backend/api/admin.py backend/deps/auth.py` returns >= 4 lines (3 writers in api/auth.py + 1 writer in api/admin.py + 1 reader in deps/auth.py)
  </acceptance_criteria>
  <done>Full suite green; all Phase 7.2 behaviors verified; no regressions.</done>
</task>

</tasks>

<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>

<verification>
- `cd backend && pytest -v` — zero failures; full suite green
- `grep -c "user_nbf" backend/api/auth.py` returns >= 3 (one per security-event handler)
- `grep -c "user_nbf" backend/api/admin.py` returns >= 1 (deactivation only)
- `grep -E "^import time" backend/api/auth.py backend/api/admin.py` returns 2 lines
- All 5 NBF-write/check/fail-open stubs from Plan 01 that were not promoted in Plan 02 now PASSED
- `cd backend && bandit -r backend/ --severity-level high` — zero HIGH severity findings introduced by this plan
</verification>

<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>

<output>
Create `.planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-03-SUMMARY.md` when done.

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).
</output>