[Security] [RememberMe] Add support for parallel requests doing remember-me re-authentication

src/Symfony/Bridge/Doctrine/CHANGELOG.md

@@ -8,6 +8,7 @@ CHANGELOG
  * Deprecate `DoctrineTestHelper` and `TestRepositoryFactory`
  * [BC BREAK] Remove `UuidV*Generator` classes
  * Add `UuidGenerator`
+ * Add support for the new security-core `TokenVerifierInterface` in `DoctrineTokenProvider`, fixing parallel requests handling in remember-me
 
 5.2.0
 -----

src/Symfony/Bridge/Doctrine/Security/RememberMe/DoctrineTokenProvider.php

@@ -19,6 +19,7 @@ use Doctrine\DBAL\Types\Types;
 use Symfony\Component\Security\Core\Authentication\RememberMe\PersistentToken;
 use Symfony\Component\Security\Core\Authentication\RememberMe\PersistentTokenInterface;
 use Symfony\Component\Security\Core\Authentication\RememberMe\TokenProviderInterface;
+use Symfony\Component\Security\Core\Authentication\RememberMe\TokenVerifierInterface;
 use Symfony\Component\Security\Core\Exception\TokenNotFoundException;
 
 /**
@@ -39,7 +40,7 @@ use Symfony\Component\Security\Core\Exception\TokenNotFoundException;
  *         `username` varchar(200) NOT NULL
  *     );
  */
-class DoctrineTokenProvider implements TokenProviderInterface
+class DoctrineTokenProvider implements TokenProviderInterface, TokenVerifierInterface
 {
     private $conn;
 
@@ -136,6 +137,65 @@ class DoctrineTokenProvider implements TokenProviderInterface
         }
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function verifyToken(PersistentTokenInterface $token, string $tokenValue): bool
+    {
+        // Check if the token value matches the current persisted token
+        if (hash_equals($token->getTokenValue(), $tokenValue)) {
+            return true;
+        }
+
+        // Generate an alternative series id here by changing the suffix == to _
+        // this is needed to be able to store an older token value in the database
+        // which has a PRIMARY(series), and it works as long as series ids are
+        // generated using base64_encode(random_bytes(64)) which always outputs
+        // a == suffix, but if it should not work for some reason we abort
+        // for safety
+        $tmpSeries = preg_replace('{=+$}', '_', $token->getSeries());
+        if ($tmpSeries === $token->getSeries()) {
+            return false;
+        }
+
+        // Check if the previous token is present. If the given $tokenValue
+        // matches the previous token (and it is outdated by at most 60seconds)
+        // we also accept it as a valid value.
+        try {
+            $tmpToken = $this->loadTokenBySeries($tmpSeries);
+        } catch (TokenNotFoundException $e) {
+            return false;
+        }
+
+        if ($tmpToken->getLastUsed()->getTimestamp() + 60 < time()) {
+            return false;
+        }
+
+        return hash_equals($tmpToken->getTokenValue(), $tokenValue);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function updateExistingToken(PersistentTokenInterface $token, string $tokenValue, \DateTimeInterface $lastUsed): void
+    {
+        if (!$token instanceof PersistentToken) {
+            return;
+        }
+
+        // Persist a copy of the previous token for authentication
+        // in verifyToken should the old token still be sent by the browser
+        // in a request concurrent to the one that did this token update
+        $tmpSeries = preg_replace('{=+$}', '_', $token->getSeries());
+        // if we cannot generate a unique series it is not worth trying further
+        if ($tmpSeries === $token->getSeries()) {
+            return;
+        }
+
+        $this->deleteTokenBySeries($tmpSeries);
+        $this->createNewToken(new PersistentToken($token->getClass(), $token->getUserIdentifier(), $tmpSeries, $token->getTokenValue(), $lastUsed));
+    }
+
     /**
      * Adds the Table to the Schema if "remember me" uses this Connection.
      */

src/Symfony/Bridge/Doctrine/Tests/Security/RememberMe/DoctrineTokenProviderTest.php

@@ -56,6 +56,56 @@ class DoctrineTokenProviderTest extends TestCase
         $provider->loadTokenBySeries('someSeries');
     }
 
