🛡️ Sentinel: [security improvement] Move staff verification to backend#37
🛡️ Sentinel: [security improvement] Move staff verification to backend#37
Conversation
This change enhances security by moving the staff password verification from the client-side JavaScript to a secure backend endpoint. - Created `StaffLogin` model in `backend/models.py`. - Implemented `/api/verify-staff` endpoint in `backend/main.py`. - Used `hmac.compare_digest` for secure, timing-attack resistant comparison. - Updated `js/main.js` to call the backend instead of using a hardcoded password. - Added `STAFF_PASSWORD` to `.env.example`. - Added comprehensive backend tests in `backend/tests/test_staff.py`. - Verified changes with Playwright frontend tests. Co-authored-by: LVT-ENG <214667862+LVT-ENG@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a backend-based staff password verification system, replacing the previous hardcoded frontend check. Key changes include the addition of a /api/verify-staff endpoint in FastAPI using hmac.compare_digest for security, a new StaffLogin Pydantic model, and updated frontend logic in js/main.js to perform an asynchronous API call. The PR also introduces unit tests for the new endpoint and a Vite proxy configuration. Feedback was provided regarding the security risk of using a hardcoded default value for the STAFF_PASSWORD environment variable, suggesting it should be mandatory instead.
|
|
||
| # 🛡️ Configuración Maestra (abvetos.com) - Secrets moved to environment variables | ||
| SECRET_KEY = os.getenv("LVT_SECRET_KEY", "DEVELOPMENT_SECRET_DO_NOT_USE_IN_PROD") | ||
| STAFF_PASSWORD = os.getenv("STAFF_PASSWORD", "SAC_MUSEUM_2026") |
There was a problem hiding this comment.
While using os.getenv is good for loading secrets from environment variables, providing a hardcoded default password directly in the application code (e.g., "SAC_MUSEUM_2026") is not ideal for a security-sensitive value like a staff password. In a production environment, this could lead to using a known, default password if the environment variable is not properly set.
Consider making the STAFF_PASSWORD environment variable mandatory for production deployments. For development, you could either:
- Set a random default if the environment variable is not found.
- Raise an error if the environment variable is not set, forcing explicit configuration.
This ensures that a secure password is always used and prevents accidental deployment with a known default.
| STAFF_PASSWORD = os.getenv("STAFF_PASSWORD", "SAC_MUSEUM_2026") | |
| STAFF_PASSWORD = os.getenv("STAFF_PASSWORD") | |
| if not STAFF_PASSWORD: | |
| raise ValueError("STAFF_PASSWORD environment variable not set.") |
1 participant
The staff password verification was previously implemented in client-side JavaScript, exposing the password 'SAC_MUSEUM_2026' to anyone who inspected the source code. This PR moves the verification logic to the FastAPI backend, utilizes environment variables for secret management, and employs
hmac.compare_digestto mitigate timing attacks. Frontend behavior remains identical but is now backed by secure server-side validation.PR created automatically by Jules for task 9127030693678809678 started by @LVT-ENG