-
Notifications
You must be signed in to change notification settings - Fork 12
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
move PasswordValidator configuration into _config #53
move PasswordValidator configuration into _config #53
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep I agree with @ScopeyNZ. The tests are good for your PR at least so you know your config is working, but there's no logic changes so no need for tests
Also, do you want to put this into the 2.2 release? |
@robbieaverill I'm keen for this to be added to the 2.2.0 release. |
Indeed it appears to only be testing config settings, however that isn't the main goal of this minor test suite. The goal is more to catch 'regressions' should someone alter the values, given that the minimums tested here are a requirement for compliance. The tests should still pass if passwords are strengthened with more checks or higher character limits, for example. The values were previously removed due to duplication. However on inspection I could not find where they were duplicated. I assume framework defaults - however I couldn't find where they were set there either. This is merely an extra layer of assurance. E.g. the This is my justification for adding the wee test suite. |
@NightJar no worries, if you think it's worth keeping it then how about we compromise and you add the test to a |
Sounds like a plan :) Also added a |
8401fed
to
efabff6
Compare
and out of _config.php i.e. use Injector rather than proceedural boot time code, as SilverStripe 4 supports this (where 3 did not). Re-introduce some security level settings that were lost at some point with no clear reason as to why.
efabff6
to
e9baf31
Compare
and out of _config.php
i.e. use Injector rather than proceedural boot time code, as
SilverStripe 4 supports this (where 3 did not).
Re-introduce some security level settings that were lost at some point
with no clear reason as to why.
Fixes #52