[User Accounts] Fix edge-case that gave a confusing error message

[johnsaigle]

Brief summary of changes

When both the password and email fields were empty, an error was displayed saying "Your password cannot match your email address"

While technically accurate, this isn't actually helpful for a user, especially since our checkbox option "Generate new password" requires leaving the password field blank.

Testing instructions (if applicable)

1. Verify the confusing error message in the linked Issue.
2. On this branch, do the same thing. The password error won't appear.
3. Repeat 2. but uncheck 'Generate new password'. Ensure that the error message "Please specify password or click Generate new password" is displayed instead.

Link(s) to related issue(s)

• Resolves [user_accounts] new user #5948

[johnsaigle]

@lingma Would you mind reviewing?

[lingma]

What about (!empty($values['Password_hash']) && !empty($values['Email']) && in_array($values['Email'], $values['Password_hash']))?

[johnsaigle]

Are you asking what happens if the Email equals the password? There should be an error condition for that. You can test and see.

[lingma]

Are you asking what happens if the Email equals the password? There should be an error condition for that. You can test and see.

No, I am wondering what if the password is email0

[lingma]

The proposed code could solve this problem. But I did not test.

[johnsaigle]

No, I am wondering what if the password is email0

I don't understand what problem you are trying to address here.

[lingma]

I am wondering what if the password is the same as the Email, only add a 0 or whatever, the program won't detect anything.

[johnsaigle]

The Password class is self-validating and contains a reference to the Zxcvbn library which evaluates password-strength. We're using this library to evaluate passwords instead of creating ad-hoc password rules for LORIS.

A benefit of this is that the modules don't have to be responsible for checking passwords. E.g. Passwords can be changed in the login module as well as in user_accounts. Instead of duplicating these checks, it's all done in Password.class.inc.

[driusan]

I just had a look at the code, and the Zxcvbn only seems to check the complexity of the input in the Password class. It doesn't validate that the blacklisted things that the library doesn't know about (ie. the email address) aren't a substring as far as I can tell.

[johnsaigle]

That's true; it's not a feature we discussed. The code has only ever checked if the Password === Email as far as I know.

We can add that in another PR?

[driusan]

Sure, I just meant that that's what @lingma seems to be saying that you're misinterpreting..