WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 19 months ago

Last modified 19 months ago

#24633 closed enhancement (fixed)

Allow admins to generate and send new passwords for users

Reported by: mordauk Owned by:
Milestone: 4.3 Priority: normal
Severity: normal Version:
Component: Users Keywords: dev-feedback has-patch
Focuses: javascript, administration Cc:

Description

For sites that have a lot of user accounts (such as community forums), it'd be really nice for admins to be able to manually generate a new password for user accounts and then automatically send that new password via email. The option to flag the user's account so they are encouraged to reset their password (exactly as they are when using an auto-generated password) would be excellent.

The Simple User Password Generator plugin by 10up does this superbly. I'd propose it be implemented exactly like they have done.

Any reason not to do this?

Attachments (33)

24633.patch (4.6 KB) - added by mordauk 4 years ago.
24633.2.patch (4.7 KB) - added by mordauk 3 years ago.
Refreshed to apply cleanly
24633-wip.patch (6.6 KB) - added by mordauk 3 years ago.
24633.3.patch (6.8 KB) - added by Mat Lipe 3 years ago.
Some js improvement over 24633-wip
24633.4.patch (53.0 KB) - added by mordauk 3 years ago.
Removes duplicate p.description element and correctly sets the generated password to both pass1 and pass2
24633.5.patch (6.8 KB) - added by mordauk 3 years ago.
Updated patch to remove accidental inclusion of theme files diff
24633.6.patch (6.9 KB) - added by mordauk 3 years ago.
Don't show password confirm when Generate Password is clicked
24633.7.patch (8.8 KB) - added by michalzuber 3 years ago.
Rewrote 24633.6.patch to be able patch new src, didn't move pass2 input
24633.8.patch (12.4 KB) - added by michalzuber 3 years ago.
Only Generate password without option checkboxes
24633.9.patch (7.2 KB) - added by mordauk 3 years ago.
24633.10.patch (7.3 KB) - added by michalzuber 3 years ago.
Fixed missing show_password.hide() in generate password, checkbox inputs had no value
24633.diff (8.0 KB) - added by adamsilverstein 3 years ago.
use visibility dash icon for password show/hide, first pass
24633.11.patch (8.0 KB) - added by adamsilverstein 3 years ago.
use visibility dash icon for password show/hide, first pass; name try two
24633.12.patch (8.0 KB) - added by adamsilverstein 3 years ago.
better line height for visibility icon
24633.13.patch (8.4 KB) - added by michalzuber 3 years ago.
Cursor pointer for icon, separate table row for repeat password, password show/hide icon visible always to toggle password visibility
24633.14.patch (9.5 KB) - added by adamsilverstein 3 years ago.
clean up visibility icon placement, action, add some inline docs
24633.15.patch (7.4 KB) - added by adamsilverstein 3 years ago.
simplified version
24633.16.patch (7.3 KB) - added by mordauk 3 years ago.
24633.17.patch (7.3 KB) - added by mordauk 3 years ago.
24633.18.patch (7.3 KB) - added by adamsilverstein 2 years ago.
change fields from text to password on blur instead of keyup
24633.19.patch (7.4 KB) - added by mordauk 2 years ago.
24633.20.patch (7.4 KB) - added by mordauk 2 years ago.
24633.21.patch (7.4 KB) - added by DrewAPicture 2 years ago.
Keepin' it fresh
24633.22.patch (5.8 KB) - added by mordauk 2 years ago.
24633.23.patch (7.8 KB) - added by mordauk 2 years ago.
24633.24.patch (8.2 KB) - added by mordauk 2 years ago.
24633.25.patch (11.1 KB) - added by adamsilverstein 2 years ago.
updates!
24633.26.patch (10.2 KB) - added by adamsilverstein 2 years ago.
24633.27.patch (10.4 KB) - added by mordauk 2 years ago.
24633.28.patch (9.8 KB) - added by mordauk 2 years ago.
24633.29.patch (9.8 KB) - added by mordauk 2 years ago.
24633.30.patch (9.8 KB) - added by DrewAPicture 2 years ago.
wp-password-universal-overlay-i1.png (146.8 KB) - added by folletto 2 years ago.
Password Updage: "Universal Overlay" concept i1

Download all attachments as: .zip

Change History (140)

#1 @SergeyBiryukov
4 years ago

  • Component changed from Validation to Users

Related: #8296, #12461.

#2 @DrewAPicture
4 years ago

  • Cc xoodrew@… added

+1 for integrating the plugin functionality in core.

#3 @aaroncampbell
4 years ago

  • Cc aaroncampbell added

#4 @nacin
4 years ago

  • Milestone changed from Awaiting Review to 3.7
  • Type changed from enhancement to task (blessed)

#5 @mordauk
4 years ago

24633.patch is a quick, functional pull from the plugin.

Some thoughts:

  1. Should a non-admin user that is editing their profile be able to see the Send password options?
  1. There should probably be a filter on the email that is sent to the user. Perhaps it should be part of a separate function?

#6 @mordauk
4 years ago

  • Keywords has-patch added

#7 @jakemgold
4 years ago

Patch needs a few tweaks, including 'hide-if-no-js' for the generate button. I'll carve out some time to modify.

