From a7ee4fbd23f4d50b51f4704c79d45d64336f1edb Mon Sep 17 00:00:00 2001 From: curo1305 Date: Fri, 5 Jun 2026 11:23:07 +0200 Subject: [PATCH] docs(07.1): capture phase context --- .planning/ROADMAP.md | 48 ++++++++ .../07.1-CONTEXT.md | 103 ++++++++++++++++++ .../07.1-DISCUSSION-LOG.md | 77 +++++++++++++ 3 files changed, 228 insertions(+) create mode 100644 .planning/phases/07.1-security-session-revocation-on-privilege-change/07.1-CONTEXT.md create mode 100644 .planning/phases/07.1-security-session-revocation-on-privilege-change/07.1-DISCUSSION-LOG.md diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 8445fd9..91e2aa5 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -444,6 +444,50 @@ Before any phase is marked complete, all three gates must pass: --- +### Phase 7.1: Security: session revocation on privilege change (CR-01..03) (INSERTED) + +**Goal**: Fix the three missing session-revocation calls in `backend/api/auth.py`: `change_password`, `enable_totp`, and `disable_totp` must all call `revoke_all_refresh_tokens()` (excluding the current session). Add `sessions_revoked` to their response shapes and a frontend toast when other sessions are terminated. +**Mode:** quick +**Depends on**: Phase 7 +**Requirements**: CR-01, CR-02, CR-03 + +**Status:** Context gathered — ready for planning + +--- + +### Phase 7.2: Security — JTI Claim + Redis Access-Token Revocation (INSERTED) + +**Goal**: Add a `jti` (JWT ID) claim to every issued access token. In `get_current_user`, check `redis.get("jti_revoked:{jti}")` and raise 401 if set. Add `revoke_access_token(jti, ttl)` helper called from `change_password`, `enable_totp`, `disable_totp`, and admin account deactivation. Closes the 15-minute window where a revoked session's live access token remains valid. +**Mode:** quick +**Depends on**: Phase 7.1 +**Requirements**: Tracked in `.planning/codebase/CONCERNS.md` §"No JTI Claim and No JTI Revocation in Redis" + +**Status:** Not planned yet + +--- + +### Phase 7.3: Security — ES256 Algorithm Upgrade (INSERTED) + +**Goal**: Replace HS256 with ES256 (ECDSA P-256) for JWT signing. Generate a P-256 key pair; store private key in `JWT_PRIVATE_KEY` env var, public key in `JWT_PUBLIC_KEY`. Update `create_access_token` and all `decode_*` functions. Rotate all active refresh tokens on first boot after deploy. A leaked public key cannot forge tokens. +**Mode:** quick +**Depends on**: Phase 7.2 +**Requirements**: Tracked in `.planning/codebase/CONCERNS.md` §"JWT Algorithm Downgrade: HS256 Instead of ES256" + +**Status:** Not planned yet + +--- + +### Phase 7.4: Security — Token Fingerprinting / Token Binding (INSERTED) + +**Goal**: Add a `fgp` (fingerprint) claim = `hmac(key, User-Agent + Accept-Language)[:16]` to every issued access token. In `get_current_user`, recompute the fingerprint from the request headers and compare with `hmac.compare_digest`. Limits replay of stolen access tokens to the original device/browser context. +**Mode:** quick +**Depends on**: Phase 7.3 +**Requirements**: Tracked in `.planning/codebase/CONCERNS.md` §"No Token Fingerprint / Token Binding" + +**Status:** Not planned yet + +--- + ## Progress Table | Phase | Plans Complete | Status | Completed | @@ -457,3 +501,7 @@ Before any phase is marked complete, all three gates must pass: | 6.1. Close v1.0 audit gaps | 2/2 | Complete | 2026-05-30 | | 6.2. Close v1 sharing + cloud-delete + CSV export gaps | 5/5 | Complete | 2026-05-31 | | 7. Redo and optimize LLM integration | 5/5 | Complete | 2026-06-05 | +| 7.1. Security: session revocation on privilege change (CR-01..03) | 0/? | Context gathered | — | +| 7.2. Security: JTI claim + Redis access-token revocation | 0/? | Not planned (INSERTED) | — | +| 7.3. Security: ES256 algorithm upgrade | 0/? | Not planned (INSERTED) | — | +| 7.4. Security: token fingerprinting / token binding | 0/? | Not planned (INSERTED) | — | diff --git a/.planning/phases/07.1-security-session-revocation-on-privilege-change/07.1-CONTEXT.md b/.planning/phases/07.1-security-session-revocation-on-privilege-change/07.1-CONTEXT.md new file mode 100644 index 0000000..184cd41 --- /dev/null +++ b/.planning/phases/07.1-security-session-revocation-on-privilege-change/07.1-CONTEXT.md @@ -0,0 +1,103 @@ +# Phase 7.1: Security — Session Revocation on Privilege Change - Context + +**Gathered:** 2026-06-05 +**Status:** Ready for planning + + +## 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. + + + + +## 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`). + + + + +## 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 + + + + +## 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 + + + + +## 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. + + + + +## 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". + + + +--- + +*Phase: 07.1-security-session-revocation-on-privilege-change* +*Context gathered: 2026-06-05* diff --git a/.planning/phases/07.1-security-session-revocation-on-privilege-change/07.1-DISCUSSION-LOG.md b/.planning/phases/07.1-security-session-revocation-on-privilege-change/07.1-DISCUSSION-LOG.md new file mode 100644 index 0000000..9d4fd35 --- /dev/null +++ b/.planning/phases/07.1-security-session-revocation-on-privilege-change/07.1-DISCUSSION-LOG.md @@ -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