IN PROCESS: Enforce stronger passwords

[gaswirth]

Currently, there are no restrictions on complexity for user passwords. Enforce a password policy on the frontend.

[gacetta]

@gaswirth - I'm reading up about this. If you tackle it, no prob, but I think I'll be able to get to it on monday morning.

What complexity restrictions are you thinking? A quick search of password complexity and length standards basically led me to these categories:

• minimum length: usually ranges from 8 (microsoft accounts) to 15 (DoD security standard)
• specifying how many chars from each (or some) of the following sets: uppercase letters, lowercase letters, numeric, special char
• (less common) dictionary check - AKA can't contain any words found in a dictionary. This could be an english dictionary, or a dictionary that contains the word 'password', the user account's name, etc.
• (less common) consecutive char check - can't contain '1234' or 'abcd' or more than two consecutive chars from user name

I found the following npm libraries. I'd have to research them more but they all seem to have the ability to check some of the above restrictions via regex: joi-password-complexity check-password-strength complexity

I just read an article about best practices, but also just read an article about why enforced complexity isn't the best practice. Good ol' tech world of differing opinions.

[gaswirth]

Awesome!!

I think an 8 char minimum is good, along with at least one cap, at least one number, and at least one special char. The check-password-strength library looks like it's the most recently published, and it seems simple enough. Wanna give that one a shot and see where it leads us?

Interesting article about why enforced complexity isn't the best practice. I think given that our target audience is so wide, while this may effectively help a brute force cracker statistically, I think a lot of our audience may use really simple passwords by default. By enforcing complexity, we're at least preventing people from using terrible passwords.

[gacetta]

Sounds good, @gaswirth!

I'll take a shot at adding check-password-strength into the project with the following settings: minimum length: 8 password diversity requirements: 1+ Capital Letter, 1+ number, 1+ special char

And I agree, that was an interesting article, but most people don't use password libraries and such...

[gacetta]

ok @gaswirth ! I've got check-password-strength working but ran into some typing issues that prevented me from making things look pretty.

At the moment I haven't implemented it fully into the code. It's just a simple console log using the check-password-strength library that occurs in the handleSubmit function in the three places passwords can be set: RegisterView.tsx, ChangePasswordView.tsx and ResetPasswordView.tsx

I have specific questions about behavior for non-valid passwords to move forward:

1. When should password validation occur?
On form submit? After typing via a debounce like the password compare? On form validation?

2. Do you have preferences for where password validation should be located (in Code)? I'm still wrapping my head around all the custom hooks and mutations, but I know there's many options for where this check might best be located. At the moment, it made the most sense to write a custom hook called useValidatePassword in hooks.tsx. But ultimately it'd be nice to have it integrated somehow with useResetPasswordError, useChangePasswordError, and useRegistrationError which are the three places password errors are handled.

3. what would you like to happen upon non-valid password?
My assumption: upon failed password validation, display error message under input component using error prop (similar to the error message behavior for "passwords don't match" or "incorrect password", etc...).

Anyways, i'll push up my code for now if you want to take a look. It's nothing fancy as it took me a while to understand how and where to integrate it (and struggle with typing until I gave up).

PS - refactor suggestion: combine useResetPasswordError, useChangePasswordError, and useRegistrationError into one?

[gaswirth]

Coolio! Publish, por favor, so I can see!

1. I'm thinking it can happen at the same time as the password compare. Probably easiest place to try it out first.
2. Might not actually need a custom hook for this -- the passwordStrength call can run inside the same useEffect that handles password compare. Probably a little less overhead this way, too.
3. We'll do an error message. On further thought, you're totally right -- I don't need these error hooks separate any more. I'm setting up a new Issue #71 to tackle this and tagged @gacetta.

I may do this in my cartoon-watching brain-off time since it's a breeze, but in case you seeeeeee it here, feel free to pounce.

[gaswirth]

(I did indeed combine my error handlers as per #71. All set there)

[gaswirth]

@gacetta I moved your implementation into the password check debouncers on the 3 applicable components. I also combined the error handler, and set up handling across the password fields -- take a look!

One unforeseen issue (though I should have trusted my instincts and kept them separate, which would have avoided this): by combining my error messages into one useErrorMessage hook, instead of keeping them in separate password, email, etc hooks, is that I now have to add conditionals any time I have a form that could have different errors. So, on the register form: there can be email errors OR password errors, and now rather than using the 2 specific hooks stored in their own state, I have to add conditionals to each field to make sure it doesn't display another field's errors. Not the end of the world, but annoying and unnecessarily complicated.

It's one of those situations where it feels like refactoring and simplifying makes code more maintainable, but in reality, the more verbose (and potentially repetitive) choice is the cleanest.

I think let's revert the combining of the error messages.

(Ignore the error message stuff above. Conditionals are needed anyway and the way I did it was cumbersome. There's definitely a way to streamline this, but for now, we're going to stick with the new combined useErrorMessage hook.)

[gacetta]

@gaswirth Looks great. I see what you mean about how it results in a lot of useEffects that are reused across Views...

Some general thoughts/questions:

• QUESTION do we need to outline the password requirements somewhere? AKA, minLength and character diversity? As a user, the error message: 'Password is too weak. Please choose a stronger password.' doesn't give me enough information. Maybe above the "password" or "new password" field you could create a PasswordRequirements component that has a simple phrase (if that's too messy, maybe a tooltip?)

"Passwords must be 8 characters long & contain at least one of each: lowercase letter, uppercase letter, number, and special character: !"#$%&'()*+,-./:;<=>?@[]^_`{|}~".

• changed passwordsMatch state to default to true where used, since two blank password fields technically match in the current implementation.
• Take a look I tried condensing some of the useEffects into one. (I didn't deal with the debounce cause async in useEffect gets confusing to me). This approach seems cleaner in ChangePasswordView.tsx and ResetPasswordView.tsx but maybe not so much in RegisterView.tsx.
• debounce timing is 500 on RegisterView and ResetPasswordView and 300 in ChangePasswordView. Updated so they're all 500 for consistency.

useErrorMessage in hooks.tsx : This is exactly what I thought combining it would be. Looks great!

• added additional comment to section off // Password Errors

useValidatePassword in hooks.tsx:

• simplified if (!password || password.length < 1) return; to if (!password) return
• this hook checks password length as part of it's validation. We might be able to lose password.length checks in other conditionals if wanted

[gaswirth]

Looks great! Thanks for your work @gacetta !