laravel / ideas

Issues board used for Laravel internals discussions.
938 stars 28 forks source link

[Security] Password requirements during registration should be applied during password reset #631

Closed jhoff closed 3 years ago

jhoff commented 7 years ago

Reposting from https://github.com/laravel/framework/issues/19524 per @themsaid.

In a default Laravel application, RegisterController::validator is defined in the "user-land" controller. It seems that the intention is to make it somewhat obvious to the developer how to change those requirements to suit their needs. For example, you might want to increase your password requirements ( currently min:6 ).

The password reset flow however has PasswordResetController::rules defined in the ResetsPassword trait, using a separately defined set of requirements for password ( same by default though ).

I imagine that most developers won't realize that if they change the password requirements in RegisterController, that the end user will not have those same requirements applied during password reset by default.

As far as I can tell this isn't documented anywhere, but I feel like it might fall under the category of "potential security issue". At a minimum, maybe the rules method should be lifted out of the ResetsPassword trait and into PasswordResetController with possibly some notes in the docblocks and / or authentication documentation.

jhoff commented 3 years ago

Certainly, this issue is no longer relevant with Laravel 7 / 8 and at a cursory glance shouldn't be an issue with Fortify, as the registration and create controllers now share the same password rules by default. Closing.