Fix: Improve email validation and typo detection in login and registration forms#1052
Fix: Improve email validation and typo detection in login and registration forms#1052VaibhavSingh2006 wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughThis pull request adds blank lines to two files for improved code formatting. The login component receives additional spacing around the destructuring statement in the handleSubmit method, while the registration component receives spacing after the CSS import and before a conditional check. No functional logic, control flow, or behavior is modified. Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| "gmal.com": "gmail.com", | ||
| "gmial.com": "gmail.com", | ||
| "gmaill.com": "gmail.com", | ||
| "gmail.co": "gmail.com", |
There was a problem hiding this comment.
WARNING: gmail.co is a legitimate domain used by Google Colombia and other country-specific Google services. Mapping it to gmail.com will incorrectly block users with valid @gmail.co email addresses from logging in or registering.
Similarly, icloud.co (line 19) is a valid domain in Colombia. These entries should be removed from the typo map to avoid false positives.
| } | ||
|
|
||
| return null; | ||
| } No newline at end of file |
There was a problem hiding this comment.
WARNING: Missing newline at end of file. Most editors and linters expect a trailing newline.
| } | |
| } |
|
|
||
| // Check email domain typo | ||
| if (username && username.includes("@")) { | ||
| const suggestion = checkDomainTypo(username); |
There was a problem hiding this comment.
WARNING: Indentation is incorrect. The body of the if (username && username.includes("@")) block (lines 213–225) is not indented relative to the if statement. This makes the code hard to read and may confuse linters.
| const suggestion = checkDomainTypo(username); | |
| const suggestion = checkDomainTypo(username); |
| username: t`Please recheck your email. Did you mean ${suggestion}?`, | ||
| }, | ||
| }); | ||
| setLoading(false); |
There was a problem hiding this comment.
WARNING: setLoading(false) is called here, but setLoading(true) is only called later at line 231 (after this early return). Calling setLoading(false) before loading was ever set to true is a no-op at best and misleading at worst. This call should be removed.
| } = this.state; | ||
|
|
||
| // Check email domain typo | ||
| if (email && email.includes("@")) { |
There was a problem hiding this comment.
WARNING: Indentation is incorrect. The if block starting here is not indented to match the surrounding method body (which uses 4-space indentation). This inconsistency is visible throughout lines 140–152 and should be fixed to match the rest of the file's style.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge This PR adds extra blank lines/whitespace to two component files. These are purely formatting changes that don't affect functionality, security, or introduce any runtime errors. Files Reviewed (2 files)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/components/login/login.js`:
- Line 3: The import ordering in client/components/login/login.js is incorrect:
move the import of checkDomainTypo (import { checkDomainTypo } from
"../../utils/check-domain-typo") so it appears after the ttag import (the module
that provides translation functions) to satisfy the import/order lint rule;
update the import block so ttag imports come first, then
"../../utils/check-domain-typo" and ensure no other imports are reordered
incorrectly.
- Around line 211-225: Only run the domain-typo suggestion for actual email
logins: guard the checkDomainTypo(username) branch so it executes only when the
current login flow is the email/password flow (e.g. when login method/state
indicates "email" or when the submit path for radius/realm is not selected), or
move the block to after the radius/realm-submit branching; keep the same
behavior for updating setState({errors: {...errors, username: t`Please recheck
your email. Did you mean ${suggestion}?`}}) and setLoading(false) but skip this
rewrite for non-email username formats to avoid rewriting real realm-style
usernames that contain "@".
In `@client/components/registration/registration.js`:
- Around line 143-149: The new interpolated message t`Please recheck your email.
Did you mean ${suggestion}?` (introduced in registration.js and also in
client/components/login/login.js) is missing from the translation catalogs; add
the exact msgid to i18n/en.po and to client/test-translation.json (using the
same English source string with a placeholder for ${suggestion}) so translations
and test fixtures stay in sync, and ensure the entry key/text matches the
template punctuation and spacing exactly.
- Line 3: Move the import of checkDomainTypo so it comes after the ttag import
to satisfy import/order: locate the current import statement "import {
checkDomainTypo } from \"../../utils/check-domain-typo\";" in registration.js
and reposition it below the ttag import (the import that brings in ttag
functions), keeping the same named import and preserving existing formatting;
run the linter to confirm the ordering error is resolved.
In `@client/utils/check-domain-typo.js`:
- Around line 22-36: The file exports a single named function checkDomainTypo
which triggers import/prefer-default-export and also misses a trailing newline;
fix by either converting the export to a default export (export default
checkDomainTypo) and updating all callers to import checkDomainTypo from the
module as the default, or keep the named export and add an
eslint-disable-next-line comment to suppress import/prefer-default-export at the
top of the file; in both cases ensure the file ends with a single trailing
newline to satisfy eol-last.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 26627439-4fa7-47ee-b794-5a79bbcebfec
📒 Files selected for processing (3)
client/components/login/login.jsclient/components/registration/registration.jsclient/utils/check-domain-typo.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Tests and Coverage
🧰 Additional context used
🪛 ESLint
client/components/login/login.js
[error] 3-3: ../../utils/check-domain-typo import should occur after import of ttag
(import/order)
client/utils/check-domain-typo.js
[error] 22-36: Prefer default export on a file with single export.
(import/prefer-default-export)
[error] 36-36: Newline required at end of file but not found.
(eol-last)
client/components/registration/registration.js
[error] 3-3: ../../utils/check-domain-typo import should occur after import of ttag
(import/order)
| /* eslint-disable camelcase */ | ||
| import "./index.css"; | ||
|
|
||
| import { checkDomainTypo } from "../../utils/check-domain-typo"; |
There was a problem hiding this comment.
Reorder this import to satisfy import/order.
../../utils/check-domain-typo needs to be placed after the ttag import to match the current lint configuration.
🧰 Tools
🪛 ESLint
[error] 3-3: ../../utils/check-domain-typo import should occur after import of ttag
(import/order)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/components/login/login.js` at line 3, The import ordering in
client/components/login/login.js is incorrect: move the import of
checkDomainTypo (import { checkDomainTypo } from
"../../utils/check-domain-typo") so it appears after the ttag import (the module
that provides translation functions) to satisfy the import/order lint rule;
update the import block so ttag imports come first, then
"../../utils/check-domain-typo" and ensure no other imports are reordered
incorrectly.
| // Check email domain typo | ||
| if (username && username.includes("@")) { | ||
| const suggestion = checkDomainTypo(username); | ||
|
|
||
| if (suggestion) { | ||
| this.setState({ | ||
| errors: { | ||
| ...errors, | ||
| username: t`Please recheck your email. Did you mean ${suggestion}?`, | ||
| }, | ||
| }); | ||
| setLoading(false); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Only run typo detection for email-based logins.
This branch treats every username containing @ as an email. That breaks non-email login modes that legitimately use user@realm style usernames, and it runs before the radius submit path on Lines 235-237. A real realm name that happens to match your typo map would be blocked and rewritten as a public email domain.
Proposed fix
- const {orgSlug, setUserData, language, settings} = this.props;
+ const {orgSlug, setUserData, language, settings, loginForm} = this.props;
const {radius_realms} = settings;
const {username, password, errors} = this.state;
+ const isEmailLogin = loginForm.input_fields.username.type === "email";
// Check email domain typo
- if (username && username.includes("@")) {
- const suggestion = checkDomainTypo(username);
+ if (isEmailLogin && username && username.includes("@")) {
+ const suggestion = checkDomainTypo(username);
- if (suggestion) {
- this.setState({
- errors: {
- ...errors,
- username: t`Please recheck your email. Did you mean ${suggestion}?`,
- },
- });
- setLoading(false);
- return;
- }
+ if (suggestion) {
+ this.setState({
+ errors: {
+ ...errors,
+ username: t`Please recheck your email. Did you mean ${suggestion}?`,
+ },
+ });
+ setLoading(false);
+ return;
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/components/login/login.js` around lines 211 - 225, Only run the
domain-typo suggestion for actual email logins: guard the
checkDomainTypo(username) branch so it executes only when the current login flow
is the email/password flow (e.g. when login method/state indicates "email" or
when the submit path for radius/realm is not selected), or move the block to
after the radius/realm-submit branching; keep the same behavior for updating
setState({errors: {...errors, username: t`Please recheck your email. Did you
mean ${suggestion}?`}}) and setLoading(false) but skip this rewrite for
non-email username formats to avoid rewriting real realm-style usernames that
contain "@".
| @@ -1,5 +1,6 @@ | |||
| /* eslint-disable camelcase */ | |||
| import "./index.css"; | |||
| import { checkDomainTypo } from "../../utils/check-domain-typo"; | |||
There was a problem hiding this comment.
Reorder this import to satisfy import/order.
../../utils/check-domain-typo needs to be placed after the ttag import to match the current lint configuration.
🧰 Tools
🪛 ESLint
[error] 3-3: ../../utils/check-domain-typo import should occur after import of ttag
(import/order)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/components/registration/registration.js` at line 3, Move the import of
checkDomainTypo so it comes after the ttag import to satisfy import/order:
locate the current import statement "import { checkDomainTypo } from
\"../../utils/check-domain-typo\";" in registration.js and reposition it below
the ttag import (the import that brings in ttag functions), keeping the same
named import and preserving existing formatting; run the linter to confirm the
ordering error is resolved.
| if (suggestion) { | ||
| this.setState({ | ||
| errors: { | ||
| ...errors, | ||
| email: t`Please recheck your email. Did you mean ${suggestion}?`, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Add this new msgid to the translation catalogs.
This new t\Please recheck your email. Did you mean ${suggestion}?`string is not present ini18n/en.poorclient/test-translation.json, so the new validation copy will stay out of sync with the locale/test fixtures. The same msgid is also introduced in client/components/login/login.js` Line 219.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/components/registration/registration.js` around lines 143 - 149, The
new interpolated message t`Please recheck your email. Did you mean
${suggestion}?` (introduced in registration.js and also in
client/components/login/login.js) is missing from the translation catalogs; add
the exact msgid to i18n/en.po and to client/test-translation.json (using the
same English source string with a placeholder for ${suggestion}) so translations
and test fixtures stay in sync, and ensure the entry key/text matches the
template punctuation and spacing exactly.
| export function checkDomainTypo(email) { | ||
| if (!email || !email.includes("@")) return null; | ||
|
|
||
| const parts = email.split("@"); | ||
| if (parts.length !== 2) return null; | ||
|
|
||
| const name = parts[0]; | ||
| const domain = parts[1].toLowerCase(); | ||
|
|
||
| if (TYPO_MAP[domain]) { | ||
| return `${name}@${TYPO_MAP[domain]}`; | ||
| } | ||
|
|
||
| return null; | ||
| } No newline at end of file |
There was a problem hiding this comment.
Fix the lint blockers in this new module.
This file currently fails the existing lint rules: import/prefer-default-export on the single export and eol-last at EOF. Either switch this helper to a default export and update its consumers, or explicitly suppress the rule for this file, then add the trailing newline so CI stays green.
🧰 Tools
🪛 ESLint
[error] 22-36: Prefer default export on a file with single export.
(import/prefer-default-export)
[error] 36-36: Newline required at end of file but not found.
(eol-last)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/utils/check-domain-typo.js` around lines 22 - 36, The file exports a
single named function checkDomainTypo which triggers
import/prefer-default-export and also misses a trailing newline; fix by either
converting the export to a default export (export default checkDomainTypo) and
updating all callers to import checkDomainTypo from the module as the default,
or keep the named export and add an eslint-disable-next-line comment to suppress
import/prefer-default-export at the top of the file; in both cases ensure the
file ends with a single trailing newline to satisfy eol-last.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
client/components/registration/registration.js (1)
114-151:⚠️ Potential issue | 🟠 MajorRegistration still submits typoed emails unchanged.
On Lines 141-151, the only client-side guard before submission is the password confirmation check.
postDataand posted verbatim, so common provider typos will not trigger the correction flow requested in issue#1044.Proposed fix
+import {checkDomainTypo} from "../../utils/check-domain-typo"; + handleSubmit(event) { const {setLoading} = this.context; event.preventDefault(); const {orgSlug, authenticate, settings, language, setUserData, navigate} = this.props; const { phone_number, email, username, first_name, last_name, birth_date, location, password1, password2, errors, selectedPlan, plans, tax_number, street, city, zipcode, country, } = this.state; + + const suggestion = checkDomainTypo(email); + if (suggestion) { + this.setState({ + errors: { + ...errors, + email: t`Please recheck your email. Did you mean ${suggestion}?`, + }, + }); + return false; + } if (password1 !== password2) { this.setState({ errors: { password2: t`PWD_CNF_ERR`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/registration/registration.js` around lines 114 - 151, The handleSubmit function currently sends this.state.email verbatim; update handleSubmit to validate and normalize the email before building postData: call the existing email-suggestion/normalization utility (or implement a simple provider-typo checker) on this.state.email, and if a suggestion is returned setState to update email (or present the suggestion and halt submission) so postData uses the corrected address; ensure the check runs after the password match block and before constructing postData/using registerApiUrl so postData.email is never the typoed value.client/components/login/login.js (1)
204-223:⚠️ Potential issue | 🟠 MajorReintroduce the email-typo gate in the login submit path.
On Lines 204-223,
handleSubmitgoes straight to the RADIUS handoff or API call without checking for known email-domain typos first. As written,user@gmal.com/user@gmial.comstill submit unchanged from this component, so the behavior described in issue#1044cannot be enforced here.Proposed fix
+import {checkDomainTypo} from "../../utils/check-domain-typo"; + handleSubmit(event, sesame_token = null) { const {setLoading} = this.context; if (event) event.preventDefault(); - const {orgSlug, setUserData, language, settings} = this.props; + const {orgSlug, setUserData, language, settings, loginForm} = this.props; const {radius_realms} = settings; const {username, password, errors} = this.state; + const isEmailLogin = loginForm.input_fields.username.type === "email"; + + if (isEmailLogin && username.includes("@")) { + const suggestion = checkDomainTypo(username); + if (suggestion) { + this.setState({ + errors: { + ...errors, + username: t`Please recheck your email. Did you mean ${suggestion}?`, + }, + }); + return; + } + } const url = loginApiUrl(orgSlug);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/login/login.js` around lines 204 - 223, handleSubmit currently proceeds to RADIUS or API submission without running the email-typo gate; add a check after extracting username (and before the radius_realms branch and any submission) that detects known email-domain typos and invokes the existing typo-handling flow (e.g. call a helper like checkEmailTypo(username) or this.handleEmailTypoGate(username)), and if that helper returns/handles the submission, return early so the corrected/confirmed email is used instead of submitting user@gmal.com unchanged; place this logic in handleSubmit, referencing the username state and the realmsRadiusLoginForm branch so it runs before realmsRadiusLoginForm.current.submit() or any API call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@client/components/login/login.js`:
- Around line 204-223: handleSubmit currently proceeds to RADIUS or API
submission without running the email-typo gate; add a check after extracting
username (and before the radius_realms branch and any submission) that detects
known email-domain typos and invokes the existing typo-handling flow (e.g. call
a helper like checkEmailTypo(username) or this.handleEmailTypoGate(username)),
and if that helper returns/handles the submission, return early so the
corrected/confirmed email is used instead of submitting user@gmal.com unchanged;
place this logic in handleSubmit, referencing the username state and the
realmsRadiusLoginForm branch so it runs before
realmsRadiusLoginForm.current.submit() or any API call.
In `@client/components/registration/registration.js`:
- Around line 114-151: The handleSubmit function currently sends
this.state.email verbatim; update handleSubmit to validate and normalize the
email before building postData: call the existing email-suggestion/normalization
utility (or implement a simple provider-typo checker) on this.state.email, and
if a suggestion is returned setState to update email (or present the suggestion
and halt submission) so postData uses the corrected address; ensure the check
runs after the password match block and before constructing postData/using
registerApiUrl so postData.email is never the typoed value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7d0525f7-d70d-4784-a75f-8892befe9d01
📒 Files selected for processing (2)
client/components/login/login.jsclient/components/registration/registration.js
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-06T23:26:11.513Z
Learnt from: nemesifier
Repo: openwisp/openwisp-wifi-login-pages PR: 0
File: :0-0
Timestamp: 2026-03-06T23:26:11.513Z
Learning: In client/components/status/status.js (openwisp-wifi-login-pages), catch blocks deliberately do NOT check `isComponentMounted` — console warnings from catch paths are considered harmless. Only real memory leaks (timers, event listeners) and "setState on unmounted" in success paths are fixed. The maintainer (nemesifier) explicitly wants to avoid over-engineering with defensive checks in every catch block.
Applied to files:
client/components/login/login.js
| const {radius_realms} = settings; | ||
| const {username, password, errors} = this.state; | ||
|
|
||
|
|
There was a problem hiding this comment.
WARNING: This line contains only whitespace characters (trailing spaces). It should be an empty line or removed entirely.
| /* eslint-disable camelcase */ | ||
| import "./index.css"; | ||
|
|
||
|
|
There was a problem hiding this comment.
WARNING: Extra blank line added between import "./index.css"; and import axios from "axios";. This adds unnecessary whitespace and doesn't follow the existing import grouping style.
| country, | ||
| } = this.state; | ||
|
|
||
|
|
There was a problem hiding this comment.
WARNING: This line contains only whitespace characters (trailing spaces). It should be an empty line or removed entirely.
Prettier Code Style FailuresHello @VaibhavSingh2006 and @stktyagi, Failures & Remediation
|
2 participants
Closes #1044
Description
The email validation logic in the login and registration forms was too permissive and allowed invalid email domains such as
user@gmal.com.This PR introduces typo detection for common email domain mistakes while ensuring that valid domains (e.g., corporate or university emails) are not blocked.
Changes
check-domain-typo.jsto detect common email domain typosuser@company.comoruser@university.eduare allowedExample
Environment
OS: macOS
Node.js: v18
Browser: Chrome