Change to prevent unauthenticated requests for CSRF modified login behaviour as well
The change that went into Foreman as 4e3a7e7a2a5 included definition of
handle_unverified_request which in turn affected not just the logout functionality, but login as well. The net effect is that it's now not possible to log in to Foreman WebUI via its logon form at /users/login without GETing that URL first, to get the authenticity_token.
Since issue #6999 was attempting to only change the logout behaviour, I'd call this a regression, unless there is a specific reason why the exception should be raised for the unauthenticated login page where just resetting the session and proceeding with the request was enough.
#5 Updated by Jan Pazdziora about 6 years ago
Dominic Cleal wrote:
Thanks for the report. Did this affect one of the login features you added? (I'm thinking the form intercept method perhaps?)
It did affect my tests of that feature. It's fairly easy to change the tests but I'm concerned that there was a wholesale change in behaviour even if the issue/commit message only point to a very narrow case (logout). Hence this issue, to spark a discussion about the intended behaviour in all the other cases.
#6 Updated by Dominic Cleal about 6 years ago
- Status changed from New to Rejected
- Legacy Backlogs Release (now unused) deleted (
Yeah, the commit message, as is typical for us, was overly narrow. We realised the scope was broader than logout only, as any cross-domain request that was made missing CSRF tokens would result in the user being "logged out" by way of their session being invalidated.
Looking at what we do during the login process, we reset the user's session immediately before we go any further, as part of the fix for another CVE (#4457). So as not to leave another case where a user could be attacked and get their session deliberately reset, I think we need to retain the CSRF authenticity check on login.
I think we can say this is deliberate and we're preventing an identical attack on the login route.