/security-auditor skills changes done

backend/routers/users.py

@@ -1,17 +1,17 @@
 """User management routes"""
-from fastapi import APIRouter, HTTPException
+import logging
+from fastapi import APIRouter, HTTPException, Depends
 from db import get_database
-from models import UserCreate, UserUpdate, User
+from models import UserCreate, UserUpdate
 from datetime import datetime
-from typing import Optional
-from bson import ObjectId
-from bson.errors import InvalidId
+from auth import get_current_user, verify_user_access
 
+log = logging.getLogger(__name__)
 router = APIRouter()
 
 
 @router.post("/register", response_model=dict)
-async def register_user(user_data: UserCreate):
+async def register_user(user_data: UserCreate, token: dict = Depends(get_current_user)):
     """
     Register or get user (idempotent).
 
@@ -19,10 +19,11 @@ async def register_user(user_data: UserCreate):
     If user already exists, returns existing user.
     Called after Firebase Google Auth on frontend.
     """
-    db = get_database()
+    if user_data.email != token.get("email"):
+        raise HTTPException(status_code=403, detail="Access denied")
 
+    db = get_database()
     try:
-        # Upsert: Update if exists, insert if not
         result = db.users.update_one(
             {"email": user_data.email},
             {
@@ -40,11 +41,9 @@ async def register_user(user_data: UserCreate):
             upsert=True
         )
 
-        # Fetch the user (either newly created or existing)
         user = db.users.find_one({"email": user_data.email})
         if not user:
-            raise HTTPException(
-                status_code=500, detail="Failed to retrieve user after upsert")
+            raise HTTPException(status_code=500, detail="Failed to retrieve user after upsert")
 
         return {
             "id": str(user["_id"]),
@@ -62,15 +61,17 @@ async def register_user(user_data: UserCreate):
     except HTTPException:
         raise
     except Exception as e:
-        raise HTTPException(
-            status_code=500, detail=f"Registration failed: {str(e)}")
+        log.exception("Registration failed")
+        raise HTTPException(status_code=500, detail="Internal server error")
 
 
 @router.get("/by-email/{email}", response_model=dict)
-async def get_user_by_email(email: str):
+async def get_user_by_email(email: str, token: dict = Depends(get_current_user)):
     """Get user profile by email (called after Firebase Auth)."""
-    db = get_database()
+    if email != token.get("email"):
+        raise HTTPException(status_code=403, detail="Access denied")
 
+    db = get_database()
     try:
         user = db.users.find_one({"email": email})
         if not user:
@@ -91,26 +92,17 @@ async def get_user_by_email(email: str):
         }
     except HTTPException:
         raise
-    except Exception as e:
-        raise HTTPException(
-            status_code=500, detail=f"Failed to fetch user: {str(e)}")
+    except Exception:
+        log.exception("Failed to fetch user by email")
+        raise HTTPException(status_code=500, detail="Internal server error")
 
 
 @router.get("/{user_id}", response_model=dict)
-async def get_user_by_id(user_id: str):
+async def get_user_by_id(user_id: str, token: dict = Depends(get_current_user)):
     """Get user profile by ID."""
     db = get_database()
-
     try:
-        user_oid = ObjectId(user_id)
-    except InvalidId:
-        raise HTTPException(status_code=400, detail="Invalid user ID format")
-
-    try:
-        user = db.users.find_one({"_id": user_oid})
-        if not user:
-            raise HTTPException(status_code=404, detail="User not found")
-
+        user = verify_user_access(user_id, db, token)
         return {
             "id": str(user["_id"]),
             "email": user["email"],
@@ -124,72 +116,54 @@ async def get_user_by_id(user_id: str):
         }
     except HTTPException:
         raise
-    except Exception as e:
-        raise HTTPException(
-            status_code=500, detail=f"Failed to fetch user: {str(e)}")
+    except Exception:
+        log.exception("Failed to fetch user by ID")
+        raise HTTPException(status_code=500, detail="Internal server error")
 
 
 @router.put("/{user_id}", response_model=dict)
-async def update_user(user_id: str, user_data: UserUpdate):
+async def update_user(user_id: str, user_data: UserUpdate, token: dict = Depends(get_current_user)):
     """Update user profile."""
     db = get_database()
-
     try:
-        user_oid = ObjectId(user_id)
-    except InvalidId:
-        raise HTTPException(status_code=400, detail="Invalid user ID format")
+        user = verify_user_access(user_id, db, token)
+        user_oid = user["_id"]
 
-    try:
-        # Prepare update data (exclude None values)
         update_data = user_data.model_dump(exclude_unset=True)
         update_data["updatedAt"] = datetime.utcnow()
 
-        result = db.users.update_one(
-            {"_id": user_oid},
-            {"$set": update_data}
-        )
+        db.users.update_one({"_id": user_oid}, {"$set": update_data})
 
-        if result.matched_count == 0:
-            raise HTTPException(status_code=404, detail="User not found")
-
-        # Fetch and return updated user
-        user = db.users.find_one({"_id": user_oid})
+        updated = db.users.find_one({"_id": user_oid})
         return {
-            "id": str(user["_id"]),
-            "email": user["email"],
-            "displayName": user.get("displayName"),
-            "photoURL": user.get("photoURL"),
-            "theme": user.get("theme", "light"),
-            "backgroundImage": user.get("backgroundImage"),
-            "backgroundImages": user.get("backgroundImages", []),
-            "tutorial": user.get("tutorial"),
-            "createdAt": user["createdAt"].isoformat(),
-            "updatedAt": user["updatedAt"].isoformat(),
+            "id": str(updated["_id"]),
+            "email": updated["email"],
+            "displayName": updated.get("displayName"),
+            "photoURL": updated.get("photoURL"),
+            "theme": updated.get("theme", "light"),
+            "backgroundImage": updated.get("backgroundImage"),
+            "backgroundImages": updated.get("backgroundImages", []),
+            "tutorial": updated.get("tutorial"),
+            "createdAt": updated["createdAt"].isoformat(),
+            "updatedAt": updated["updatedAt"].isoformat(),
             "message": "User updated successfully"
         }
     except HTTPException:
         raise
-    except Exception as e:
-        raise HTTPException(status_code=500, detail=f"Update failed: {str(e)}")
+    except Exception:
+        log.exception("User update failed")
+        raise HTTPException(status_code=500, detail="Internal server error")
 
 
 @router.delete("/{user_id}")
-async def delete_user(user_id: str):
+async def delete_user(user_id: str, token: dict = Depends(get_current_user)):
     """Delete user account and all associated data."""
     db = get_database()
-
     try:
-        user_oid = ObjectId(user_id)
-    except InvalidId:
-        raise HTTPException(status_code=400, detail="Invalid user ID format")
+        user = verify_user_access(user_id, db, token)
+        user_oid = user["_id"]
 
-    try:
-        # Delete user
         user_result = db.users.delete_one({"_id": user_oid})
-        if user_result.deleted_count == 0:
-            raise HTTPException(status_code=404, detail="User not found")
-
-        # Delete all user's entries
         entry_result = db.entries.delete_many({"userId": user_oid})
 
         return {
@@ -199,6 +173,6 @@ async def delete_user(user_id: str):
         }
     except HTTPException:
         raise
-    except Exception as e:
-        raise HTTPException(
-            status_code=500, detail=f"Deletion failed: {str(e)}")
+    except Exception:
+        log.exception("User deletion failed")
+        raise HTTPException(status_code=500, detail="Internal server error")