diff --git a/backend/app/api/activities.py b/backend/app/api/activities.py index 66f6f08..c68c5ee 100644 --- a/backend/app/api/activities.py +++ b/backend/app/api/activities.py @@ -1,7 +1,7 @@ from fastapi import APIRouter, Depends, HTTPException, Query from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy import select, func, desc, delete -from sqlalchemy.orm import selectinload +from sqlalchemy.orm import selectinload, aliased from pydantic import BaseModel from typing import Optional, List from datetime import datetime @@ -267,17 +267,27 @@ async def get_data_points( if not act.scalar_one_or_none(): raise HTTPException(status_code=404, detail="Activity not found") - q = select(ActivityDataPoint).where( - ActivityDataPoint.activity_id == activity_id - ).order_by(ActivityDataPoint.timestamp) + if downsample > 1: + # Stride in SQL (keep every Nth row by ordered row number) so the DB + # never ships the rows we'd immediately discard — a long per-second + # activity is tens of thousands of points and the UI only charts ~1/N. + rn = func.row_number().over(order_by=ActivityDataPoint.timestamp).label("rn") + sub = ( + select(ActivityDataPoint, rn) + .where(ActivityDataPoint.activity_id == activity_id) + .subquery() + ) + adp = aliased(ActivityDataPoint, sub) + q = select(adp).where(sub.c.rn % downsample == 1).order_by(sub.c.timestamp) + else: + q = ( + select(ActivityDataPoint) + .where(ActivityDataPoint.activity_id == activity_id) + .order_by(ActivityDataPoint.timestamp) + ) result = await db.execute(q) - points = result.scalars().all() - - if downsample > 1: - points = points[::downsample] - - return points + return result.scalars().all() @router.get("/{activity_id}/laps", response_model=List[LapOut]) diff --git a/backend/app/api/auth.py b/backend/app/api/auth.py index fe726ef..85e0aa1 100644 --- a/backend/app/api/auth.py +++ b/backend/app/api/auth.py @@ -9,7 +9,7 @@ from jose import jwt, JWTError import httpx from app.core.database import get_db -from app.core.security import verify_password, create_access_token, get_current_user +from app.core.security import verify_password, dummy_verify_password, create_access_token, get_current_user from app.core.config import settings from app.models.user import User @@ -141,6 +141,9 @@ async def login( result = await db.execute(select(User).where(User.username == form_data.username)) user = result.scalar_one_or_none() if not user: + # Spend the same time as a real verify so timing can't reveal whether + # the username exists. + dummy_verify_password() raise HTTPException(status_code=400, detail="Invalid credentials") if user.pocketid_sub is not None: raise HTTPException( diff --git a/backend/app/api/upload.py b/backend/app/api/upload.py index 459db8d..65c658a 100644 --- a/backend/app/api/upload.py +++ b/backend/app/api/upload.py @@ -17,6 +17,34 @@ MAX_FILE_SIZE = 500 * 1024 * 1024 # 500 MB upload cap MAX_EXTRACT_SIZE = 4 * 1024 * 1024 * 1024 # 4 GB total uncompressed cap (zip-bomb guard) _CHUNK = 1024 * 1024 +_TASK_OWNER_TTL = 86400 # 24h — long enough to outlive any upload's polling + + +def _remember_task_owner(task_id: str, user_id: int) -> None: + """Record which user a pollable task belongs to, so the status endpoint can + refuse to surface another user's task result (the Celery task id is the only + thing the client presents). Best-effort: Redis hiccups must not fail uploads.""" + if not task_id: + return + try: + import redis as redis_lib + redis_lib.Redis.from_url(settings.redis_url).set( + f"upload_task_owner:{task_id}", user_id, ex=_TASK_OWNER_TTL + ) + except Exception: + pass + + +def _task_owner(task_id: str) -> int | None: + try: + import redis as redis_lib + v = redis_lib.Redis.from_url(settings.redis_url).get(f"upload_task_owner:{task_id}") + if v is None: + return None + return int(v.decode() if isinstance(v, (bytes, bytearray)) else v) + except Exception: + return None + def _safe_name(filename: str) -> str: """Reduce an uploaded filename to a safe basename — no path traversal.""" @@ -119,6 +147,7 @@ async def upload_activity( # Queue processing task = process_activity_file.delay(str(dest), current_user.id, suffix[1:]) + _remember_task_owner(task.id, current_user.id) return {"task_id": task.id, "status": "queued", "filename": file.filename} @@ -180,6 +209,7 @@ async def upload_garmin_export( # Queue health/wellness data extraction health_task = process_garmin_health_zip.delay(str(dest), current_user.id) + _remember_task_owner(health_task.id, current_user.id) return { "status": "queued", @@ -247,6 +277,7 @@ async def upload_strava_export( # Preview only — classify new/duplicate without writing. The extracted # files are kept so the confirm step can import them without re-uploading. task = analyze_strava_export.delay([str(p) for p in files], current_user.id) + _remember_task_owner(task.id, current_user.id) return {"status": "analyzing", "task_id": task.id, "token": dest.stem, "activity_files": len(files)} @@ -255,10 +286,12 @@ async def upload_strava_export( prefer_existing=True).id for p in files ] + polled = task_ids[-1] if task_ids else None + _remember_task_owner(polled, current_user.id) return { "status": "queued", "activity_tasks": len(task_ids), - "task_id": task_ids[-1] if task_ids else None, + "task_id": polled, } @@ -294,6 +327,13 @@ async def check_task_status( current_user: User = Depends(get_current_user), ): """Check the status of an upload processing task.""" + # A task result can carry the owner's activity data, so don't surface another + # user's task. We fail closed only on a positive owner mismatch; a missing + # record (Redis down / TTL expired) stays permissive so polling never breaks. + owner = _task_owner(task_id) + if owner is not None and owner != current_user.id: + raise HTTPException(status_code=404, detail="Task not found") + from app.workers.celery_app import celery_app result = celery_app.AsyncResult(task_id) return { diff --git a/backend/app/api/users.py b/backend/app/api/users.py index 61f067a..f2b1e1a 100644 --- a/backend/app/api/users.py +++ b/backend/app/api/users.py @@ -19,7 +19,8 @@ from app.core.security import get_current_user from app.core.config import settings from app.models.user import ( User, Activity, ActivityDataPoint, ActivityLap, NamedRoute, - Segment, SegmentEffort, PersonalRecord, HealthMetric, WeightLog, GarminConnectConfig, + Segment, SegmentEffort, PersonalRecord, HealthMetric, WeightLog, + GarminConnectConfig, StravaConfig, ) router = APIRouter() @@ -134,6 +135,11 @@ async def delete_user( await db.execute(delete(HealthMetric).where(HealthMetric.user_id == user_id)) await db.execute(delete(WeightLog).where(WeightLog.user_id == user_id)) await db.execute(delete(GarminConnectConfig).where(GarminConnectConfig.user_id == user_id)) + # StravaConfig holds Fernet-encrypted OAuth tokens and has a NOT-NULL FK to + # users with no DB-level cascade; the Core deletes above bypass the ORM + # relationship cascade, so it must be removed explicitly or the final + # DELETE on users raises a ForeignKey violation (and leaves tokens orphaned). + await db.execute(delete(StravaConfig).where(StravaConfig.user_id == user_id)) await db.execute(delete(User).where(User.id == user_id)) await db.commit() diff --git a/backend/app/core/security.py b/backend/app/core/security.py index 0bff6ac..c3803c4 100644 --- a/backend/app/core/security.py +++ b/backend/app/core/security.py @@ -18,6 +18,13 @@ def verify_password(plain: str, hashed: str) -> bool: return pwd_context.verify(plain, hashed) +def dummy_verify_password() -> None: + """Run a throwaway bcrypt verification so the username-not-found login path + takes the same time as a real password check, preventing username + enumeration via response-timing differences.""" + pwd_context.dummy_verify() + + def hash_password(password: str) -> str: return pwd_context.hash(password) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index d9aa96d..52742ea 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -10,14 +10,12 @@ "dependencies": { "@tanstack/react-query": "^5.40.0", "axios": "^1.7.2", - "clsx": "^2.1.1", "date-fns": "^3.6.0", "leaflet": "^1.9.4", "react": "^18.3.1", "react-dom": "^18.3.1", "react-dropzone": "^14.2.3", "react-grid-layout": "^1.5.3", - "react-leaflet": "^4.2.1", "react-router-dom": "^6.23.1", "recharts": "^2.12.7", "zustand": "^4.5.2" @@ -813,17 +811,6 @@ "node": ">= 8" } }, - "node_modules/@react-leaflet/core": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/@react-leaflet/core/-/core-2.1.0.tgz", - "integrity": "sha512-Qk7Pfu8BSarKGqILj4x7bCSZ1pjuAPZ+qmRwH5S7mDS91VSbVVsJSrW4qA+GPrro8t69gFYVMWb1Zc4yFmPiVg==", - "license": "Hippocratic-2.1", - "peerDependencies": { - "leaflet": "^1.9.0", - "react": "^18.0.0", - "react-dom": "^18.0.0" - } - }, "node_modules/@remix-run/router": { "version": "1.23.3", "resolved": "https://registry.npmjs.org/@remix-run/router/-/router-1.23.3.tgz", @@ -2882,20 +2869,6 @@ "integrity": "sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ==", "license": "MIT" }, - "node_modules/react-leaflet": { - "version": "4.2.1", - "resolved": "https://registry.npmjs.org/react-leaflet/-/react-leaflet-4.2.1.tgz", - "integrity": "sha512-p9chkvhcKrWn/H/1FFeVSqLdReGwn2qmiobOQGO3BifX+/vV/39qhY8dGqbdcPh1e6jxh/QHriLXr7a4eLFK4Q==", - "license": "Hippocratic-2.1", - "dependencies": { - "@react-leaflet/core": "^2.1.0" - }, - "peerDependencies": { - "leaflet": "^1.9.0", - "react": "^18.0.0", - "react-dom": "^18.0.0" - } - }, "node_modules/react-refresh": { "version": "0.17.0", "resolved": "https://registry.npmjs.org/react-refresh/-/react-refresh-0.17.0.tgz", diff --git a/frontend/package.json b/frontend/package.json index 932858d..4b8508e 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -11,14 +11,12 @@ "dependencies": { "@tanstack/react-query": "^5.40.0", "axios": "^1.7.2", - "clsx": "^2.1.1", "date-fns": "^3.6.0", "leaflet": "^1.9.4", "react": "^18.3.1", "react-dom": "^18.3.1", "react-dropzone": "^14.2.3", "react-grid-layout": "^1.5.3", - "react-leaflet": "^4.2.1", "react-router-dom": "^6.23.1", "recharts": "^2.12.7", "zustand": "^4.5.2"