#8 @jakemgold
4 years ago

  • Cc jake@… added

#9 @mordauk
4 years ago

Updated patch attached for hide-if-no-js.

#10 follow-up: @jakemgold
4 years ago

While you're making tweaks:

  • user-profile.js line 40 has two line ends (semicolons).
  • Any reason for the inline style for the button?
  • Dumb question: do we have to update the translation files? My past core patches never dealt with new language strings.

@mordauk
4 years ago

#11 in reply to: ↑ 10 @DrewAPicture
4 years ago

Replying to jakemgold:

  • Dumb question: do we have to update the translation files? My past core patches never dealt with new language strings.

Nope, just localize the strings sans textdomain and they'll be added to the pot via some kind of magical process.

#12 @mordauk
4 years ago

Corrected, thanks for the catch.

The inline styles were left over from your plugin, not sure why they were added there. I just missed removing it during the copy over.

Last edited 4 years ago by mordauk (previous) (diff)

#13 @wedi
4 years ago

#8296 was marked as a duplicate.

#14 @SergeyBiryukov
4 years ago

#12461 was marked as a duplicate.

#15 @adamsilverstein
4 years ago

This is a great addition!

Also seems like this would be a useful addition to the password reset screen, as suggested in IRC. In that case the password would need to be visible so the user can record it since it won't be emailed.

This probably deserves a separate ticket: Plaintext email is inherently insecure, so when passwords *are* emailed, we should force users to reset their password on the next login.

#16 @adamsilverstein
4 years ago

  • Cc adamsilverstein@… added

#17 @toscho
4 years ago

  • Cc info@… added

#18 @jeremyfelt
4 years ago

Tested patch. Everything appears to be in order.

On a side note, the text Type your new password again on the edit user screen reads weird as an admin. Not really related to this ticket, but noted.

#19 @jeremyfelt
4 years ago

  • Milestone changed from 3.7 to 3.8

Moving to 3.8. Per conversation with @nacin, it's pretty late in the cycle to push this in.

@mordauk
3 years ago

Refreshed to apply cleanly

#20 @mordauk
3 years ago

After some discussion with Nacin and Jaquith, the password generation should probably be added to all screens where a password can be set (aka, install, prodit, user edit, etc).

I'm working on getting the flow right for profile and will then work on apply it to all other locations.

Currently, my proposed flow looks like this:

On load:
http://i.imgur.com/XBWFUmF.png

On click of Show Password:
http://i.imgur.com/bcn1i8l.png

If editing a user (instead of changing your own password), the additional options for sending the password via email and encouraging the user to reset their password are shown:

http://i.imgur.com/bmFYtC9.png

Last edited 3 years ago by mordauk (previous) (diff)

#21 @mordauk
3 years ago

Notes for potential new behavior:

  • show password as plaintext when generating your own password
  • hide password when generating one for another user
  • only ever show repeat password input if password manually edited
  • hide password when manually entering

@mordauk
3 years ago

#22 @mordauk
3 years ago

24633-wip.patch is a (mostly) working implementation but it is still rough. It's just meant to show how it could behave and is in no way finished (the JS could be greatly improved).

#23 @matt
3 years ago

  • Milestone changed from 3.8 to Future Release

Moving off 3.8 due to lack of review and activity, sorry about that.

#24 @Mat Lipe
3 years ago

  • Cc Mat Lipe added

@Mat Lipe
3 years ago

Some js improvement over 24633-wip

#25 @Mat Lipe
3 years ago

  • Keywords needs-testing added

@mordauk
3 years ago

Removes duplicate p.description element and correctly sets the generated password to both pass1 and pass2

#26 @mordauk
3 years ago

Any chance of getting attention on this for 3.9?

#27 @toddlahman
3 years ago

Will there be a a global option to apply the two options?:

Send this password to the user by email.
Encourage the user to change their password, once logged in.

A global set of options would prevent having to check the boxes for each and every user, and instead allow this to be set as a global policy if desired.

A required password length, and format requiring such items as an uppercase letter and a number, etc., should also be available as a global option.

Perhaps a new submenu under Settings could be added, like Security, where these options would be available, and where plugins could add more options that could be applied to other areas like the new WP-API from Ryan McCue.

#28 follow-up: @mordauk
3 years ago

I don't see a real need to provide UI options for those. Allowing them to be changed via plugins should be sufficient.

#29 in reply to: ↑ 28 @Mat Lipe
3 years ago

Replying to mordauk:

I don't see a real need to provide UI options for those. Allowing them to be changed via plugins should be sufficient.

If this is needed it should be a separate ticket.

@mordauk
3 years ago

Updated patch to remove accidental inclusion of theme files diff

@mordauk
3 years ago

Don't show password confirm when Generate Password is clicked

#30 @mordauk
3 years ago

24633.6.patch prevents the Repeat New Password field from showing when the Generate Password button is clicked.

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


3 years ago

#32 @PubDirLtd
3 years ago

There is a need for a global settings panel in multisite networks and also, where sites require greater security, an additional option to force users to change their password, once they've logged in for the first time.

#33 @DrewAPicture
3 years ago

Just making a note that it would be nice to handle #27192 if/when then gets merged.

