-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fixes white-label with auto password #18453
Conversation
add modal background field readonly
$('.modal.open #currentUserPassword').focus(); | ||
$('.modal.open #currentUserPassword').off('keypress').keypress(onEnter); | ||
$('.pluginSettings input').prop("readonly",true); | ||
$('.confirm-password-modal input').prop("readonly",false); | ||
}, | ||
onCloseStart: function () { | ||
$('.pluginSettings input').prop('readonly', false); | ||
}, | ||
}).modal('open'); |
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.
I'm not sure if that will really solve the issue. I guess the readonly
attributes are set to prevent password managers from filling in the field. On the one side, it might actually be too late to set that attribute when opening the modal. The HTML is already placed earlier and a password manager could already fill the field. On the other side I doubt that password managers will all respect the readonly
attribute. There should also be a autocomplete=off
, which is already ignore by certain password managers, so why should they respect other attributes?
I guess the cleanest solution would be to disconnect the password input field from angular and set the passwordConfirmation
property to the input fields value once the modal is closed. That way the modal should always be shown, even if it is already filled.
@sgiehl I did a screen capture on that one, it seems like autocomplete=off didn't prevent the autofill, but the read-only does, for Bitwarden and another third part password. See the screen capture. screen-capture.mp4 |
Yes. It works for my password manager too. But I actually can only repeat my last comment. It works for the tested password managers. But as soon as any password manager is able to set a value for that field it will break again. |
update white label
fix a typo
@sgiehl did a simple approach, hopefully that disconnect the password input |
@peterhashair That wasn't what I had in my mind, also it actually changes the behavior. Before the password confirm was only shown when saving some admin configuration. Now it is also shown for user configuration. What I had in mind, was removing the |
disconnect password
Yep. closing this one in favor of the vue PR. |
Description:
Fixes white-label, when modal open password manager autofill the field in the back, update to read-only fields on modal opens. Revert to normal when modal closed.
https://innocraft.atlassian.net/jira/software/projects/L3/boards/20?label=Core&selectedIssue=L3-161
Review