[5.7] MethodNotAllowedHTTPException on Intended Redirect

[mcordingley]

Environment:

• Laravel Version: 5.5.40
• PHP Version: 7.0
• Database Driver & Version: N/A

Description:

redirect()->intended() can redirect the user to a bad route if the user interacts with a form after session timeout. This leads to errors being shown to a user from reasonable use of the application and is surprisingly difficult to avoid or work around.

Steps To Reproduce:

• Have an expired session or no session.
• POST to a route protected by the 'auth' middleware that does not have a corresponding GET route.
  • Easy way to do this is to login, navigate to a form, wait a while, then submit the form.
• Authenticate and be redirected to the POST route.
• Receive MethodNotAllowedHTTPException.

Proposed Solution:

Alter \Illuminate\Routing\Redirector::guest() to check if the current request is suitable for redirection and if the requested route isn't suitable for redirect, don't store it as url.intended and instead store the "previous" URL as the intended route to get the user back to being as close as possible to the failed request.

[ankurk91]

Is there any way to exclude some URLs from intended behavior?

[mcordingley]

@ankurk91 Not that I see. However, this issue affects all routes that do not accept GET requests so a blacklist wouldn't help even if we could specify one.

[laurencei]

Out of curiosity - why is ajax specifically excluded?

[mcordingley]

Because we don't want to redirect the user to an AJAX route. We want to only redirect someplace that can be meaningfully loaded up in the browser.

See also Illuminate\Session\Middleware\StartSession::storeCurrentUrl().

[mcordingley]

An unlikely --but possible-- scenario is that the user has an expired session, makes an AJAX request, then manually goes over to the login page to log back in. In that case, the AJAX request could have gotten stored as the intended route, since there'd be nothing to overwrite the value in session.

[taylorotwell]

What if the previous route isn't valid either?

[mcordingley]

@taylorotwell It has to be, else it wouldn't have been placed into the session for previous to fetch. Again, see Illuminate\Session\Middleware\StartSession::storeCurrentUrl() where that gets set. And assuming that there's nothing even in there, it'll return a null. That'll have the intended redirect then go to the default location.

[mcordingley]

Note that I added one more commit to try to bullet-proof this change. With the PR closed, that commit does not appear to show up in here.

[X-Coder264]

@mcordingley Please create a new PR then since Taylor does not read PR comments after he already closed it.

[mcordingley]

@X-Coder264 I created a bug report over this. It got ignored for a month and then closed. I then created this PR. It immediately got closed. In both cases, it seemed that the people closing them didn't even bother to read what I had written. What's to say that opening another one won't simply get closed without being read?

[mcordingley]

Reopened as #25739