[Security] Custom Authenticator: Adding info about session #19813
Conversation
Page: https://symfony.com/doc/5.x/security/custom_authenticator.html This line was really missing on this page: ```php $request->getSession()->set(SecurityRequestAttributes::AUTHENTICATION_ERROR, $exception); ``` I hope the code block formatting (inside this list) works. If not, the list could be changed to sub-headings. In preparation of this, I also moved the box about which class to extend upwards (to the other info about extending).
| .. tip:: | ||
|
|
||
| If your login method is interactive, which means that the user actively | ||
| logged into your application, you may want your authenticator to implement the | ||
| :class:`Symfony\\Component\\Security\\Http\\Authenticator\\InteractiveAuthenticatorInterface` | ||
| so that it dispatches an | ||
| :class:`Symfony\\Component\\Security\\Http\\Event\\InteractiveLoginEvent` |
There was a problem hiding this comment.
The definition of "interactive authentication" has become very messy in Symfony and if it was possible to deprecate events, this will be the first candidate I think of.
For this reason, I don't want to give this mention such a high-prio location in the document... it's almost better if you don't implement it :) I think current location at the end of the section makes sense: you can find it if you want to search for it, but you probably don't read it when you're implementing a custom authenticator.
There was a problem hiding this comment.
When trying to figure out why I can't get it to work, I stumbled over the issue at symfony/symfony#49166 (comment) - where this was the solution ;-)
I do get your point; but you can't show people a full example, and in the end say something like "You should have started differently..."
So what about:
- Removing it completely?
- Or reword it to something like "In previous versions, it was recommended to .... but this is no longer the case."
| with the login errors. In order to access the login error in the controller | ||
| with ``$authenticationUtils->getLastAuthenticationError()``, you need to | ||
| store it in the session now:: | ||
|
|
||
| use Symfony\Component\Security\Http\SecurityRequestAttributes; |
There was a problem hiding this comment.
Like your other PR, this is pretty much a convention of how the build-in form login authenticator works. When building a custom authenticator, you can invent anything you want to return errors to the application.
I would generally skip the controller in custom authenticators and render a Twig template with errors directly from the authenticator.
Also, the code example starts a session. My prediction is that 80% of the use-cases for a custom authenticator is in a stateless security setting. I'm a bit wary about showcasing this, as it has a high chance of leading to people starting sessions in stateless applications.
There was a problem hiding this comment.
What I didn't like is the sentence before it:
This is useful for e.g. login forms, where the login controller is run again with the login errors.
This sounds like the login errors will work out the box - which isn't the case. So the message I'm trying to get across is: "If you want to use the AuthenticationUtils somewhere, you need to store this in the session here".
So in total:
- Let's change the list to sub-headings!
- This gives more room to really explain the session thing...
What do you think?
Page: https://symfony.com/doc/5.x/security/custom_authenticator.html
This line was really missing on this page:
I hope the code block formatting (inside this list) works. If not, the list could be changed to sub-headings. In preparation of this, I also moved the box about which class to extend upwards (to the other info about extending).