Added password strength gauge in registration form.

[yeraydiazdiaz]

Fixes #6

Added vendor JS gulp step to include zxcvbn.js and include it in the register template.
Added password strength gauge in registration form.

[yeraydiazdiaz]

In order to follow the zxcvbn recommendations I made some changes on the frontend pipeline, specifically creating a vendor subdirectory in warehouse/static/js, adding a gulp step to simply copy them to a js/vendor subdirectory in dist and finally ignoring it in the linting. Seemed like the best approach to achieve the recommendations although we're now adding a specific version of zxcvbn.js on the repo that will need to be manually updated, I couldn't think of a better approach so suggestions are welcome.

The UI is of course up for discussion and there's no validation step, it's simply a graphical representation of the result of zxcvbn score, any comments and suggestions are appreciated.

[di]

@yeraydiazdiaz This is great, thanks!

I think vendoring the library is definitely the right approach. We do something similar for the Admin UI.

The UI is of course up for discussion and there's no validation step, it's simply a graphical representation of the result of zxcvbn score, any comments and suggestions are appreciated.

The UI looks great to me but @nlhkabu will have to approve. We do validate this in the backend (the score must be >= 2) so it would be reasonable to disable form submission if the password doesn't meet this score.

[yeraydiazdiaz]

Cool, I didn't realise we were doing it already. I'll add the disabling of the submit button for low scores to this PR and wait for feedback on the UI.

[yeraydiazdiaz]

I've updated this to include a slightly more advanced validation in the frontend side, enabling the submit button only if all required fields have values, the password score is >= 2 and the passwords match. Here's a screengrab of the flow:

[dstufft]

The gif of this change looks really good to me, I'll try to get it reviewed this week and if @nlhkabu hasn't had a chance to review it before then, I feel comfortable merging it and tagging it for her to do a post-commit review.

[di]

@yeraydiazdiaz After taking a second look at this, I think that I wasn't clear enough when I said " it would be reasonable to disable form submission".

What I meant was that attempting to submit the form when the password doesn't meet these requirements should not submit the form, and instead show a tooltip indicating that the password is insufficient, similar to how it works for other fields:

By disabling the submit button, the form no longer gives this feedback when a non-password field is invalid, which is not a great UX.

[yeraydiazdiaz]

@di I agree it's not great UX. I did consider adding information via the tooltip, however it is actually implemented using the :required pseudo-class which is browser specific and could not find a way to hook into it.

I do think disabling the submit button if the required fields are missing helps the UX though, if you agree maybe the easiest way forward is to show a message and/or color code the guage to show that the password is not secure enough?

[yeraydiazdiaz]

I've repurposed this PR implementing the behaviour described by @di. I had to build a custom tooltip system in a similar fashion to that of Chrome but I think the flow is now quite nice. Here's the result:

[yeraydiazdiaz]

Huh, didn't realise the above gif shows a backend validation error on the password strength while the frontend allows submission. I must have had local changes in the backend. Here's the actual flow:

Currently tooltips are only cleared when attempting another form submission. I am wondering if they should disappear after a timeout, an event (maybe when focusing on the infringing or any field in the form) or both?

It'd be good to introduce some testing around frontend, not sure if there's plans for that.

[brainwane]

Thanks for your PR @yeraydiazdiaz and sorry for the slow response!

The folks working on Warehouse have gotten funding to concentrate on improving and deploying Warehouse, and have kicked off work towards our development roadmap.

Issue #6 is in our "shut down Legacy PyPI" milestone which we might not get to during the next few months of grant-funded effort, but we are working to review open PRs while we have multiple maintainers concentrating on improving Warehouse right now. So if you fix the merge conflict on this PR and let us know that you've done so by leaving a comment, the next few months are a good time to get it re-reviewed.

Thanks and sorry again for the wait.

[yeraydiazdiaz]

Hi @brainwane, no worries, I've rebased and updated the PR, let me know if it needs any changes.

[brainwane]

@lgh2 the next time you and I chat in IRC, let's talk about you reviewing this PR. Thanks!

[lgh2]

Sounds good!

[lgh2]

Hi @yeraydiazdiaz -- thank you very much for the PR!

I am using Chrome Version 64.0.3282.119 (Official Build) beta (64-bit) on Linux Mint 18.1 (Ubuntu Xenial 16.04)

I tested it and when I entered "12345678" as a password, the tooltip appeared up at the top of the screen, and I had to scroll up in order to see it:

Since I block ads pretty aggressively in Chrome, Sumana suggested that some of my plugins might be causing the issue, so I created a new profile in Chrome for testing, and had the same problem:

The form did not submit when the "password strength" bar was red or orange, but would submit when it was yellow or green. I think moving the tooltip to a more visible place might help users see why the form is not submitting.

[yeraydiazdiaz]

@nlhkabu I've implemented your suggestions, in particular using the existing tooltip system which I somehow managed completely miss before 😅

stale

[di]

@yeraydiazdiaz This looks great! One small change before merging:

The "Confirm Password" input is starting to look a little out of place with everything between it and the "Password" input. Could you change the class for it's label from sr-only to form-group__label so it is displayed?

[nlhkabu]

perfect :)

[nlhkabu]

We should not document the containing block here. The following is enough:

<span class="password-strength">
  <span class="password-strength__guage"></span>
</span>

[nlhkabu]

Hi @yeraydiazdiaz and @di, I've just added a couple of nitpick comments.

One thing we're not considering here is the experience of users who are using screen readers.
Would it be possible to do something like this:

<span class="password-strength">
  <span class="password-strength__guage">
    <span class="sr-only">weak</span><!-- text is updated as password becomes stronger -->
  </span>
</span>

If this is too much to include in this PR, we can merge without and open a separate ticket and PR later?

[nlhkabu]

Can we split this out into 2 <p> s?

<p class="form-group__help-text">
  Choose a strong password that contains letters (uppercase and lowercase), numbers and special characters. Avoid common words or repetition.
</p>
<p class="form-group__help-text">
  <strong>Password strength</strong>
  <span class="password-strength">
    <span class="password-strength__guage"></span>
  </span>
</p>

[di]

@nlhkabu We are setting the aria-label on the parent of the input at the same time as when we show the tooltip, does that suffice or do we need to have the <span class="sr-only"> bit as well (or instead)?

[nlhkabu]

@di, I think we should do both. The experience for someone using a screen reader will be to hear "password strength" and then ... nothing. Adding something here to indicate the strength before they submit the form would be useful :)

[di]

Thanks @yeraydiazdiaz!

[brainwane]

Thank you so much, @yeraydiazdiaz! Let us know if you're interested in an idea for the next issue you could work on. :)

[yeraydiazdiaz]

No worries at all, I tried out pypi.org last night and it was an excellent experience, really happy to contribute, just point me to an issue 👍

[nlhkabu]

hi @yeraydiazdiaz - I was actually thinking about this this morning - it would be great if we could add the password strength indicator onto 2 other pages:

1. the page where you choose a new password, during the 'reset password' workflow
2. the account settings page (where a logged in user can change their password)

What do you think?