* Hide password when registering or modifying users #5414
Conversation
dosubot
bot
added
the
size:M
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
| @@ -28,6 +28,7 @@ function UserController($scope, $window, $translate, toastr, AppUtil, UserServic | |||
| $scope.changeStatus = changeStatus | |||
| $scope.searchUsers = searchUsers | |||
| $scope.resetSearchUser = resetSearchUser | |||
| $scope.validatePwdMatch = validatePwdMatch | |||
WalkthroughPassword handling in the user management interface was enhanced by adding a password confirmation field, client-side validation for matching passwords, and masking password inputs. Localization files were updated with new entries for these features, and the changelog documents the security-related update. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as user-manage.html
participant Ctrl as UserController.js
User->>UI: Enter password and confirm password
UI->>Ctrl: validatePwdMatch()
Ctrl->>UI: Set pwdNotMatch flag
alt Passwords do not match
UI->>User: Show "Passwords do not match" error
UI-->>Ctrl: Prevent form submission
else Passwords match
User->>UI: Click submit
UI->>Ctrl: createOrUpdateUser()
Ctrl->>Ctrl: validatePwdMatch()
alt pwdNotMatch is true
Ctrl-->>UI: Abort submission
else
Ctrl->>Server: Submit user data
end
end
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
♻️ Duplicate comments (2)
apollo-portal/src/main/resources/static/scripts/controller/UserController.js (2)
31-31: Missing semicolon consistent with codebase style.The missing semicolon follows the same pattern as previous review comments on this codebase.
89-94: Improve password validation logic.The current validation only checks if
passwordexists but doesn't validateconfirmPasswordpresence. This could result in false validation whenconfirmPasswordis undefined or empty.function validatePwdMatch() { $scope.pwdNotMatch = false - if ($scope.user.password && $scope.user.password != $scope.user.confirmPassword) { + if ($scope.user.password && $scope.user.confirmPassword && + $scope.user.password !== $scope.user.confirmPassword) { $scope.pwdNotMatch = true } }Also note the missing semicolons on lines 90 and 92, which is consistent with the existing codebase style issues flagged in previous reviews.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGES.md(1 hunks)apollo-portal/src/main/resources/static/i18n/en.json(1 hunks)apollo-portal/src/main/resources/static/i18n/zh-CN.json(1 hunks)apollo-portal/src/main/resources/static/scripts/controller/UserController.js(3 hunks)apollo-portal/src/main/resources/static/user-manage.html(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (8)
🔇 Additional comments (7)
apollo-portal/src/main/resources/static/i18n/zh-CN.json (1)
625-626: New keys look correct – please double-check for duplicates in the bundleTranslations are accurate and the naming convention (
UserMange.*) stays consistent with the (unfortunate) historic typo.
Before merging, run the usual “duplicate-key” lint to be certain these two keys are not declared elsewhere in the file – the JSON is already >950 lines and easy to overlook.apollo-portal/src/main/resources/static/i18n/en.json (1)
625-626: Strings added correctly, but verify usage spellingThe English strings are fine. Make sure the UI code references the exact keys
UserMange.ConfirmPwd/UserMange.PwdNotMatch(including the original “Mange” typo); otherwise the labels will fall back to the key name.apollo-portal/src/main/resources/static/user-manage.html (4)
134-134: Excellent security improvement!Changing the password input type from "text" to "password" properly masks the password characters, preventing visual exposure during input.
135-138: Good UX for password validation feedback.The conditional error message provides clear feedback when passwords don't match, improving user experience during form completion.
140-147: Well-implemented password confirmation field.The password confirmation field follows consistent form structure and triggers validation on blur event for immediate feedback without being intrusive.
172-172: Proper form submission prevention.The submit button correctly prevents submission when passwords don't match, ensuring data integrity before form submission.
apollo-portal/src/main/resources/static/scripts/controller/UserController.js (1)
115-118: Excellent placement of password validation.The validation is correctly placed before user creation/update, ensuring passwords match before submission and providing proper early return to prevent invalid operations.
CHANGES.md
Outdated
| @@ -16,5 +16,6 @@ Apollo 2.5.0 | |||
| * [Security: Prevent unauthorized access to other users' apps in /apps/by-owner endpoint](https://github.com/apolloconfig/apollo/pull/5396) | |||
| * [Fix: Bump h2database and snakeyaml version](https://github.com/apolloconfig/apollo/pull/5406) | |||
| * [Bugfix: Correct permission target format to appId+env+namespace/cluster](https://github.com/apolloconfig/apollo/pull/5407) | |||
| * [Security: Hide password when registering or modifying users](https://github.com/apolloconfig/apollo/pull/5413) | |||
There was a problem hiding this comment.
Incorrect PR number in changelog entry
The PR referenced in the changelog (#5413) does not match the current PR (#5414).
Fix the link to avoid pointing users at the wrong discussion.
-* [Security: Hide password when registering or modifying users](https://github.com/apolloconfig/apollo/pull/5413)
+* [Security: Hide password when registering or modifying users](https://github.com/apolloconfig/apollo/pull/5414)📝 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.
| * [Security: Hide password when registering or modifying users](https://github.com/apolloconfig/apollo/pull/5413) | |
| * [Security: Hide password when registering or modifying users](https://github.com/apolloconfig/apollo/pull/5414) |
🤖 Prompt for AI Agents
In CHANGES.md at line 19, the PR number in the changelog entry is incorrect; it
references #5413 instead of the correct PR #5414. Update the PR link to use
#5414 so it points to the correct pull request discussion.
What's the purpose of this PR
Passwords displayed in plain text pose security risks
Brief changelog
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean testto make sure this pull request doesn't break anything.CHANGESlog.Summary by CodeRabbit
New Features
Localization
Documentation