New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Require password confirmation when removing users through UI #19586
Conversation
249789d
to
c8e5624
Compare
c8e5624
to
e7707f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested deleting single and multiple users, the password prompt is always shown. Errors are shown when deleting if removing one user fails but the other succeeds. 👍
I noticed the password confirmation dialog 'No' button is missing it's formatting.
(plugins/CorePluginsAdmin/vue/src/PasswordConfirmation.vue
line 37, the no button link is missing the btn-flat
class)
Not caused by this PR but maybe could be fixed here?
There are a couple of UI test that are might need an update, otherwise this seems ready to merge.
- Functional review done
- Code review done
- Can't see any issues with this Security review done
- Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
- Works well and is consistent with the rest of the UI Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
- Wording review done
- Test have been added Tests were added if useful/possible
- Has breaking changesReviewed for breaking changes
- Change log is updated Developer changelog updated if needed
- Not needed Documentation added if needed
- Existing documentation updated if needed
I wasn't sure, if that was done on purpose before. But I'll push a change nevertheless. Guess it looks better if the button is displayed correctly. |
f897ccd
to
179b718
Compare
0f67fe6
to
65c9037
Compare
Description:
As removing users is an action that can't be undone, it's more secure to ask for a password confirmation in the UI.
This will prevent accidentally removing users and will make it e.g. impossible to use an active session of someone else to remove users without knowing his password.
Note: This PR also improves the error handling of bulk requests in AjaxHandler. When e.g. removing multiple users, this is done in a bulk request. If an error occurs (like incorrect password), an error notification was previously created containing each error response of the bulk request. This could e.g. cause that the incorrect password error was show hundreds of times in the error notification. With the changes in this PR, the error messages will be counted and only printed once (together with the count).
Review