docs(07.1): capture phase context
This commit is contained in:
curo1305
+103
@@ -0,0 +1,103 @@
|
||||
# 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*
|
||||
+77
@@ -0,0 +1,77 @@
|
||||
# Phase 7.1: Security — Session Revocation on Privilege Change - Discussion Log
|
||||
|
||||
> **Audit trail only.** Do not use as input to planning, research, or execution agents.
|
||||
> Decisions are captured in CONTEXT.md — this log preserves the alternatives considered.
|
||||
|
||||
**Date:** 2026-06-05
|
||||
**Phase:** 07.1-security-session-revocation-on-privilege-change
|
||||
**Areas discussed:** Current-session behavior, Scope (CR-01..03), API response + frontend UX
|
||||
|
||||
---
|
||||
|
||||
## Current-Session Behavior
|
||||
|
||||
| Option | Description | Selected |
|
||||
|--------|-------------|----------|
|
||||
| Revoke all, including current | Cleanest security posture — matches GitHub, Google. Attacker cut off immediately. User must re-login within 15 min (access token TTL). | |
|
||||
| Exclude current session | Revoke all OTHER sessions, keep the current one alive. More user-friendly; requires reading and hashing the current refresh cookie to exclude it. | ✓ |
|
||||
|
||||
**User's choice:** Exclude current session
|
||||
**Notes:** No follow-up needed — clear preference.
|
||||
|
||||
### Implementation mechanism
|
||||
|
||||
| Option | Description | Selected |
|
||||
|--------|-------------|----------|
|
||||
| Read raw cookie, hash it, match to DB | Read `refresh_token` cookie, compute SHA-256, pass as `skip_token_hash` to `revoke_all_refresh_tokens`. Consistent with how `rotate_refresh_token` already works. | ✓ |
|
||||
| Pass DB row ID | Look up the RefreshToken row during the request, pass its UUID to exclude. More explicit but needs a new helper. | |
|
||||
| You decide | Let the planner choose the cleanest approach. | |
|
||||
|
||||
**User's choice:** Read raw cookie, hash it, match to DB
|
||||
|
||||
---
|
||||
|
||||
## Scope: CR-01..03
|
||||
|
||||
| Option | Description | Selected |
|
||||
|--------|-------------|----------|
|
||||
| Only CR-01..03 | Three missing `revoke_all_refresh_tokens()` calls. Each ~2 lines. Fast, contained blast radius. Other issues deferred to 7.2–7.4. | ✓ |
|
||||
| CR-01..03 + JTI revocation | Also add JTI claims + Redis-based access-token revocation. Much larger change. | |
|
||||
| All CONCERNS.md issues | CR-01..03 + JTI + ES256 + token fingerprinting. Major auth rewrite, 3–4 plans. | |
|
||||
|
||||
**User's choice:** "I want to focus now on CR-01..03 but I want you to add a 7.2-4 for the other three concerns right now."
|
||||
**Notes:** Phases 7.2 (JTI), 7.3 (ES256), 7.4 (token fingerprinting) added to ROADMAP.md as part of this session.
|
||||
|
||||
---
|
||||
|
||||
## API Response + Frontend UX
|
||||
|
||||
### Response shape
|
||||
|
||||
| Option | Description | Selected |
|
||||
|--------|-------------|----------|
|
||||
| Add `sessions_revoked` count | Return `{"message": "...", "sessions_revoked": N}`. Minimal change, useful signal for frontend. | ✓ |
|
||||
| Silent revocation | Don't change response shape. Revocation happens transparently. | |
|
||||
|
||||
**User's choice:** Add `sessions_revoked` count
|
||||
|
||||
### Frontend handling
|
||||
|
||||
| Option | Description | Selected |
|
||||
|--------|-------------|----------|
|
||||
| Show toast notification | "Other sessions have been terminated." — shown briefly when `sessions_revoked > 0`. | ✓ |
|
||||
| No frontend change | Ignore `sessions_revoked` in response. Backend-only phase. | |
|
||||
|
||||
**User's choice:** Show toast notification
|
||||
|
||||
---
|
||||
|
||||
## Claude's Discretion
|
||||
|
||||
None — all areas were resolved with explicit user preferences.
|
||||
|
||||
## Deferred Ideas
|
||||
|
||||
- **Phase 7.2**: JTI claim + Redis access-token revocation (closes 15-min grace window)
|
||||
- **Phase 7.3**: ES256 algorithm upgrade (HS256 → ECDSA P-256)
|
||||
- **Phase 7.4**: Token fingerprinting / `fgp` claim + validation
|
||||
Reference in New Issue
Block a user