#34 @DrewAPicture
3 years ago

  • Focuses administration added
  • Milestone changed from Future Release to 4.0

Looks like we're going to try to make some progress on this during BeachPress.

#35 follow-up: @knutsp
3 years ago

I think passwords should not be sent via email at all. Send a link to the password reset form, as when the lost password form is used.

At least passwords should no be sent from sites with a secure admin (https).

If WordPress has sent a password via email there should be a nag, at least, as when the initial password is not changed yet. This is not very user friendly since the user must use two passwords, first the generated one and then the changed one.

And a nag that is just ignored for long time doesn't make the password invalid.

Emails can be intercepted, as can http. But emails are usually stored for years, and if they are exposed by accident, an old password may still be valid. One may argue that if an email client is exposed or an account is hacked, then a wrong person may change the password. Such change may be detected by the owner and legitimately changed. But a leaked, working password is worse, since no one might even get suspicious.

But enhancement proposed in this ticket will make WordPress a little bit easy to use, especially for the admin, but far from more secure, and a bit less user friendly for the non-admin (being nagged instead of changing the password once and securely).

The existence of the nag itself indicates that sending passwords via email is not regarded a secure practice.

#36 in reply to: ↑ 35 @ryanduff
3 years ago

Replying to knutsp:

I think passwords should not be sent via email at all. Send a link to the password reset form, as when the lost password form is used.

At least passwords should no be sent from sites with a secure admin (https).

If WordPress has sent a password via email there should be a nag, at least, as when the initial password is not changed yet. This is not very user friendly since the user must use two passwords, first the generated one and then the changed one.

And a nag that is just ignored for long time doesn't make the password invalid.

Was talking about this today at BeachPress and just bouncing ideas. One thing that sounded better from a security standpoint was to flag the account and force a reset.

Most of the plugins, and the manual way to do it is just reset the password for the user but not send via email. That way they need to request a password reset so they get the link to reset it themselves. Downside is that in theory they can just reset their new password to the same as the old password.

If we added a user meta flag that their account was locked or due for a password reset, we could compare the hash of the new password to what's currently in the database and force them to set a different password.

Probably candidate for discussion in a new ticket rather than here.

#37 @mordauk
3 years ago

I do like the idea of having a way to truly force a user to reset a password.

@michalzuber
3 years ago

Rewrote 24633.6.patch to be able patch new src, didn't move pass2 input

@michalzuber
3 years ago

Only Generate password without option checkboxes

#38 @mordauk
3 years ago

There is some incorrect (at least in relation to previous patches) behavior with 24633.8.patch.

The second password input should not be visible and "Show Password" should not be visible until a password is generated: https://cloudup.com/ci7dJ0jUENz

Generating a password causes the visibility status to toggle, making the password visible in plaintext: https://cloudup.com/cRhsMMQ8PMD - This shouldn't happen until "Show Password" is clicked.

#39 @mordauk
3 years ago

I think having the checkboxes to encourage a user change the password and to send it to them via email is important.

@mordauk
3 years ago

#40 @mordauk
3 years ago

24633.9.patch is a refreshed patch that updates the diff for /src/ and fixes a few issues in the javascript.

@michalzuber
3 years ago

Fixed missing show_password.hide() in generate password, checkbox inputs had no value

#41 @michalzuber
3 years ago

I would replace the Show/Hide Password anchor with an icon in the password input.
http://i.imgur.com/qQxDDrm.png

#42 @mordauk
3 years ago

I do like the eye icon.

#43 @adamsilverstein
3 years ago

I also like the eye icon, however i'm wondering about compatibility with password helper plugins - specifically, I use lastpass and it puts a background image into password fields in just the spot you indicated for the 'eye' icon:

http://f.cl.ly/items/0Z3k0E3v2K2i3C020l3k/Profile%20%E2%80%B9%20sadasd%20%E2%80%94%20WordPress%202014-07-15%2008-25-32%202014-07-15%2008-25-41.jpg

How about placing the eye icon just to the right of the form field? we have nice dashicon we can use (visibility):

http://f.cl.ly/items/2E1t460T0Y2Q1g2e1v1g/Profile%20%E2%80%B9%20sadasd%20%E2%80%94%20WordPress%202014-07-15%2008-32-37%202014-07-15%2008-32-44.jpg

#44 @mordauk
3 years ago

Putting the icon to the right of the field would probably be better.

@adamsilverstein
3 years ago

use visibility dash icon for password show/hide, first pass

@adamsilverstein
3 years ago

use visibility dash icon for password show/hide, first pass; name try two

#45 @adamsilverstein
3 years ago

In 24633.11.patch - use dashicon for show/hide password instead of text strings; first pass, I attempted to add some color indication of the status; (black/gray); placed icon outside field to avoid conflicts with lastpass.

@adamsilverstein
3 years ago

better line height for visibility icon

@michalzuber
3 years ago

Cursor pointer for icon, separate table row for repeat password, password show/hide icon visible always to toggle password visibility

#46 @michalzuber
3 years ago

Nice job adamsilverstein, thank you.
I made some tweaks:

  • cursor: pointer; for icon hover
  • separate row for repeat password
  • show icon always to toggle password visibility
  • added padding-left for .indicator-hint
  • inputs should have class="regular-text" to match with others in the form

