Skip to content

[Security] Custom Authenticator: Adding info about session #19813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 5.4
Choose a base branch
from
Open

[Security] Custom Authenticator: Adding info about session #19813

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions security/custom_authenticator.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@

Authenticators should implement the
:class:`Symfony\\Component\\Security\\Http\\Authenticator\\AuthenticatorInterface`.

.. 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`
Comment on lines +13 to +19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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."


You can also extend
:class:`Symfony\\Component\\Security\\Http\\Authenticator\\AbstractAuthenticator`,
which has a default implementation for the ``createToken()``
Expand Down Expand Up @@ -176,7 +185,12 @@

If ``null`` is returned, the request continues like normal. This is
useful for e.g. login forms, where the login controller is run again
with the login errors.
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;

Check failure on line 192 in security/custom_authenticator.rst

View workflow job for this annotation

GitHub Actions / Code Blocks 

[Missing class] Class, interface or trait with name "Symfony\Component\Security\Http\SecurityRequestAttributes" does not exist
Comment on lines +188 to +192
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

$request->getSession()->set(SecurityRequestAttributes::AUTHENTICATION_ERROR, $exception);

If you're using :ref:`login throttling <security-login-throttling>`,
you can check if ``$exception`` is an instance of
Expand All @@ -190,13 +204,6 @@
above. Use :class:`Symfony\\Component\\Security\\Core\\Exception\\CustomUserMessageAuthenticationException`
if you want to set custom error messages.

.. 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`

.. _security-passport:

Expand Down
Loading