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

Minimum password length #373

Closed
tobias-kuendig opened this issue Feb 23, 2019 · 8 comments
Closed

Minimum password length #373

tobias-kuendig opened this issue Feb 23, 2019 · 8 comments

Comments

@tobias-kuendig
Copy link
Contributor

The current minimum length for a password is 4 characters. By today's security standards this is pretty bad and shouldn't be the default this plugin ships with.

Furthermore, changing the password length currently is very cumbersome or even impossible without forking or manually extending the Account component.

Why not use the validation rules of the model everywhere so a user can easily cusomize them with plugin extensions?

User::extend(function (User $user) {
    $user->rules['password'] = 'required:create|between:8,255|confirmed';
    $user->rules['password_confirmation'] = 'required_with:password|between:8,255';
});

What do you guys think?

I would like to create a PR that changes the minimum password length to 8 characters and re-use the same validation rules of the User model in the different components.

A problem I see with that is the possibility of changing the default signup rules for a lot of production sites.

#353 could be done in the same PR.

@LukeTowers
Copy link
Contributor

@tobias-kuendig sounds good. Also, I don't think the max length supported is actually 255 given that the default length of the string column in the default DB collation for October is 191 and then the password gets hashed on top of that.

@tobias-kuendig
Copy link
Contributor Author

#375

This is my take on the problem. It essentially parses the model rules and re-uses the between constraints if available.

I'd say either go this direction or merge #354 so the password length becomes a backend setting.

@bcwaretx
Copy link

@LukeTowers It looks like this is in the 1.5.3 release but just didn't make the fix list and release notes.

@LukeTowers
Copy link
Contributor

@bcwaretx this particular change was actually in the 1.5.0 release. @bennothommo are the release notes correct?

@bcwaretx
Copy link

@LukeTowers My apologies, the Nov 19 commit references this issue. The fix to re-use the Model validation in the Component doesn't appear in the code until 1.5.3, like what is referenced in #375?
7307e99

@LukeTowers
Copy link
Contributor

That's correct @bcwaretx

@bennothommo
Copy link
Contributor

@LukeTowers the original password length changes in 1.5.0 were documented for that release, but your change to sync the validation rules in 1.5.3 was not - I have updated the release notes to reflect that change.

@LukeTowers
Copy link
Contributor

Thanks @bennothommo

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

No branches or pull requests

5 participants