Files
kite/.planning/phases/07.1-security-session-revocation-on-privilege-change/07.1-01-PLAN.md
T

189 lines
11 KiB
Markdown

---
phase: 07.1
plan: 01
type: execute
wave: 1
depends_on: []
files_modified:
- backend/services/auth.py
- backend/api/auth.py
autonomous: true
requirements:
- CR-01
- CR-02
- CR-03
must_haves:
truths:
- "Changing password revokes all other refresh tokens; current session stays alive"
- "Enabling TOTP revokes all other refresh tokens; current session stays alive"
- "Disabling TOTP revokes all other refresh tokens; current session stays alive"
- "All three endpoints return sessions_revoked: int in their response body"
- "The revocation count is written to the audit log metadata_ for all three operations"
artifacts:
- path: "backend/services/auth.py"
provides: "revoke_all_refresh_tokens with skip_token_hash optional param"
contains: "skip_token_hash: Optional[str] = None"
- path: "backend/api/auth.py"
provides: "revoke call + sessions_revoked in change_password, enable_totp, disable_totp"
contains: "sessions_revoked"
key_links:
- from: "backend/api/auth.py (change_password)"
to: "backend/services/auth.py (revoke_all_refresh_tokens)"
via: "await auth_service.revoke_all_refresh_tokens(session, current_user.id, skip_token_hash=...)"
pattern: "skip_token_hash"
- from: "backend/api/auth.py (enable_totp)"
to: "backend/services/auth.py (revoke_all_refresh_tokens)"
via: "await auth_service.revoke_all_refresh_tokens(session, current_user.id, skip_token_hash=...)"
pattern: "skip_token_hash"
- from: "backend/api/auth.py (disable_totp)"
to: "backend/services/auth.py (revoke_all_refresh_tokens)"
via: "await auth_service.revoke_all_refresh_tokens(session, current_user.id, skip_token_hash=...)"
pattern: "skip_token_hash"
---
<objective>
Implement the three missing session-revocation calls (CR-01, CR-02, CR-03) by:
1. Extending `revoke_all_refresh_tokens` in `services/auth.py` with an optional `skip_token_hash` parameter so callers can exclude the current session.
2. Wiring the revoke call into the `change_password`, `enable_totp`, and `disable_totp` handlers in `api/auth.py`, deriving `skip_token_hash` from the request's refresh token cookie, writing the count to the audit log, and returning `sessions_revoked` in each response.
Purpose: Enforces the CLAUDE.md invariant — "Password change, TOTP enroll/revoke, and account deactivation immediately revoke all active sessions."
Output: Modified `services/auth.py` and `api/auth.py`; no migrations, no new routes.
</objective>
<execution_context>
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
@$HOME/.claude/get-shit-done/templates/summary.md
</execution_context>
<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/phases/07.1-security-session-revocation-on-privilege-change/07.1-CONTEXT.md
</context>
<tasks>
<task type="auto">
<name>Task 1: Add skip_token_hash param to revoke_all_refresh_tokens</name>
<files>backend/services/auth.py</files>
<read_first>
backend/services/auth.py — read the full function at lines 218-237 (revoke_all_refresh_tokens), lines 154-172 (create_refresh_token, shows SHA-256 hash pattern), and lines 176-215 (rotate_refresh_token, shows how raw cookie value is hashed for DB lookup — identical pattern needed here).
</read_first>
<action>
Modify `revoke_all_refresh_tokens(session: AsyncSession, user_id: uuid.UUID)` to accept an additional optional parameter `skip_token_hash: Optional[str] = None`.
Update the WHERE clause in the SQLAlchemy `select(RefreshToken)` query to also filter out the token to skip when `skip_token_hash` is not None. The filter must add `RefreshToken.token_hash != skip_token_hash` as an additional condition in the `where()` call — only when the param is not None. When `skip_token_hash is None` (the existing `logout_all` caller), behavior is completely unchanged: all revoked=False tokens for user_id are revoked.
The `Optional` import is already present in the file (verify before adding). Do not change the function signature for any other caller — the default `None` value ensures backwards compatibility.
Do NOT change the row-by-row loop revocation logic. Bulk UPDATE optimization is explicitly out of scope per CONTEXT.md.
</action>
<verify>
<automated>cd /Users/nik/Documents/Progamming/document_scanner/backend && grep -n "skip_token_hash" services/auth.py</automated>
</verify>
<acceptance_criteria>
- `services/auth.py` contains `async def revoke_all_refresh_tokens(session: AsyncSession, user_id: uuid.UUID, skip_token_hash: Optional[str] = None) -> int:`
- The WHERE clause excludes `skip_token_hash` when it is not None: `RefreshToken.token_hash != skip_token_hash` appears in the updated query body
- The existing `logout_all` caller in `api/auth.py` (line ~410) continues to call `revoke_all_refresh_tokens(session, current_user.id)` with no third argument — no change required there
- `grep -n "skip_token_hash" backend/services/auth.py` returns at least 2 lines (signature + WHERE usage)
</acceptance_criteria>
</task>
<task type="auto">
<name>Task 2: Wire revoke into change_password, enable_totp, disable_totp</name>
<files>backend/api/auth.py</files>
<read_first>
backend/api/auth.py — read the following sections:
- Lines 399-423 (logout_all handler) — this is the canonical pattern: revoke + write_audit_log metadata + commit + response
- Lines 454-503 (change_password handler) — add revoke before the existing session.commit()
- Lines 545-590 (enable_totp handler) — add revoke before the existing session.commit()
- Lines 595-624 (disable_totp handler) — add revoke before the existing session.commit()
Also verify the import block at the top of auth.py contains `import hashlib` (needed for SHA-256 hashing).
</read_first>
<action>
Apply the same pattern to all three handlers. The pattern is identical for each:
1. Derive the skip hash from the request cookie before the revoke call:
Read `raw_cookie = request.cookies.get("refresh_token")` then compute
`skip_hash = hashlib.sha256(raw_cookie.encode()).hexdigest() if raw_cookie else None`
2. Call revoke with the skip hash after the existing audit_log call but before `session.commit()`:
`revoked = await auth_service.revoke_all_refresh_tokens(session, current_user.id, skip_token_hash=skip_hash)`
3. Add `sessions_revoked` to the audit log metadata_ in the existing `write_audit_log` call for each handler — extend the existing `metadata_={}` dict, or add `metadata_={"sessions_revoked": revoked}` if the current call has no metadata_ argument. Per D-06, this mirrors the `logout_all` pattern at line 419.
4. Return `sessions_revoked` in the response dict. Exact shapes:
- `change_password` currently returns `{"message": "Password updated"}` — change to `{"message": "Password updated", "sessions_revoked": revoked}`
- `enable_totp` currently returns `{"backup_codes": plain_codes}` — change to `{"backup_codes": plain_codes, "sessions_revoked": revoked}`
- `disable_totp` currently returns `{"message": "TOTP disabled"}` — change to `{"message": "TOTP disabled", "sessions_revoked": revoked}`
Per D-02/D-03: `request` is already a parameter on all three handlers (check the handler signatures; `change_password` and `disable_totp` already have `request: Request`; `enable_totp` already has `request: Request` for rate limiting). No signature changes needed.
Ensure `import hashlib` is present in the import section — it is already used in `services/auth.py:161`; verify it is also imported in `api/auth.py` before adding the hash computation.
Placement rule: `revoke_all_refresh_tokens` call goes BEFORE `session.commit()` in each handler (so it participates in the same transaction flush). The existing `write_audit_log` call uses `session.flush()` internally, so the order is: derive skip_hash → revoke → extend audit metadata → commit → return response.
Do NOT modify the `logout_all` handler — it already correctly revokes without skip (all sessions, including current).
</action>
<verify>
<automated>cd /Users/nik/Documents/Progamming/document_scanner/backend && grep -n "sessions_revoked" api/auth.py</automated>
</verify>
<acceptance_criteria>
- `grep -n "sessions_revoked" backend/api/auth.py` returns at least 6 lines (3 revoke calls + 3 response dicts)
- `grep -n "skip_token_hash" backend/api/auth.py` returns at least 3 lines (one per handler)
- `grep -n "hashlib.sha256" backend/api/auth.py` returns at least 3 lines (one per handler deriving the skip hash)
- The `logout_all` handler is unchanged: `grep -n "logout_all" backend/api/auth.py` shows the handler still calls `revoke_all_refresh_tokens(session, current_user.id)` with no `skip_token_hash` argument
- `cd backend && python -c "import api.auth"` exits with code 0 (no import errors)
</acceptance_criteria>
</task>
</tasks>
<threat_model>
## Trust Boundaries
| Boundary | Description |
|----------|-------------|
| client cookie → API | Raw refresh token arrives in httpOnly cookie; must not be logged or echoed |
## STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-7.1-01 | Tampering | skip_token_hash derivation | mitigate | Hash computed server-side with hashlib.sha256 — cookie value never compared in plaintext; constant-time exclusion via WHERE clause (not Python ==) |
| T-7.1-02 | Information Disclosure | sessions_revoked response field | accept | Count of revoked sessions is low-sensitivity metadata; exact token values never exposed |
| T-7.1-03 | Elevation of Privilege | missing skip on logout_all | accept | logout_all intentionally revokes ALL sessions (no skip) — behaviour unchanged, tested separately |
| T-7.1-SC | Tampering | npm/pip installs | accept | No new packages installed in this plan — no legitimacy gate required |
</threat_model>
<verification>
After both tasks complete:
1. `cd /Users/nik/Documents/Progamming/document_scanner/backend && python -c "import api.auth; import services.auth"` — exits 0
2. `grep -c "sessions_revoked" backend/api/auth.py` — returns 6 or more
3. `grep -c "skip_token_hash" backend/services/auth.py` — returns 2 or more
4. `grep -c "skip_token_hash" backend/api/auth.py` — returns 3 or more
5. `logout_all` handler unchanged: still calls `revoke_all_refresh_tokens(session, current_user.id)` without skip arg
</verification>
<success_criteria>
- `revoke_all_refresh_tokens` has signature `(session, user_id, skip_token_hash: Optional[str] = None) -> int` and filters by token_hash when skip is set
- All three handlers (`change_password`, `enable_totp`, `disable_totp`) call `revoke_all_refresh_tokens` with the derived `skip_token_hash` before `session.commit()`
- All three handlers include `"sessions_revoked": revoked` in their response dict
- All three handlers include `"sessions_revoked": revoked` in their `write_audit_log` `metadata_` kwarg
- No existing tests broken (run `pytest -x -q` after changes to confirm)
</success_criteria>
<output>
Create `.planning/phases/07.1-security-session-revocation-on-privilege-change/07.1-01-SUMMARY.md` when done.
</output>