Harden JWT: 8-hour expiry, add JWT vulnerability checks
- Reduce ACCESS_TOKEN_EXPIRE_MINUTES from 24h to 8h (no permanent sessions) - Add JWT_PATTERNS to security_check.py: algorithm=none, verify_exp=False, multi-day timedelta, oversized EXPIRE_MINUTES, hardcoded secret - Add JWT security checklist to security-auditor agent - Document auth/session security items in TODO.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -54,6 +54,24 @@ Systematically review in this order:
|
|||||||
6. CORS configuration (`app/main.py`)
|
6. CORS configuration (`app/main.py`)
|
||||||
7. Frontend — token storage, XSS vectors, any `dangerouslySetInnerHTML`
|
7. Frontend — token storage, XSS vectors, any `dangerouslySetInnerHTML`
|
||||||
|
|
||||||
|
## JWT security checklist
|
||||||
|
|
||||||
|
When reviewing any authentication code, verify all of the following:
|
||||||
|
|
||||||
|
| Check | What to look for | Severity |
|
||||||
|
|---|---|---|
|
||||||
|
| Algorithm confusion | `algorithms=["none"]` or `algorithm="none"` in `jwt.decode()` | Critical |
|
||||||
|
| Expiry enforcement | `verify_exp=False` or `options={"verify_exp": False}` | Critical |
|
||||||
|
| Token lifetime | `ACCESS_TOKEN_EXPIRE_MINUTES` — must be ≤ 480 (8 h); flag `timedelta(days=...)` in token creation | High |
|
||||||
|
| Secret key strength | `SECRET_KEY` must come from env var, ≥ 32 random chars; flag hardcoded strings | High |
|
||||||
|
| Algorithm pinned | `jwt.decode()` must pass `algorithms=["HS256"]` (or project algorithm) explicitly — never a variable | High |
|
||||||
|
| Missing claims | Token payload should include `sub`, `exp`, `iat`; flag if `iat` is absent | Medium |
|
||||||
|
| Token storage | Frontend stores JWT in `localStorage` — note the XSS exposure tradeoff; recommend `httpOnly` cookie migration when hardening | Medium |
|
||||||
|
| No refresh tokens | Project policy: no permanent sessions, no refresh tokens. Flag any `refresh_token` implementation | Medium |
|
||||||
|
| No "remember me" | No `remember_me` or extended-expiry paths in auth flow | Medium |
|
||||||
|
|
||||||
|
Current project policy: **8-hour JWT, no refresh tokens, no permanent login.**
|
||||||
|
|
||||||
## Hard rules
|
## Hard rules
|
||||||
|
|
||||||
- Never weaken an existing security control
|
- Never weaken an existing security control
|
||||||
|
|||||||
@@ -8,6 +8,12 @@
|
|||||||
- [ ] **Decide on UI component library** — shadcn/ui (recommended: Tailwind-based, unstyled accessible primitives, white-label friendly) vs MUI vs other; decision affects both Penpot design system and frontend implementation
|
- [ ] **Decide on UI component library** — shadcn/ui (recommended: Tailwind-based, unstyled accessible primitives, white-label friendly) vs MUI vs other; decision affects both Penpot design system and frontend implementation
|
||||||
- [ ] **Connect ux-designer agent** — confirm Penpot API reachable, provide instance URL + token to agent at session start
|
- [ ] **Connect ux-designer agent** — confirm Penpot API reachable, provide instance URL + token to agent at session start
|
||||||
|
|
||||||
|
## Auth / session security
|
||||||
|
|
||||||
|
- [x] **8-hour JWT expiry** — `ACCESS_TOKEN_EXPIRE_MINUTES = 60 * 8`; no permanent login
|
||||||
|
- [ ] **No refresh tokens** — refresh token flow not implemented; if added later, must use `httpOnly` cookies and rotation
|
||||||
|
- [ ] **`httpOnly` cookie migration** — currently storing JWT in `localStorage` (XSS-exposed); migrate to `httpOnly` cookie when hardening for production
|
||||||
|
|
||||||
## App permissions
|
## App permissions
|
||||||
|
|
||||||
- [ ] **Permissions registry** — admin-managed table that controls which apps each user can access. Schema: `user_app_permissions (user_id FK, app_key)`. Admin UI lets the admin grant/revoke per-app access per user. The Apps page only shows apps the current user has been granted access to.
|
- [ ] **Permissions registry** — admin-managed table that controls which apps each user can access. Schema: `user_app_permissions (user_id FK, app_key)`. Admin UI lets the admin grant/revoke per-app access per user. The Apps page only shows apps the current user has been granted access to.
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ class Settings(BaseSettings):
|
|||||||
|
|
||||||
SECRET_KEY: str = "change-me-in-production"
|
SECRET_KEY: str = "change-me-in-production"
|
||||||
ALGORITHM: str = "HS256"
|
ALGORITHM: str = "HS256"
|
||||||
ACCESS_TOKEN_EXPIRE_MINUTES: int = 60 * 24 # 1 day
|
ACCESS_TOKEN_EXPIRE_MINUTES: int = 60 * 8 # 8 hours — no permanent sessions
|
||||||
|
|
||||||
CORS_ORIGINS: list[str] = ["http://localhost:5173"]
|
CORS_ORIGINS: list[str] = ["http://localhost:5173"]
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,14 @@
|
|||||||
|
# 2026-04-13 — JWT token expiry hardened to 8 hours
|
||||||
|
|
||||||
|
**Timestamp:** 2026-04-13T04:00:00
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Reduced JWT token lifetime from 24 hours to 8 hours with no permanent session option. Added JWT vulnerability detection to the pre-commit security check and a JWT security checklist to the security-auditor agent. Updated TODO with auth/session security items.
|
||||||
|
|
||||||
|
## Files Modified
|
||||||
|
|
||||||
|
- `backend/app/core/config.py` — `ACCESS_TOKEN_EXPIRE_MINUTES` changed from `60 * 24` to `60 * 8`; added comment "no permanent sessions"
|
||||||
|
- `scripts/security_check.py` — added `JWT_PATTERNS` category: algorithm confusion (`none`), disabled expiry verification, multi-day token lifetime, oversized EXPIRE_MINUTES, hardcoded secret; wired into `ALL_PATTERNS` and updated docstring
|
||||||
|
- `.claude/agents/security-auditor.md` — added JWT security checklist table covering algorithm confusion, expiry enforcement, token lifetime, secret key strength, missing claims, localStorage storage, no refresh tokens policy
|
||||||
|
- `TODO.md` — added "Auth / session security" section: 8-hour JWT checked off, refresh token and httpOnly cookie migration as future items
|
||||||
@@ -9,8 +9,9 @@ Checks:
|
|||||||
3. Weak cryptography (MD5, SHA1 for passwords, DES)
|
3. Weak cryptography (MD5, SHA1 for passwords, DES)
|
||||||
4. SQL injection risk (f-strings / .format() / % in execute/query/text())
|
4. SQL injection risk (f-strings / .format() / % in execute/query/text())
|
||||||
5. Missing input sanitization (raw request attributes passed to DB)
|
5. Missing input sanitization (raw request attributes passed to DB)
|
||||||
6. Debug/development flags left in code
|
6. JWT vulnerabilities (algorithm=none, verify_exp=False, long-lived tokens)
|
||||||
7. bandit static analysis on Python files
|
7. Debug/development flags left in code
|
||||||
|
8. bandit static analysis on Python files
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import os
|
import os
|
||||||
@@ -74,6 +75,28 @@ SANITIZATION_PATTERNS = [
|
|||||||
"raw request attribute passed to DB — route through a Pydantic schema first"),
|
"raw request attribute passed to DB — route through a Pydantic schema first"),
|
||||||
]
|
]
|
||||||
|
|
||||||
|
JWT_PATTERNS = [
|
||||||
|
# Algorithm confusion attack — accepting 'none' algorithm
|
||||||
|
(r'algorithms?\s*=\s*\[.*["\']none["\']',
|
||||||
|
"JWT algorithm 'none' accepted — algorithm confusion attack (Critical)"),
|
||||||
|
(r'algorithm\s*=\s*["\']none["\']',
|
||||||
|
"JWT algorithm set to 'none' — algorithm confusion attack (Critical)"),
|
||||||
|
# Disabling expiry verification
|
||||||
|
(r'verify_exp\s*[=:]\s*False',
|
||||||
|
"JWT expiry verification disabled — tokens never expire (Critical)"),
|
||||||
|
(r'options\s*=\s*\{[^}]*["\']verify_exp["\'].*False',
|
||||||
|
"JWT expiry verification disabled in options dict (Critical)"),
|
||||||
|
# Long-lived tokens: timedelta(days=...) in JWT context is suspicious
|
||||||
|
(r'timedelta\s*\(\s*days\s*=\s*[1-9]',
|
||||||
|
"JWT token with multi-day expiry — use hours, not days (High)"),
|
||||||
|
# Overly large EXPIRE_MINUTES constant (>9999 min ≈ 7 days)
|
||||||
|
(r'EXPIRE_MINUTES\s*[=:]\s*[1-9]\d{4,}',
|
||||||
|
"JWT EXPIRE_MINUTES value > 9999 (> 7 days) — reduce token lifetime (High)"),
|
||||||
|
# Hardcoded JWT secret
|
||||||
|
(r'SECRET_KEY\s*=\s*["\'][a-zA-Z0-9_\-]{4,}["\'](?!.*env|.*change)',
|
||||||
|
"possible hardcoded JWT secret — use env var (High)"),
|
||||||
|
]
|
||||||
|
|
||||||
DEBUG_PATTERNS = [
|
DEBUG_PATTERNS = [
|
||||||
(r'\bdebug\s*=\s*True\b', "debug=True left in code"),
|
(r'\bdebug\s*=\s*True\b', "debug=True left in code"),
|
||||||
(r'print\s*\(.*password', "possible password printed to stdout"),
|
(r'print\s*\(.*password', "possible password printed to stdout"),
|
||||||
@@ -85,6 +108,7 @@ ALL_PATTERNS = (
|
|||||||
+ [("CRYPTO", p, m) for p, m in WEAK_CRYPTO_PATTERNS]
|
+ [("CRYPTO", p, m) for p, m in WEAK_CRYPTO_PATTERNS]
|
||||||
+ [("SQLINJ", p, m) for p, m in SQL_INJECTION_PATTERNS]
|
+ [("SQLINJ", p, m) for p, m in SQL_INJECTION_PATTERNS]
|
||||||
+ [("SANIT", p, m) for p, m in SANITIZATION_PATTERNS]
|
+ [("SANIT", p, m) for p, m in SANITIZATION_PATTERNS]
|
||||||
|
+ [("JWT", p, m) for p, m in JWT_PATTERNS]
|
||||||
+ [("DEBUG", p, m) for p, m in DEBUG_PATTERNS]
|
+ [("DEBUG", p, m) for p, m in DEBUG_PATTERNS]
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user