Visual comparison of 24633.13.patch and 24633.12.patch @ http://i.imgur.com/VSUZuE1.png

#47 @DrewAPicture
3 years ago

Hmm, I definitely like the left one (24633.13.patch?) better, though the "eye" icon is riding a little high.

#48 follow-up: @mordauk
3 years ago

I prefer the large input fields as well, but I'm not really in love with the eye icon in between the input field and the button. It just looks a bit off.

It makes sense for the icon to be as close to the input as possible, but it's throwing it off. Nothing immediately comes to mind with how to improve it; thoughts?

#49 in reply to: ↑ 48 ; follow-up: @adamsilverstein
3 years ago

adding some space to the right of the icon might help connect it more to the input on its left. i also liked the original idea of putting the eye inside the field (pretty straightforward with css) - but found it directly conflicted with the icon (and click handler) lastpass placed there - see cl.ly/image/2U0D2H2A390b. we should be able to tell lastpass to skip these fields by setting autocomplete="off" for the form (see http://stackoverflow.com/questions/20954944/stop-lastpass-filling-out-a-form). you could still use lastpass, just not from the form fields.

Replying to mordauk:

I prefer the large input fields as well, but I'm not really in love with the eye icon in between the input field and the button. It just looks a bit off.

It makes sense for the icon to be as close to the input as possible, but it's throwing it off. Nothing immediately comes to mind with how to improve it; thoughts?

#50 in reply to: ↑ 49 ; follow-up: @aaroncampbell
3 years ago

Replying to adamsilverstein:

we should be able to tell lastpass to skip these fields by setting autocomplete="off" for the form

I don't love that idea. As a LastPass user, I definitely prefer to generate passwords using LastPass rather than WordPress, partially because I can share it to another LastPass user without using E-Mail, and partially because I can control how long and complex it is.

#51 in reply to: ↑ 50 ; follow-up: @adamsilverstein
3 years ago

Replying to aaroncampbell:

Replying to adamsilverstein:

we should be able to tell lastpass to skip these fields by setting autocomplete="off" for the form

I don't love that idea. As a LastPass user, I definitely prefer to generate passwords using LastPass rather than WordPress, partially because I can share it to another LastPass user without using E-Mail, and partially because I can control how long and complex it is.

I hear ya! I'm also a lastpass fan and would rather not disable it - although I would note that you can still use if with the autofill turned off on these fields, you would have to use the extension/add on icon to activate the generate password feature.

what about if lastpass was active, putting the eye/visibility icon a bit to the left or right of the lastpass icon (meaning you would have two icons in the field - is that too weird? or put it outside the field if lastpass active, inside otherwise?

#52 in reply to: ↑ 51 ; follow-up: @mordauk
3 years ago

Replying to adamsilverstein:

Replying to aaroncampbell:

Replying to adamsilverstein:

we should be able to tell lastpass to skip these fields by setting autocomplete="off" for the form

I don't love that idea. As a LastPass user, I definitely prefer to generate passwords using LastPass rather than WordPress, partially because I can share it to another LastPass user without using E-Mail, and partially because I can control how long and complex it is.

I hear ya! I'm also a lastpass fan and would rather not disable it - although I would note that you can still use if with the autofill turned off on these fields, you would have to use the extension/add on icon to activate the generate password feature.

what about if lastpass was active, putting the eye/visibility icon a bit to the left or right of the lastpass icon (meaning you would have two icons in the field - is that too weird? or put it outside the field if lastpass active, inside otherwise?

Putting in a specific rule for lastpass seems like a slippery slope to me. What about when a new password management app comes along and it also adds an icon? And then another? I'd rather see a generic setup that can work with any app.

#53 in reply to: ↑ 52 ; follow-up: @DrewAPicture
3 years ago

Replying to mordauk:

Putting in a specific rule for lastpass seems like a slippery slope to me. What about when a new password management app comes along and it also adds an icon? And then another? I'd rather see a generic setup that can work with any app.

+1 for a generic approach. Obviously it would nice to work nicely with LastPass, but we shouldn't specifically tailor it for LastPass compatibility.

#54 in reply to: ↑ 53 @adamsilverstein
3 years ago

Replying to DrewAPicture:

Replying to mordauk:

Putting in a specific rule for lastpass seems like a slippery slope to me. What about when a new password management app comes along and it also adds an icon? And then another? I'd rather see a generic setup that can work with any app.

+1 for a generic approach. Obviously it would nice to work nicely with LastPass, but we shouldn't specifically tailor it for LastPass compatibility.

Ok, since lastpass already has this spot, lets stick with the 'right of the field' position and add some more space to the right of the icon to better tie the icon to the field.

@adamsilverstein
3 years ago

clean up visibility icon placement, action, add some inline docs

#55 @adamsilverstein
3 years ago

In 24633.14.patch:

  • refresh of .13 against trunk
  • better placement of visibility icon in chrome & firefox, beter vertical centering, more space to right before generate password button
  • added some inline docs
  • code cleanup
  • tighter spacing between password and repeat password fields

looks like this:

http://f.cl.ly/items/0R1B3C223N0K2D3a3c40/Profile%20%E2%80%B9%20sadasd%20%E2%80%94%20WordPress%202014-07-16%2022-57-49%202014-07-16%2022-57-51.jpg

#56 follow-up: @DrewAPicture
3 years ago

I've been thinking about this quite a bit the last couple of days, and I feel like we can make this a little more automagical by forgoing the "eye" icon altogether.

The current flow is that the fields are both masked. So I suggest only revealing the passwords if you're using the password generator.

And on a side note, because we're talking about it ... LastPass/1Password/other has separate functionality to reveal it to you/store it, whatever anyway.

#57 in reply to: ↑ 56 ; follow-up: @michalzuber
3 years ago

Replying to DrewAPicture:

The current flow is that the fields are both masked. So I suggest only revealing the passwords if you're using the password generator.

I think it's usable in both scenarios, if I'm typing a hard password I would like to switch to see and don't need to retype that passwords to match.

It looks near perfect, but in the JS console the following appears:
JQMIGRATE: Can't change the 'type' of an input or button in IE 6/7/8

What about IE 6/7/8 support, do we need to fix this? I'm on a Mac, somebody on Win could try it with IETester ?

#58 in reply to: ↑ 57 @DrewAPicture
3 years ago

Replying to michalzuber:

It looks near perfect, but in the JS console the following appears:
JQMIGRATE: Can't change the 'type' of an input or button in IE 6/7/8

What about IE 6/7/8 support, do we need to fix this? I'm on a Mac, somebody on Win could try it with IETester ?

I think it looks cluttered, and I'm not really sure why the icon is by itself like that. The padding on the right looks odd. And where else in the admin do we have a standalone icon that's clickable. I feel if we're going to keep it, we should just put in a button and remove the padding to right of it.

Should be:
IE6: No
IE7: Not horribly broken
IE8: Do the best we can

#59 @mordauk
3 years ago

I'm with @DrewAPicture and forgoing the icon all together. I loved the idea initially but now seeing it, I can't picture a way that it really works.

#60 follow-up: @DrewAPicture
3 years ago

  • Milestone changed from 4.0 to Future Release

Annnnnnnd this is coming together too late for 4.0 -- we're almost in beta 2, afterall.

And yes, I realize the frustration punting this will cause. It's been marked as a task since 3.7 but we still don't have the UI nailed down, and this should really get some feedback from the UI team and/or user testing before any hard decisions are made.

Let's keep working on this and see if we can't get it in early in 4.1 :/

#61 in reply to: ↑ 60 ; follow-up: @mordauk
3 years ago

Replying to DrewAPicture:

Annnnnnnd this is coming together too late for 4.0 -- we're almost in beta 2, afterall.

And yes, I realize the frustration punting this will cause. It's been marked as a task since 3.7 but we still don't have the UI nailed down, and this should really get some feedback from the UI team and/or user testing before any hard decisions are made.

Let's keep working on this and see if we can't get it in early in 4.1 :/

Could it be milestoned for 4.1 at least to help encourage people to look at it?

#62 in reply to: ↑ 61 @DrewAPicture
3 years ago

  • Keywords 4.1-early added
  • Type changed from task (blessed) to enhancement

Replying to mordauk:

Could it be milestoned for 4.1 at least to help encourage people to look at it?

The 4.1 milestone isn't in the list yet, but I'll mark it 4.1-early and re-milestone on the other side.

@adamsilverstein
3 years ago

simplified version

#63 @adamsilverstein
3 years ago

  • Focuses javascript added

24633.15.patch

  • Simplified workflow as suggested by @drewapicture, drops the eye icon - password is visible right after generation, and only then
  • selects password after generation to simplify copying
  • increase generated password length to 30
  • any change in the field shows the second match field and turns them both back to password fields
  • passes jshint :)
  • note: field type change only works in ie >8

#64 @DrewAPicture
3 years ago

  • Keywords needs-refresh added

Patch needs to be refreshed for /wp-admin/user-edit.php

@mordauk
3 years ago

#65 @mordauk
3 years ago

24633.16.patch is a refreshed patch that applies cleanly!

#66 @mordauk
3 years ago

  • Keywords needs-refresh removed

#67 @DrewAPicture
3 years ago

Looks like there's a little weirdness with the interaction after generating a password with 24633.16.patch.

I ran through four common scenarios (imo), and my general conclusion is that we should at the very least always show both fields and the strength meter whenever the first field is interacted with, whether that be by clicking the button, pasting something in, typing something in, or clicking the generate button after some text has already been entered in one or both of the fields. Or just show all three elements all the time.

Here are the results of the four scenarios:

1. Clicking the Generate Password button:

  • The password is generated and sent to the password field
  • The password is visible
  • The second password field and strength meter don't appear
  • If you select or otherwise interact with the generated password, the second password field and strength meter suddenly show up and both fields mask the password

2. Pasting a password in to the single-visible password field:

  • The password pasted into the first password field is masked
  • The second password field (empty) and strength meter appear

3. Typing a password in to the single-visible password field:

  • The password when entered into the first password field is masked
  • The second password field (empty) and strength meter appear

4. Clicking the Generate Password button when the password fields already contain something:

  • The second password field and strength meter disappear
  • Interacting in some way with the text in the first field then causes the second password field and strength meter to appear, with both password fields filled and masked

@mordauk
3 years ago

#68 @mordauk
3 years ago

I agree with @DrewAPicture

24633.17.patch is an updated patch that makes the fields visible at all times.

It still feels a little "off" to me so I'd love to hear some feedback on this altered version. The main issue I see still is that if you generate a password, it's shown in plaintext but then gets masked if you click into the field and type CMD/CTL+C to copy it. I feel it should stay as plain text if copying it to the clipboard.

@adamsilverstein
2 years ago

change fields from text to password on blur instead of keyup

#69 @adamsilverstein
2 years ago

  • Keywords dev-feedback added

24633.18.patch:

  • Change from 24633.17.patch - change passwords fields back from text to password on blur instead of keyup. Fixes an issue in firefox where hitting ctrl/cmd-c to copy also changed the field back to password (masked)

@mordauk
2 years ago

#70 @mordauk
2 years ago

I really like 24633.18.patch though it doesn't apply cleanly any more, so 24633.19.patch is a refreshed patch.

#71 @DrewAPicture
2 years ago

  • Keywords needs-refresh 4.2-early added; 4.1-early removed

Sorry, I meant to test the latest developments at the summit and I never got around to it. Looks like 24633.19.patch needs a refresh for admin-ajax.php.

@mordauk
2 years ago

#72 @mordauk
2 years ago

  • Keywords needs-refresh removed

Patch refreshed.

@DrewAPicture
2 years ago

Keepin' it fresh

#73 @iseulde
2 years ago

  • Milestone changed from Future Release to 4.2

has-patch 4.2-early so moving to 4.2.

This ticket was mentioned in Slack in #core by drew. View the logs.


2 years ago

#75 @limecanvas
2 years ago

How about logging the password change datetime and possibly password strength (and other data over time) so that if somebody wanted to implement a password changing policy, site-wide, they would have that information to hand. i.e. notify users who haven't changed password in 120 days etc.

Last edited 2 years ago by limecanvas (previous) (diff)

#76 @DrewAPicture
2 years ago

I had a chance to chat with Pippin (@mordauk) about this a little bit last week and I think we both agree that this task may be best served by approaching the password generation functionality as something that should be developed in a standalone way.

The thinking is that if we abstract out the password generation functionality, we can design it in such a way that it could easily be reused in several different areas of core. For the sake of completeness, I envision those areas would include:

  • user-edit – Generating passwords when editing a user profile
  • user-new – Generating passwords when creating a new user
  • password reset – Generating passwords when resetting a password
  • installation – Generating passwords when creating the initial account on installation

As for the interaction stuff related to sending passwords to a user and/or recommending they change passwords on first login, I think those should be handled separately. If we really want to have a win here, we need something stable and reusable.

To that end, I'm not sure that necessarily supporting things like Lastpass and all the UI things that come along with it would be necessary to get to a minimum viable product. We'd also probably want to consider whether the password meter UI would be useful in these other contexts as well, but that's probably also a separate ticket.

Next Steps

I think the next step here would be to take the latest patch and break out the generation code to a point where you could simply point it at a group of selectors, e.g. button, first password, second password, and it would just work.

On top of that, we need to be cognizant of the mobile and accessibility implications that come along with the introduction of any kind of new core UI, so getting a consult from those teams would be a good step too.

There's some work yet to do here, but I think we could really get a great feature out of this.

#77 @DrewAPicture
2 years ago

  • Keywords needs-patch added; has-patch needs-testing 4.2-early removed

This ticket was mentioned in Slack in #core by drew. View the logs.


2 years ago

@mordauk
2 years ago

#79 @mordauk
2 years ago

I'm starting to work on this now. To start, I've gone ahead and refreshed the latest patch so it applies cleanly.

@mordauk
2 years ago

#80 follow-up: @mordauk
2 years ago

24633.23.patch is my first pass at creating an abstracted password generation API that will make it much simpler to add password generation to profiles, user registration, user creation, etc.

To set up password generation, we can call the new passwords object like so:

passwords.init( '#pass1', '#pass2', '#generate-password' );

The first password, confirmation, and submit field selectors are passed as parameters to the function, which will then handle generating the password, checking the strength, and displaying the results.

Would anyone like to lend their opinions of this initial approach?

#81 in reply to: ↑ 80 ; follow-up: @adamsilverstein
2 years ago

Thanks! This looks really good, I love the idea of abstracting the functionality so we can use it everywhere we create passwords in core.

Wondering if should namespace into wp global to make object wp.passwords? Also, maybe move to wp-util instead of user-profile.js? Will plugins/themes be able to leverage the password generator?

I will review in more detail, test and see about extending to other password fields to see how straightforward that is.

Thanks for keeping this ticket alive :)

Replying to mordauk:

24633.23.patch is my first pass at creating an abstracted password generation API that will make it much simpler to add password generation to profiles, user registration, user creation, etc.

To set up password generation, we can call the new passwords object like so:

passwords.init( '#pass1', '#pass2', '#generate-password' );

The first password, confirmation, and submit field selectors are passed as parameters to the function, which will then handle generating the password, checking the strength, and displaying the results.

Would anyone like to lend their opinions of this initial approach?

#82 @mordauk
2 years ago

I wasn't completely sure if the profile JS is loaded globally so left it there for the time being, but I do like the idea of namespacing it. I'll move it over now.

@mordauk
2 years ago

#83 @mordauk
2 years ago

24633.24.patch moves passwords object to wp-utils.js and renames it to wp.passwords.

#84 in reply to: ↑ 81 ; follow-up: @azaozz
2 years ago

Replying to adamsilverstein:

Wondering if should namespace into wp global to make object wp.passwords? Also, maybe move to wp-util instead of user-profile.js? Will plugins/themes be able to leverage the password generator?

Namespacing it sounds good. However moving it to wp-util.js doesn't seem right. Currently user-profile.js requires password-strength-meter.js which loads zxcvbn.min.js which is a 683KB minified JS file. There is no way we are going to load that "globally" :)

Perhaps better to add the password checking/generating functions to wp.passwordStrength in password-strength-meter.js? Then we can enqueue that file everywhere a password can be typed. I know the names aren't that good. We could probably rename the file, but should keep the script-loader "handle" name for back-compat.

Last edited 2 years ago by azaozz (previous) (diff)

#85 in reply to: ↑ 84 @mordauk
2 years ago

Replying to azaozz:

Perhaps better to add the password checking/generating functions to wp.passwordStrength in password-strength-meter.js? Then we can enqueue that file everywhere a password can be typed. I know the names aren't that good. We could probably rename the file, but should keep the script-loader "handle" name for back-compat.

I like this idea.

@adamsilverstein
2 years ago

updates!

#86 @adamsilverstein
2 years ago

  • Keywords has-patch added; needs-patch removed

In 24633.26.patch:

  • Move utility code to password-strength-meter.js (haven't renamed file yet to keep the change set cleaner, although I like the idea)
  • Add the 'Generate Password' button to the user-new screen (button needs some css attention, not sure best approach to override style it is inheriting from the form inputs)
  • changed name to wp.passwordGenerator
Last edited 2 years ago by adamsilverstein (previous) (diff)

@mordauk
2 years ago

#87 @mordauk
2 years ago

24633.27.patch refreshes the patch so it applies cleanly. It also sets the width of the #generate-password input on user-new.php to auto. Without setting the width to auto, the submit button gets set to 25em, the same width as the input fields. To be consistent with user-edit.php, the width should be automatic based on the text length.

#88 @ocean90
2 years ago

Some thoughts to 24633.27.patch:

  • The JavaScript needs an overhaul. Why is wp.passwordGenerator used so often? Can't it be this or a shorter variable? Same for wp.passwordGenerator
  • [31483] should be included
  • check_strength: function() { should be checkStrength: function() { (Why does the generator has do this? Can we trigger a .change() event?)
  • $.post() should be replaced with wp.ajax.post()
  • To be consistent, the AJAX action should be generate-password (no underscore)

@mordauk
2 years ago

#89 @mordauk
2 years ago

24633.28.patch addresses suggestions from @ocean90

@mordauk
2 years ago

#90 @mordauk
2 years ago

Oops, missed changes from [31483]. Fixed in 24633.29.patch.

Last edited 2 years ago by SergeyBiryukov (previous) (diff)

This ticket was mentioned in Slack in #core by drew. View the logs.


2 years ago

This ticket was mentioned in Slack in #core by drew. View the logs.


2 years ago

#93 @DrewAPicture
2 years ago

  • Milestone changed from 4.2 to Future Release

Despite the recent surge in activity, this isn't going to make it for 4.2 :(

Maybe we can get it done in 4.3.

This ticket was mentioned in Slack in #core-passwords by drew. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-passwords by rmarks. View the logs.


2 years ago

@folletto
2 years ago

Password Updage: "Universal Overlay" concept i1

#96 follow-up: @folletto
2 years ago

As the above, I've tried to come up with an integrated concept, trying to balance in a single solution all the issues discussed above. Here's attached above the "Universal Overlay" concept:

  1. The UI is designed as a modular component: abstract from its context, but with a visual design that fits in every (hopefully) WordPress context. In the above you see it used both in wp-admin and in the login page (note that the width stretches to match the text-field, so it's designed to fit the smaller of the two).
  2. The field starts blank, so as a user you don't have to read information that you need only when you start typing. The idea is to have the clean field, that expands with the overlay and the password hint once it gets focused. This introduces some moving of the fields around, which is not ideal, but I feel the benefits outbalance that slight movement.
  3. By default we use "Visible", so there's just one field. Better to avoid mis-typing frustration, and avoids using a negative-for-positive meaning UI ("check to hide" vs "check to show").
  4. If "Visible" gets unchecked we animate the second verification line in (fold-open animation), empty.
  5. Visible and Generate go hand by hand: clicking generate turns on automatically the visibility, and un-checking then the visibility checkbox shows the verification field empty, so you have to type the generated password to make it validate.
  6. If JavaScript is off we still get a single field that works (with a visible password), so it's still functional.
  7. The strength indicator uses the padlock icon to reinforce the security theme (I know, this sounds silly, but there are studies showing that the icon of a padlock helps perceiving a form more secure).

That's all I came up with. I feel it's a good direction, even if maybe there are some details that need more fine-tuning. :)

I hope this helps. :)

This ticket was mentioned in Slack in #core by folletto. View the logs.


23 months ago

#98 in reply to: ↑ 96 @GaryJ
23 months ago

Replying to folletto:

maybe there are some details that need more fine-tuning. :)

Will the various types of colour blindness be able to distinguish between the green padlocks and the grey padlock?

The field starts blank, so as a user you don't have to read information that you need only when you start typing.

The UX then means a user has to focus in the field just to get the Generate button appear before they can click it, and that's one step too many and more confusing for non-visual users. Could the Generate button always be shown, and only have the password strength indicator be shown when the field is not empty, not even just on focus, as per the previous screenshots in this ticket? They are two separate tasks - generation of a password, and analysis of a password.

As a general side note for some of the other screenshots - screen reader users should probably be given some clue that the Generate button exists, before or as they focus on the password field.

#99 follow-up: @folletto
23 months ago

Will the various types of colour blindness be able to distinguish between the green padlocks and the grey padlock?

Sure. A color combination that is distinguishable is doable.
The text will still give that information regardless. :)

The UX then means a user has to focus in the field just to get the Generate button appear before they can click it, and that's one step too many and more confusing for non-visual users.

There should be no issue there: the form could easily pop up on tabbing as any normal hidden field like the "Skip to content" field do, thus being accessible, and would be an experience functionally identical to someone clicking on the text-field (the reader would read "Change Password" and then access the fields in sequence). But I'm sure there are other techniques that could be used too.

Can you expand on your perspective behind this?

#100 in reply to: ↑ 99 @GaryJ
23 months ago

Replying to folletto:

Can you expand on your perspective behind this?

There are really three issues here, which I didn't make clear.

  1. Having to click into the text field, before the Generate button becomes available. That's like having to put the car key in the lock before one can open it remotely with the fob. The initial step is redundant. Make the Generate button always be available, not just on text field focus.
  1. Having the Generate button after the field means a screen reader may have an experience of:
  • Read the Change Password label.
  • Navigate to the corresponding field and focus it.
  • Manually enter a made-up password.
  • Navigate to the next item (which may be the Generate button).
  • Realise that the Generate button exists, and that they didn't need to make up a password manually.

Having the button before (in the source) and/or use of aria-* attributes to describe the password field to indicate that the button exists, should help mitigate that.

  1. Likewise, having the strength indicator before just seems odd to me. It's appearance is in response to the password field being empty, so why have it visually appear before the field? Imagine if the comment preview here in Trac appeared above the Add Comment field. From an accessibility point of view, aria-live should help screen reader users, so the placement is a little less important for them.

#101 @folletto
23 months ago

Now I understand, thank you. :)

