189 lines
11 KiB
Markdown
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>
|