Fix #387: Prevent password from matching username#1608
Fix #387: Prevent password from matching username#1608mambax7 wants to merge 2 commits intoXOOPS:masterfrom
Conversation
Add a validation check in XoopsUserUtility::validate() that rejects a password identical to the username (case-insensitive), covering both registration and profile edit flows. Add the corresponding language string _US_PWDEQUALSUNAME to htdocs/language/english/user.php. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
WalkthroughAdds a case-insensitive validation that disallows a password equal to the username when a non-empty password is provided; introduces a new language string _US_PWDEQUALSUNAME for the error message. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements a security enhancement to prevent users from setting their password equal to their username during registration and profile editing. The change addresses issue #387 by adding a case-insensitive validation check in the core XoopsUserUtility::validate() method.
Changes:
- Added case-insensitive password-equals-username check in
XoopsUserUtility::validate() - Added English language string
_US_PWDEQUALSUNAMEfor the validation error message
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| htdocs/class/userutility.php | Added validation logic to reject passwords matching username (case-insensitive) |
| htdocs/language/english/user.php | Added error message constant for password-equals-username validation |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@htdocs/class/userutility.php`:
- Around line 223-224: The condition uses strtolower($pass) without guaranteeing
$pass is not null, causing deprecation/errors; update the conditional in the
user utility check (the branch that appends _US_PWDEQUALSUNAME to $stop) to
ensure $pass is not null (e.g., add $pass !== null) or replace the $pass check
with a null/empty-safe check like !empty($pass) before calling strtolower($pass)
so strtolower is never invoked with a null argument.
- userutility.php: add $pass !== null guard before strtolower($pass) to prevent PHP 8.1 deprecation / PHP 9.0 TypeError when $pass is null - edituser.php: add username=password check to the inline validation block that bypasses XoopsUserUtility::validate() (password change during profile edit) - modules/profile/changepass.php: add username=password check to the profile module's dedicated change-password page (also bypassed XoopsUserUtility::validate()) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@htdocs/class/userutility.php`:
- Around line 223-224: Add unit tests exercising the new password≠username rule
in userutility.php: write tests that call the user validation/registration
routine that uses $pass and $uname (the code path where strtolower($pass) ===
strtolower($uname) triggers) and assert that when password equals username
(case-insensitive) the validation fails and returns/contains the
_US_PWDEQUALSUNAME error, and also assert that when password and username differ
validation succeeds; place tests alongside existing registration/profile unit
tests to cover both registration and profile-update flows so this core behavior
is protected from regressions.
In `@htdocs/edituser.php`:
- Around line 64-65: The type cast "(string)" in the conditional using
strtolower((string)$xoopsUser->getVar(...)) violates PSR-12; add a space after
the cast so it reads "(string) $xoopsUser->getVar(...)" in the strtolower call
within the block that sets $errors[] = _US_PWDEQUALSUNAME; ensure the change is
applied to the conditional using strtolower and the getVar invocation.
In `@htdocs/modules/profile/changepass.php`:
- Around line 58-59: The condition in changepass.php uses a type cast without
the PSR-12 required space: replace occurrences like
(string)$GLOBALS['xoopsUser']->getVar('uname', 'n') in the password check (the
elseif that compares $password to
strtolower((string)$GLOBALS['xoopsUser']->getVar(...))) with a space after the
cast so it becomes (string) $GLOBALS['xoopsUser']->getVar(...); ensure the same
spacing rule is applied anywhere else using (string) casts in this file.
| } elseif ($pass !== '' && $pass !== null && $uname !== null && strtolower($pass) === strtolower($uname)) { | ||
| $stop .= _US_PWDEQUALSUNAME . '<br>'; |
There was a problem hiding this comment.
Add unit tests for the new password≠username rule.
This is new core validation behavior and should be covered by tests to prevent regressions in registration/profile flows.
Based on learnings: "New functionality has corresponding unit tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@htdocs/class/userutility.php` around lines 223 - 224, Add unit tests
exercising the new password≠username rule in userutility.php: write tests that
call the user validation/registration routine that uses $pass and $uname (the
code path where strtolower($pass) === strtolower($uname) triggers) and assert
that when password equals username (case-insensitive) the validation fails and
returns/contains the _US_PWDEQUALSUNAME error, and also assert that when
password and username differ validation succeeds; place tests alongside existing
registration/profile unit tests to cover both registration and profile-update
flows so this core behavior is protected from regressions.
| } elseif (strtolower($password) === strtolower((string)$xoopsUser->getVar('uname', 'n'))) { | ||
| $errors[] = _US_PWDEQUALSUNAME; |
There was a problem hiding this comment.
PSR-12: add space after type cast.
Type casts should include a space before the variable.
Suggested fix
- } elseif (strtolower($password) === strtolower((string)$xoopsUser->getVar('uname', 'n'))) {
+ } elseif (strtolower($password) === strtolower((string) $xoopsUser->getVar('uname', 'n'))) {As per coding guidelines: "**/*.php: Follow PSR-12 coding standard".
📝 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.
| } elseif (strtolower($password) === strtolower((string)$xoopsUser->getVar('uname', 'n'))) { | |
| $errors[] = _US_PWDEQUALSUNAME; | |
| } elseif (strtolower($password) === strtolower((string) $xoopsUser->getVar('uname', 'n'))) { | |
| $errors[] = _US_PWDEQUALSUNAME; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@htdocs/edituser.php` around lines 64 - 65, The type cast "(string)" in the
conditional using strtolower((string)$xoopsUser->getVar(...)) violates PSR-12;
add a space after the cast so it reads "(string) $xoopsUser->getVar(...)" in the
strtolower call within the block that sets $errors[] = _US_PWDEQUALSUNAME;
ensure the change is applied to the conditional using strtolower and the getVar
invocation.
| } elseif (!empty($password) && strtolower($password) === strtolower((string)$GLOBALS['xoopsUser']->getVar('uname', 'n'))) { | ||
| $errors[] = _US_PWDEQUALSUNAME; |
There was a problem hiding this comment.
PSR-12: add space after type cast.
Type casts should include a space before the variable.
Suggested fix
- } elseif (!empty($password) && strtolower($password) === strtolower((string)$GLOBALS['xoopsUser']->getVar('uname', 'n'))) {
+ } elseif (!empty($password) && strtolower($password) === strtolower((string) $GLOBALS['xoopsUser']->getVar('uname', 'n'))) {As per coding guidelines: "**/*.php: Follow PSR-12 coding standard".
📝 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.
| } elseif (!empty($password) && strtolower($password) === strtolower((string)$GLOBALS['xoopsUser']->getVar('uname', 'n'))) { | |
| $errors[] = _US_PWDEQUALSUNAME; | |
| } elseif (!empty($password) && strtolower($password) === strtolower((string) $GLOBALS['xoopsUser']->getVar('uname', 'n'))) { | |
| $errors[] = _US_PWDEQUALSUNAME; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@htdocs/modules/profile/changepass.php` around lines 58 - 59, The condition in
changepass.php uses a type cast without the PSR-12 required space: replace
occurrences like (string)$GLOBALS['xoopsUser']->getVar('uname', 'n') in the
password check (the elseif that compares $password to
strtolower((string)$GLOBALS['xoopsUser']->getVar(...))) with a space after the
cast so it becomes (string) $GLOBALS['xoopsUser']->getVar(...); ensure the same
spacing rule is applied anywhere else using (string) casts in this file.
1 participant
Summary
XoopsUserUtility::validate()that rejects a password identical to the usernameregister.php) and profile edit (edituser.php) since both callXoopsUserUtility::validate()_US_PWDEQUALSUNAMEtolanguage/english/user.phpTest plan
Fixes #387
🤖 Generated with Claude Code
Summary by CodeRabbit