Yes (1) and (3) are choices made in this design suggestion. I feel the overall balance of it works well, but sure, it's just a choice out of many. I don't think there's a clear winner here: you either compromize cleanliness, flow, legibility or overall hierarchy. In this scenario I make explicit choices in one direction, which I feel works best, but yes, it's a choice. :)

Regarding (2), I don't think it's an issue since the DOM could be arranged in a way that makes it properly ordered, having the "Generate" button come first (i.e. having the box before the field, or other technical solutions).

#102 @adamsilverstein
19 months ago

  • Resolution set to fixed
  • Status changed from new to closed

Closing this as I think the new password generation in 4.3 solves the main issue @mordauk opened the ticket for. If that is not the case, please re-open.

#103 @SergeyBiryukov
19 months ago

  • Milestone changed from Future Release to 4.3

#104 follow-up: @Ipstenu
19 months ago

@adamsilverstein - You mean becuase of #34180 ?

The groundwork is there but what that _doesn't_ do is make a button to email someone a password.

it'd be really nice for admins to be able to manually generate a new password for user accounts and then automatically send that new password via email.

I made a sample plugin to add a button like we have for generate new password - https://cloudup.com/cWNmv6T0SXI

I was trying to futz with ajax enough to make it a patch and not a plugin but here's the code: https://cloudup.com/cmnPAmv3Qlq

It doesn't show on YOUR page, only on other users'.

#105 in reply to: ↑ 104 @adamsilverstein
19 months ago

Replying to Ipstenu:

@adamsilverstein - You mean becuase of #34180 ?

Right; because we built on the work started here, I thought this ticket was resolved. I can see how adding the 'send reset link' button would improve what we have here.

Thanks for your code, thats most of the work to get a patch! I am a little hesitant to re-open this ticket, mostly because it has 104 comments already! Would you be willing to create a new ticket to cover this specific feature?

I will happily work on converting your plugin code into a core patch, seems like a good idea! Would make a nice bulk action for the user screen as well for a quick way to reset a bunch of user passwords.

#106 follow-up: @Ipstenu
19 months ago

Sure - https://core.trac.wordpress.org/ticket/34281#ticket

If the section was extendable (like I didn't have to make a new 'mischief' section in the profile) a plugin would work fine for the long run, but being able to just send folks a password reset link will help support a ton.

#107 in reply to: ↑ 106 @adamsilverstein
19 months ago

Replying to Ipstenu:

Sure - https://core.trac.wordpress.org/ticket/34281#ticket

Thank you, I will work on a patch. This seems like a worthy core enhancement.

Note: See TracTickets for help on using tickets.