From c1941ed9acd3ef45f0eecc47493d25a2c1509b36 Mon Sep 17 00:00:00 2001 From: Mistral Vibe Date: Mon, 30 Mar 2026 21:11:53 +0200 Subject: [PATCH] security: httpOnly cookies, rate limiting, nginx headers, SSE sanitization Auth / token storage: - JWT is now set as an httpOnly Secure SameSite=Lax cookie on login - Add POST /auth/logout endpoint that clears the cookie - get_current_member falls back to rh_token cookie when no Authorization header - WebSocket auth now accepts cookie (rh_token) or optional ?token= query param - Frontend removes all localStorage JWT access; uses credentials:"include" on every fetch so the httpOnly cookie is sent automatically - Replace clearToken() with logout() that calls the server logout endpoint - Non-sensitive rh_session flag in localStorage used only for client-side routing Rate limiting: - Add slowapi>=0.1.9 dependency - /auth/login limited to 10 req/min per IP - /auth/register limited to 5 req/min per IP Nginx security headers: - Add X-Frame-Options, X-Content-Type-Options, Referrer-Policy, X-XSS-Protection, Permissions-Policy to all responses SSE error leakage: - songs.py nc-scan/stream no longer leaks str(exc) to clients Co-Authored-By: Claude Sonnet 4.6 --- api/pyproject.toml | 1 + api/src/rehearsalhub/dependencies.py | 13 ++++++++--- api/src/rehearsalhub/main.py | 8 +++++++ api/src/rehearsalhub/routers/auth.py | 33 ++++++++++++++++++++++++--- api/src/rehearsalhub/routers/songs.py | 4 ++-- api/src/rehearsalhub/routers/ws.py | 14 ++++++++---- web/nginx.conf | 7 ++++++ web/src/App.tsx | 4 ++-- web/src/api/auth.ts | 13 +++++++++-- web/src/api/client.ts | 30 ++++++++++++++---------- web/src/hooks/useWaveform.ts | 5 ++-- web/src/pages/BandPage.tsx | 6 ++--- web/src/pages/HomePage.tsx | 5 ++-- web/src/pages/InvitePage.tsx | 6 ++--- 14 files changed, 109 insertions(+), 40 deletions(-) diff --git a/api/pyproject.toml b/api/pyproject.toml index 59d2f6c..b583cfb 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -20,6 +20,7 @@ dependencies = [ "redis[hiredis]>=5.0", "python-multipart>=0.0.9", "Pillow>=10.0", + "slowapi>=0.1.9", ] [project.optional-dependencies] diff --git a/api/src/rehearsalhub/dependencies.py b/api/src/rehearsalhub/dependencies.py index 892b07c..635a7dc 100644 --- a/api/src/rehearsalhub/dependencies.py +++ b/api/src/rehearsalhub/dependencies.py @@ -4,7 +4,7 @@ from __future__ import annotations import uuid -from fastapi import Depends, HTTPException, status +from fastapi import Depends, HTTPException, Request, status from fastapi.security import OAuth2PasswordBearer from sqlalchemy.ext.asyncio import AsyncSession @@ -13,18 +13,25 @@ from rehearsalhub.db.models import Member from rehearsalhub.services.auth import decode_token from rehearsalhub.repositories.member import MemberRepository -oauth2_scheme = OAuth2PasswordBearer(tokenUrl="/api/v1/auth/login") +# auto_error=False so we can fall back to cookie auth without a 401 from the scheme itself +oauth2_scheme = OAuth2PasswordBearer(tokenUrl="/api/v1/auth/login", auto_error=False) async def get_current_member( - token: str = Depends(oauth2_scheme), + request: Request, + bearer_token: str | None = Depends(oauth2_scheme), session: AsyncSession = Depends(get_session), ) -> Member: + # Prefer Authorization: Bearer header; fall back to httpOnly cookie + token = bearer_token or request.cookies.get("rh_token") + credentials_exc = HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid or expired token", headers={"WWW-Authenticate": "Bearer"}, ) + if not token: + raise credentials_exc try: payload = decode_token(token) member_id_str: str | None = payload.get("sub") diff --git a/api/src/rehearsalhub/main.py b/api/src/rehearsalhub/main.py index c9f630e..6c4def4 100644 --- a/api/src/rehearsalhub/main.py +++ b/api/src/rehearsalhub/main.py @@ -6,6 +6,9 @@ import os from fastapi import FastAPI, Request, Response from fastapi.middleware.cors import CORSMiddleware from fastapi.staticfiles import StaticFiles +from slowapi import Limiter, _rate_limit_exceeded_handler +from slowapi.errors import RateLimitExceeded +from slowapi.util import get_remote_address from rehearsalhub.config import get_settings from rehearsalhub.routers import ( @@ -20,6 +23,8 @@ from rehearsalhub.routers import ( ws_router, ) +limiter = Limiter(key_func=get_remote_address) + @asynccontextmanager async def lifespan(app: FastAPI): @@ -43,6 +48,9 @@ def create_app() -> FastAPI: lifespan=lifespan, ) + app.state.limiter = limiter + app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) + app.add_middleware( CORSMiddleware, allow_origins=[f"https://{settings.domain}", "http://localhost:3000"], diff --git a/api/src/rehearsalhub/routers/auth.py b/api/src/rehearsalhub/routers/auth.py index 90b504c..6f30a16 100644 --- a/api/src/rehearsalhub/routers/auth.py +++ b/api/src/rehearsalhub/routers/auth.py @@ -2,10 +2,13 @@ import logging import os import uuid -from fastapi import APIRouter, Depends, File, HTTPException, UploadFile, status +from fastapi import APIRouter, Depends, File, HTTPException, Request, Response, UploadFile, status from PIL import Image, UnidentifiedImageError +from slowapi import Limiter +from slowapi.util import get_remote_address from sqlalchemy.ext.asyncio import AsyncSession +from rehearsalhub.config import get_settings from rehearsalhub.db.engine import get_session from rehearsalhub.db.models import Member from rehearsalhub.dependencies import get_current_member @@ -17,13 +20,15 @@ from rehearsalhub.services.auth import AuthService log = logging.getLogger(__name__) router = APIRouter(prefix="/auth", tags=["auth"]) +limiter = Limiter(key_func=get_remote_address) _ALLOWED_IMAGE_EXTENSIONS = {"jpg", "jpeg", "png", "gif", "webp"} _MAX_AVATAR_SIZE = 5 * 1024 * 1024 # 5 MB @router.post("/register", response_model=MemberRead, status_code=status.HTTP_201_CREATED) -async def register(req: RegisterRequest, session: AsyncSession = Depends(get_session)): +@limiter.limit("5/minute") +async def register(request: Request, req: RegisterRequest, session: AsyncSession = Depends(get_session)): svc = AuthService(session) try: member = await svc.register(req) @@ -33,16 +38,38 @@ async def register(req: RegisterRequest, session: AsyncSession = Depends(get_ses @router.post("/login", response_model=TokenResponse) -async def login(req: LoginRequest, session: AsyncSession = Depends(get_session)): +@limiter.limit("10/minute") +async def login( + request: Request, + req: LoginRequest, + response: Response, + session: AsyncSession = Depends(get_session), +): svc = AuthService(session) token = await svc.login(req.email, req.password) if token is None: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid credentials" ) + settings = get_settings() + response.set_cookie( + key="rh_token", + value=token.access_token, + httponly=True, + secure=not settings.debug, + samesite="lax", + max_age=settings.access_token_expire_minutes * 60, + path="/", + ) return token +@router.post("/logout", status_code=status.HTTP_204_NO_CONTENT) +async def logout(response: Response): + response.delete_cookie(key="rh_token", path="/") + return None + + @router.get("/me", response_model=MemberRead) async def get_me(current_member: Member = Depends(get_current_member)): return MemberRead.from_model(current_member) diff --git a/api/src/rehearsalhub/routers/songs.py b/api/src/rehearsalhub/routers/songs.py index 219ff00..cb0a9ee 100644 --- a/api/src/rehearsalhub/routers/songs.py +++ b/api/src/rehearsalhub/routers/songs.py @@ -180,9 +180,9 @@ async def scan_nextcloud_stream( yield json.dumps(event) + "\n" if event.get("type") in ("song", "session"): await db.commit() - except Exception as exc: + except Exception: log.exception("SSE scan error for band %s", band_id) - yield json.dumps({"type": "error", "message": str(exc)}) + "\n" + yield json.dumps({"type": "error", "message": "Scan failed due to an internal error."}) + "\n" finally: await db.commit() diff --git a/api/src/rehearsalhub/routers/ws.py b/api/src/rehearsalhub/routers/ws.py index 8759b37..b328d60 100644 --- a/api/src/rehearsalhub/routers/ws.py +++ b/api/src/rehearsalhub/routers/ws.py @@ -16,13 +16,19 @@ router = APIRouter(tags=["websocket"]) async def version_ws( version_id: uuid.UUID, websocket: WebSocket, - token: str = Query(...), + token: str | None = Query(None), ): - """WebSocket endpoint. Requires a valid JWT passed as ?token=.""" - # Validate token before accepting the connection + """ + WebSocket endpoint. Authentication via: + - ?token= query parameter, or + - rh_token httpOnly cookie (sent automatically by the browser) + """ + raw_token = token or websocket.cookies.get("rh_token") async for session in get_session(): try: - payload = decode_token(token) + if not raw_token: + raise ValueError("no token") + payload = decode_token(raw_token) member_id = uuid.UUID(payload["sub"]) member = await MemberRepository(session).get_by_id(member_id) if member is None: diff --git a/web/nginx.conf b/web/nginx.conf index c05e301..a563fb5 100644 --- a/web/nginx.conf +++ b/web/nginx.conf @@ -3,6 +3,13 @@ server { root /usr/share/nginx/html; index index.html; + # Security headers + add_header X-Frame-Options "DENY" always; + add_header X-Content-Type-Options "nosniff" always; + add_header Referrer-Policy "strict-origin-when-cross-origin" always; + add_header X-XSS-Protection "0" always; + add_header Permissions-Policy "camera=(), microphone=(), geolocation=()" always; + # Allow avatar uploads up to 10MB (API enforces a 5MB limit) client_max_body_size 10m; diff --git a/web/src/App.tsx b/web/src/App.tsx index 0443bfd..5a17e4d 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -2,6 +2,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { BrowserRouter, Route, Routes, Navigate } from "react-router-dom"; import "./index.css"; import { ThemeProvider, useTheme } from "./theme"; +import { isLoggedIn } from "./api/client"; import { LoginPage } from "./pages/LoginPage"; import { HomePage } from "./pages/HomePage"; import { BandPage } from "./pages/BandPage"; @@ -15,8 +16,7 @@ const queryClient = new QueryClient({ }); function PrivateRoute({ children }: { children: React.ReactNode }) { - const token = localStorage.getItem("rh_token"); - return token ? <>{children} : ; + return isLoggedIn() ? <>{children} : ; } function ThemeToggle() { diff --git a/web/src/api/auth.ts b/web/src/api/auth.ts index 7c51fa3..e1381e7 100644 --- a/web/src/api/auth.ts +++ b/web/src/api/auth.ts @@ -1,4 +1,4 @@ -import { api, setToken } from "./client"; +import { api, markLoggedIn, markLoggedOut } from "./client"; export interface LoginRequest { email: string; @@ -20,10 +20,19 @@ export interface MemberRead { export async function login(req: LoginRequest): Promise { const resp = await api.post("/auth/login", req); - setToken(resp.access_token); + markLoggedIn(); return resp; } +export async function logout(): Promise { + try { + await api.post("/auth/logout", {}); + } finally { + markLoggedOut(); + window.location.href = "/login"; + } +} + export async function register(req: { email: string; password: string; diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 6914219..d8e02ad 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -1,15 +1,21 @@ const BASE = "/api/v1"; -function getToken(): string | null { - return localStorage.getItem("rh_token"); +// A non-sensitive flag in localStorage that tells the SPA whether the user has +// an active session. The actual JWT lives in an httpOnly cookie and is never +// readable by JavaScript. Clearing this flag is sufficient for client-side +// route guards; the server still validates the cookie on every request. +const SESSION_KEY = "rh_session"; + +export function markLoggedIn(): void { + localStorage.setItem(SESSION_KEY, "1"); } -export function setToken(token: string): void { - localStorage.setItem("rh_token", token); +export function markLoggedOut(): void { + localStorage.removeItem(SESSION_KEY); } -export function clearToken(): void { - localStorage.removeItem("rh_token"); +export function isLoggedIn(): boolean { + return localStorage.getItem(SESSION_KEY) === "1"; } async function request( @@ -17,20 +23,20 @@ async function request( options: RequestInit = {}, isFormData = false ): Promise { - const token = getToken(); const headers: Record = { ...(options.headers as Record), }; if (!isFormData) { headers["Content-Type"] = "application/json"; } - if (token) { - headers["Authorization"] = `Bearer ${token}`; - } - const resp = await fetch(`${BASE}${path}`, { ...options, headers }); + const resp = await fetch(`${BASE}${path}`, { + ...options, + headers, + credentials: "include", // send httpOnly cookie on every request + }); if (!resp.ok) { if (resp.status === 401) { - clearToken(); + markLoggedOut(); window.location.href = "/login"; throw new Error("Session expired"); } diff --git a/web/src/hooks/useWaveform.ts b/web/src/hooks/useWaveform.ts index aff3f23..be2f204 100644 --- a/web/src/hooks/useWaveform.ts +++ b/web/src/hooks/useWaveform.ts @@ -40,9 +40,8 @@ export function useWaveform( normalize: true, }); - const token = localStorage.getItem("rh_token"); - const audioUrl = token ? `${options.url}?token=${encodeURIComponent(token)}` : options.url; - ws.load(audioUrl); + // The rh_token httpOnly cookie is sent automatically by the browser. + ws.load(options.url); ws.on("ready", () => { setIsReady(true); diff --git a/web/src/pages/BandPage.tsx b/web/src/pages/BandPage.tsx index f9c7447..cc2a20b 100644 --- a/web/src/pages/BandPage.tsx +++ b/web/src/pages/BandPage.tsx @@ -123,11 +123,11 @@ export function BandPage() { setScanMsg(null); setScanProgress("Starting scan…"); - const token = localStorage.getItem("rh_token"); - const url = `/api/v1/bands/${bandId}/nc-scan/stream${token ? `?token=${encodeURIComponent(token)}` : ""}`; + const url = `/api/v1/bands/${bandId}/nc-scan/stream`; try { - const resp = await fetch(url); + // credentials: "include" sends the rh_token httpOnly cookie automatically + const resp = await fetch(url, { credentials: "include" }); if (!resp.ok || !resp.body) { const text = await resp.text().catch(() => resp.statusText); throw new Error(text || `HTTP ${resp.status}`); diff --git a/web/src/pages/HomePage.tsx b/web/src/pages/HomePage.tsx index 73652ef..ab6a9ee 100644 --- a/web/src/pages/HomePage.tsx +++ b/web/src/pages/HomePage.tsx @@ -2,7 +2,7 @@ import { useState } from "react"; import { useNavigate } from "react-router-dom"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import { listBands, createBand } from "../api/bands"; -import { clearToken } from "../api/client"; +import { logout } from "../api/auth"; export function HomePage() { const navigate = useNavigate(); @@ -29,8 +29,7 @@ export function HomePage() { }); function handleSignOut() { - clearToken(); - navigate("/login"); + logout(); } const inputStyle: React.CSSProperties = { diff --git a/web/src/pages/InvitePage.tsx b/web/src/pages/InvitePage.tsx index 134fcb2..186e328 100644 --- a/web/src/pages/InvitePage.tsx +++ b/web/src/pages/InvitePage.tsx @@ -1,6 +1,6 @@ import { useEffect, useState } from "react"; import { useParams, useNavigate } from "react-router-dom"; -import { api } from "../api/client"; +import { api, isLoggedIn } from "../api/client"; interface InviteInfo { id: string; @@ -19,7 +19,7 @@ export function InvitePage() { const [accepting, setAccepting] = useState(false); const [done, setDone] = useState(false); - const isLoggedIn = !!localStorage.getItem("rh_token"); + const loggedIn = isLoggedIn(); useEffect(() => { if (!token) return; @@ -74,7 +74,7 @@ export function InvitePage() { {invite.used_at && " · Already used"}

- {isLoggedIn ? ( + {loggedIn ? (