Skip to content

feat: enhance email and password validation with typo detection#1045

Open
vikram-2101 wants to merge 2 commits into
openwisp:masterfrom
vikram-2101:feature/enhanced-validation
Open

feat: enhance email and password validation with typo detection#1045
vikram-2101 wants to merge 2 commits into
openwisp:masterfrom
vikram-2101:feature/enhanced-validation

Conversation

@vikram-2101
Copy link
Copy Markdown

Checklist

  • [✔️] I have read the OpenWISP Contributing Guidelines.
  • [✔️] I have manually tested the changes proposed in this pull request.
  • [✔️] I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • [✔️] I have updated the documentation.

Reference to Existing Issue

Closes #1044.

Description of Changes

This PR improves registration and login security and UX:

Centralized Validation: Moved all validation logic to
client/utils/validation.js
.
Email Improvements: Added a stricter regex and domain typo detection (e.g., gmal.com prompt).
Password Hardening: Enforced 8-character minimum and mandatory complexity (uppercase, lowercase, digits, symbols).
UX: specific error feedback for password strength and email typos.
Build: Replaced deprecated brotli-webpack-plugin with compression-webpack-plugin for Webpack 5.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 2, 2026

📝 Walkthrough

Walkthrough

This pull request introduces enhanced email and password validation across the authentication flows. A new centralized validation utility module is created, exposing validateEmail and validatePassword functions that enforce a stricter email regex pattern and implement 8-character minimum password requirements with complexity checks (uppercase, lowercase, digit, special character). The validateEmail function additionally detects common domain typos and returns suggestions. These validators are integrated into the login and registration components to validate inputs before form submission, with user prompts for email correction suggestions. The test configuration is updated with stricter validation patterns, comprehensive unit tests are added for the validators, and the Webpack configuration is updated to replace the deprecated BrotliPlugin with CompressionPlugin for Webpack 5 compatibility.

Sequence Diagram

