Skip to content
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

NIST Password Rules #1258

Merged
merged 5 commits into from
Aug 21, 2019
Merged

NIST Password Rules #1258

merged 5 commits into from
Aug 21, 2019

Conversation

DivineOmega
Copy link
Contributor

@DivineOmega DivineOmega commented Aug 12, 2019

This PR adds new default validation rules that follow the password related recommendations found in NIST Special Publication 800-63B section 5. It does so by making use of the langleyfoxall/laravel-nist-password-rules pacakge.

This supersedes the current direct use of my package (divineomega/laravel-password-exposed-validation-rule) as the password exposed validation rule is included in this new package.

I believe this password policy will help new developers make secure choices by default, with the ability to scale complexity up or down as necessary.

Note that relevant tests have been modified, and all pass successfully.

@rappasoft
Copy link
Owner

rappasoft commented Aug 21, 2019

Do you think we need the current ChangePassword anymore? I feel like it's just added fluff with this new added package. If you think not i'll remove it from all the required places.

Edit: Also can you explain why the changePassword for UpdatePasswordRequest is different than the UpdateUserPasswordRequest and ResetPasswordRequest. Specifically this added line: https://github.com/rappasoft/laravel-boilerplate/pull/1258/files#diff-4c94b4ce32364d12ecbbd3404025fc06R41

Edit 2: I understand the first edit, just the question about the other rule being needed is left.

I have never used this package so i'm just trying to understand. After these are added and I modify whatever i'll merge it in.

@rappasoft
Copy link
Owner

If we're going strictly by NIST, then I vote the ChangePassword rule be removed as NIST says 'No other complexity requirements for memorized secrets SHOULD be imposed.' So i'll go ahead and do that, because they really only add the uppercase/number rules and as of now there's like 10 errors if you set the password as say 'secret'.

@rappasoft
Copy link
Owner

Made some changes and ended up with this: 721d0b5

rappasoft added a commit that referenced this pull request Aug 21, 2019
- Added Azerbaijan language (#1254)
- Added NIST Password Rules (#1258)

- Assign all permissions to the Admin role without the need to explicitly assign the roles/permissions to the user. (#1227)

- Removed default Google scopes (https://github.com/rappasoft/laravel-boilerplate/pull/1253/files)
- Removed ChangePassword rule as the new NIST rules cover it
@rappasoft rappasoft merged commit b0ad954 into rappasoft:master Aug 21, 2019
@DivineOmega
Copy link
Contributor Author

Thanks for getting this all merged in. 👍 🙂

Give me a shout if you have any problems, or file an issue on the package repo.

@DivineOmega DivineOmega deleted the feature/nist-rules branch August 22, 2019 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants