104 lines
6.8 KiB
Markdown
104 lines
6.8 KiB
Markdown
# Phase 7.1: Security — Session Revocation on Privilege Change - Context
|
||
|
||
**Gathered:** 2026-06-05
|
||
**Status:** Ready for planning
|
||
|
||
<domain>
|
||
## Phase Boundary
|
||
|
||
Phase 7.1 delivers exactly three security bug fixes: adding the missing `revoke_all_refresh_tokens()` call to `change_password`, `enable_totp`, and `disable_totp` in `backend/api/auth.py`. These are the three findings labeled CR-01, CR-02, and CR-03 in `.planning/v0.1-MILESTONE-AUDIT.md`. No other changes are in scope.
|
||
|
||
</domain>
|
||
|
||
<decisions>
|
||
## Implementation Decisions
|
||
|
||
### Scope
|
||
- **D-01:** Phase fixes only CR-01..03 — the three missing `revoke_all_refresh_tokens()` calls. The other CONCERNS.md security issues (ES256, JTI, token fingerprinting, default secrets, etc.) are deferred to phases 7.2–7.4.
|
||
|
||
### Session Revocation Behavior
|
||
- **D-02:** The calling user's own current refresh token is **excluded** from revocation. Other sessions (other devices/browsers) are revoked; the session making the request stays alive. The user does not need to re-login.
|
||
- **D-03:** To identify and exclude the current session: read the raw refresh token from the `refresh_token` cookie (`request.cookies.get("refresh_token")`), SHA-256 hash it (consistent with how `rotate_refresh_token` works in `services/auth.py:189`), and pass the hash as a new optional `skip_token_hash` parameter to `revoke_all_refresh_tokens`. The function skips any row where `token_hash == skip_token_hash`.
|
||
|
||
### API Responses
|
||
- **D-04:** All three endpoints gain a `sessions_revoked: int` field in their response body, reporting how many OTHER sessions were terminated (not counting the current session). Example: `{"message": "Password updated", "sessions_revoked": 2}`.
|
||
|
||
### Frontend UX
|
||
- **D-05:** When `sessions_revoked > 0` is returned from any of the three endpoints, the frontend shows a brief toast notification: "Other sessions have been terminated." No redirect or re-login required (current session stays alive per D-02).
|
||
|
||
### Audit Logging
|
||
- **D-06:** Log the revocation count in the existing audit log event's `metadata_` field for all three operations (consistent with the pattern used in `logout_all` at `auth.py:419`).
|
||
|
||
</decisions>
|
||
|
||
<canonical_refs>
|
||
## Canonical References
|
||
|
||
**Downstream agents MUST read these before planning or implementing.**
|
||
|
||
### Target code — the three missing calls
|
||
- `backend/api/auth.py` — `change_password` handler (~line 447): add revoke before `session.commit()` at line 493
|
||
- `backend/api/auth.py` — `enable_totp` handler (~line 539): add revoke before `session.commit()` at line 580
|
||
- `backend/api/auth.py` — `disable_totp` handler (~line 588): add revoke before `session.commit()` at line 614
|
||
|
||
### Existing revocation infrastructure
|
||
- `backend/services/auth.py:218` — `revoke_all_refresh_tokens(session, user_id)`: the function being extended with an optional `skip_token_hash` param
|
||
- `backend/services/auth.py:154` — `create_refresh_token`: shows token is stored as SHA-256 hash (`token_hash = hashlib.sha256(raw.encode()).hexdigest()`)
|
||
- `backend/services/auth.py:176` — `rotate_refresh_token`: shows how raw token from cookie is hashed for DB lookup — same pattern needed for skip logic
|
||
- `backend/api/auth.py:394` — `logout_all`: working reference for revoke + audit log pattern; D-04 responses follow this shape
|
||
|
||
### Audit
|
||
- `.planning/v0.1-MILESTONE-AUDIT.md` — CR-01, CR-02, CR-03 definitions with exact file/line references and required fixes
|
||
- `.planning/codebase/CONCERNS.md` — full security concern descriptions with fix approaches
|
||
|
||
### CLAUDE.md security invariant
|
||
- `CLAUDE.md` §"Login token hardening" — "Password change, TOTP enroll/revoke, and account deactivation immediately revoke all active sessions." This is the rule these fixes enforce.
|
||
|
||
### Frontend toast
|
||
- `frontend/src/` — check existing toast/notification component for the correct notification API before adding new toast calls
|
||
|
||
</canonical_refs>
|
||
|
||
<code_context>
|
||
## Existing Code Insights
|
||
|
||
### Reusable Assets
|
||
- `revoke_all_refresh_tokens(session, user_id)` in `services/auth.py:218` — already returns `int` count; needs one new optional param (`skip_token_hash: Optional[str] = None`) and a WHERE-clause addition to exclude the skip hash
|
||
- SHA-256 cookie hashing pattern in `rotate_refresh_token` (`services/auth.py:189`) — identical pattern needed in all three handlers to derive `skip_token_hash` from the raw cookie value
|
||
- `logout_all` handler (`auth.py:401`) — working reference for the revoke + audit log + response pattern that all three handlers will now mirror
|
||
|
||
### Established Patterns
|
||
- Refresh token identity: raw token in cookie → `sha256(raw.encode()).hexdigest()` → `token_hash` column. No plaintext stored in DB.
|
||
- Audit metadata: `metadata_={"sessions_revoked": count}` passed to `write_audit_log` (see `logout_all` line 419)
|
||
- Response shapes: all three endpoints currently return simple `{"message": "..."}` dicts — D-04 adds `sessions_revoked` to each
|
||
|
||
### Integration Points
|
||
- `backend/api/auth.py` — only file changed in the backend; no new routes, no new models, no migrations needed
|
||
- `backend/services/auth.py` — `revoke_all_refresh_tokens` needs the optional `skip_token_hash` param; this is the only service change
|
||
- `frontend/src/` — toast notification triggered when `sessions_revoked > 0` in the response from these three endpoints
|
||
|
||
</code_context>
|
||
|
||
<specifics>
|
||
## Specific Ideas
|
||
|
||
- The `skip_token_hash` param on `revoke_all_refresh_tokens` should be `Optional[str] = None`. When `None` (the existing `logout_all` caller), behavior is unchanged — all tokens revoked. When set, the single WHERE clause addition is `RefreshToken.token_hash != skip_token_hash`.
|
||
- If the refresh cookie is absent (e.g., request authenticated via access token only), `skip_token_hash` is `None` and all tokens are revoked — safe fallback.
|
||
- The `revoke_all_refresh_tokens` bulk-UPDATE optimization noted in CONCERNS.md (one UPDATE per token → single bulk UPDATE) is out of scope for this phase; don't fix it here.
|
||
|
||
</specifics>
|
||
|
||
<deferred>
|
||
## Deferred Ideas
|
||
|
||
- **Phase 7.2 — JTI claim + Redis access-token revocation**: Closes the 15-min grace window; revoking refresh tokens still leaves the live access token valid until expiry. Tracked in CONCERNS.md §"No JTI Claim and No JTI Revocation in Redis".
|
||
- **Phase 7.3 — ES256 algorithm upgrade**: Replace HS256 with ECDSA P-256; generate a key pair, update `create_access_token` and decode functions, rotate all refresh tokens. Tracked in CONCERNS.md §"JWT Algorithm Downgrade: HS256 Instead of ES256".
|
||
- **Phase 7.4 — Token fingerprinting / token binding**: Add `fgp` (fingerprint) claim = HMAC of `User-Agent + Accept-Language` to access tokens; validate on every request. Tracked in CONCERNS.md §"No Token Fingerprint / Token Binding".
|
||
|
||
</deferred>
|
||
|
||
---
|
||
|
||
*Phase: 07.1-security-session-revocation-on-privilege-change*
|
||
*Context gathered: 2026-06-05*
|