This middleware no longer supports get requests?
skeets23 opened this issue · 1 comments
So I recently updated this package in my project, and I traced back a bug to this change in the source code that I can't understand for the life of me:
Response.php:
...
protected function makeStatusCode()
{
if ($this->getRequest()->isMethod('get') || ($this->checkOTP() === Constants::OTP_VALID)) {
return SymfonyResponse::HTTP_OK;
}
...
.... so.... any request that is a "get" is OK? How does that make sense?
Actually, if they're not authenticated, it's definitely not ok, and all this does is hide the error message, and display the page asking for the code again with no explanation as to what happened.
Unless I'm misunderstanding something, $this->getRequest()->isMethod('get') ||
needs to be removed from the if statement.
This change happened in commit 92b9b8c
Yes, I see that the documentation says to use <form method="POST" action="/some-action">
, but this is an inferior approach -- you then have to set a static redirect for the post request, so that no matter what the original URL is, they're redirected to the same page.
Example: User goes to mysite.com/settings/some-setting
, and they hit the 2FA middleware. What I would expect is that once you complete the 2FA challenge successfully, it takes you to mysite.com/settings/some-setting
, but that won't be possible using this library currently because it requires a POST request, and mysite.com/settings/some-setting
is a GET route.
And you can't just use back()
in the post request either, that will blow up if they messed up the 2FA code 2 or 3 times from too many redirects.
It was much more simple before, just create a form exactly like this <form>
, and then the original URL is retained, and a GET request is sent to the original URL, passes through the 2FA middleware, and puts the user exactly where they expect to end up.
I see no valid reason to require POST
in the middleware. If there is an easy way to get the user back to their original GET request while requiring that the 2FA code is sent over a POST request, then it's not documented, so either way there's a problem.
Hey @antonioribeiro any insight into this?