WIP checkpoint: Address review feedback: Address review feedback: Scaffold FastAPI backend project structure

backend/app/routers/assets.py

@@ -1,5 +1,7 @@
 """Asset management router."""
 
+import os
+from pathlib import Path
 from typing import Optional
 
 from fastapi import APIRouter, Depends, HTTPException, UploadFile, File, Query
@@ -51,6 +53,17 @@ class AssetUpdate(BaseModel):
     is_public: Optional[bool] = None
 
 
+# --- Helpers ---
+
+def _sanitize_filename(filename: str) -> str:
+    """Strip path components and unsafe characters from a filename."""
+    # Remove any directory components (path traversal protection).
+    base = os.path.basename(filename.replace("\\", "/"))
+    # Keep only safe characters.
+    safe = "".join(c for c in base if c.isalnum() or c in "._-")
+    return safe or "file"
+
+
 # --- Endpoints ---
 
 @router.post("/", response_model=AssetResponse, status_code=201)
@@ -155,11 +168,11 @@ async def delete_asset(
     await db.commit()
 
 
-@router.post("/{asset_id}/upload")
+@router.post("/{asset_id}/upload", response_model=AssetResponse)
 async def upload_asset_file(
     asset_id: int,
     file: UploadFile = File(...),
-    current_user: CurrentUser = None,
+    current_user: CurrentUser = Depends(),
     db: AsyncSession = Depends(get_db),
 ):
     """Upload a file for an existing asset."""
@@ -167,11 +180,10 @@ async def upload_asset_file(
     if asset is None or asset.owner_id != current_user.id:
         raise HTTPException(status_code=404, detail="Asset not found")
 
-    # Sanitize filename to prevent path traversal
-    safe_filename = file.filename.replace("/", "_").replace("\\", "_") if file.filename else f"asset_{asset_id}"
-    file_path = f"uploads/{asset_id}_{safe_filename}"
+    # Sanitize filename to prevent path traversal.
+    safe_filename = _sanitize_filename(file.filename or f"asset_{asset_id}")
 
-    # Validate file size against configured maximum
+    # Validate file size against configured maximum.
     max_bytes = settings.MAX_UPLOAD_SIZE_MB * 1024 * 1024
     content = await file.read()
     file_size = len(content)
@@ -181,12 +193,17 @@ async def upload_asset_file(
             detail=f"File size {file_size} exceeds maximum allowed size {max_bytes} bytes",
         )
 
-    # TODO: Implement actual file write logic (S3, local filesystem, etc.)
-    asset.file_path = file_path
+    # Write the file to the upload directory.
+    upload_dir = Path(settings.UPLOAD_DIR)
+    upload_dir.mkdir(parents=True, exist_ok=True)
+    stored_name = f"{asset_id}_{safe_filename}"
+    file_path = upload_dir / stored_name
+    file_path.write_bytes(content)
+
+    asset.file_path = str(file_path)
     asset.mime_type = file.content_type
     asset.file_size = file_size
 
     await db.commit()
     await db.refresh(asset)
-
-    return {"message": "File upload endpoint", "asset_id": asset_id, "file_path": file_path}
+    return asset

backend/app/routers/users.py

@@ -30,7 +30,7 @@ class UserUpdate(BaseModel):
     is_active: bool | None = None
 
 
-# ─── Endpoints ────────────────────────────────────────────────────────────────
+# ─── Endpoints ───────────────────────────────────────────────────────────────
 
 @router.get("/me", response_model=UserResponse)
 async def get_current_user_profile(
@@ -46,15 +46,14 @@ async def get_user(
     current_user: CurrentUser,
     db: Annotated[AsyncSession, Depends(get_db)],
 ):
-    """Get a user by ID.  Owners and admins can see any user; others see
-    only their own profile."""
+    """Get a user by ID.  Only the owner can view their own profile."""
     result = await db.execute(select(User).where(User.id == user_id))
     user = result.scalar_one_or_none()
     if not user:
         raise HTTPException(status_code=404, detail="User not found")
 
-    # Allow viewing own profile or if user is active (admins always allowed)
-    if user.id != current_user.id and not current_user.is_active:
+    # Only the owner may view their own profile.
+    if user.id != current_user.id:
         raise HTTPException(status_code=403, detail="Not authorized to view this user")
 
     return user
@@ -67,13 +66,14 @@ async def update_user(
     current_user: CurrentUser,
     db: Annotated[AsyncSession, Depends(get_db)],
 ):
-    """Update a user.  Only the owner (or an active admin) can update."""
+    """Update a user.  Only the owner can update their own account."""
     result = await db.execute(select(User).where(User.id == user_id))
     user = result.scalar_one_or_none()
     if not user:
         raise HTTPException(status_code=404, detail="User not found")
 
-    if user.id != current_user.id and not current_user.is_active:
+    # Only the owner may update their own account.
+    if user.id != current_user.id:
         raise HTTPException(status_code=403, detail="Not authorized to update this user")
 
     if user_update.email is not None:
@@ -92,13 +92,14 @@ async def delete_user(
     current_user: CurrentUser,
     db: Annotated[AsyncSession, Depends(get_db)],
 ):
-    """Delete a user.  Only the owner (or an active admin) can delete."""
+    """Delete a user.  Only the owner can delete their own account."""
     result = await db.execute(select(User).where(User.id == user_id))
     user = result.scalar_one_or_none()
     if not user:
         raise HTTPException(status_code=404, detail="User not found")
 
-    if user.id != current_user.id and not current_user.is_active:
+    # Only the owner may delete their own account.
+    if user.id != current_user.id:
         raise HTTPException(status_code=403, detail="Not authorized to delete this user")
 
     await db.delete(user)