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

Register Password to be min 8 to match ResetPassword and laravel default #4916

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ziming
Copy link
Contributor

@ziming ziming commented Feb 1, 2023

Laravel 10 and Backpack 6 is very near, so I thought I revive this old issue. Only bump the password min from 6 to 8, nothing about Password::defaults() or how the config rules should be like, be it in config file or ServiceProvider

WHY

BEFORE - What was wrong? What was happening before this PR?

Backpack Register password minimum is 6, but ResetPassword in Backpack and Laravel Default is Min 8. This is not consistent. NIST also recommends password to be minimum length 8

AFTER - What is happening after this PR?

Backpack Register Controller password minimum to be min length 8 to match Reset Password and Laravel Default

HOW

How did you achieve that, in technical terms?

Update the validation Rule

Is it a breaking change?

Well Laravel 10 is and backpack 6 is coming, I doubt new users registering will ever realise min password length has increased by 2 characters too.

How can we test the before & after?

Try and register with a password less than 8 characters?

@scrutinizer-notifier
Copy link

The inspection completed: No new issues

@ziming
Copy link
Contributor Author

ziming commented Feb 23, 2023

Not sure if this is stupid or clever, but just before I go to sleep, I happen to think about my previous approach to stronger password rules for backpack (#4442) and thought of a new approach that would require much less maintenance from you all.

Instead of creating a seperate Backpack Password Rules object (that is essentially a duplicate of Laravel own Password object) like my previous PR

We take advantage of the fact that route middlewares are executed after Service Providers later in the request and provide a command like this

Command to publish a route middleware called BackpackPasswordRulesRouteMiddleware or any other name

Then after that route middleware is published, the user can use Laravel own Password class in the middleware

// [Password::defaults()](https://laravel.com/docs/10.x/validation#validating-passwords)
public function handle($request, Closure $next) {
    // stronger password rules for admin
    Password::defaults(function () { // exists in laravel 9
        return Password::min(12)->mixedCase()->uncompromised();
    });
}

Then backpack register, edit profile, reset password routes just need to use this route middleware (which is by default is an empty handle() method.

It will be okay if the user did all these Password::defaults() customization stuffs in their service provider, being in a route middleware, this only executes for specific routes that apply this middleware and override the original Password::defaults() in the ServiceProvider for that request only.

And thus this will only require 3 things

  • Register, Reset, Edit Profile routes in backpack to use this route middleware
  • Password::defaults() as the last password rule in the password validation in those 3 forms above
  • a command to publish this empty route middleware with a comment in the handle method on it's purpose and how to use it

How that sounds?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants