Hidden password input fields should default to disabled="disabled"

[raamdev]

There have been ​several ​reports of WordPress v4.3 sending the "notice of password change" email every time a user's profile is updated. I'm aware of a ​similar issue where some plugins are improperly calling wp_update_user() with a user_pass field, but what I'm describing in this ticket is separate and unrelated.

Some browsers (reproduced sporadically with Firefox and Chrome) with the "Save Passwords" or "Auto Fill" options enabled are submitting the hidden pass1 input field on the Edit User page, with a password auto-filled from the browser, resulting in the updated user's password being reset whenever the "Update User" button is clicked, even if nothing on the users profile was changed.

Since WordPress v4.3 now hides the password reset input box behind the "Generate Password" button, the hidden password input field should default to having the ​HTML attribute disabled="disabled" so that even if it is automatically filled by the browser behind-the-scenes, it is never POSTd; i.e., if the field is not in view, disable it entirely, to avoid the potential for this to occur.

The JavaScript used to show the password input field whenever you click "Generate Password" in WP v4.3+, could then remove the disabled="disabled" attribute when it is in view, so that you can interact with it and so that the browser will POST when it is in view.

[raamdev]

Hidden password input field should have disabled="disabled" attribute

[adamsilverstein]

@raamdev: Thanks for the bug report.

I can reproduce a similar issue by going to the edit profile screen, then telling LastPass to autofill. I'm not sure how to reproduce with one of the browser password helpers you mentioned.

Can you give the steps to reproduce and/or try adding disabled="disabled" attribute to all the hidden fields, including the generated one, to see if it helps.

I'm not sure disabling the fields will change anything - currently all fields have the parameter autocomplete="off". This is the setting that is supposed to tell LastPass and other password managers "don't autofill this field".

Unfortunately, it looks like LastPass ignores these settings BY DEFAULT. To fix this issue you can set LastPass to honor the autofill parameter:

I tested this and verified it works correctly, preventing LastPass from filling the hidden fields even when I try to manually 'autofill'.

Can you please give it a try and let me know if this resolves your issue?

Recommend wontfix pending feedback from reporter.

[JasWSInc]

Can you give the steps to reproduce and/or try adding disabled="disabled" attribute to all the hidden fields, including the generated one, to see if it helps.

I can confirm that adding disabled="disabled" resolves this in Chrome, Firefox, and Opera. Adding disabled="disabled" seems the way to go! :-) Any field that is not being displayed should be disabled to avoid this conflict. That way any attempt to auto-fill a field (by whatever software; e.g., Chrome, LastPass, etc.) will have no adverse effect. Note that a field that is disabled (even if POSTd) will not be sent to the server. That's exactly what we need in this scenario.

[adamsilverstein]

Ok @JasWSInc thanks for the feedback and confirming this fixes the issue.

I will work up a patch with the disabled attribute applied. Happy to add that if it resolves the issue! I use chrome password manager but never seem to have this issue - i wonder if my settings are different somehow? any special steps to reproduce?

Replying to JasWSInc:

Can you give the steps to reproduce and/or try adding disabled="disabled" attribute to all the hidden fields, including the generated one, to see if it helps.

I can confirm that adding disabled="disabled" resolves this in Chrome, Firefox, and Opera. Adding disabled="disabled" seems the way to go! :-) Any field that is not being displayed should be disabled to avoid this conflict. That way any attempt to auto-fill a field (by whatever software; e.g., Chrome, LastPass, etc.) will have no adverse effect. Note that a field that is disabled (even if POSTd) will not be sent to the server. That's exactly what we need in this scenario.

[adamsilverstein]

In 33699.diff​:

• Disable all hidden input fields on initial load, when the 'Generate Password' button is visible and the fields are hidden.
• Enable all inputs when 'Generate Password' button clicked.
• Disable all inputs when 'Cancel' clicked.

Prevents an issue where password helpers would autofill the hidden password fields, inadvertently causing password changes when editing other fields on the profile page among other issues. This patch keeps the fields 'disabled' when they are hidden which should prevent submission even if they are filled.

@JasWSInc: Can you please check the patch to see if this resolves the issue you see?

Thanks!

[JasWSInc]

@adamsilverstein I applied your patch to trunk and ran a few tests. No problems. Looks like that does the trick!

[JasWSInc]

@adamsilverstein writes...

any special steps to reproduce?

Referencing my notes here also, where I detailed how I was able to reproduce this in the very beginning: ​https://github.com/websharks/s2member/issues/705#issuecomment-137253739

So it's a bit tricky to reproduce. You need to have an installation of WordPress to test with that already has a stored password (in your browser) for the user profile that you're editing. In other words, the browser needs to make an attempt to auto-fill the password field on the user profile editing page.

Even then, when I'm testing this in Chrome with the hidden password field it returns inconsistent results. Approx. 1 in 10 attempts results in Chrome attempting to auto-fill the hidden input field. Anyway, your patch makes that a moot point now. Good!

[raamdev]

I'm not sure disabling the fields will change anything - currently all fields have the parameter autocomplete="off". This is the setting that is supposed to tell LastPass and other password managers "don't autofill this field".

@adamsilverstein Disabling the fields (via the disabled="disabled" attribute) ensures that the browser does not send the field in the POST at all, even if that field somehow contains a value.

I haven't been able to reproduce the issue that I described with any regularity, but what's happening, as @JasWSInc mentioned, is that the browser is sometimes sending the pass1 and/or pass2 field with the POST, even when the field is hidden, and when the browser (or perhaps an extension) does not obey the autocomplete="off" setting and fills in the hidden field with a value this results in a WordPress Password Reset inadvertently occurring when updating the users profile.

Since there's no reason to send the field with the POST when the field is not visible, setting disabled="disabled" is what we want.

I reviewed your patch and it looks good to me. :-) Thank you!

[adamsilverstein]

Super, thanks for clarifying. Appreciate the bug report and testing.

[adamsilverstein]

#34108 was marked as a duplicate.

[adamsilverstein]

Related: #33441

The patch on this ticket resolves the issues raised in #33441.

[ocean90]

#33441 was marked as a duplicate.

[ocean90]

In 35603:

Passwords: Disable hidden input fields on profile/user edit page.

Prevents an issue where password helpers would autofill the hidden password fields and inadvertently causing password changes when editing other fields on the profile page.

Props adamsilverstein.
Fixes #33699.

[flixos90]

On the installation screen the password input field is now disabled since it is initially opened already.

[flixos90]

Fixed JS disabling the password field on installation routine

[flixos90]

The patch above fixes the issue by checking the password field's name attribute ('admin_password' on the installation screen, but different in a user's profile in backend).

[flixos90]

fixed further JS issues during installation / lostpassword form

[flixos90]

The above patch fixes the previous one which did not work properly on the password reset form.

Furthermore the JS files needed during installation are only enqueued in the steps where they're necessary to prevent JS errors.

[adamsilverstein]

@flixos90 Good catch. I think this affects the password reset screen as well where the password is initially shown.

I like your suggestion of breaking out the script if needed, however lets leave that for another ticket.

In 33699.4.diff​ :

• Only disable text password field if it is hidden, corrects a bug where the visible password field would be disabled on the password reset and WordPress install screens since r35603.
• Verified passes JSHint

[flixos90]

@adamsilverstein sounds good. I created the ticket at #34700, including a patch like the fix from above.

[ocean90]

In 35649:

Passwords: Only disable hidden password fields if they are really hidden.

Makes the password field on install and for password resets editable again. Both fields were accidentally set to disabled in [35603].

Props adamsilverstein, flixos90.
Fixes #33699.

[SergeyBiryukov]

[35603] and [35649] broke creating a new user via Add New User screen.

Notice: Undefined index: user_pass in wp-includes/user-functions.php on line 1295

The password is no longer passed to edit_user() and wp_insert_user().

[ocean90]

#34750 was marked as a duplicate.

[wonderboymusic]

your journey

[adamsilverstein]

33699.6.diff​ resolves the underlying issue where we were inadvertently submitting the fields disabled and their value was never passed to PHP. This patch ensures the fields get enabled before the submission occurs. I verified that I no longer get a PHP notice when adding a new user.

Once possible issue would be that if validation fails (for example, omitting the email address), the fields would be enabled again. I don't think the password managers would then attempt to autofill these fields, because I think they only check for password fields on page load; however I am unable to reproduce the original issue consistently.

@raamdev or @JasWSInc can you check @ocean90's patch to see if you see any of the original issue recurring after applying this patch: if possible, please test the edit profile screen and try submitting the form with a validation error (for example, change the email to something thats not an email). submit, get the error, correct and resubmit; then verify the password wasn’t inadvertently changed.

Thanks!

[ocean90]

Anyone using a password manager, testing highly appreciated.

[ocean90]

In 35733:

Passwords: Re-enable password fields before submitting the form.

Avoids an PHP undefined notice when creating new users.

Fixes #33699.

[swissspidy]

#34815 was marked as a duplicate.

[swissspidy]

#33992 was marked as a duplicate.