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

[6.x] Alleviate breaking change introduced by password confirm feature #30389

Merged
merged 2 commits into from
Oct 23, 2019

Conversation

netpok
Copy link
Contributor

@netpok netpok commented Oct 22, 2019

In Laravel 7.2 the password confirmation was introduced, this feature sadly brought a breaking change with it:
The Route::auth method registers the confirmation routes by default.

This results in:

  • The breakage of route:list command
  • Server error 500 response upon accessing the password/confirm routes
  • Addition of two new route endpoint (which does not introduce any exploitable behavior even if the controller is present)

The best solution for this would be to disable this function by default but after 15 days this could introduce problems in many live systems. This pull request does the second best solution and enables these routes only if the controller exists.

Because this middleware targets a narrow range of the userbase (just like the VerifiesEmail middleware) I'm also proposing to make these routes disabled by default in 7.x.

Fixes #30382

@SagarNaliyapara
Copy link
Contributor

@netpok can't we move options to config?

@netpok
Copy link
Contributor Author

netpok commented Oct 23, 2019

I think it would not make much sense since it is directly related to the method itself. Also it is more related to the routes than the auth and currently the route config is the web.php where you can set these options.

Furthermore even if it's decided to be moved that could only happen in the next version.

@taylorotwell taylorotwell merged commit 4d2d79a into laravel:6.x Oct 23, 2019
@netpok netpok deleted the bugfix/confirm-break branch October 23, 2019 14:26
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.

Addition of ConfirmPasswordController.php breaking change
3 participants