-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Security] Use auth trust resolver to determine anonymous in ContextListener #18211
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.
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
[Security] Use auth trust resolver to determine anonymous in ContextListener #18211
Conversation
@@ -15,6 +15,8 @@ | |||
use Symfony\Component\HttpKernel\Event\GetResponseEvent; | |||
use Symfony\Component\HttpKernel\Event\FilterResponseEvent; | |||
use Symfony\Component\HttpKernel\KernelEvents; | |||
use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver; | |||
use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface; | |||
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is no longer used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, fixed
c2e4d54
to
a695
8000
652
Compare
@@ -121,7 +124,7 @@ public function onKernelResponse(FilterResponseEvent $event) | |||
$request = $event->getRequest(); | |||
$session = $request->getSession(); | |||
|
|||
if ((null === $token = $this->tokenStorage->getToken()) || ($token instanceof AnonymousToken)) { | |||
if ((null === $token = $this->tokenStorage->getToken()) || ($this->trustResolver->isAnonymous($token))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove one pair of parentheses here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -58,6 +60,7 @@ public function __construct(TokenStorageInterface $tokenStorage, array $userProv | |||
$this->sessionKey = '_security_'.$contextKey; | |||
$this->logger = $logger; | |||
$this->dispatcher = $dispatcher; | |||
$this->trustResolver = $trustResolver ?: new AuthenticationTrustResolver('Symfony\Component\Security\Core\Authentication\Token\AnonymousToken', 'Symfony\Component\Security\Core\Authentication\Token\RememberMeToken'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use ::class
constants here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree
👍 |
1 similar comment
👍 |
Thanks @wouterj |
…ous in ContextListener (WouterJ) This PR was squashed before being merged into the 3.1-dev branch (closes #18211). Discussion ---------- [Security] Use auth trust resolver to determine anonymous in ContextListener | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | not done yet There is a nice class in Symfony that is used to check whether a token is anonymously: `AuthenticationTrustResolver`. However, its logic was still hard coded in the `ContextListener`, making it impossible to customize it (e.g. using another anonymous token class). I think it makes lots of sense to use the dedicated class. Commits ------- ab5578e [Security] Use auth trust resolver to determine anonymous in ContextListener
This PR was merged into the 3.1-dev branch. Discussion ---------- use class constants instead of FQCN strings | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #18211 (comment) | License | MIT | Doc PR | Commits ------- d4ec7dd use class constants instead of FQCN strings
There is a nice class in Symfony that is used to check whether a token is anonymously:
AuthenticationTrustResolver
. However, its logic was still hard coded in theContextListener
, making it impossible to customize it (e.g. using another anonymous token class). I think it makes lots of sense to use the dedicated class.