fix: cleanup StravaConfig on user delete (FK violation); equalize login timing (user enum); scope upload task-status to owner; SQL-side data-point downsampling; drop unused clsx/react-leaflet deps
This commit is contained in:
@@ -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])
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
Generated
-27
@@ -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",
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user