Bug 198391 – Add support of zxcvbn password strength checker to bugs.webkit.org website.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Website

[Ling Ho]

Currently bugs.webkit.org check for password complexity based on several simple rules that allows many weak passwords to be used. We would like to make sure our users, especially contributors use strong passwords.

Installing a password strength estimator will better serve the purpose.  zxcvbn is a reputable password strength estimator, and currently also deployed at trac.webkit.org.

[Ling Ho]

Created attachment 370985 [details]
Added option to select zxcvbn password estimator

[[e-mail]]

Created attachment 370994 [details]
Patch

[David Kilzer (:ddkilzer)]

Comment on attachment 370994 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370994&action=review

r- only to update Bugzilla/Install/Requirements.pm to add the Data::Password::zxcvbn module as required.  The rest of the changes are optional.

> Websites/bugs.webkit.org/Bugzilla/Config/Auth.pm:115
> +                'letters_numbers_specialchars', 'zxcvbn' ],

Nit:  I hadn't heard of zxcvbn before, so using something a little more description (like 'zxcvbn_password_checker') would be easier to grok the first time you see it.

Not necessary to land the patch.

> Websites/bugs.webkit.org/Bugzilla/User.pm:34
> +use Data::Password::zxcvbn 'password_strength'; # WEBKIT_CHANGES

I think it's more idiomatic Perl to use qw() here:

use Data::Password::zxcvbn qw(password_strength); # WEBKIT_CHANGES

--

You also need to update Bugzilla/Install/Requirements.pm to list the Data::Password::zxcvbn module as required.

> Websites/bugs.webkit.org/Bugzilla/User.pm:2492
> +    } elsif ($complexity_level eq 'zxcvbn') {

Update 'zxcvbn' if you changed the name above.

[[e-mail]]

Created attachment 371428 [details]
Patch

[[e-mail]]

- Renamed selection "zxcvbn" to "zxcvbn_password_checker".
- Add Data::Password::zxcvbn module to Bugzilla/Install/Requirements.pm.
- Changed "use Data::Password::zxcvbn 'password_strength';" to "use Data::Password::zxcvbn qw(password_strength);"

[David Kilzer (:ddkilzer)]

Comment on attachment 371428 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=371428&action=review

r=me, but please fix the [% error filter html %] issue at minimum.  After that, this is good to go.

> Websites/bugs.webkit.org/Bugzilla/User.pm:2495
> +        return 'Password is weak. ' . $est_strength->{feedback}->{warning} . '. '

Nit:  Format of this message is not like the others; would like something like `password_too_weak_SCORE' where "SCORE" is replaced with the numeric score, then you could change the template to pull out the score (which may not even be needed--if so, just drop "_SCORE" from the return value) and write the text to display to the user there (which would be easier to translate if we cared about this).

You can land the patch without this change, but please consider it.

> Websites/bugs.webkit.org/template/en/default/global/user-error.html.tmpl:1468
> +    [% error %]

This should be:

    [% error filter html %]

[David Kilzer (:ddkilzer)]

Comment on attachment 371428 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=371428&action=review

>> Websites/bugs.webkit.org/template/en/default/global/user-error.html.tmpl:1468
>> +    [% error %]
> 
> This should be:
> 
>     [% error filter html %]

If you make the other change, you can just put the literal text here like this:

    The password is too weak.

[[e-mail]]

Created attachment 371435 [details]
Patch

[[e-mail]]

Created attachment 371436 [details]
Patch

[[e-mail]]

- Added "FILTER html" to [% error %]
- Keeping the way message is displayed because it will relay warning message string generated by the zxcvbn module. Removed the trailing period since the string could be empty.

[David Kilzer (:ddkilzer)]

Comment on attachment 371436 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 371436 [details]
Patch

Clearing flags on attachment: 371436

Committed r246140: <https://trac.webkit.org/changeset/246140>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/51467679>