sequenceDiagram
    actor User
    participant LoginComp as Login Component
    participant Validator as Validation Utils
    participant UI as User Prompt

    User->>LoginComp: Submit form with email
    LoginComp->>Validator: validateEmail(email)
    Validator-->>LoginComp: {isValid, suggestion}
    
    alt Email Invalid
        LoginComp->>LoginComp: Set email error
        LoginComp-->>User: Show error message
    else Email Valid + Suggestion
        LoginComp->>UI: Prompt user for correction
        UI-->>User: "Did you mean...?"
        alt User Accepts
            User->>UI: Confirm
            UI->>LoginComp: Update email
            LoginComp->>LoginComp: Abort current submission
        else User Rejects
            User->>UI: Reject
            UI->>LoginComp: Abort submission
        end
    else Email Valid + No Suggestion
        LoginComp->>LoginComp: Proceed with submission
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main feature: enhanced email and password validation with typo detection, which aligns with the primary changes across validation components.
Description check ✅ Passed The description covers all required sections: completed checklist items, issue reference (#1044), detailed change description, and addresses the core objectives including centralized validation, email/password improvements, and build fixes.
Linked Issues check ✅ Passed The PR fully addresses issue #1044 requirements: implements stricter email regex pattern, adds domain typo detection with user prompts, and strengthens password validation with 8-character minimum and complexity requirements.
Out of Scope Changes check ✅ Passed All changes directly support the linked issue requirements: validation logic centralization, email/password improvements, configuration updates, comprehensive tests, and the Webpack 5 deprecation fix is a necessary build maintenance item.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🤖 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`:
- Around line 228-238: Replace the blocking window.confirm call in the login
component with a non-blocking UI flow: remove window.confirm and instead trigger
your app's modal/toast utility (or create a small EmailSuggestionModal
component) from the same handler so the suggestion UI is asynchronous and
stylable; when the user accepts, call this.setState({username: suggestion}) (as
in the current code) and return/continue flow via the modal's promise/callback,
and when they reject simply continue without changing state. Also remove the
unnecessary setLoading(false) call since loading was never set true in this
branch. Ensure the new flow is wired into the same method where the code lives
(the handler that currently uses window.confirm and this.setState) so no other
logic needs changing.
- Around line 219-227: The validation branch that handles invalid email
currently calls setLoading(false) unnecessarily; remove the setLoading(false)
call inside the if (!isValid) block so only this.setState({ errors: { username:
t`USERNAME_LOG_TITL` } }) and return false remain, leaving setLoading to be
managed where loading is actually started (see the isValid check and the
surrounding login flow that later calls setLoading(true)).

In `@client/components/registration/registration.js`:
- Around line 158-165: The two premature setLoading(false) calls inside the
email suggestion/confirmation branch should be removed because loading is not
set true earlier; in the block that checks suggestion and confirmTypo (the code
that calls window.confirm and this.setState({email: suggestion})), delete the
setLoading(false) invocations so you only manage loading where setLoading(true)
is actually used (e.g., around the submission logic), ensuring you leave the
suggestion flow to just update state and return without toggling loading.

In `@client/test-config.json`:
- Around line 62-66: The HTML/email regex in the JSON ("pattern" for the email
field) only allows lowercase letters, which conflicts with the case-insensitive
validateEmail function in client/utils/validation.js; update the email "pattern"
to accept uppercase letters as well (e.g., expand [a-z0-9...] to [A-Za-z0-9...],
including the TLD portion) or remove the HTML pattern entirely and rely on
validateEmail, and apply the same change to the other occurrence referenced
(lines 179-191 equivalent) so both HTML pattern entries match the behavior of
validateEmail.
- Line 62: The email regex in the JSON config uses
"[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$" which lacks the start anchor; update
the pattern value in client/test-config.json to include a leading caret so it
becomes "^[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$" to ensure full-string
validation and match the regex style used in client/utils/validation.js.

In `@client/utils/validation.js`:
- Around line 1-7: Update the COMMON_TYPOS mapping in validation.js to include
additional frequent domain typos (for example add "outlok.com":"outlook.com",
"gmial.com":"gmail.com", "gamil.com":"gmail.com", "gmil.com":"gmail.com",
"yahho.com":"yahoo.com", etc.) so the email correction logic in the code uses
these extra mappings; locate the COMMON_TYPOS constant and add the new key→value
pairs (keeping the same object shape and style) and run any existing validation
tests to ensure behavior remains correct.
- Around line 19-24: The domain lookup uses a lowercased domain but then calls
email.replace with that lowercased domain, which fails for mixed-case inputs;
fix by splitting the email at the "@" (use the original localPart and the
original domain string), compute domainLower = domain.toLowerCase() to check
COMMON_TYPOS, and if a suggestion exists build suggestion =
`${localPart}@${COMMON_TYPOS[domainLower]}` (or equivalent) so you replace only
the domain part while preserving the original localPart and handling
case-insensitive matches; update the code paths that assign suggestion and
reference COMMON_TYPOS accordingly.

In `@client/utils/validation.test.js`:
- Around line 3-45: Add unit tests in the validateEmail and validatePassword
suites to cover defensive and boundary edge cases: include tests that pass empty
string, null, and undefined to validateEmail and validatePassword and assert
they return isValid: false (and appropriate errors/suggestions when applicable);
add a test for an email with multiple '@' symbols asserting isValid: false; add
a test for a very long password (e.g., >1000 chars) to validatePassword
asserting expected behavior (either valid if policy allows length or returns a
specific length-related error); reference validateEmail and validatePassword in
the new tests to ensure these inputs are handled without throwing.
- Around line 15-17: Add a new unit test in client/utils/validation.test.js that
calls validateEmail with an address using uppercase local or domain (e.g.,
"User@GMAL.COM") to ensure the suggestion logic is case-insensitive; assert that
validateEmail("User@GMAL.COM").suggestion equals "User@gmail.com" so the
local-part case is preserved while the domain is corrected and lowercased by the
validateEmail function.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 71c1db2 and 574f9f2.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (7)
  • CHANGES.md
  • client/components/login/login.js
  • client/components/registration/registration.js
  • client/test-config.json
  • client/utils/validation.js
  • client/utils/validation.test.js
  • config/webpack.config.js
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (3)
config/webpack.config.js (1)
server/index.js (1)
  • DEFAULT_PORT (28-28)
client/utils/validation.test.js (1)
client/utils/validation.js (4)
  • validateEmail (14-27)
  • validateEmail (14-27)
  • validatePassword (35-57)
  • validatePassword (35-57)
client/components/registration/registration.js (1)
client/utils/validation.js (6)
  • validateEmail (14-27)
  • validateEmail (14-27)
  • isValid (16-16)
  • suggestion (18-18)
  • validatePassword (35-57)
  • validatePassword (35-57)
🔇 Additional comments (6)
config/webpack.config.js (2)

21-21: LGTM!

The fallback to DEFAULT_PORT when process.env.CLIENT is not set is a sensible improvement for better developer experience.


82-92: LGTM!

The CompressionPlugin configuration for Brotli compression is well-configured:

  • level: 11 provides maximum compression
  • threshold: 10240 avoids compressing small files where overhead isn't worthwhile
  • deleteOriginalAssets: false correctly preserves originals for browsers without Brotli support
client/utils/validation.js (1)

35-57: LGTM!

The password validation logic is comprehensive and correctly validates all required complexity rules. Returning all errors at once provides good UX by showing users everything they need to fix.

client/components/login/login.js (1)

215-238: LGTM on validation integration.

The email validation is correctly integrated - it validates before the API call, handles invalid input with appropriate error state, and offers correction suggestions for typos.

client/components/registration/registration.js (1)

148-176: Validation logic is well-structured.

Good implementation:

  • Email validation happens before password validation
  • Password mismatch check (lines 139-146) runs first before complexity validation
  • Only the first password error is shown to avoid overwhelming the user
CHANGES.md (1)

5-13: LGTM!

The changelog entries accurately document the new features (enhanced email validation, password complexity, centralized validation) and the bugfix (Webpack 5 deprecation fix). Clear and well-organized.

Comment on lines +219 to +227
if (!isValid) {
this.setState({
errors: {
username: t`USERNAME_LOG_TITL`,
},
});
setLoading(false);
return false;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unnecessary setLoading(false) call.

At line 225, setLoading(false) is called when email validation fails, but setLoading(true) (line 239) hasn't been called yet at this point in the execution flow. This is harmless but unnecessary and could cause confusion.

🔧 Suggested fix
       if (!isValid) {
         this.setState({
           errors: {
             username: t`USERNAME_LOG_TITL`,
           },
         });
-        setLoading(false);
         return false;
       }
🤖 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 219 - 227, The validation
branch that handles invalid email currently calls setLoading(false)
unnecessarily; remove the setLoading(false) call inside the if (!isValid) block
so only this.setState({ errors: { username: t`USERNAME_LOG_TITL` } }) and return
false remain, leaving setLoading to be managed where loading is actually started
(see the isValid check and the surrounding login flow that later calls
setLoading(true)).

Comment on lines +228 to +238
if (suggestion) {
const confirmTypo = window.confirm(
`${t`EMAIL_TYPO_ERR`} ${suggestion}?`,
);
if (confirmTypo) {
this.setState({username: suggestion});
setLoading(false);
return false;
}
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

window.confirm blocks the UI thread.

Using window.confirm for the email typo suggestion is functional but provides a poor user experience as it's a blocking synchronous call that can't be styled to match the app's design. Consider using a non-blocking modal or toast notification that allows users to accept/reject the suggestion asynchronously.

Also, the setLoading(false) on line 234 is unnecessary since loading hasn't been set to true yet.

🤖 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 228 - 238, Replace the
blocking window.confirm call in the login component with a non-blocking UI flow:
remove window.confirm and instead trigger your app's modal/toast utility (or
create a small EmailSuggestionModal component) from the same handler so the
suggestion UI is asynchronous and stylable; when the user accepts, call
this.setState({username: suggestion}) (as in the current code) and
return/continue flow via the modal's promise/callback, and when they reject
simply continue without changing state. Also remove the unnecessary
setLoading(false) call since loading was never set true in this branch. Ensure
the new flow is wired into the same method where the code lives (the handler
that currently uses window.confirm and this.setState) so no other logic needs
changing.

Comment on lines +158 to +165
if (suggestion) {
const confirmTypo = window.confirm(`${t`EMAIL_TYPO_ERR`} ${suggestion}?`);
if (confirmTypo) {
this.setState({email: suggestion});
setLoading(false);
return false;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unnecessary setLoading(false) calls before loading is set.

At lines 162 and 174, setLoading(false) is called but setLoading(true) is only called later at line 233. These calls are unnecessary since the loading state isn't true at this point.

🔧 Suggested fix
     if (suggestion) {
       const confirmTypo = window.confirm(`${t`EMAIL_TYPO_ERR`} ${suggestion}?`);
       if (confirmTypo) {
         this.setState({email: suggestion});
-        setLoading(false);
         return false;
       }
     }
 
     const passwordValidation = validatePassword(password1);
     if (!passwordValidation.isValid) {
       this.setState({
         errors: {
           password1: passwordValidation.errors[0],
         },
       });
-      setLoading(false);
       return false;
     }
🤖 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 158 - 165, The
two premature setLoading(false) calls inside the email suggestion/confirmation
branch should be removed because loading is not set true earlier; in the block
that checks suggestion and confirmTypo (the code that calls window.confirm and
this.setState({email: suggestion})), delete the setLoading(false) invocations so
you only manage loading where setLoading(true) is actually used (e.g., around
the submission logic), ensuring you leave the suggestion flow to just update
state and return without toggling loading.

Comment thread client/test-config.json
Comment on lines +62 to +66
"pattern": "[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$"
},
"password": {
"type": "password",
"pattern": ".{6,}"
"pattern": "^(?=.*[a-z])(?=.*[A-Z])(?=.*\\d)(?=.*[!@#$%^&*()_+={}\\[\\]:;\"'<>,.?/\\\\|~`]).{8,}$"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Consider case-insensitivity mismatch between HTML pattern and JS validation.

