From 4c48a5cf946d70204ad45dfcc345d0e9cfbd07a2 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 4 Apr 2026 01:09:42 +0200 Subject: [PATCH] Security hardening, async TeachChat, rate limit UX, unthrottle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security: - admin.py: move api_key from URL query params to POST body (litellm/models, tts/voices) — prevents key logging - admin.py: sanitize exception messages in voice discovery — log internally, return generic errors - teach.py: log LLM errors server-side, show friendly message to user - nextcloud.py: normalize path with posixpath.normpath to prevent ../ traversal - auth.py: check_rate_limit now accepts user param — admins/moderators/unthrottled always exempt Performance: - teach.py: make /chat endpoint async, use litellm.acompletion() — no longer blocks a uvicorn thread per request Features: - users: add is_unthrottled column (DB migration in setup_pgvector) - admin.py: PUT /users/{id}/unthrottle endpoint - AdminPage.jsx: Unlimited/Throttle toggle button per user, shows "unlimited" badge - Rate limit messages improved with user-friendly context Co-Authored-By: Claude Sonnet 4.6 --- backend/app/main.py | 3 +- backend/app/models/user.py | 1 + backend/app/routers/admin.py | 69 ++++++++++++++++++++++++-------- backend/app/routers/nextcloud.py | 4 +- backend/app/routers/teach.py | 13 +++--- backend/app/routers/tts.py | 5 ++- backend/app/schemas/auth.py | 1 + backend/app/utils/auth.py | 9 ++++- frontend/src/pages/AdminPage.jsx | 42 ++++++++++++++----- 9 files changed, 110 insertions(+), 37 deletions(-) diff --git a/backend/app/main.py b/backend/app/main.py index 4fae014..c1ff323 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -228,7 +228,8 @@ def setup_pgvector(): UNIQUE(user_id, question_id) ) """)) - conn.commit() + # Unthrottle flag for users (exempt from AI/TTS rate limits) + conn.execute(text("ALTER TABLE users ADD COLUMN IF NOT EXISTS is_unthrottled INTEGER DEFAULT 0")) conn.commit() diff --git a/backend/app/models/user.py b/backend/app/models/user.py index 4ecdc7e..81de4ae 100644 --- a/backend/app/models/user.py +++ b/backend/app/models/user.py @@ -14,6 +14,7 @@ class User(Base): hashed_password = Column(String, nullable=False) name = Column(String, nullable=False) role = Column(String, default="user") # admin, moderator, user + is_unthrottled = Column(Integer, default=0) # 1 = exempt from rate limits created_at = Column(DateTime, default=datetime.utcnow) documents = relationship("PDFDocument", back_populates="user") diff --git a/backend/app/routers/admin.py b/backend/app/routers/admin.py index 857616d..82b31a0 100644 --- a/backend/app/routers/admin.py +++ b/backend/app/routers/admin.py @@ -46,6 +46,23 @@ def update_user_role( return user +@router.put("/users/{user_id}/unthrottle", response_model=UserResponse) +def set_user_unthrottle( + user_id: int, + data: dict, + db: Session = Depends(get_db), + admin: User = Depends(require_admin), +): + """Set or clear the unthrottle flag for a user — exempt from AI/TTS rate limits.""" + user = db.query(User).filter(User.id == user_id).first() + if not user: + raise HTTPException(status_code=404, detail="User not found") + user.is_unthrottled = 1 if data.get("unthrottled") else 0 + db.commit() + db.refresh(user) + return user + + @router.post("/users", response_model=UserResponse) def create_user( user_data: UserCreate, @@ -99,34 +116,41 @@ def list_available_models( return result -@router.get("/litellm/models") +class LiteLLMSearchRequest(BaseModel): + api_key: str | None = None + api_base: str | None = None + + +@router.post("/litellm/models") def search_litellm_models( - api_key: str | None = None, - api_base: str | None = None, + data: LiteLLMSearchRequest, admin: User = Depends(require_admin), ): """Query available models from LiteLLM proxy or OpenAI-compatible API.""" - base = (api_base or settings.LITELLM_API_BASE or "").rstrip("/") - key = api_key or settings.LITELLM_API_KEY + import logging + log = logging.getLogger(__name__) + base = (data.api_base or settings.LITELLM_API_BASE or "").rstrip("/") + key = data.api_key or settings.LITELLM_API_KEY if base: try: headers = {"Authorization": f"Bearer {key}"} if key else {} - resp = httpx.get(f"{base}/v1/models", headers=headers, timeout=10) + resp = httpx.post(f"{base}/v1/models", headers=headers, timeout=10) if False else \ + httpx.get(f"{base}/v1/models", headers=headers, timeout=10) resp.raise_for_status() - data = resp.json() - models = sorted([m["id"] for m in data.get("data", [])]) + models = sorted([m["id"] for m in resp.json().get("data", [])]) return {"models": models, "source": base} except Exception as e: - raise HTTPException(status_code=400, detail=f"Failed to query models API: {e}") + log.warning(f"LiteLLM model search failed: {e}") + raise HTTPException(status_code=400, detail="Failed to query models API. Check the API base URL and key.") - # Fall back to LiteLLM's built-in model list try: import litellm models = sorted(litellm.utils.get_valid_models()) return {"models": models, "source": "litellm-builtin"} except Exception as e: - raise HTTPException(status_code=500, detail=f"Failed to get LiteLLM models: {e}") + log.warning(f"LiteLLM builtin model list failed: {e}") + raise HTTPException(status_code=500, detail="Failed to retrieve LiteLLM built-in model list.") @router.get("/models", response_model=list[AIModelConfigResponse]) @@ -238,14 +262,23 @@ def test_model( raise HTTPException(status_code=502, detail=str(e)) -@router.get("/tts/voices") +class TTSVoiceSearchRequest(BaseModel): + provider: str + api_key: str | None = None + region: str | None = None + + +@router.post("/tts/voices") def search_tts_voices( - provider: str, - api_key: str | None = None, - region: str | None = None, + data: TTSVoiceSearchRequest, admin: User = Depends(require_admin), ): """Discover available TTS voices from ElevenLabs, AWS Polly, or return OpenAI hardcoded list.""" + import logging + log = logging.getLogger(__name__) + provider = data.provider + api_key = data.api_key + region = data.region if provider == "elevenlabs": key = api_key or settings.ELEVENLABS_API_KEY @@ -270,7 +303,8 @@ def search_tts_voices( except HTTPException: raise except Exception as e: - raise HTTPException(status_code=400, detail=f"ElevenLabs API error: {e}") + log.warning(f"ElevenLabs voice discovery failed: {e}") + raise HTTPException(status_code=400, detail="ElevenLabs API error — check your API key and try again.") elif provider == "polly": access_key = api_key or settings.AWS_ACCESS_KEY_ID @@ -302,7 +336,8 @@ def search_tts_voices( except HTTPException: raise except Exception as e: - raise HTTPException(status_code=400, detail=f"AWS Polly error: {e}") + log.warning(f"AWS Polly voice discovery failed: {e}") + raise HTTPException(status_code=400, detail="AWS Polly error — check your credentials and region.") elif provider == "openai": voices = [] diff --git a/backend/app/routers/nextcloud.py b/backend/app/routers/nextcloud.py index 9c37ff5..39be305 100644 --- a/backend/app/routers/nextcloud.py +++ b/backend/app/routers/nextcloud.py @@ -31,8 +31,10 @@ class NCRequest(BaseModel): def _dav_url(server: str, username: str, path: str) -> str: + import posixpath base = server.rstrip("/") - p = path.lstrip("/") + # Normalize to collapse any ../ sequences before building URL + p = posixpath.normpath("/" + path).lstrip("/") return f"{base}/remote.php/dav/files/{username}/{p}" diff --git a/backend/app/routers/teach.py b/backend/app/routers/teach.py index 8690e0e..e9c99de 100644 --- a/backend/app/routers/teach.py +++ b/backend/app/routers/teach.py @@ -124,18 +124,19 @@ def list_teach_models( @router.post("/chat") -def chat( +async def chat( req: ChatRequest, db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ): """Send a message to the teach AI with full question context.""" - # Rate limit: 30 AI chat messages per user per 10 minutes + # Rate limit: 30 AI chat messages per user per 10 minutes (admins/unthrottled users exempt) check_rate_limit( key=f"teach_chat:{current_user.id}", max_calls=30, window_seconds=600, - detail="Too many AI chat messages. Please wait a few minutes before continuing.", + detail="You've sent too many messages to the AI tutor. Please wait a few minutes before continuing. If you need this limit raised, contact an admin.", + user=current_user, ) model_info = _get_teach_model(db, req.model_id) if not model_info: @@ -175,8 +176,10 @@ def chat( kwargs["api_key"] = settings.LITELLM_API_KEY if settings.LITELLM_API_BASE: kwargs["api_base"] = settings.LITELLM_API_BASE - response = litellm.completion(**kwargs) + response = await litellm.acompletion(**kwargs) reply = response.choices[0].message.content.strip() return {"reply": reply} except Exception as e: - raise HTTPException(status_code=502, detail=f"AI model error: {str(e)}") + import logging + logging.getLogger(__name__).error(f"TeachChat error for user {current_user.id} model {model_id}: {e}") + raise HTTPException(status_code=502, detail="The AI tutor is temporarily unavailable. Please try again in a moment.") diff --git a/backend/app/routers/tts.py b/backend/app/routers/tts.py index 296bb45..c9c6d51 100644 --- a/backend/app/routers/tts.py +++ b/backend/app/routers/tts.py @@ -51,12 +51,13 @@ def text_to_speech( current_user: User = Depends(get_current_user), ): """Convert text to speech using configured or user-selected TTS model.""" - # Rate limit: 60 TTS requests per user per hour + # Rate limit: 60 TTS requests per user per hour (admins/unthrottled users exempt) check_rate_limit( key=f"tts_speak:{current_user.id}", max_calls=60, window_seconds=3600, - detail="TTS rate limit reached. You can generate up to 60 audio clips per hour.", + detail="You've reached the audio limit (60 clips/hour). The limit resets automatically — try again shortly. Contact an admin if you need this raised.", + user=current_user, ) if not request.text.strip(): raise HTTPException(status_code=400, detail="Text cannot be empty") diff --git a/backend/app/schemas/auth.py b/backend/app/schemas/auth.py index 37b508f..06eb712 100644 --- a/backend/app/schemas/auth.py +++ b/backend/app/schemas/auth.py @@ -14,6 +14,7 @@ class UserResponse(BaseModel): email: str name: str role: str + is_unthrottled: int = 0 created_at: datetime class Config: diff --git a/backend/app/utils/auth.py b/backend/app/utils/auth.py index 32bfaf4..6b1a0d0 100644 --- a/backend/app/utils/auth.py +++ b/backend/app/utils/auth.py @@ -119,8 +119,13 @@ class TokenRefreshMiddleware(BaseHTTPMiddleware): return response -def check_rate_limit(key: str, max_calls: int, window_seconds: int, detail: str): - """Generic Redis-backed rate limiter. Raises 429 if limit exceeded. Degrades gracefully if Redis is down.""" +def check_rate_limit(key: str, max_calls: int, window_seconds: int, detail: str, user=None): + """Generic Redis-backed rate limiter. Admins and unthrottled users are always exempt. + Degrades gracefully if Redis is down.""" + # Admins, moderators, and explicitly unthrottled users bypass all rate limits + if user is not None: + if getattr(user, 'role', '') in ('admin', 'moderator') or getattr(user, 'is_unthrottled', 0): + return try: import redis as redis_lib r = redis_lib.from_url(settings.REDIS_URL, decode_responses=True, socket_connect_timeout=1) diff --git a/frontend/src/pages/AdminPage.jsx b/frontend/src/pages/AdminPage.jsx index 73deab8..9dd44ae 100644 --- a/frontend/src/pages/AdminPage.jsx +++ b/frontend/src/pages/AdminPage.jsx @@ -87,6 +87,16 @@ export default function AdminPage() { } } + const toggleUnthrottle = async (userId, currentVal) => { + try { + await api.put(`/admin/users/${userId}/unthrottle`, { unthrottled: !currentVal }) + setSuccess(currentVal ? 'Rate limits restored for user' : 'Rate limits removed for user') + loadData() + } catch (err) { + setError(err.response?.data?.detail || 'Failed to update throttle setting') + } + } + const createModel = async (e) => { e.preventDefault() setError('') @@ -151,10 +161,10 @@ export default function AdminPage() { setSearchResults([]) setSearchLoading(true) try { - const params = new URLSearchParams() - if (searchApiKey) params.set('api_key', searchApiKey) - if (searchApiBase) params.set('api_base', searchApiBase) - const res = await api.get(`/admin/litellm/models?${params}`) + const res = await api.post('/admin/litellm/models', { + api_key: searchApiKey || null, + api_base: searchApiBase || null, + }) setSearchResults(res.data.models) } catch (err) { setSearchError(err.response?.data?.detail || 'Failed to query models') @@ -168,7 +178,7 @@ export default function AdminPage() { setEmbedSearchResults([]) setEmbedSearchLoading(true) try { - const res = await api.get('/admin/litellm/models') + const res = await api.post('/admin/litellm/models', {}) const all = res.data.models || [] // Filter to likely embedding models setEmbedSearchResults(all) @@ -231,9 +241,10 @@ export default function AdminPage() { setTtsVoices([]) setTtsVoicesLoading(true) try { - const params = new URLSearchParams({ provider: ttsProvider }) - if (ttsSearchKey) params.set('api_key', ttsSearchKey) - const res = await api.get(`/admin/tts/voices?${params}`) + const res = await api.post('/admin/tts/voices', { + provider: ttsProvider, + api_key: ttsSearchKey || null, + }) setTtsVoices(res.data.voices) } catch (err) { setTtsVoicesError(err.response?.data?.detail || 'Failed to fetch voices') @@ -578,10 +589,15 @@ export default function AdminPage() { {u.name}
{u.email} · joined {new Date(u.created_at).toLocaleDateString()}
-
+
{u.role} + {u.is_unthrottled ? ( + + unlimited + + ) : null} +
))}