Require users to use strong(er) passwords

[0xDaedalus]

Closes #1329

This PR requires users to have create a somewhat-strong password to use the extension.

How strong of a password?
3 strong of a password!
How strong is 3?
Well - the tool we're using has the following scale:

 // 0 - too guessable: risky password. (guesses < 10^3)
 // 1 - very guessable: protection from throttled online attacks. (guesses < 10^6)
 // 2 - somewhat guessable: protection from unthrottled online attacks. (guesses < 10^8)
 // 3 - safely unguessable: moderate protection from offline slow-hash scenario. (guesses < 10^10)
 // 4 - very unguessable: strong protection from offline slow-hash scenario. (guesses >= 10^10)

While we all want to be good security-conscious citizens of the web I do think that requiring users to have complex passwords of more than 16 characters (anecdotally what it seems to take to score 4) might cause a bit of friction. @mhluongo @mr-michael what do you think?

[mhluongo]

Considering the key derivation function we're using is parallelizeable, 3 feels like the right balance..? Maybe we lower it to a 2 once we have Argon2, maybe we leave it where it is. 4 is too much for a consumer product

[mhluongo]

We should move to a password strength meter, though this might be okay for the start... @VladUXUI @mr-michael thoughts on whether a length error works in the interim?

[Shadowfiend]

Ngl, getting "Password too weak" without any guidance on how to make it stronger would make me want to throw my laptop at the wall and sling Mjölnir after it lol.

On the other hand I would probably use a password generator and not run into the problem in the first place so maybe the issue is imaginary 😅

[jagodarybacka]

I agree with @Shadowfiend - we have to give user info about what we consider a strong password. Maybe a tooltip somewhere? cc @VladUXUI

Second thing - it will drive me crazy during development where I'm reinstalling the extension very often to first copy the password and then to copy the seed. Can we consider allowing super easy passwords under the feature flag?

[greg-nagy]

I do like this lib, but does it really worth including 800 kb source to have this level of sophistication?

And I also wonder does it worth it for us to be this sophisticated?

What were your thoughts @0xDaedalus?
It's a non blocker, left an approve, but I would like to have a back and forth before merging.

[0xDaedalus]

I do like this lib, but does it really worth including 800 kb source to have this level of sophistication?

I thought of this as well - we're an extension not a website so I'm personally not as sensitive to bundle size (perhaps erroneously?)

And I also wonder does it worth it for us to be this sophisticated?

I think the worth here comes from not only actual security (for example this helps mitigate users using passwords that are vulnerable to dictionary attacks) - but the perceived security as well. I think there is a non-zero benefit to being able to clearly point to exactly why and where our wallet is more secure than others.

I agree with @Shadowfiend - we have to give user info about what we consider a strong password. Maybe a tooltip somewhere? cc @VladUXUI

We can do this but there are at least ~20 different reasons a password can fail right now (from playing around with it) - I intended this PR to be a quick win while we're pushing for polish and polygon so if we want to go that way I'd rather close it and then pick this up again once its properly been scoped.

[mhluongo]

I'd like to see this merged with a follow-up "password strength bar" design personally cc @VladUXUI

I do believe this level of password scrutiny is right and worth the lib weight.

[Shadowfiend]

For context this is like 10% of our existing bundle size right?

I would consider making this a recommendation, not a requirement, which is how password strength is typically implemented. We should warn, but blocking specific passwords seems like a path to frustration, even more so if, again, we can't guide the user to something better.

Lastly I'll softly suggest that making a change like this that throws up an explicit barrier to potentially completing our equivalent of "signup" should maybe start at checking in with product and design--even if the implementation itself is easy. Ultimately up to @VladUXUI/@mr-michael whether that is true though.

[mhluongo]

Good point. I'd assumed the issue behind it (#1329) was already reviewed by product, or it wouldn't have made it into the shortlist for the sprint or in @mr-michael's polish ticket though...?

[0xDaedalus]

I would consider making this a recommendation, not a requirement, which is how password strength is typically implemented.

FWIW I think this is generally a requirement and not a recommendation for financial applications (certainly for trading applications) which is a category that I think its fair to say that we fall into.

[henryboldi]

Ngl, getting "Password too weak" without any guidance on how to make it stronger would make me want to throw my laptop at the wall and sling Mjölnir after it lol.

Same 😬

We can do this but there are at least ~20 different reasons a password can fail right now

This makes it even more frustrating

[mhluongo]

Let's solve the problem. What do you guys think of a password bar that directly corresponds to the levels we get back from the library? Something sort of like this

https://dribbble.com/shots/4069878-Password-Strength-Meter

... and in the meantime, maybe we set the strength lower?

[henryboldi]

^ Big fan of this solution!

[Shadowfiend]

Def comfortable with a password strength meter, though I think it's only a partial solve.

My 2c on why I don't think this rises to the level of a trading account (incidentally these are usually implemented in terms of dumb but easily explained rules, at least the ones I've seen) in terms of requirements is that this isn't known to be a large account, and we support hardware wallets for higher security.

We've briefly discussed in the past a suggested gradual ramp of security for users based on the funds they have... Keeping that in mind as a potentially interesting model, requiring somewhat-opaquely "high" security vs suggesting it seems wobbly to me.

With that said, I'll quiet down on the issue and leave it to product and design and user input to guide things from here 😉

[greg-nagy]

I think it's useful to always err on the side of paranoia in terms of security. That's why I liked the solution and went on with the merge. In this case, I think it's more important if we consider how we handle our keychains, not just the actual UX of the wallet. Personally, I am ok releasing it as is, with the strength bar as a fast follow.

We've briefly discussed in the past a suggested gradual ramp of security for users based on the funds they have

This is def a good direction we should go in the future.

[Shadowfiend]

For the individual user, paranoia is not the wrong strategy. In my view, however, what we're erring on the side of here is paternalism because we are removing user choice instead of guiding them to better ones with information.

It may still be the right trade off, but it makes me itch.