The email pattern in the config uses only lowercase [a-z], but the validateEmail function in client/utils/validation.js uses the /i flag for case-insensitivity. HTML pattern attributes are case-sensitive by default, so uppercase letters in emails (e.g., User@Example.COM) will fail HTML validation but pass JS validation. This inconsistency could cause confusing UX where the form prevents submission even though the JS validator would accept the email.

Consider either:

  1. Making the HTML pattern case-insensitive: [a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$
  2. Or relying solely on the JS validation and removing the HTML pattern

Also applies to: 179-191

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/test-config.json` around lines 62 - 66, The HTML/email regex in the
JSON ("pattern" for the email field) only allows lowercase letters, which
conflicts with the case-insensitive validateEmail function in
client/utils/validation.js; update the email "pattern" to accept uppercase
letters as well (e.g., expand [a-z0-9...] to [A-Za-z0-9...], including the TLD
portion) or remove the HTML pattern entirely and rely on validateEmail, and
apply the same change to the other occurrence referenced (lines 179-191
equivalent) so both HTML pattern entries match the behavior of validateEmail.

Comment thread client/test-config.json
"email": {
"type": "email",
"pattern": ".+@.+\\..+"
"pattern": "[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Email pattern missing start anchor ^.

The email pattern [a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$ lacks a ^ anchor at the beginning. Without it, when used as an HTML pattern attribute, the browser may accept input like invalid@[a-z0-9._%+-]+@example.com because the pattern only needs to match at the end.

For consistency with the regex in client/utils/validation.js (which uses ^...$), add the start anchor:

🔧 Suggested fix
-            "pattern": "[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$"
+            "pattern": "^[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"pattern": "[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$"
"pattern": "^[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/test-config.json` at line 62, The email regex in the JSON config uses
"[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$" which lacks the start anchor; update
the pattern value in client/test-config.json to include a leading caret so it
becomes "^[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$" to ensure full-string
validation and match the regex style used in client/utils/validation.js.

Comment on lines +1 to +7
const COMMON_TYPOS = {
"gmal.com": "gmail.com",
"gmai.com": "gmail.com",
"gnail.com": "gmail.com",
"hotmal.com": "hotmail.com",
"yaho.com": "yahoo.com",
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider expanding common typo coverage.

The typo dictionary covers a few common misspellings but could be expanded to include additional common domains and their typos (e.g., outlok.comoutlook.com, gmial.comgmail.com, gamil.comgmail.com). This is optional but would improve UX.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/utils/validation.js` around lines 1 - 7, Update the COMMON_TYPOS
mapping in validation.js to include additional frequent domain typos (for
example add "outlok.com":"outlook.com", "gmial.com":"gmail.com",
"gamil.com":"gmail.com", "gmil.com":"gmail.com", "yahho.com":"yahoo.com", etc.)
so the email correction logic in the code uses these extra mappings; locate the
COMMON_TYPOS constant and add the new key→value pairs (keeping the same object
shape and style) and run any existing validation tests to ensure behavior
remains correct.

Comment on lines +19 to +24
if (isValid) {
const domain = email.split("@")[1].toLowerCase();
if (COMMON_TYPOS[domain]) {
suggestion = email.replace(domain, COMMON_TYPOS[domain]);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Case-sensitivity bug in email suggestion replacement.

When generating the suggestion, the code lowercases the domain for lookup but then tries to replace that lowercase string in the original (potentially mixed-case) email. If the user enters User@GMAL.COM, the domain becomes gmal.com after lowercase, but email.replace("gmal.com", "gmail.com") won't find a match in User@GMAL.COM.

🐛 Proposed fix
 let suggestion = null;
 if (isValid) {
   const domain = email.split("@")[1].toLowerCase();
   if (COMMON_TYPOS[domain]) {
-    suggestion = email.replace(domain, COMMON_TYPOS[domain]);
+    const localPart = email.split("@")[0];
+    suggestion = `${localPart}@${COMMON_TYPOS[domain]}`;
   }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (isValid) {
const domain = email.split("@")[1].toLowerCase();
if (COMMON_TYPOS[domain]) {
suggestion = email.replace(domain, COMMON_TYPOS[domain]);
}
}
if (isValid) {
const domain = email.split("@")[1].toLowerCase();
if (COMMON_TYPOS[domain]) {
const localPart = email.split("@")[0];
suggestion = `${localPart}@${COMMON_TYPOS[domain]}`;
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/utils/validation.js` around lines 19 - 24, The domain lookup uses a
lowercased domain but then calls email.replace with that lowercased domain,
which fails for mixed-case inputs; fix by splitting the email at the "@" (use
the original localPart and the original domain string), compute domainLower =
domain.toLowerCase() to check COMMON_TYPOS, and if a suggestion exists build
suggestion = `${localPart}@${COMMON_TYPOS[domainLower]}` (or equivalent) so you
replace only the domain part while preserving the original localPart and
handling case-insensitive matches; update the code paths that assign suggestion
and reference COMMON_TYPOS accordingly.

Comment on lines +3 to +45
describe("Validation utilities", () => {
describe("validateEmail", () => {
it("should return isValid: true for correct email formats", () => {
expect(validateEmail("test@example.com").isValid).toBe(true);
expect(validateEmail("user.name@domain.co").isValid).toBe(true);
});

it("should return isValid: false for incorrect email formats", () => {
expect(validateEmail("plainaddress").isValid).toBe(false);
expect(validateEmail("@example.com").isValid).toBe(false);
});

it("should return a suggestion for common typos", () => {
expect(validateEmail("user@gmal.com").suggestion).toBe("user@gmail.com");
});
});

describe("validatePassword", () => {
it("should return isValid: true for strong passwords", () => {
expect(validatePassword("StrongPass123!").isValid).toBe(true);
});

it("should return isValid: false for weak passwords", () => {
expect(validatePassword("weak").isValid).toBe(false); // too short
expect(validatePassword("lowercase123!").isValid).toBe(false); // no uppercase
expect(validatePassword("UPPERCASE123!").isValid).toBe(false); // no lowercase
expect(validatePassword("NoDigits!").isValid).toBe(false); // no digits
expect(validatePassword("NoSymbol123").isValid).toBe(false); // no symbol
});

it("should return specific error messages", () => {
const result = validatePassword("short");
expect(result.errors).toContain(
"Password must be at least 8 characters long.",
);
expect(result.errors).toContain(
"Password must contain at least one uppercase letter.",
);
expect(result.errors).toContain(
"Password must contain at least one special character.",
);
});
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding edge case tests.

The test suite covers the main scenarios well, but consider adding tests for edge cases:

  • Empty string input for both validators
  • null or undefined inputs (defensive testing)
  • Very long passwords (boundary testing)
  • Emails with multiple @ symbols
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/utils/validation.test.js` around lines 3 - 45, Add unit tests in the
validateEmail and validatePassword suites to cover defensive and boundary edge
cases: include tests that pass empty string, null, and undefined to
validateEmail and validatePassword and assert they return isValid: false (and
appropriate errors/suggestions when applicable); add a test for an email with
multiple '@' symbols asserting isValid: false; add a test for a very long
password (e.g., >1000 chars) to validatePassword asserting expected behavior
(either valid if policy allows length or returns a specific length-related
error); reference validateEmail and validatePassword in the new tests to ensure
these inputs are handled without throwing.

Comment on lines +15 to +17
it("should return a suggestion for common typos", () => {
expect(validateEmail("user@gmal.com").suggestion).toBe("user@gmail.com");
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add test for uppercase email typo suggestion.

Consider adding a test case for emails with uppercase domains to verify the suggestion works correctly (this would catch the case-sensitivity bug in the implementation):

it("should return a suggestion for common typos regardless of case", () => {
  expect(validateEmail("User@GMAL.COM").suggestion).toBe("User@gmail.com");
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/utils/validation.test.js` around lines 15 - 17, Add a new unit test in
client/utils/validation.test.js that calls validateEmail with an address using
uppercase local or domain (e.g., "User@GMAL.COM") to ensure the suggestion logic
is case-insensitive; assert that validateEmail("User@GMAL.COM").suggestion
equals "User@gmail.com" so the local-part case is preserved while the domain is
corrected and lowercased by the validateEmail function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] Weak Email Validation Pattern in Registration and Login Forms

1 participant