fix(auth): check username and email duplicates independently with specific error messages#39
fix(auth): check username and email duplicates independently with specific error messages#39ojaswa072 wants to merge 3 commits intoOWASP-BLT:mainfrom
Conversation
…cific error messages
|
👋 Thanks for opening this pull request, @ojaswa072! Before your PR is reviewed, please ensure:
🔍 Our team will review your PR shortly. If you have questions, feel free to ask in the comments. 🚀 Keep up the great work! — OWASP BLT |
📊 Monthly LeaderboardHi @ojaswa072! Here's how you rank for March 2026:
Scoring this month (across OWASP-BLT org): Open PRs (+1 each), Merged PRs (+10), Closed (not merged) (−2), Reviews (+5; first two per PR in-month), Comments (+2, excludes CodeRabbit). Run |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Repository: OWASP-BLT/coderabbit/.coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/handlers/auth.py (1)
8-8: Unused import shadows local variable.The import
from src.libs import dbis not used anywhere in this file. Additionally, the local variabledbassigned on line 89 shadows this imported module name, which could cause confusion.🧹 Proposed fix
-from src.libs import db🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/handlers/auth.py` at line 8, The top-level import "from src.libs import db" is unused and conflicts with the local variable named db (assigned at the db variable declaration around line 89), so either remove that unused import or rename one of them to avoid shadowing; the simplest fix is to delete the unused module import "from src.libs import db" in src/handlers/auth.py, otherwise alias the import (e.g., libs_db) or rename the local variable (e.g., local_db) consistently wherever it’s referenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/handlers/auth.py`:
- Around line 94-100: The pre-checks (existing_username/existing_email) still
allow a TOCTOU race so wrap the User.create(...) call in a specific try/except
that detects unique-constraint violations from the DB driver and maps them to
the appropriate 400 responses (use error_response("Username already taken", 400)
or error_response("Email already registered", 400) depending on which constraint
failed); leave other exceptions to propagate to the existing outer handler.
Update the exception detection to inspect the DB error type/message returned by
your driver (SQLite/D1) and ensure the handler references User.create,
existing_username, existing_email, and error_response so the correct
client-facing 400 is returned on constraint violations.
---
Nitpick comments:
In `@src/handlers/auth.py`:
- Line 8: The top-level import "from src.libs import db" is unused and conflicts
with the local variable named db (assigned at the db variable declaration around
line 89), so either remove that unused import or rename one of them to avoid
shadowing; the simplest fix is to delete the unused module import "from src.libs
import db" in src/handlers/auth.py, otherwise alias the import (e.g., libs_db)
or rename the local variable (e.g., local_db) consistently wherever it’s
referenced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: OWASP-BLT/coderabbit/.coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 0eaabcfb-6eaa-4810-80fb-cfcf6ee58519
📒 Files selected for processing (1)
src/handlers/auth.py
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/handlers/auth.py`:
- Line 8: Remove the unused top-level import "from src.libs import db" in
src/handlers/auth.py because the file uses the local variable db returned by
get_db_safe(env) (see get_db_safe and the local variable db around where DB is
used), so delete that import line and run linters/tests to ensure nothing else
relied on the module-level name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: OWASP-BLT/coderabbit/.coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 4943e7f9-a6e1-4e0c-860d-7d98b1eab6b9
📒 Files selected for processing (1)
src/handlers/auth.py
1 participant
Summary
Fixed a logic bug in the signup endpoint where email uniqueness was only checked if the username was not already taken.
Problem
The original code used a chained
if not existing_usercheck, meaning:This also returned a generic "User already exists" message with no indication of which field caused the conflict.
Fix
Files Changed
src/handlers/auth.py— signup duplicate check logicSummary by CodeRabbit