docs(07.2): research phase — JTI claim + Redis access-token revocation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
eda91f7512
commit
35ff0d52fa
+510
@@ -0,0 +1,510 @@
|
|||||||
|
# Phase 7.2: Security — JTI Claim + Redis Access-Token Revocation - Research
|
||||||
|
|
||||||
|
**Researched:** 2026-06-05
|
||||||
|
**Domain:** JWT security, Redis-backed token revocation, FastAPI dependency injection
|
||||||
|
**Confidence:** HIGH
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
<user_constraints>
|
||||||
|
## User Constraints (from CONTEXT.md)
|
||||||
|
|
||||||
|
### Locked Decisions
|
||||||
|
|
||||||
|
**D-01 — JTI claim**
|
||||||
|
Add `jti=str(uuid.uuid4())` to the `create_access_token` payload in `backend/services/auth.py`. The `jti` claim is included on every token but is used only as an identity handle — per-JTI Redis tracking is NOT implemented (user-level NBF replaces it).
|
||||||
|
|
||||||
|
**D-02 — Revocation approach: user-level NBF key, not per-JTI**
|
||||||
|
Set `user_nbf:{user_id}` = Unix timestamp in Redis (TTL = 15 min, the access token lifetime) when any of these security events fire: `change_password`, `enable_totp`, `disable_totp`, admin account deactivation.
|
||||||
|
|
||||||
|
In `get_current_user`, after a successful decode, read `user_nbf:{user_id}` from Redis. If the key exists and `token["iat"] < nbf_timestamp` → raise HTTP 401.
|
||||||
|
|
||||||
|
**D-03 — Redis access pattern in get_current_user**
|
||||||
|
Use `request.app.state.redis` directly inside `get_current_user`. Consistent with existing pattern in `backend/api/auth.py` lines 197 and 566. No new `get_redis` dependency function.
|
||||||
|
|
||||||
|
**D-04 — Redis failure handling: fail-open**
|
||||||
|
If the Redis `GET user_nbf:{user_id}` call raises an exception, log a warning and allow the request to proceed. Mirrors the HIBP fail-open pattern in `services/auth.py`.
|
||||||
|
|
||||||
|
**D-05 — Admin account deactivation coverage**
|
||||||
|
The existing deactivation handler in `backend/api/admin.py:351` already calls `revoke_all_refresh_tokens`. Add the `user_nbf` write there too.
|
||||||
|
|
||||||
|
**D-06 — TTL for user_nbf key**
|
||||||
|
15 minutes (900 seconds) — matches the access token TTL.
|
||||||
|
|
||||||
|
### Claude's Discretion
|
||||||
|
|
||||||
|
None specified.
|
||||||
|
|
||||||
|
### Deferred Ideas (OUT OF SCOPE)
|
||||||
|
|
||||||
|
- **Per-JTI Redis tracking** — storing each issued JTI in Redis for individual token revocation.
|
||||||
|
- **Phase 7.3 — ES256 algorithm upgrade**: Replace HS256 with ECDSA P-256. The jti claim added in this phase is a prerequisite.
|
||||||
|
- **Phase 7.4 — Token fingerprinting / token binding**: Add `fgp` (fingerprint) claim = HMAC of `User-Agent + Accept-Language`.
|
||||||
|
</user_constraints>
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Phase 7.2 closes a 15-minute security gap: after Phase 7.1 revokes all refresh tokens on a security event (password change, TOTP enroll/revoke, account deactivation), the attacker's live access token remains valid until its 15-minute TTL expires. This phase eliminates that window by embedding a `jti` UUID claim in every new access token and introducing a `user_nbf:{user_id}` Redis key that invalidates all tokens issued before the security event.
|
||||||
|
|
||||||
|
The implementation touches exactly four files: `backend/services/auth.py` (add `jti` to `create_access_token`), `backend/deps/auth.py` (add NBF check in `get_current_user`), `backend/api/auth.py` (`change_password`, `enable_totp`, `disable_totp` — write Redis key), and `backend/api/admin.py` (deactivation handler — write Redis key). No schema migrations, no new endpoints, no frontend changes.
|
||||||
|
|
||||||
|
The approach is a user-level "not-before" timestamp rather than per-JTI revocation. Writing one Redis key per security event is O(1) and leaves no accumulation. The key auto-expires after 15 minutes (matching access token TTL), so Redis memory pressure is negligible.
|
||||||
|
|
||||||
|
**Primary recommendation:** Implement exactly as specified in D-01 through D-06. The patterns are already proven in this codebase (TOTP replay prevention uses the identical Redis key-style and TTL approach). The only new import needed in `api/auth.py` and `api/admin.py` is `import time` — all other dependencies are already present.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Architectural Responsibility Map
|
||||||
|
|
||||||
|
| Capability | Primary Tier | Secondary Tier | Rationale |
|
||||||
|
|------------|-------------|----------------|-----------|
|
||||||
|
| JTI claim generation | API / Backend (`services/auth.py`) | — | Token creation is a pure service function; JTI is a UUID added to the payload dict before signing |
|
||||||
|
| NBF check on every request | API / Backend (`deps/auth.py`) | — | FastAPI dep chain; runs before every protected route handler |
|
||||||
|
| user_nbf Redis write on security event | API / Backend (`api/auth.py`, `api/admin.py`) | — | Handlers own the security event logic; Redis write belongs alongside the existing `revoke_all_refresh_tokens` call |
|
||||||
|
| Redis storage for user_nbf | Database / Storage (Redis) | — | TTL-keyed ephemeral state; same store as TOTP replay prevention and rate limiting |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Standard Stack
|
||||||
|
|
||||||
|
### Core (no new packages required)
|
||||||
|
|
||||||
|
| Library | Current Version | Purpose | Already Used |
|
||||||
|
|---------|----------------|---------|-------------|
|
||||||
|
| `PyJWT` | 2.13.0 [VERIFIED: runtime] | JWT encoding/decoding; `jti` is an arbitrary payload key — accepted verbatim | Yes, `services/auth.py` |
|
||||||
|
| `redis` (async) | `>=4.6.0` in requirements.txt | `await redis.get(key)` / `await redis.set(key, value, ex=seconds)` | Yes, `app.state.redis` |
|
||||||
|
| `uuid` (stdlib) | 3.x | `str(uuid.uuid4())` for JTI value | Yes, `services/auth.py` |
|
||||||
|
| `time` (stdlib) | 3.x | `int(time.time())` for Unix timestamp in user_nbf key | Needs `import time` added to `api/auth.py` and `api/admin.py` |
|
||||||
|
|
||||||
|
### No new packages to install
|
||||||
|
|
||||||
|
This phase adds zero new dependencies. All required functionality is already present in the installed libraries and Python stdlib.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Package Legitimacy Audit
|
||||||
|
|
||||||
|
> No external packages are added in this phase. This section is intentionally blank.
|
||||||
|
|
||||||
|
**Packages removed due to slopcheck [SLOP] verdict:** none
|
||||||
|
**Packages flagged as suspicious [SUS]:** none
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Architecture Patterns
|
||||||
|
|
||||||
|
### System Architecture Diagram
|
||||||
|
|
||||||
|
```
|
||||||
|
Security Event (change_password / enable_totp / disable_totp / admin deactivate)
|
||||||
|
│
|
||||||
|
▼
|
||||||
|
api/auth.py or api/admin.py
|
||||||
|
│ (already) revoke_all_refresh_tokens(session, user_id, ...)
|
||||||
|
│ (NEW) await redis.set(f"user_nbf:{user_id}", int(time.time()), ex=900)
|
||||||
|
│
|
||||||
|
▼
|
||||||
|
Redis: user_nbf:{user_id} = <unix_ts> [TTL=900s]
|
||||||
|
|
||||||
|
Every API request (any protected route):
|
||||||
|
Authorization: Bearer <access_token>
|
||||||
|
│
|
||||||
|
▼
|
||||||
|
deps/auth.py :: get_current_user
|
||||||
|
│
|
||||||
|
├─ decode_access_token → payload (sub, role, iat, jti, ...)
|
||||||
|
│
|
||||||
|
├─ (NEW) nbf_bytes = await request.app.state.redis.get(f"user_nbf:{payload['sub']}")
|
||||||
|
│ if nbf_bytes and payload["iat"] < int(nbf_bytes.decode()):
|
||||||
|
│ raise HTTP 401 "Session invalidated"
|
||||||
|
│
|
||||||
|
└─ session.get(User, user_uuid) → return user
|
||||||
|
|
||||||
|
Token issuance (every login / refresh):
|
||||||
|
services/auth.py :: create_access_token
|
||||||
|
│ (NEW) jti=str(uuid.uuid4()) added to payload
|
||||||
|
└─ jwt.encode(payload, ...) → signed JWT
|
||||||
|
```
|
||||||
|
|
||||||
|
### Recommended Project Structure
|
||||||
|
|
||||||
|
No new files. Changes are surgical edits to existing files:
|
||||||
|
|
||||||
|
```
|
||||||
|
backend/
|
||||||
|
├── services/auth.py # add jti= to create_access_token payload
|
||||||
|
├── deps/auth.py # add user_nbf Redis check in get_current_user
|
||||||
|
├── api/auth.py # add redis write to change_password, enable_totp, disable_totp
|
||||||
|
└── api/admin.py # add redis write to deactivation handler
|
||||||
|
```
|
||||||
|
|
||||||
|
### Pattern 1: Adding jti to create_access_token
|
||||||
|
|
||||||
|
**What:** Insert `jti=str(uuid.uuid4())` into the payload dict before signing.
|
||||||
|
**When to use:** Always. PyJWT encodes arbitrary payload keys verbatim. The `jti` key is RFC 7519 standard (lowercase).
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Source: CONTEXT.md D-01 + PyJWT 2.13.0 verified behavior
|
||||||
|
import uuid
|
||||||
|
|
||||||
|
def create_access_token(user_id: str, role: str) -> str:
|
||||||
|
now = datetime.now(timezone.utc)
|
||||||
|
payload = {
|
||||||
|
"sub": str(user_id),
|
||||||
|
"role": role,
|
||||||
|
"typ": "access",
|
||||||
|
"iat": now,
|
||||||
|
"exp": now + timedelta(minutes=settings.access_token_expire_minutes),
|
||||||
|
"jti": str(uuid.uuid4()), # ← only change
|
||||||
|
}
|
||||||
|
return jwt.encode(payload, settings.secret_key, algorithm="HS256")
|
||||||
|
```
|
||||||
|
|
||||||
|
**Verified fact:** PyJWT 2.13.0 encodes `iat` as a Unix integer when a `datetime` object is passed. `payload["iat"]` after `decode()` is an `int`. `int(time.time())` is also an `int`. Direct `<` comparison works correctly. [VERIFIED: runtime test — `iat type: <class 'int'>` confirmed in this session]
|
||||||
|
|
||||||
|
### Pattern 2: user_nbf Redis write on security event
|
||||||
|
|
||||||
|
**What:** Write `user_nbf:{user_id}` = `int(time.time())` with TTL=900 to Redis immediately before `session.commit()` on any security event.
|
||||||
|
**When to use:** In `change_password`, `enable_totp`, `disable_totp`, and the admin deactivation handler.
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Source: CONTEXT.md D-02, D-03, D-06; mirrors totp_used pattern in services/auth.py
|
||||||
|
import time # new import
|
||||||
|
|
||||||
|
# Inside handler, after revoke_all_refresh_tokens, before session.commit():
|
||||||
|
redis_client = request.app.state.redis
|
||||||
|
await redis_client.set(f"user_nbf:{current_user.id}", int(time.time()), ex=900)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Key facts:**
|
||||||
|
- TTL = 900 = `ACCESS_TOKEN_EXPIRE_MINUTES * 60` = 15 * 60. Keys auto-expire when the oldest possible old token would have expired anyway. [VERIFIED: config.py `access_token_expire_minutes: int = 15`]
|
||||||
|
- `request.app.state.redis` is the aioredis client already confirmed at lines 197 and 566 of `api/auth.py`.
|
||||||
|
- Admin deactivation uses `request` from handler signature — same access pattern applies.
|
||||||
|
|
||||||
|
### Pattern 3: NBF check in get_current_user
|
||||||
|
|
||||||
|
**What:** After `decode_access_token` succeeds, check Redis for `user_nbf:{user_id}`. If set and `payload["iat"] < stored_timestamp`, raise HTTP 401.
|
||||||
|
**When to use:** In `deps/auth.py::get_current_user`, after the decode block, before the DB lookup.
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Source: CONTEXT.md D-02, D-03, D-04; mirrors HIBP fail-open pattern in services/auth.py
|
||||||
|
import logging
|
||||||
|
_logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
async def get_current_user(
|
||||||
|
request: Request,
|
||||||
|
credentials: HTTPAuthorizationCredentials = Depends(security),
|
||||||
|
session: AsyncSession = Depends(get_db),
|
||||||
|
) -> User:
|
||||||
|
try:
|
||||||
|
payload = auth_service.decode_access_token(credentials.credentials)
|
||||||
|
except ValueError as exc:
|
||||||
|
raise HTTPException(status_code=401, ...) from exc
|
||||||
|
|
||||||
|
# ── user_nbf check (NEW) ────────────────────────────────────────────
|
||||||
|
try:
|
||||||
|
redis = request.app.state.redis
|
||||||
|
nbf_bytes = await redis.get(f"user_nbf:{payload['sub']}")
|
||||||
|
if nbf_bytes is not None and payload["iat"] < int(nbf_bytes.decode()):
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||||
|
detail="Session invalidated",
|
||||||
|
headers={"WWW-Authenticate": "Bearer"},
|
||||||
|
)
|
||||||
|
except HTTPException:
|
||||||
|
raise # re-raise the 401 we just constructed
|
||||||
|
except Exception as exc:
|
||||||
|
_logger.warning("Redis user_nbf check failed (fail-open): %s", exc)
|
||||||
|
# ── end user_nbf check ──────────────────────────────────────────────
|
||||||
|
|
||||||
|
try:
|
||||||
|
user_uuid = uuid.UUID(payload["sub"])
|
||||||
|
except (KeyError, ValueError) as exc:
|
||||||
|
raise HTTPException(status_code=401, ...) from exc
|
||||||
|
|
||||||
|
user = await session.get(User, user_uuid)
|
||||||
|
if user is None or not user.is_active:
|
||||||
|
raise HTTPException(status_code=401, ...)
|
||||||
|
|
||||||
|
request.state.current_user = user
|
||||||
|
return user
|
||||||
|
```
|
||||||
|
|
||||||
|
**Critical detail:** The `except Exception` that implements fail-open must explicitly re-raise `HTTPException` first, otherwise it would swallow the intentional 401 we just raised. [ASSUMED — standard Python try/except behavior, no special framework wrapping needed here, but the planner must verify the exception-in-except pattern is correct]
|
||||||
|
|
||||||
|
### Pattern 4: Admin deactivation handler (api/admin.py ~line 351)
|
||||||
|
|
||||||
|
**What:** Add Redis write immediately after the existing `revoke_all_refresh_tokens` call.
|
||||||
|
**When to use:** When `not body.is_active` — same condition as the existing revocation.
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Source: CONTEXT.md D-05; existing handler at admin.py:351
|
||||||
|
if not body.is_active:
|
||||||
|
await revoke_all_refresh_tokens(session, user.id)
|
||||||
|
# (NEW) immediately block any live access token
|
||||||
|
await request.app.state.redis.set(
|
||||||
|
f"user_nbf:{user.id}", int(time.time()), ex=900
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Import needed:** `import time` at the top of `api/admin.py` (check if already present).
|
||||||
|
|
||||||
|
### Anti-Patterns to Avoid
|
||||||
|
|
||||||
|
- **Do not swallow HTTPException in the fail-open catch:** The `except Exception as exc` block must have a `raise` for `HTTPException` before the general catch. Otherwise a valid revocation check (iat < nbf) will silently pass through as fail-open.
|
||||||
|
- **Do not use `datetime`-based comparison for iat vs nbf:** `payload["iat"]` is an int (verified above). `int(time.time())` is also an int. Use integer comparison — no timezone conversion needed.
|
||||||
|
- **Do not write user_nbf for successful activation (`body.is_active == True`):** The Redis write is only needed on deactivation. Writing it on activation would block the just-reactivated user's next request.
|
||||||
|
- **Do not add `import time` inside handler bodies:** Add it at the module level of `api/auth.py` and `api/admin.py` following the existing module-level imports.
|
||||||
|
- **Do not set TTL to `settings.access_token_expire_minutes * 60` computed inline:** Hardcode 900 or define `ACCESS_TOKEN_NBF_TTL = 15 * 60` as a module-level constant next to `create_access_token`. The constant TTL is intentional and should be obvious.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Don't Hand-Roll
|
||||||
|
|
||||||
|
| Problem | Don't Build | Use Instead | Why |
|
||||||
|
|---------|-------------|-------------|-----|
|
||||||
|
| UUID generation for jti | Custom random hex | `str(uuid.uuid4())` | Already imported in `services/auth.py`; RFC 7519 compliant; collision-free |
|
||||||
|
| Unix timestamp | Custom datetime arithmetic | `int(time.time())` | Direct, correct, no timezone confusion |
|
||||||
|
| Redis TTL management | Python-side expiry tracking | `redis.set(..., ex=900)` | Atomic server-side TTL; correct even if the app crashes after writing |
|
||||||
|
| Fail-open Redis errors | Try/except that re-raises | Logger warning + proceed | Matches HIBP pattern already in codebase; availability > availability of revocation during outage |
|
||||||
|
|
||||||
|
**Key insight:** This phase is almost entirely plumbing existing infrastructure. The TOTP replay prevention (`totp_used:{user_id}:{code}` with TTL=90) is a direct template for `user_nbf:{user_id}` with TTL=900.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Common Pitfalls
|
||||||
|
|
||||||
|
### Pitfall 1: Swallowing the HTTPException in fail-open
|
||||||
|
**What goes wrong:** The `except Exception as exc` catch intended for Redis errors silently catches the `HTTPException(401)` raised by the iat-vs-nbf check, allowing a revoked token through.
|
||||||
|
**Why it happens:** Python's `except Exception` catches all exceptions including `HTTPException`.
|
||||||
|
**How to avoid:** Always add `except HTTPException: raise` before the broad `except Exception` catch in the user_nbf check block.
|
||||||
|
**Warning signs:** Test that verifies NBF revocation passes in isolation but fails when run inside a try/except block — the 401 never reaches the test assertion.
|
||||||
|
|
||||||
|
### Pitfall 2: Wrong comparison direction
|
||||||
|
**What goes wrong:** Checking `nbf_timestamp < payload["iat"]` instead of `payload["iat"] < nbf_timestamp` — logic is inverted; tokens issued AFTER the event are blocked, tokens issued BEFORE are allowed.
|
||||||
|
**Why it happens:** Easy to flip the inequality when reading "token issued before NBF is invalid."
|
||||||
|
**How to avoid:** Read it as: "token's issue time (iat) is EARLIER THAN the not-before timestamp (nbf) → invalid." `payload["iat"] < nbf_timestamp` → raise 401.
|
||||||
|
**Warning signs:** Security test where a token issued before the event still works — the revocation is a no-op.
|
||||||
|
|
||||||
|
### Pitfall 3: Blocking the recovering session
|
||||||
|
**What goes wrong:** The user changes their password (security event) → user_nbf is written → user's own current session's access token is also blocked → next API call returns 401 → frontend must refresh the access token.
|
||||||
|
**Why this is intentional, not a bug:** The CONTEXT.md D-02 explicitly states: "the calling session's own token is also immediately blocked (its iat predates now()), but it recovers transparently by exchanging its still-live refresh token (which Phase 7.1 D-02 preserved) for a new access token with iat > nbf." The frontend's silent refresh path handles this. No code change is needed to "fix" this — it is the intended behavior.
|
||||||
|
**Warning signs:** A test that calls change_password and then immediately calls `/api/auth/me` WITHOUT refreshing the access token → will get 401. This is correct. The test must refresh first.
|
||||||
|
|
||||||
|
### Pitfall 4: Redis not available in test fixtures
|
||||||
|
**What goes wrong:** `get_current_user` calls `request.app.state.redis` but the test app (created via `make_test_app()` in `test_auth_deps.py`) has no `app.state.redis` set → `AttributeError`.
|
||||||
|
**Why it happens:** The minimal test app in `test_auth_deps.py` does not mount the full lifespan that creates `app.state.redis`. The new NBF check will crash with `AttributeError` on any test using that fixture.
|
||||||
|
**How to avoid:** In `test_auth_deps.py`, add a FakeRedis instance to `app.state.redis` before tests run. Since the NBF key will not be set in the fake Redis, the check will pass through (no key = no block). The existing FakeRedis class in `test_auth_api.py` already supports `get` and `set` — reuse it or import it.
|
||||||
|
**Warning signs:** `AttributeError: 'State' object has no attribute 'redis'` in any test that exercises `get_current_user`.
|
||||||
|
|
||||||
|
### Pitfall 5: admin.py does not import `time`
|
||||||
|
**What goes wrong:** `int(time.time())` raises `NameError: name 'time' is not defined`.
|
||||||
|
**Why it happens:** `api/admin.py` does not currently import the `time` stdlib module (grep confirms no `import time` there).
|
||||||
|
**How to avoid:** Add `import time` to the module-level imports of `api/admin.py`.
|
||||||
|
**Warning signs:** `NameError` in the deactivation handler during testing.
|
||||||
|
|
||||||
|
### Pitfall 6: TTL mismatch between user_nbf write sites
|
||||||
|
**What goes wrong:** One handler writes TTL=900 and another writes a different value, creating inconsistent revocation windows.
|
||||||
|
**How to avoid:** Define `_USER_NBF_TTL = 900` as a module-level constant in `api/auth.py` (or in `config.py` as `access_token_expire_minutes * 60`). Use the constant in all four write sites. In `api/admin.py`, import the constant or hardcode 900 with a comment referencing it.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Code Examples
|
||||||
|
|
||||||
|
### Verified: PyJWT iat encoding behavior
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Verified in this research session with PyJWT 2.13.0
|
||||||
|
# payload["iat"] after jwt.decode() is always an int (Unix timestamp)
|
||||||
|
# int(time.time()) is also an int
|
||||||
|
# Direct integer comparison works: payload["iat"] < int(nbf_bytes.decode())
|
||||||
|
decoded = jwt.decode(token, settings.secret_key, algorithms=["HS256"])
|
||||||
|
type(decoded["iat"]) # <class 'int'> ← verified
|
||||||
|
```
|
||||||
|
[VERIFIED: runtime test — see research session output]
|
||||||
|
|
||||||
|
### Verified: Redis set with TTL
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Source: services/auth.py::verify_totp (existing pattern, line 296)
|
||||||
|
await redis_client.set(replay_key, "1", ex=90)
|
||||||
|
# Same pattern for user_nbf:
|
||||||
|
await redis_client.set(f"user_nbf:{user_id}", int(time.time()), ex=900)
|
||||||
|
```
|
||||||
|
[VERIFIED: codebase — `services/auth.py:296`]
|
||||||
|
|
||||||
|
### Verified: Redis get returning bytes
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Source: services/auth.py::verify_totp (existing pattern, line 289)
|
||||||
|
if await redis_client.get(replay_key):
|
||||||
|
return False
|
||||||
|
# For user_nbf, need the value (not just truthiness):
|
||||||
|
nbf_bytes = await redis_client.get(f"user_nbf:{payload['sub']}")
|
||||||
|
if nbf_bytes is not None and payload["iat"] < int(nbf_bytes.decode()):
|
||||||
|
raise HTTPException(...)
|
||||||
|
```
|
||||||
|
[VERIFIED: codebase — `services/auth.py:289`; `redis.get()` returns `bytes | None`]
|
||||||
|
|
||||||
|
### Verified: Fail-open pattern
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Source: services/auth.py::check_hibp (lines 394-396) — exact template
|
||||||
|
try:
|
||||||
|
# ... Redis call ...
|
||||||
|
except Exception as exc:
|
||||||
|
logger.warning("HIBP check failed (fail-open): %s", exc)
|
||||||
|
return False
|
||||||
|
```
|
||||||
|
[VERIFIED: codebase — `services/auth.py:394`]
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## State of the Art
|
||||||
|
|
||||||
|
| Old Approach | Current Approach | When Changed | Impact |
|
||||||
|
|--------------|------------------|--------------|--------|
|
||||||
|
| No token revocation after security event (access token valid until TTL) | user-level NBF key in Redis; closes 15-min gap | Phase 7.2 | Deactivated users cannot continue using live access tokens |
|
||||||
|
| No jti claim | `jti=str(uuid.uuid4())` in every access token | Phase 7.2 | Prerequisite for Phase 7.3 ES256 migration; enables future per-JTI revocation |
|
||||||
|
|
||||||
|
**Deferred / out of scope:**
|
||||||
|
- Per-JTI revocation (one Redis key per token): more granular but not needed; user-NBF covers all sessions uniformly and is O(1) writes vs O(N) for N active sessions.
|
||||||
|
- ES256 algorithm upgrade (Phase 7.3): jti claim is its prerequisite.
|
||||||
|
- Token fingerprinting (Phase 7.4): fgp claim not in scope here.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Assumptions Log
|
||||||
|
|
||||||
|
| # | Claim | Section | Risk if Wrong |
|
||||||
|
|---|-------|---------|---------------|
|
||||||
|
| A1 | The `except Exception` catch in the fail-open block must have an explicit `except HTTPException: raise` guard before it | Architecture Patterns, Pattern 3 | If wrong (e.g., FastAPI wraps HTTPException in a different base), the guard might be unnecessary boilerplate — low risk, safe to include regardless |
|
||||||
|
| A2 | `api/admin.py` does not currently import `time` | Common Pitfalls 5 | If wrong, no change needed — planner should verify with grep |
|
||||||
|
|
||||||
|
**If this table is empty:** Not applicable — A1 and A2 are documented above.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Open Questions
|
||||||
|
|
||||||
|
1. **Does `test_auth_deps.py` need a FakeRedis fixture?**
|
||||||
|
- What we know: `test_auth_deps.py` creates a minimal test app that calls `get_current_user`. After this phase, `get_current_user` will call `request.app.state.redis`. The minimal app has no lifespan handler.
|
||||||
|
- What's unclear: Whether the existing `auth_client` fixture sets `app.state.redis` anywhere.
|
||||||
|
- Recommendation: Planner should add `test_app.state.redis = FakeRedis()` in `make_test_app()` or in the fixture. No Redis key will be set so all existing tests continue to pass.
|
||||||
|
|
||||||
|
2. **Should `_USER_NBF_TTL` be a constant in `config.py` or a module constant in `api/auth.py`?**
|
||||||
|
- What we know: `access_token_expire_minutes` is already in `config.py`; TTL = `access_token_expire_minutes * 60`.
|
||||||
|
- What's unclear: Whether the user prefers `settings.access_token_expire_minutes * 60` (derived at runtime) or a hardcoded `900`.
|
||||||
|
- Recommendation: Use `settings.access_token_expire_minutes * 60` in all write sites so that if the TTL ever changes in config, the NBF TTL stays synchronized automatically. This is slightly cleaner than a separate constant.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Environment Availability
|
||||||
|
|
||||||
|
| Dependency | Required By | Available | Version | Fallback |
|
||||||
|
|------------|------------|-----------|---------|----------|
|
||||||
|
| Redis (`app.state.redis`) | user_nbf writes and reads | Yes (runtime via Docker Compose; FakeRedis in tests) | redis>=4.6.0 | Fail-open (D-04) |
|
||||||
|
| PyJWT | `create_access_token`, `decode_access_token` | Yes | 2.13.0 [VERIFIED: runtime] | — |
|
||||||
|
| Python `uuid` stdlib | `str(uuid.uuid4())` for jti | Yes (stdlib) | always available | — |
|
||||||
|
| Python `time` stdlib | `int(time.time())` for user_nbf value | Yes (stdlib, needs `import time`) | always available | — |
|
||||||
|
|
||||||
|
**Missing dependencies with no fallback:** None.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Validation Architecture
|
||||||
|
|
||||||
|
### Test Framework
|
||||||
|
|
||||||
|
| Property | Value |
|
||||||
|
|----------|-------|
|
||||||
|
| Framework | pytest + pytest-asyncio |
|
||||||
|
| Config file | `backend/pytest.ini` (or inline `pyproject.toml` — check existing) |
|
||||||
|
| Quick run command | `pytest tests/test_auth_deps.py tests/test_auth_api.py -v` |
|
||||||
|
| Full suite command | `pytest -v` |
|
||||||
|
|
||||||
|
### Phase Requirements → Test Map
|
||||||
|
|
||||||
|
| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|
||||||
|
|--------|----------|-----------|-------------------|-------------|
|
||||||
|
| SEC (JTI) | Every access token contains a `jti` UUID claim | Unit | `pytest tests/test_task2_auth_service.py -k jti -v` | Wave 0 stub |
|
||||||
|
| SEC (NBF-write) | change_password writes user_nbf Redis key | Integration | `pytest tests/test_auth_api.py -k nbf -v` | Wave 0 stub |
|
||||||
|
| SEC (NBF-write) | enable_totp writes user_nbf Redis key | Integration | `pytest tests/test_auth_api.py -k nbf -v` | Wave 0 stub |
|
||||||
|
| SEC (NBF-write) | disable_totp writes user_nbf Redis key | Integration | `pytest tests/test_auth_api.py -k nbf -v` | Wave 0 stub |
|
||||||
|
| SEC (NBF-write) | admin deactivation writes user_nbf Redis key | Integration | `pytest tests/test_admin_api.py -k nbf -v` | Wave 0 stub |
|
||||||
|
| SEC (NBF-check) | Token with iat before user_nbf returns 401 | Integration | `pytest tests/test_auth_deps.py -k nbf -v` | Wave 0 stub |
|
||||||
|
| SEC (fail-open) | Redis error in NBF check does not block request | Unit | `pytest tests/test_auth_deps.py -k failopen -v` | Wave 0 stub |
|
||||||
|
| SEC (NBF-check) | Token with iat after user_nbf is accepted | Integration | `pytest tests/test_auth_deps.py -k nbf_after -v` | Wave 0 stub |
|
||||||
|
|
||||||
|
### Sampling Rate
|
||||||
|
|
||||||
|
- **Per task commit:** `pytest tests/test_auth_deps.py tests/test_auth_api.py tests/test_admin_api.py -v`
|
||||||
|
- **Per wave merge:** `pytest -v` (full suite)
|
||||||
|
- **Phase gate:** Full suite green before `/gsd:verify-work`
|
||||||
|
|
||||||
|
### Wave 0 Gaps
|
||||||
|
|
||||||
|
- [ ] `tests/test_auth_api.py` — add NBF-write tests for change_password, enable_totp, disable_totp
|
||||||
|
- [ ] `tests/test_auth_deps.py` — add NBF-check tests (iat < nbf → 401, iat > nbf → pass, Redis error → pass); add FakeRedis to `auth_client` fixture or `make_test_app()`
|
||||||
|
- [ ] `tests/test_admin_api.py` — add NBF-write test for deactivation handler
|
||||||
|
- [ ] `tests/test_task2_auth_service.py` — add JTI presence test for `create_access_token`
|
||||||
|
|
||||||
|
*(Existing 34 auth tests in `test_auth_deps.py` and `test_auth_api.py` must remain green with zero regressions.)*
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Security Domain
|
||||||
|
|
||||||
|
### Applicable ASVS Categories
|
||||||
|
|
||||||
|
| ASVS Category | Applies | Standard Control |
|
||||||
|
|---------------|---------|-----------------|
|
||||||
|
| V2 Authentication | Yes | user_nbf key closes post-revocation window |
|
||||||
|
| V3 Session Management | Yes | user_nbf + existing refresh revocation = complete session invalidation |
|
||||||
|
| V4 Access Control | No | No new access control decisions |
|
||||||
|
| V5 Input Validation | No | No new user input |
|
||||||
|
| V6 Cryptography | No | jti is UUID (not a secret); no new crypto |
|
||||||
|
|
||||||
|
### Known Threat Patterns for JWT + Redis stack
|
||||||
|
|
||||||
|
| Pattern | STRIDE | Standard Mitigation |
|
||||||
|
|---------|--------|---------------------|
|
||||||
|
| Stolen access token replay after deactivation | Elevation of Privilege | user_nbf Redis key blocks iat-predating tokens (this phase) |
|
||||||
|
| Redis outage used to bypass revocation | Denial of Service / EoP | Fail-open (D-04) — availability preferred over blocking; known and accepted trade-off |
|
||||||
|
| user_nbf key TTL mismatch | Elevation of Privilege | Derive TTL from `settings.access_token_expire_minutes * 60` to stay synchronized |
|
||||||
|
| HTTPException swallowed by fail-open catch | Elevation of Privilege | Explicit `except HTTPException: raise` guard before broad `except Exception` |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Sources
|
||||||
|
|
||||||
|
### Primary (HIGH confidence)
|
||||||
|
- PyJWT 2.13.0 — runtime verified: `iat` decoded as `int`, `jti` custom claim encoded verbatim
|
||||||
|
- `backend/services/auth.py` — TOTP replay prevention pattern (lines 288-296) as direct template for user_nbf
|
||||||
|
- `backend/api/auth.py` — `request.app.state.redis` access pattern (lines 197, 566)
|
||||||
|
- `backend/deps/auth.py` — `get_current_user` signature and exception handling structure
|
||||||
|
- `backend/config.py` — `access_token_expire_minutes: int = 15` confirms 900s TTL
|
||||||
|
- `.planning/phases/07.2-security-jti-claim-redis-access-token-revocation-inserted/07.2-CONTEXT.md` — locked decisions D-01 through D-06
|
||||||
|
|
||||||
|
### Secondary (MEDIUM confidence)
|
||||||
|
- `backend/services/auth.py::check_hibp` — fail-open exception handling template
|
||||||
|
- `backend/api/admin.py:351` — deactivation handler structure for D-05 write site
|
||||||
|
- `backend/tests/test_auth_api.py::FakeRedis` — test infrastructure reusable for Phase 7.2 tests
|
||||||
|
|
||||||
|
### Tertiary (LOW confidence)
|
||||||
|
- None — all claims verified against codebase or runtime.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Metadata
|
||||||
|
|
||||||
|
**Confidence breakdown:**
|
||||||
|
- Standard stack: HIGH — no new packages; all libraries verified in runtime
|
||||||
|
- Architecture: HIGH — patterns directly copied from existing codebase (TOTP replay, HIBP fail-open, redis access)
|
||||||
|
- Pitfalls: HIGH — derived from direct code inspection of exception handling and test infrastructure
|
||||||
|
- Test gaps: HIGH — confirmed by reading test files; FakeRedis missing from auth_deps fixture is a real gap
|
||||||
|
|
||||||
|
**Research date:** 2026-06-05
|
||||||
|
**Valid until:** Stable (no fast-moving dependencies; Python stdlib and PyJWT 2.x API is stable)
|
||||||
Reference in New Issue
Block a user