+    public function testVerifyOutdatedTokenAfterParallelRequest()
+    {
+        $provider = $this->bootstrapProvider();
+        $series = base64_encode(random_bytes(64));
+        $oldValue = 'oldValue';
+        $newValue = 'newValue';
+
+        // setup existing token
+        $token = new PersistentToken('someClass', 'someUser', $series, $oldValue, new \DateTime('2013-01-26T18:23:51'));
+        $provider->createNewToken($token);
+
+        // new request comes in requiring remember-me auth, which updates the token
+        $provider->updateExistingToken($token, $newValue, new \DateTime('-5 seconds'));
+        $provider->updateToken($series, $newValue, new \DateTime('-5 seconds'));
+
+        // parallel request comes in with the old remember-me cookie and session, which also requires reauth
+        $token = $provider->loadTokenBySeries($series);
+        $this->assertEquals($newValue, $token->getTokenValue());
+
+        // new token is valid
+        $this->assertTrue($provider->verifyToken($token, $newValue));
+        // old token is still valid
+        $this->assertTrue($provider->verifyToken($token, $oldValue));
+    }
+
+    public function testVerifyOutdatedTokenAfterParallelRequestFailsAfter60Seconds()
+    {
+        $provider = $this->bootstrapProvider();
+        $series = base64_encode(random_bytes(64));
+        $oldValue = 'oldValue';
+        $newValue = 'newValue';
+
+        // setup existing token
+        $token = new PersistentToken('someClass', 'someUser', $series, $oldValue, new \DateTime('2013-01-26T18:23:51'));
+        $provider->createNewToken($token);
+
+        // new request comes in requiring remember-me auth, which updates the token
+        $provider->updateExistingToken($token, $newValue, new \DateTime('-61 seconds'));
+        $provider->updateToken($series, $newValue, new \DateTime('-5 seconds'));
+
+        // parallel request comes in with the old remember-me cookie and session, which also requires reauth
+        $token = $provider->loadTokenBySeries($series);
+        $this->assertEquals($newValue, $token->getTokenValue());
+
+        // new token is valid
+        $this->assertTrue($provider->verifyToken($token, $newValue));
+        // old token is not valid anymore after 60 seconds
+        $this->assertFalse($provider->verifyToken($token, $oldValue));
+    }
+
     /**
      * @return DoctrineTokenProvider
      */

src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/CleanRememberMeVerifierPass.php

@@ -0,0 +1,33 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+/**
+ * Cleans up the remember me verifier cache if cache is missing.
+ *
+ * @author Jordi Boggiano <j.boggiano@seld.be>
+ */
+class CleanRememberMeVerifierPass implements CompilerPassInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function process(ContainerBuilder $container)
+    {
+        if (!$container->hasDefinition('cache.system')) {
+            $container->removeDefinition('cache.security_token_verifier');
+        }
+    }
+}

src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php

@@ -19,10 +19,12 @@ use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
 use Symfony\Component\DependencyInjection\ChildDefinition;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\HttpFoundation\Cookie;
