[RFC] Check the roles in the legacy controllers

[leofeyer]

Instead of calling $this->User->authenticate() twice (one time in the Token class and again in the controllers), we should remove the second call and check the roles instead.

[aschempp]

Previously, $this->User->authenticate() redirected to the login screen without access, e.g. also for valid requests but on session timeout. Now we're throwing an exception. That's not really wrong, but is the exception handler somewhere and redirecting to the login screen?

I think if we have Symfony security and form_login firewall (which we will have in the backend), it will redirect if we use the correct exception.

[aschempp]

Also, you want to change this in 4.5 or also in the LTS version to prevent multiple authenticate runs?

[bytehead]

I think this gets obsolete with Symfony security. It will check the roles.

[leofeyer]

Previously, $this->User->authenticate() redirected to the login screen without access, e.g. also for valid requests but on session timeout. Now we're throwing an exception.

$this->User->authenticate() is already executed in the token, so if the user is not authenticated, they will be redirected to the login screen before the controller is even constructed.

The point of this PR is to prevent the authenticate() method to run a second time – you recall?

The role check is a completely new feature, which does not change the current behavior at all. In fact, the exception will never be thrown, because if the user would not have the roles, they would have been redirected to the login screen before.

[leofeyer]

The point of this PR is to prevent the authenticate() method to run a second time – you recall?

Actually, we should change this in Contao 4.4, too, because right now the whole authentication routine is run twice. This includes the setUserFromDb() method as well as the hooks.

@contao/developers What do you think?

[leofeyer]

I think this gets obsolete with Symfony security. It will check the roles.

@bytehead How is this going to work in the front end? Right now we only check the roles if the page is protected.

[bytehead]

@leofeyer looks and acts mostly the same as your proposal here. I'm fine with this. My PR still needs some time.

[aschempp]

Actually, we should change this in Contao 4.4, too, because right now the whole authentication routine is run twice. This includes the setUserFromDb() method as well as the hooks.

Exactly what I asked in #1067 (comment) 😂

[leofeyer]

See 1ab1524.