Update email regex pattern and improve error messages in auth handlers' . #122
Conversation
WalkthroughChanged two room handlers' error text for non-string email context values. Modified email-name extraction to only return the local part when the input fully matches an email pattern; otherwise the original input is returned. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/utils/auth.go (1)
38-46: Critical: Regex missing capturing group breaks username extraction.The regex pattern lacks capturing groups, so
FindStringSubmatchreturns only the full matched email inmatch[0]. The conditionlen(match) < 2will always be true, causing the function to return the full email instead of extracting the username. This breaks the function's documented purpose and affects multiple callers.Usages impacted:
backend/controllers/auth.go:59— fallback when nickname is emptybackend/controllers/auth.go:160–161— setting DisplayName and Nickname on user creationbackend/controllers/leaderboard.go:70— fallback when DisplayName is emptyApply this diff to fix:
- re := regexp.MustCompile(`^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`) + re := regexp.MustCompile(`^([a-zA-Z0-9._%+-]+)@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`)
Naitik1886
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/utils/auth.go(1 hunks)
🔇 Additional comments (1)
backend/utils/auth.go (1)
39-46: Email validation via struct binding tags mitigates behavior change concerns.The
SignUpRequeststruct enforces email validation withbinding:"required,email"on the Email field, which Gin validates before the handler executes. This ensuresrequest.Emailat lines 160–161 is always a valid email format. Other call sites use existing user emails (from the database) or trusted sources (Google tokens). The new behavior—returning the original input when the pattern doesn't match—is actually safer than unconditionally extracting before '@'. No code changes required.
| // ExtractNameFromEmail extracts the username before '@' | ||
| func ExtractNameFromEmail(email string) string { | ||
| re := regexp.MustCompile(`^([^@]+)`) | ||
| re := regexp.MustCompile(`^([a-zA-Z0-9._%+-]+)@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`) |
There was a problem hiding this comment.
Move regex compilation to package level and address pattern edge cases.
The regex is compiled on every function call, which is inefficient. Additionally, the current pattern has edge cases that allow technically invalid emails:
- Consecutive dots:
[email protected] - Leading/trailing dots:
[email protected]or[email protected] - Leading/trailing hyphens in domain:
[email protected]or[email protected]
Apply this diff to move the regex to package level and tighten the pattern:
+var emailRegex = regexp.MustCompile(`^([a-zA-Z0-9]+(?:[._%-][a-zA-Z0-9]+)*)@[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?)*\.[a-zA-Z]{2,}$`)
+
// ExtractNameFromEmail extracts the username before '@'
func ExtractNameFromEmail(email string) string {
- re := regexp.MustCompile(`^([a-zA-Z0-9._%+-]+)@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`)
- match := re.FindStringSubmatch(email)
+ match := emailRegex.FindStringSubmatch(email)
if len(match) < 2 {
return email
}The improved pattern:
- Prevents consecutive special characters
- Prevents leading/trailing special characters
- Ensures domain labels don't start/end with hyphens
Alternatively, if the current pattern is intentionally lenient for your use case, at minimum move the compilation to package level for performance.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/utils/auth.go around line 40, the email regexp is compiled on each
call and allows edge cases (consecutive or leading/trailing dots in local part
and domain labels starting/ending with hyphens); move the regexp compilation to
a package-level variable and replace the per-call Compile/Match with that
precompiled var, and tighten the pattern to disallow consecutive special
characters and leading/trailing dots in the local part and disallow domain
labels that start or end with hyphens (i.e. use a single compiled pattern that
enforces no consecutive special chars, no leading/trailing special chars, and
domain label rules).
|
|
2 participants
Fix: Issue #66 - Incorrect Input Validation for Email/Password in Authentication
Solved - Updated the old email regex with the standard one.
Also fixed the unclear error messaging .
Summary by CodeRabbit
Bug Fixes
Behavior Changes
✏️ Tip: You can customize this high-level summary in your review settings.