+use Symfony\Component\Security\Core\Authentication\RememberMe\CacheTokenVerifier;
 use Symfony\Component\Security\Http\EventListener\RememberMeLogoutListener;
 
 /**
@@ -116,10 +118,12 @@ class RememberMeFactory implements SecurityFactoryInterface, AuthenticatorFactor
                 ->addTag('security.remember_me_handler', ['firewall' => $firewallName]);
         } elseif (isset($config['token_provider'])) {
             $tokenProviderId = $this->createTokenProvider($container, $firewallName, $config['token_provider']);
+            $tokenVerifier = $this->createTokenVerifier($container, $firewallName, $config['token_verifier'] ?? null);
             $container->setDefinition($rememberMeHandlerId, new ChildDefinition('security.authenticator.persistent_remember_me_handler'))
                 ->replaceArgument(0, new Reference($tokenProviderId))
                 ->replaceArgument(2, new Reference($userProviderId))
                 ->replaceArgument(4, $config)
+                ->replaceArgument(6, $tokenVerifier)
                 ->addTag('security.remember_me_handler', ['firewall' => $firewallName]);
         } else {
             $signatureHasherId = 'security.authenticator.remember_me_signature_hasher.'.$firewallName;
@@ -214,6 +218,9 @@ class RememberMeFactory implements SecurityFactoryInterface, AuthenticatorFactor
                         ->end()
                     ->end()
                 ->end()
+            ->end()
+            ->scalarNode('token_verifier')
+                ->info('The service ID of a custom rememberme token verifier.')
             ->end();
 
         foreach ($this->options as $name => $value) {
@@ -304,4 +311,20 @@ class RememberMeFactory implements SecurityFactoryInterface, AuthenticatorFactor
 
         return $tokenProviderId;
     }
+
+    private function createTokenVerifier(ContainerBuilder $container, string $firewallName, ?string $serviceId): Reference
+    {
+        if ($serviceId) {
+            return new Reference($serviceId);
+        }
+
+        $tokenVerifierId = 'security.remember_me.token_verifier.'.$firewallName;
+
+        $container->register($tokenVerifierId, CacheTokenVerifier::class)
+            ->addArgument(new Reference('cache.security_token_verifier', ContainerInterface::NULL_ON_INVALID_REFERENCE))
+            ->addArgument(60)
+            ->addArgument('rememberme-'.$firewallName.'-stale-');
+
+        return new Reference($tokenVerifierId, ContainerInterface::NULL_ON_INVALID_REFERENCE);
+    }
 }

src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd

@@ -350,6 +350,7 @@
         <xsd:attribute name="secret" type="xsd:string" use="required" />
         <xsd:attribute name="service" type="xsd:string" />
         <xsd:attribute name="token-provider" type="xsd:string" />
+        <xsd:attribute name="token-verifier" type="xsd:string" />
         <xsd:attribute name="catch-exceptions" type="xsd:boolean" />
         <xsd:attribute name="secure" type="remember_me_secure" />
         <xsd:attribute name="samesite" type="remember_me_samesite" />

src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_remember_me.php

@@ -51,6 +51,7 @@ return static function (ContainerConfigurator $container) {
                 service('request_stack'),
                 abstract_arg('options'),
                 service('logger')->nullOnInvalid(),
+                abstract_arg('token verifier'),
             ])
             ->tag('monolog.logger', ['channel' => 'security'])
 
@@ -87,5 +88,11 @@ return static function (ContainerConfigurator $container) {
                 service('logger')->nullOnInvalid(),
             ])
             ->tag('monolog.logger', ['channel' => 'security'])
+
+        // Cache
+        ->set('cache.security_token_verifier')
+            ->parent('cache.system')
+            ->private()
+            ->tag('cache.pool')
     ;
 };

src/Symfony/Bundle/SecurityBundle/SecurityBundle.php

@@ -14,6 +14,7 @@ namespace Symfony\Bundle\SecurityBundle;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
+use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\CleanRememberMeVerifierPass;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterCsrfFeaturesPass;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterEntryPointPass;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterGlobalSecurityEventListenersPass;
@@ -76,6 +77,7 @@ class SecurityBundle extends Bundle
         $container->addCompilerPass(new AddExpressionLanguageProvidersPass());
         $container->addCompilerPass(new AddSecurityVotersPass());
         $container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_BEFORE_REMOVING);
+        $container->addCompilerPass(new CleanRememberMeVerifierPass());
         $container->addCompilerPass(new RegisterCsrfFeaturesPass());
         $container->addCompilerPass(new RegisterTokenUsageTrackingPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 200);
         $container->addCompilerPass(new RegisterLdapLocatorPass());

src/Symfony/Component/Security/CHANGELOG.md

@@ -35,6 +35,8 @@ The CHANGELOG for version 5.4 and newer can be found in the security sub-package
  * Randomize CSRF tokens to harden BREACH attacks
  * Deprecated voters that do not return a valid decision when calling the `vote` method.
  * Flag `Serializable` implementation of `NullToken` as `@internal` and `@final`
+ * Add `TokenVerifierInterface` to allow fixing parallel requests handling in remember-me
+ * Add a `CacheTokenVerifier` implementation that stores outdated token in a cache, which is more correct and efficient as the default `DoctrineTokenProvider` implementation
 
 5.2.0
 -----

src/Symfony/Component/Security/Core/Authentication/RememberMe/CacheTokenVerifier.php

@@ -0,0 +1,68 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Core\Authentication\RememberMe;
+
+use Psr\Cache\CacheItemPoolInterface;
+
+/**
+ * @author Jordi Boggiano <j.boggiano@seld.be>
+ */
+class CacheTokenVerifier implements TokenVerifierInterface
+{
+    private $cache;
+    private $outdatedTokenTtl;
+    private $cacheKeyPrefix;
+
+    /**
+     * @param int $outdatedTokenTtl How long the outdated token should still be considered valid. Defaults
+     *                              to 60, which matches how often the PersistentRememberMeHandler will at
+     *                              most refresh tokens. Increasing to more than that is not recommended,
+     *                              but you may use a lower value.
+     */
+    public function __construct(CacheItemPoolInterface $cache, int $outdatedTokenTtl = 60, string $cacheKeyPrefix = 'rememberme-stale-')
+    {
+        $this->cache = $cache;
+        $this->outdatedTokenTtl = $outdatedTokenTtl;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function verifyToken(PersistentTokenInterface $token, string $tokenValue): bool
+    {
+        if (hash_equals($token->getTokenValue(), $tokenValue)) {
+            return true;
+        }
+
+        if (!$this->cache->hasItem($this->cacheKeyPrefix.$token->getSeries())) {
+            return false;
+        }
+
+        $item = $this->cache->getItem($this->cacheKeyPrefix.$token->getSeries());
+        $outdatedToken = $item->get();
+
+        return hash_equals($outdatedToken, $tokenValue);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function updateExistingToken(PersistentTokenInterface $token, string $tokenValue, \DateTimeInterface $lastUsed): void
+    {
+        // When a token gets updated, persist the outdated token for $outdatedTokenTtl seconds so we can
+        // still accept it as valid in verifyToken
+        $item = $this->cache->getItem($this->cacheKeyPrefix.$token->getSeries());
+        $item->set($token->getTokenValue());
+        $item->expiresAfter($this->outdatedTokenTtl);
+        $this->cache->save($item);
+    }
+}

src/Symfony/Component/Security/Core/Authentication/RememberMe/TokenVerifierInterface.php

@@ -0,0 +1,32 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Core\Authentication\RememberMe;
+
+/**
+ * @author Jordi Boggiano <j.boggiano@seld.be>
+ */
+interface TokenVerifierInterface
+{
+    /**
+     * Verifies that the given $token is valid.
+     *
+     * This lets you override the token check logic to for example accept slightly outdated tokens.
+     *
+     * Do not forget to implement token comparisons using hash_equals for a secure implementation.
+     */
+    public function verifyToken(PersistentTokenInterface $token, string $tokenValue): bool;
+
+    /**
+     * Updates an existing token with a new token value and lastUsed time.
+     */
+    public function updateExistingToken(PersistentTokenInterface $token, string $tokenValue, \DateTimeInterface $lastUsed): void;
+}

src/Symfony/Component/Security/Core/Tests/Authentication/RememberMe/CacheTokenVerifierTest.php

@@ -0,0 +1,43 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Core\Tests\Authentication\RememberMe;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Cache\Adapter\ArrayAdapter;
+use Symfony\Component\Security\Core\Authentication\RememberMe\CacheTokenVerifier;
+use Symfony\Component\Security\Core\Authentication\RememberMe\PersistentToken;
+
+class CacheTokenVerifierTest extends TestCase
+{
+    public function testVerifyCurrentToken()
+    {
+        $verifier = new CacheTokenVerifier(new ArrayAdapter());
+        $token = new PersistentToken('class', 'user', 'series1', 'value', new \DateTime());
+        $this->assertTrue($verifier->verifyToken($token, 'value'));
+    }
+
+    public function testVerifyFailsInvalidToken()
+    {
+        $verifier = new CacheTokenVerifier(new ArrayAdapter());
+        $token = new PersistentToken('class', 'user', 'series1', 'value', new \DateTime());
+        $this->assertFalse($verifier->verifyToken($token, 'wrong-value'));
+    }
+
+    public function testVerifyOutdatedToken()
+    {
+        $verifier = new CacheTokenVerifier(new ArrayAdapter());
+        $outdatedToken = new PersistentToken('class', 'user', 'series1', 'value', new \DateTime());
+        $newToken = new PersistentToken('class', 'user', 'series1', 'newvalue', new \DateTime());
+        $verifier->updateExistingToken($outdatedToken, 'newvalue', new \DateTime());
+        $this->assertTrue($verifier->verifyToken($newToken, 'value'));
+    }
+}

src/Symfony/Component/Security/Core/composer.json

@@ -25,6 +25,8 @@
     },
     "require-dev": {
         "psr/container": "^1.0|^2.0",
+        "psr/cache": "^1.0|^2.0|^3.0",
+        "symfony/cache": "^4.4|^5.0",
         "symfony/event-dispatcher": "^4.4|^5.0",
         "symfony/expression-language": "^4.4|^5.0",
         "symfony/http-foundation": "^5.3",

src/Symfony/Component/Security/Http/RememberMe/PersistentRememberMeHandler.php

@@ -15,6 +15,7 @@ use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Security\Core\Authentication\RememberMe\PersistentToken;
 use Symfony\Component\Security\Core\Authentication\RememberMe\TokenProviderInterface;
+use Symfony\Component\Security\Core\Authentication\RememberMe\TokenVerifierInterface;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Core\Exception\CookieTheftException;
 use Symfony\Component\Security\Core\User\UserInterface;
@@ -32,13 +33,18 @@ use Symfony\Component\Security\Core\User\UserProviderInterface;
 final class PersistentRememberMeHandler extends AbstractRememberMeHandler
 {
     private $tokenProvider;
+    private $tokenVerifier;
     private $secret;
 
-    public function __construct(TokenProviderInterface $tokenProvider, string $secret, UserProviderInterface $userProvider, RequestStack $requestStack, array $options, ?LoggerInterface $logger = null)
+    public function __construct(TokenProviderInterface $tokenProvider, string $secret, UserProviderInterface $userProvider, RequestStack $requestStack, array $options, ?LoggerInterface $logger = null, ?TokenVerifierInterface $tokenVerifier = null)
     {
         parent::__construct($userProvider, $requestStack, $options, $logger);
 
+        if (!$tokenVerifier && $tokenProvider instanceof TokenVerifierInterface) {
+            $tokenVerifier = $tokenProvider;
+        }
         $this->tokenProvider = $tokenProvider;
+        $this->tokenVerifier = $tokenVerifier;
         $this->secret = $secret;
     }
 
@@ -66,7 +72,13 @@ final class PersistentRememberMeHandler extends AbstractRememberMeHandler
 
         [$series, $tokenValue] = explode(':', $rememberMeDetails->getValue());
         $persistentToken = $this->tokenProvider->loadTokenBySeries($series);
-        if (!hash_equals($persistentToken->getTokenValue(), $tokenValue)) {
+
+        if ($this->tokenVerifier) {
+            $isTokenValid = $this->tokenVerifier->verifyToken($persistentToken, $tokenValue);
+        } else {
+            $isTokenValid = hash_equals($persistentToken->getTokenValue(), $tokenValue);
+        }
+        if (!$isTokenValid) {
             throw new CookieTheftException('This token was already used. The account is possibly compromised.');
         }
 
@@ -78,7 +90,12 @@ final class PersistentRememberMeHandler extends AbstractRememberMeHandler
         // if multiple concurrent requests reauthenticate a user we do not want to update the token several times
         if ($persistentToken->getLastUsed()->getTimestamp() + 60 < time()) {
             $tokenValue = base64_encode(random_bytes(64));
-            $this->tokenProvider->updateToken($series, $this->generateHash($tokenValue), new \DateTime());
+            $tokenValueHash = $this->generateHash($tokenValue);
+            $tokenLastUsed = new \DateTime();
+            if ($this->tokenVerifier) {
+                $this->tokenVerifier->updateExistingToken($persistentToken, $tokenValueHash, $tokenLastUsed);
+            }
+            $this->tokenProvider->updateToken($series, $tokenValueHash, $tokenLastUsed);
         }
 
         $this->createCookie($rememberMeDetails->withValue($tokenValue));