I don’t have a proposal for a fix but I am currently pretty much bothered by the way we use the authenticate method. Basically it’s impossible to control that you actually authenticate in a LoginController. Most of the times authenticate will have been called before and IMHO that’s bad.
A very interesting effect I had with one of our customers now was that a LoginController would fail with an access denied exception evven though valid credentials were given. What happened was that authenticate was called before hitting the controller and so you were logged in already which triggered CSRF protection which obviously failed as no CSRF token was delivered with the login request. Sure that can easily be fixed by ignoring CSRF for this action but it nicely shows the problem I have with authenticate at the moment.
So I would like to start a discussion, specifically about the fact that PolicyEnforcement will call authenticate. Is that necessary?
I’d say it’s not necessarily needed to authenticate always automatically. I just considered it helpful, to not have to take care yourself. I see the need of more control in the whole authentication process. On the other hand I don’t think it’s nice to handle, if it’s totally manual, always. Just imagine you need to trigger it in 20 different controllers, just because you don’t know which exact controller the user calls. Or you change the policy and therefore need to change the controllers to trigger authentication in different places. That would be against the whole decoupled “touchless” idea.
I’d propose to look for a solutions that does not necessarily create a dependency between controllers and the authentication process. However, I totally agree that it should be possible if you need more control.
I have no concrete solution in mind, though. Hope my arguments make sense, still
I also had some issues with that recently. Especially because the AuthenticationManager has a runtime cache that can lead to no/wrong roles to be authenticated in certain cases… I need to dig into that a but more, just two comments for now so I don’t forget about it:
The authenticateAction needs to skip the CSRF token check because it is somewhat special: the CSRF token is required for POST requests if a user is authenticated. Forms therefore only add a hidden field for the token if a user is authenticated, so that’s a little bit of a hen/egg situation.
That’s why the AbstractAuthenticationController defines the authenticateAction including the @Flow\SkipCsrfProtection (@christianm any particular reason why you can’t extend that controller in your usecase?).
But: The CsrfProtection Request Pattern currently requires the CSRF token whenever the target action/method is covered by a privilege. Including privileges Everybody has access to(!)
So when using a whitelist approach by disallowing actions by default and then allowing specific ones (like we do with Neos) the request is intercepted more often than it should.