Skip to content

Commit bff0e21

Browse files
authored
feat: Add /api/v1 prefix and security hardening (#16)
* feat: Add /api/v1 prefix and security hardening API Versioning: - Add /api/v1 prefix to all user data endpoints - Keep /health and /oauth at root level (no version prefix) Security Fixes (High Priority): - Tighten CSRF exclusions - remove /admin/sync, /admin/api-keys/, /admin/settings (HTMX already sends CSRF token) - Add callback URL validation for OAuth - require HTTPS in production, validate URL is well-formed - Add login rate limiting - lock out after 5 failed attempts for 15 min Security Fixes (Medium Priority): - Bound OAuth state dictionaries with maxsize=1000 to prevent memory exhaustion attacks - Add explicit session cookie security: secure, httponly, samesite=lax - Add SRI integrity hashes to CDN scripts (htmx, chart.js, date-fns) Documentation: - Update README API examples with /api/v1 prefix Stu Mason + AI <me@stumason.dev> * style: Fix ruff formatting Stu Mason + AI <me@stumason.dev> * fix: Address security review feedback Security fixes: - Fix IP spoofing: only trust proxy headers from localhost (reverse proxy) - Fix callback URL validation: explicit production check, strict localhost - Add async locks to rate limiter and OAuth caches (thread safety) - Fix race conditions: use dict comprehension for atomic cleanup - Lower OAuth state cache to 100 entries, log when full - Add client_id length validation (max 255 chars) - Simplify OAuth callback error handling The CSRF exclusion concern in the review was a misread - we already removed /admin/sync, /admin/api-keys/, /admin/settings from exclusions. Stu Mason + AI <me@stumason.dev>
1 parent 2d41a5c commit bff0e21

File tree

6 files changed

+393
-75
lines changed

6 files changed

+393
-75
lines changed

README.md

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().d
132132
```bash
133133
# With per-user API key (includes rate limit headers)
134134
curl -H "X-API-Key: pfk_your_api_key_here" \
135-
http://localhost:8000/users/{user_id}/sleep?days=7
135+
http://localhost:8000/api/v1/users/{user_id}/sleep?days=7
136136

137137
# Response headers include:
138138
# X-RateLimit-Limit: 1000
@@ -211,7 +211,7 @@ Store `api_key` and `polar_user_id` for this user. Use the API key for all data
211211

212212
```bash
213213
curl -H "X-API-Key: pfk_abc123..." \
214-
"https://your-polar-server.com/users/12345678/sleep?days=7"
214+
"https://your-polar-server.com/api/v1/users/12345678/sleep?days=7"
215215
```
216216

217217
### Polar Admin Setup
@@ -226,43 +226,43 @@ https://your-polar-server.com/oauth/callback
226226

227227
```bash
228228
# Get key info
229-
GET /users/{user_id}/api-key/info
229+
GET /api/v1/users/{user_id}/api-key/info
230230
X-API-Key: pfk_...
231231

232232
# Regenerate key (invalidates old key)
233-
POST /users/{user_id}/api-key/regenerate
233+
POST /api/v1/users/{user_id}/api-key/regenerate
234234
X-API-Key: pfk_...
235235

236236
# Revoke key
237-
POST /users/{user_id}/api-key/revoke
237+
POST /api/v1/users/{user_id}/api-key/revoke
238238
X-API-Key: pfk_...
239239
```
240240

241241
## API Endpoints
242242

243243
```bash
244-
# Health check
244+
# Health check (no auth required)
245245
curl http://localhost:8000/health
246246

247247
# Get sleep data (last 7 days)
248248
curl -H "X-API-Key: pfk_..." \
249-
"http://localhost:8000/users/{user_id}/sleep?days=7"
249+
"http://localhost:8000/api/v1/users/{user_id}/sleep?days=7"
250250

251251
# Get activity data
252252
curl -H "X-API-Key: pfk_..." \
253-
"http://localhost:8000/users/{user_id}/activity?days=7"
253+
"http://localhost:8000/api/v1/users/{user_id}/activity?days=7"
254254

255255
# Get nightly recharge (HRV)
256256
curl -H "X-API-Key: pfk_..." \
257-
"http://localhost:8000/users/{user_id}/recharge?days=7"
257+
"http://localhost:8000/api/v1/users/{user_id}/recharge?days=7"
258258

259259
# Get exercises
260260
curl -H "X-API-Key: pfk_..." \
261-
"http://localhost:8000/users/{user_id}/exercises?days=30"
261+
"http://localhost:8000/api/v1/users/{user_id}/exercises?days=30"
262262

263263
# Export summary
264264
curl -H "X-API-Key: pfk_..." \
265-
"http://localhost:8000/users/{user_id}/export/summary?days=30"
265+
"http://localhost:8000/api/v1/users/{user_id}/export/summary?days=30"
266266
```
267267

268268
## Development

src/polar_flow_server/admin/routes.py

Lines changed: 196 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
"""Admin panel routes."""
22

3+
import asyncio
34
import csv
45
import io
6+
import logging
57
import os
68
import re
79
import secrets
10+
from collections import OrderedDict
811
from datetime import UTC, date, datetime, timedelta
912
from typing import Any
1013
from urllib.parse import urlencode
@@ -52,9 +55,150 @@
5255
from polar_flow_server.services.scheduler import get_scheduler
5356
from polar_flow_server.services.sync import SyncService
5457

55-
# In-memory OAuth state storage (for self-hosted single-instance use)
56-
# In production SaaS, use Redis or database with TTL
57-
_oauth_states: dict[str, datetime] = {}
58+
logger = logging.getLogger(__name__)
59+
60+
61+
# =============================================================================
62+
# Bounded TTL Cache for OAuth States (prevents memory exhaustion)
63+
# =============================================================================
64+
65+
66+
class BoundedTTLCache:
67+
"""Simple bounded cache with TTL for OAuth states.
68+
69+
Prevents memory exhaustion attacks by limiting max entries.
70+
Automatically evicts expired entries on access.
71+
Thread-safe via asyncio lock.
72+
"""
73+
74+
def __init__(self, maxsize: int = 100, ttl_minutes: int = 10) -> None:
75+
self._cache: OrderedDict[str, datetime] = OrderedDict()
76+
self._maxsize = maxsize
77+
self._ttl = timedelta(minutes=ttl_minutes)
78+
self._lock = asyncio.Lock()
79+
80+
async def set(self, key: str, expires_at: datetime | None = None) -> None:
81+
"""Add or update a key with expiry time."""
82+
async with self._lock:
83+
self._cleanup_expired()
84+
# If at max, evict oldest entry and log warning
85+
if len(self._cache) >= self._maxsize:
86+
logger.warning(f"OAuth state cache full ({self._maxsize}), evicting oldest entries")
87+
while len(self._cache) >= self._maxsize:
88+
self._cache.popitem(last=False)
89+
self._cache[key] = expires_at or (datetime.now(UTC) + self._ttl)
90+
91+
async def get(self, key: str) -> datetime | None:
92+
"""Get expiry time for a key, or None if not found/expired."""
93+
async with self._lock:
94+
self._cleanup_expired()
95+
return self._cache.get(key)
96+
97+
async def pop(self, key: str) -> datetime | None:
98+
"""Remove and return expiry time for a key."""
99+
async with self._lock:
100+
return self._cache.pop(key, None)
101+
102+
async def contains(self, key: str) -> bool:
103+
"""Check if key exists (async version of __contains__)."""
104+
async with self._lock:
105+
self._cleanup_expired()
106+
return key in self._cache
107+
108+
def _cleanup_expired(self) -> None:
109+
"""Remove expired entries. Must be called with lock held."""
110+
now = datetime.now(UTC)
111+
# Use dict comprehension for atomic update
112+
self._cache = OrderedDict((k, exp) for k, exp in self._cache.items() if exp >= now)
113+
114+
115+
# OAuth state storage with bounded size (prevents memory exhaustion)
116+
_oauth_states = BoundedTTLCache(maxsize=100, ttl_minutes=10)
117+
118+
119+
# =============================================================================
120+
# Login Rate Limiting (prevents brute force attacks)
121+
# =============================================================================
122+
123+
124+
class LoginRateLimiter:
125+
"""Simple in-memory rate limiter for login attempts.
126+
127+
Tracks failed attempts by IP address and locks out after threshold.
128+
Thread-safe via asyncio lock.
129+
"""
130+
131+
def __init__(
132+
self, max_attempts: int = 5, lockout_minutes: int = 15, cleanup_interval: int = 100
133+
) -> None:
134+
self._attempts: dict[str, list[datetime]] = {}
135+
self._lockouts: dict[str, datetime] = {}
136+
self._max_attempts = max_attempts
137+
self._lockout_duration = timedelta(minutes=lockout_minutes)
138+
self._attempt_window = timedelta(minutes=15)
139+
self._cleanup_counter = 0
140+
self._cleanup_interval = cleanup_interval
141+
self._lock = asyncio.Lock()
142+
143+
async def is_locked_out(self, ip: str) -> bool:
144+
"""Check if IP is currently locked out."""
145+
async with self._lock:
146+
self._maybe_cleanup()
147+
lockout_until = self._lockouts.get(ip)
148+
if lockout_until and lockout_until > datetime.now(UTC):
149+
return True
150+
# Clear expired lockout
151+
if lockout_until:
152+
del self._lockouts[ip]
153+
return False
154+
155+
async def record_failure(self, ip: str) -> bool:
156+
"""Record a failed login attempt. Returns True if now locked out."""
157+
async with self._lock:
158+
now = datetime.now(UTC)
159+
self._maybe_cleanup()
160+
161+
# Get recent attempts within window
162+
attempts = self._attempts.get(ip, [])
163+
cutoff = now - self._attempt_window
164+
attempts = [t for t in attempts if t > cutoff]
165+
attempts.append(now)
166+
self._attempts[ip] = attempts
167+
168+
# Check if should lock out
169+
if len(attempts) >= self._max_attempts:
170+
self._lockouts[ip] = now + self._lockout_duration
171+
logger.warning(f"Login rate limit exceeded for IP {ip}, locked out")
172+
return True
173+
return False
174+
175+
async def record_success(self, ip: str) -> None:
176+
"""Clear attempts on successful login."""
177+
async with self._lock:
178+
self._attempts.pop(ip, None)
179+
self._lockouts.pop(ip, None)
180+
181+
def _maybe_cleanup(self) -> None:
182+
"""Periodically clean up old entries. Must be called with lock held."""
183+
self._cleanup_counter += 1
184+
if self._cleanup_counter < self._cleanup_interval:
185+
return
186+
self._cleanup_counter = 0
187+
188+
now = datetime.now(UTC)
189+
cutoff = now - self._attempt_window
190+
191+
# Atomic cleanup using dict comprehension
192+
self._attempts = {
193+
ip: [t for t in attempts if t > cutoff]
194+
for ip, attempts in self._attempts.items()
195+
if any(t > cutoff for t in attempts)
196+
}
197+
self._lockouts = {ip: exp for ip, exp in self._lockouts.items() if exp >= now}
198+
199+
200+
# Global rate limiter instance
201+
_login_rate_limiter = LoginRateLimiter(max_attempts=5, lockout_minutes=15)
58202

59203
# Simple email validation pattern
60204
_EMAIL_PATTERN = re.compile(r"^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$")
@@ -347,11 +491,50 @@ async def login_form(request: Request[Any, Any, Any], session: AsyncSession) ->
347491
)
348492

349493

494+
def _get_client_ip(request: Request[Any, Any, Any]) -> str:
495+
"""Get client IP from request, checking proxy headers only from trusted sources.
496+
497+
Only trusts X-Forwarded-For/X-Real-IP when request comes from localhost
498+
(i.e., from a reverse proxy like nginx/Coolify running on the same host).
499+
This prevents IP spoofing attacks where attackers set fake headers.
500+
"""
501+
client = request.client
502+
direct_ip = client.host if client else "unknown"
503+
504+
# Only trust proxy headers if request comes from localhost (reverse proxy)
505+
trusted_proxies = {"127.0.0.1", "::1", "localhost"}
506+
if direct_ip in trusted_proxies:
507+
# Check X-Forwarded-For header (set by proxies like nginx, Coolify)
508+
forwarded_for = request.headers.get("x-forwarded-for")
509+
if forwarded_for:
510+
# Take the first IP (original client)
511+
return forwarded_for.split(",")[0].strip()
512+
# Check X-Real-IP header
513+
real_ip = request.headers.get("x-real-ip")
514+
if real_ip:
515+
return real_ip
516+
517+
return direct_ip
518+
519+
350520
@post("/login", sync_to_thread=False)
351521
async def login_submit(
352522
request: Request[Any, Any, Any], session: AsyncSession
353523
) -> Template | Redirect:
354524
"""Process login form submission."""
525+
client_ip = _get_client_ip(request)
526+
527+
# Check if IP is locked out due to too many failed attempts
528+
if await _login_rate_limiter.is_locked_out(client_ip):
529+
return Template(
530+
template_name="admin/login.html",
531+
context={
532+
"error": "Too many failed attempts. Please try again later.",
533+
"email": "",
534+
"csrf_token": _get_csrf_token(request),
535+
},
536+
)
537+
355538
form_data = await request.form()
356539
email = form_data.get("email", "").strip()
357540
password = form_data.get("password", "")
@@ -368,6 +551,8 @@ async def login_submit(
368551

369552
admin = await authenticate_admin(str(email), str(password), session)
370553
if not admin:
554+
# Record failed attempt
555+
await _login_rate_limiter.record_failure(client_ip)
371556
return Template(
372557
template_name="admin/login.html",
373558
context={
@@ -377,6 +562,8 @@ async def login_submit(
377562
},
378563
)
379564

565+
# Successful login - clear any failed attempts
566+
await _login_rate_limiter.record_success(client_ip)
380567
login_admin(request, admin)
381568
return Redirect(path="/admin", status_code=HTTP_303_SEE_OTHER)
382569

@@ -777,15 +964,9 @@ async def oauth_authorize(request: Request[Any, Any, Any], session: AsyncSession
777964
# No OAuth credentials configured, redirect to setup
778965
return Redirect(path="/admin", status_code=HTTP_303_SEE_OTHER)
779966

780-
# Generate CSRF state token
967+
# Generate CSRF state token (BoundedTTLCache handles cleanup and size limits)
781968
state = secrets.token_urlsafe(32)
782-
_oauth_states[state] = datetime.now(UTC) + timedelta(minutes=10)
783-
784-
# Clean up expired states
785-
now = datetime.now(UTC)
786-
expired = [s for s, exp in _oauth_states.items() if exp < now]
787-
for s in expired:
788-
del _oauth_states[s]
969+
await _oauth_states.set(state)
789970

790971
# Build authorization URL with state for CSRF protection
791972
base_url = _get_base_url(request)
@@ -819,20 +1000,19 @@ async def oauth_callback(
8191000
)
8201001

8211002
# Validate CSRF state token
822-
if not state or state not in _oauth_states:
1003+
if not state or not await _oauth_states.contains(state):
8231004
return Template(
8241005
template_name="admin/partials/sync_error.html",
8251006
context={"error": "Invalid OAuth state - possible CSRF attack. Please try again."},
8261007
)
8271008

828-
# Check state hasn't expired and remove it (one-time use)
829-
if _oauth_states[state] < datetime.now(UTC):
830-
del _oauth_states[state]
1009+
# Get and remove state (one-time use) - also checks expiry
1010+
state_expires = await _oauth_states.pop(state)
1011+
if state_expires and state_expires < datetime.now(UTC):
8311012
return Template(
8321013
template_name="admin/partials/sync_error.html",
8331014
context={"error": "OAuth state expired. Please try again."},
8341015
)
835-
del _oauth_states[state]
8361016

8371017
# Get OAuth credentials from database
8381018
stmt = select(AppSettings).where(AppSettings.id == 1)
Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""API routes."""
22

3+
from litestar import Router
4+
35
from polar_flow_server.api.baselines import baselines_router
46
from polar_flow_server.api.data import data_router
57
from polar_flow_server.api.health import health_router
@@ -9,17 +11,24 @@
911
from polar_flow_server.api.sleep import sleep_router
1012
from polar_flow_server.api.sync import sync_router
1113

12-
# All API routers
13-
api_routers = [
14-
health_router,
14+
# Versioned API routers (user data endpoints)
15+
# These get the /api/v1 prefix
16+
_v1_routers = [
1517
sleep_router,
1618
sync_router,
1719
data_router,
1820
baselines_router, # Analytics baselines
1921
patterns_router, # Pattern detection and anomalies
2022
insights_router, # Unified insights API
21-
oauth_router, # OAuth flow and code exchange
2223
keys_router, # Key management (regenerate, revoke, status)
2324
]
2425

26+
api_v1_router = Router(path="/api/v1", route_handlers=_v1_routers)
27+
28+
# Export: health (root), oauth (root), v1 (prefixed)
29+
# - health_router: /health - no auth needed, no version prefix
30+
# - oauth_router: /oauth/* - external OAuth flow, no version prefix
31+
# - api_v1_router: /api/v1/* - all user data endpoints
32+
api_routers = [health_router, oauth_router, api_v1_router]
33+
2534
__all__ = ["api_routers"]

0 commit comments

Comments
 (0)