🛡️ Sentinel: [CRITICAL] Fix hardcoded admin credentials and exception leakage

.jules/sentinel.md

@@ -9,3 +9,16 @@ Always use an environment variable (like `ALLOWED_ORIGINS`) to strictly define a
 **Prevention:**
 1. Do not use wildcard `["*"]` for CORS in production setups, especially with authenticated routes.
 2. Verify CORS setups using testing frameworks like Pytest or by configuring restricted inputs dynamically through the `.env` configuration.
+
+## 2024-11-20 - Hardcoded Secrets and Timing Attack in Authentication Fixed
+
+**Vulnerability:**
+The `authenticate_admin` function in `backend/auth.py` contained hardcoded credentials (`"admin"` and `"admin123"`), exposing sensitive info in the codebase. Additionally, the string comparison operator (`!=`) was used for checking credentials, making the authentication vulnerable to timing attacks. Finally, the exception handling in `main.py` for the login route exposed internal error messages by returning `str(e)` in an `HTTPException`.
+
+**Learning:**
+Never commit secrets or default credentials into the repository. Authentication should use secure, constant-time comparisons to prevent information leakage through timing attacks. Error handling should avoid leaking sensitive internal details like stack traces or unhandled exception strings.
+
+**Prevention:**
+1. Use environment variables (like `ADMIN_USERNAME` and `ADMIN_PASSWORD`) to provide configuration without hardcoding secrets.
+2. Use constant-time comparison methods like `secrets.compare_digest()` for password and username checks.
+3. Catch unexpected errors and return generic error messages instead of leaking `str(e)` to the client.

backend/auth.py

@@ -1,7 +1,18 @@
 from fastapi import HTTPException
+import os
+import secrets
 
 def authenticate_admin(username: str, password: str):
-    # Dummy authentication logic
-    if username != "admin" or password != "admin123":
+    admin_username = os.getenv("ADMIN_USERNAME")
+    admin_password = os.getenv("ADMIN_PASSWORD")
+
+    if not admin_username or not admin_password:
+        raise HTTPException(status_code=500, detail="Admin credentials are not configured")
+
+    is_username_correct = secrets.compare_digest(username.encode("utf-8"), admin_username.encode("utf-8"))
+    is_password_correct = secrets.compare_digest(password.encode("utf-8"), admin_password.encode("utf-8"))
+
+    if not (is_username_correct and is_password_correct):
         raise HTTPException(status_code=401, detail="Invalid username or password")
+
     return {"message": "Login successful"}

main.py

@@ -92,9 +92,14 @@ async def admin_login(login_request: models.LoginRequest, response: Response):
         sessions[session_id] = login_request.username  # Map session ID to username
         response.set_cookie("session_id", session_id, httponly=True)
         return {"message": "Login successful"}
+    except HTTPException as e:
+        # Re-raise HTTP exceptions (e.g. 500 for missing config or 401 for bad auth)
+        logging.error(f"Login logic failed for {login_request.username}: {e.detail}")
+        raise
     except Exception as e:
-        logging.error(f"Login failed for {login_request.username}: {str(e)}")
-        raise HTTPException(status_code=401, detail=str(e))  # Unauthorized
+        logging.error(f"Unexpected login error for {login_request.username}: {str(e)}")
+        # Do not expose internal details in the generic exception response
+        raise HTTPException(status_code=401, detail="Invalid username or password")
 
 # Admin logout route
 @app.post("/admin/logout/")