-
Notifications
You must be signed in to change notification settings - Fork 821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reduce default strength of EntropyPasswordValidator #11413
Comments
Couple of questions that I think I know the answer to but want to confirm:
IMO I think it's worth using the jump to 6 to meaningfully improve security, so I'm more in favour of a jump to It means we have to trust people upgrading will look at the docs - but I think that should be a given. Overall, I'm in favour of going to |
Correct - there is no retrospective validation of passwords. Password validation only happens when you're setting a new password.
Yup, unless there's some custom logic changing how that works.
Very easy, yes
The old behaviour is retained in the
The validation error message in There are a variety of messages in the |
I agree that we should meaningfully increase security, though I think I'd still prefer Ideally we'd have a UX component in the password field that provides feedback to the user about password strength as you type, it could be as simple as a line of text below the input element that says "Password strength: weak". |
This sounds like a good compromise to me. Increase security but without making it insanely difficult for the average user. I'm a 👍🏼 to using Thanks for the detailed feedback @GuySartorelli, all sounds great to me, and awesome that there's the It's also worth thinking about our overall strategy around passwords here - one of the reasons for long / complex passwords is to avoid brute force attacks. That's not as relevant for Silverstripe because of the lockout system that only allows you a small number of tries within 15minutes before getting locked out - meaning that brute forcing is essentially impossible for passwords a lot shorter than 16 characters. |
Brute force attacks can also be used if someone gets a dump of the database (though granted the attack strategy changes a little in that scenario), so any password strategy should always take them into account. |
Yeah, that is certainly true and is a valid consideration (nice shout @GuySartorelli, thanks!), although there's a lot worse that attackers are likely to do with a full database dump from any site that isn't purely public information. I reckon |
Thanks for the feedback, sounds like we should set it to MEDIUM, which is the default level in symfony/validator I've made a proof of concept for the UX feedback, Here's a video of the POC https://www.youtube.com/watch?v=HAFfEhTzcBo |
Overall what you've shown in the video looks neat. Won't make sense when a different validation callback is passed into |
PR merged. No docs change required. |
In the use symfony/validator work the new EntropyPasswordValidator was made the default PasswordValidator for CMS 6, however this appears to be a bit too strong compared to CMS 5
We set the EntropyPasswordValidator to have a default strength on
PasswordStrength::STRENGH_STRONG
. This means you now require a password of at least 16 characters, and 19 characters if using a passphraseThe default password validation in CMS 5 is only 8 characters. This goes up to 10 if using cwp-core or similar configuration was used, and you also needed some uppercase / punctuation characters in there too
Given that STRENGTH_STRONG is such a departure from CMS 5, it seems like we should either (in order of smallest to biggest change)
@madmatt Do you have any views on this?
Comparison of what passes the password validators
CMS 5 - silverstripe/installer
CMS 5 - silverstripe/installer with cwp/cwp-core config
https://github.com/silverstripe/cwp-core/blob/3/_config/security.yml#L9
CMS 6 - PasswordStrength::STRENGTH_VERY_WEAK;
Symfony throws an Exception :-) Minimum score is actually STRENGTH_WEAK
CMS 6 - PasswordStrength::STRENGTH_WEAK;
CMS 6 - PasswordStrength::STRENGTH_MEDIUM;
CMS 6 - PasswordStrength::STRENGTH_STRONG;
CMS 6 - PasswordStrength::STRENGTH_VERY_STRONG;
Didn't test
New issues created
Acceptance criteria
PRs
The text was updated successfully, but these errors were encountered: