8000 bug #42776 [Security] Do not deauthenticate token on user change if n… · symfony/symfony@3c40300 · GitHub
[go: up one dir, main page]
Skip to content

Commit 3c40300

Browse files
fabpotfabpot
committed
bug #42776 [Security] Do not deauthenticate token on user change if not an AbstractToken (chalasr)
This PR was merged into the 5.4 branch. Discussion ---------- [Security] Do not deauthenticate token on user change if not an AbstractToken | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - In #42050, we moved the `hasUserChanged()` logic used for deauthentication from AbstractToken to ContextListener. Problem is that this check is now done against on all kind of tokens, whereas it was only for `AbstractToken` instances before. That breaks https://github.com/scheb/2fa, tokens get wrongly deauthenticated in the middle of the 2fa auth process. This fixes it by skipping non-AbstractToken implementations. We may want to provide a way to opt-in/out the `hasUserChanged()` logic on a custom token with e.g. a marker interface, but that's not necessarily worth it for now IMHO. Commits ------- fe31fcb [Security] Do not deauthenticate token on user change if not an AbstractToken
2 parents 7b52f24 + fe31fcb commit 3c40300

File tree

2 files changed

+125
-1
lines changed

2 files changed

+125
-1
lines changed

‎src/Symfony/Component/Security/Http/Firewall/ContextListener.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Symfony\Component\HttpKernel\KernelEvents;
2222
use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver;
2323
use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface;
24+
use Symfony\Component\Security\Core\Authentication\Token\AbstractToken;
2425
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
2526
use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken;
2627
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
@@ -237,7 +238,7 @@ protected function refreshUser(TokenInterface $token): ?TokenInterface
237238
$newToken->setUser($refreshedUser, false);
238239

239240
// tokens can be deauthenticated if the user has been changed.
240-
if ($this->hasUserChanged($user, $newToken)) {
241+
if ($token instanceof AbstractToken && $this->hasUserChanged($user, $newToken)) {
241242
$userDeauthenticated = true;
242243
// @deprecated since Symfony 5.4
243244
if (method_exists($newToken, 'setAuthenticated')) {

‎src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
3030
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
3131
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage;
32+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
3233
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
3334
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
3435
use Symfony\Component\Security\Core\Exception\UserNotFoundException;
@@ -233,6 +234,27 @@ public function testIfTokenIsDeauthenticated()
233234
$this->assertNull($tokenStorage->getToken());
234235
}
235236

237+
public function testTokenIsNotDeauthenticatedOnUserChangeIfNotAnInstanceOfAbstractToken()
238+
{
239+
$tokenStorage = new TokenStorage();
240+
$refreshedUser = new InMemoryUser('changed', 'baz');
241+
242+
$token = new CustomToken(new InMemoryUser('original', 'foo'), ['ROLE_FOO']);
243+
244+
$session = new Session(new MockArraySessionStorage());
245+
$session->set('_security_context_key', serialize($token));
246+
247+
$request = new Request();
248+
$request->setSession($session);
249+
$request->cookies->set('MOCKSESSID', true);
250+
251+
$listener = new ContextListener($tokenStorage, [new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider($refreshedUser)], 'context_key');
252+
$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST));
253+
254+
$this->assertInstanceOf(CustomToken::class, $tokenStorage->getToken());
255+
$this->assertSame($refreshedUser, $tokenStorage->getToken()->getUser());
256+
}
257+
236258
public function testIfTokenIsNotDeauthenticated()
237259
{
238260
$tokenStorage = new TokenStorage();
@@ -523,3 +545,104 @@ public function supportsClass($class): bool
523545
return InMemoryUser::class === $class;
524546
}
525547
}
548+
549+
class CustomToken implements TokenInterface
550+
{
551+
private $user;
552+
private $roles;
553+
554+
public function __construct(UserInterface $user, array $roles)
555+
{
556+
$this->user = $user;
557+
$this->roles = $roles;
558+
}
559+
560+
public function __serialize(): array
561+
{
562+
return [$this->user, $this->roles];
563+
}
564+
565+
public function serialize(): string
566+
{
567+
return serialize($this->__serialize());
568+
}
569+
570+
public function __unserialize(array $data): void
571+
{
572+
[$this->user, $this->roles] = $data;
573+
}
574+
575+
public function unserialize($serialized)
576+
{
577+
$this->__unserialize(\is_array($serialized) ? $serialized : unserialize($serialized));
578+
}
579+
580+
public function __toString(): string
581+
{
582+
return $this->user->getUserIdentifier();
583+
}
584+
585+
public function getRoleNames(): array
586+
{
587+
return $this->roles;
588+
}
589+
590+
public function getCredentials()
591+
{
592+
}
593+
594+
public function getUser(): UserInterface
595+
{
596+
return $this->user;
597+
}
598+
599+
public function setUser($user)
600+
{
601+
$this->user = $user;
602+
}
603+
604+
public function getUsername(): string
605+
{
606+
return $this->user->getUserIdentifier();
607+
}
608+
609+
public function getUserIdentifier(): string
610+
{
611+
return $this->getUserIdentifier();
612+
}
613+
614+
public function isAuthenticated(): bool
615+
{
616+
return true;
617+
}
618+
619+
public function setAuthenticated(bool $isAuthenticated)
620+
{
621+
}
622+
623+
public function eraseCredentials()
624+
{
625+
}
626+
627+
public function getAttributes(): array
628+
{
629+
return [];
630+
}
631+
632+
public function setAttributes(array $attributes)
633+
{
634+
}
635+
636+
public function hasAttribute(string $name): bool
637+
{
638+
return false;
639+
}
640+
641+
public function getAttribute(string $name)
642+
{
643+
}
644+
645+
public function setAttribute(string $name, $value)
646+
{
647+
}
648+
}

0 commit comments

Comments
 (0)
0