Add option to automatically set trusted device cookie when remember-m…

…e is activated (#66)

doc/configuration.rst

@@ -118,6 +118,7 @@ Firewall Configuration
                                                          # (supports symfony/property-access notation for nested values)
                    trusted_parameter_name: _trusted      # Name of the parameter for the trusted device option
                                                          # (supports symfony/property-access notation for nested values)
+                   remember_me_sets_trusted: false       # If remember-me option should also set the trusted device cookie
                    multi_factor: false                   # If ALL active two-factor methods need to be fulfilled
                                                          # (multi-factor authentication)
                    success_handler: acme.custom_success_handler  # Use a custom success handler instead of the default one

src/bundle/DependencyInjection/Factory/Security/TwoFactorFactory.php

@@ -26,6 +26,7 @@ class TwoFactorFactory implements SecurityFactoryInterface, FirewallListenerFact
     public const DEFAULT_TARGET_PATH = '/';
     public const DEFAULT_AUTH_CODE_PARAMETER_NAME = '_auth_code';
     public const DEFAULT_TRUSTED_PARAMETER_NAME = '_trusted';
+    public const DEFAULT_REMEMBER_ME_SETS_TRUSTED = false;
     public const DEFAULT_MULTI_FACTOR = false;
     public const DEFAULT_PREPARE_ON_LOGIN = false;
     public const DEFAULT_PREPARE_ON_ACCESS_DENIED = false;
@@ -88,6 +89,7 @@ public function addConfiguration(NodeDefinition $builder): void
                 ->scalarNode('authentication_required_handler')->defaultNull()->end()
                 ->scalarNode('auth_code_parameter_name')->defaultValue(self::DEFAULT_AUTH_CODE_PARAMETER_NAME)->end()
                 ->scalarNode('trusted_parameter_name')->defaultValue(self::DEFAULT_TRUSTED_PARAMETER_NAME)->end()
+                ->scalarNode('remember_me_sets_trusted')->defaultValue(self::DEFAULT_REMEMBER_ME_SETS_TRUSTED)->end()
                 ->booleanNode('multi_factor')->defaultValue(self::DEFAULT_MULTI_FACTOR)->end()
                 ->booleanNode('prepare_on_login')->defaultValue(self::DEFAULT_PREPARE_ON_LOGIN)->end()
                 ->booleanNode('prepare_on_access_denied')->defaultValue(self::DEFAULT_PREPARE_ON_ACCESS_DENIED)->end()

src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php

@@ -122,15 +122,23 @@ public function authenticate(Request $request): PassportInterface
             $passport->addBadge(new CsrfTokenBadge($tokenId, $tokenValue));
         }
 
-        if ($this->twoFactorFirewallConfig->hasTrustedDeviceParameterInRequest($request)
-            && class_exists(TrustedDeviceBadge::class) // Make sure the package is installed
-        ) {
+        // Make sure the trusted device package is installed
+        if (class_exists(TrustedDeviceBadge::class) && $this->shouldSetTrustedDevice($request, $passport)) {
             $passport->addBadge(new TrustedDeviceBadge());
         }
 
         return $passport;
     }
 
+    private function shouldSetTrustedDevice(Request $request, TwoFactorPassport $passport): bool
+    {
+        return $this->twoFactorFirewallConfig->hasTrustedDeviceParameterInRequest($request)
+            || (
+                $this->twoFactorFirewallConfig->isRememberMeSetsTrusted()
+                && $passport->hasBadge(RememberMeBadge::class)
+            );
+    }
+
     public function createAuthenticatedToken(PassportInterface $passport, string $firewallName): TokenInterface
     {
         /** @var TwoFactorPassport $passport */

src/bundle/Security/Http/Firewall/TwoFactorListener.php

@@ -188,8 +188,8 @@ private function onSuccess(Request $request, TokenInterface $token, TwoFactorTok
         $this->dispatchTwoFactorAuthenticationEvent(TwoFactorAuthenticationEvents::COMPLETE, $request, $token);
 
         $firewallName = $previousTwoFactorToken->getProviderKey(true);
-        if ($this->trustedDeviceManager
-            && $this->twoFactorFirewallConfig->hasTrustedDeviceParameterInRequest($request)
+        if ($this->trustedDeviceManager // Make sure the trusted device package is installed
+            && $this->shouldSetTrustedDevice($request, $previousTwoFactorToken)
             && $this->trustedDeviceManager->canSetTrustedDevice($token->getUser(), $request, $firewallName)
         ) {
             $this->trustedDeviceManager->addTrustedDevice($token->getUser(), $firewallName);
@@ -201,6 +201,15 @@ private function onSuccess(Request $request, TokenInterface $token, TwoFactorTok
         return $response;
     }
 
+    private function shouldSetTrustedDevice(Request $request, TwoFactorTokenInterface $previousTwoFactorToken): bool
+    {
+        return $this->twoFactorFirewallConfig->hasTrustedDeviceParameterInRequest($request)
+            || (
+                $this->twoFactorFirewallConfig->isRememberMeSetsTrusted()
+                && $previousTwoFactorToken->hasAttribute(TwoFactorTokenInterface::ATTRIBUTE_NAME_REMEMBER_ME_COOKIE)
+            );
+    }
+
     private function dispatchTwoFactorAuthenticationEvent(string $eventType, Request $request, TokenInterface $token): void
     {
         $event = new TwoFactorAuthenticationEvent($request, $token);

src/bundle/Security/TwoFactor/TwoFactorFirewallConfig.php

@@ -67,6 +67,11 @@ public function getTrustedParameterName(): string
         return $this->options['trusted_parameter_name'] ?? TwoFactorFactory::DEFAULT_TRUSTED_PARAMETER_NAME;
     }
 
+    public function isRememberMeSetsTrusted(): bool
+    {
+        return $this->options['remember_me_sets_trusted'] ?? TwoFactorFactory::DEFAULT_REMEMBER_ME_SETS_TRUSTED;
+    }
+
     public function isCsrfProtectionEnabled(): bool
     {
         return $this->options['enable_csrf'] ?? TwoFactorFactory::DEFAULT_ENABLE_CSRF;

tests/DependencyInjection/Factory/Security/TwoFactorFactoryTest.php

@@ -73,6 +73,7 @@ private function getFullConfig(): array
     authentication_required_handler: my_authentication_required_handler
     auth_code_parameter_name: auth_code_param_name
     trusted_parameter_name: trusted_param_name
+    remember_me_sets_trusted: true
     multi_factor: true
     prepare_on_login: true
     prepare_on_access_denied: true
@@ -220,6 +221,7 @@ public function addConfiguration_emptyConfig_setDefaultValues(): void
         $this->assertNull($processedConfiguration['authentication_required_handler']);
         $this->assertEquals(TwoFactorFactory::DEFAULT_AUTH_CODE_PARAMETER_NAME, $processedConfiguration['auth_code_parameter_name']);
         $this->assertEquals(TwoFactorFactory::DEFAULT_TRUSTED_PARAMETER_NAME, $processedConfiguration['trusted_parameter_name']);
+        $this->assertEquals(TwoFactorFactory::DEFAULT_REMEMBER_ME_SETS_TRUSTED, $processedConfiguration['remember_me_sets_trusted']);
         $this->assertEquals(TwoFactorFactory::DEFAULT_MULTI_FACTOR, $processedConfiguration['multi_factor']);
         $this->assertEquals(TwoFactorFactory::DEFAULT_PREPARE_ON_LOGIN, $processedConfiguration['prepare_on_login']);
         $this->assertEquals(TwoFactorFactory::DEFAULT_PREPARE_ON_ACCESS_DENIED, $processedConfiguration['prepare_on_access_denied']);
@@ -246,6 +248,7 @@ public function addConfiguration_fullConfig_setConfigValues(): void
         $this->assertEquals('my_authentication_required_handler', $processedConfiguration['authentication_required_handler']);
         $this->assertEquals('auth_code_param_name', $processedConfiguration['auth_code_parameter_name']);
         $this->assertEquals('trusted_param_name', $processedConfiguration['trusted_parameter_name']);
+        $this->assertEquals(true, $processedConfiguration['remember_me_sets_trusted']);
         $this->assertTrue($processedConfiguration['multi_factor']);
         $this->assertTrue($processedConfiguration['prepare_on_login']);
         $this->assertTrue($processedConfiguration['prepare_on_access_denied']);

tests/Security/Http/Authenticator/TwoFactorAuthenticatorTest.php

@@ -140,6 +140,14 @@ private function stubRequestHasTrustedDeviceParameter(bool $hasTrustedDevicePara
             ->willReturn($hasTrustedDeviceParam);
     }
 
+    private function stubRememberMeSetsTrusted(bool $rememberMeSetsTrusted): void
+    {
+        $this->twoFactorFirewallConfig
+            ->expects($this->any())
+            ->method('isRememberMeSetsTrusted')
+            ->willReturn($rememberMeSetsTrusted);
+    }
+
     /**
      * @return MockObject|TwoFactorTokenInterface
      */
@@ -331,6 +339,7 @@ public function authenticate_csrfEnabled_csrfBadgeAdded(): void
     public function authenticate_trustedDeviceParameterNotSet_noTrustedDeviceBadge(): void
     {
         $this->stubRequestHasTrustedDeviceParameter(false);
+        $this->stubRememberMeSetsTrusted(false);
         $this->stubTokenStorageHasTwoFactorToken();
 
         $returnValue = $this->authenticator->authenticate($this->request);
@@ -344,13 +353,33 @@ public function authenticate_trustedDeviceParameterNotSet_noTrustedDeviceBadge()
     public function authenticate_trustedDeviceParameterSet_addTrustedDeviceBadge(): void
     {
         $this->stubRequestHasTrustedDeviceParameter(true);
+        $this->stubRememberMeSetsTrusted(false);
         $this->stubTokenStorageHasTwoFactorToken();
 
         $returnValue = $this->authenticator->authenticate($this->request);
         $this->assertInstanceOf(TwoFactorPassport::class, $returnValue);
         $this->assertTrue($returnValue->hasBadge(TrustedDeviceBadge::class));
     }
 
+    /**
+     * @test
+     */
+    public function authenticate_rememberMeSetsTrustedWithRememberMeEnabled_addTrustedDeviceBadge(): void
+    {
+        $this->stubRequestHasTrustedDeviceParameter(false);
+        $this->stubRememberMeSetsTrusted(true);
+        $twoFactorToken = $this->stubTokenStorageHasTwoFactorToken();
+        $twoFactorToken
+            ->expects($this->any())
+            ->method('hasAttribute')
+            ->with(TwoFactorTokenInterface::ATTRIBUTE_NAME_USE_REMEMBER_ME)
+            ->willReturn(true);
+
+        $returnValue = $this->authenticator->authenticate($this->request);
+        $this->assertInstanceOf(TwoFactorPassport::class, $returnValue);
+        $this->assertTrue($returnValue->hasBadge(TrustedDeviceBadge::class));
+    }
+
     /**
      * @test
      */

tests/Security/Http/Firewall/TwoFactorListenerTest.php

@@ -201,6 +201,30 @@ private function createTwoFactorToken($firewallName = self::FIREWALL_NAME, $auth
         return $twoFactorToken;
     }
 
+    /**
+     * @return MockObject|TokenInterface
+     */
+    private function createTwoFactorTokenWithRememberMeCookie(Cookie $rememberMeCookie): MockObject
+    {
+        $attributes = [TwoFactorTokenInterface::ATTRIBUTE_NAME_REMEMBER_ME_COOKIE => [$rememberMeCookie]];
+
+        return $this->createTwoFactorToken(self::FIREWALL_NAME, null, $attributes);
+    }
+
+    /**
+     * @return MockObject|TokenInterface
+     */
+    private function createAuthenticatedToken(string $user): MockObject
+    {
+        $authenticatedToken = $this->createMock(TokenInterface::class);
+        $authenticatedToken
+            ->expects($this->any())
+            ->method('getUser')
+            ->willReturn($user);
+
+        return $authenticatedToken;
+    }
+
     private function stubTokenStorageHasToken(TokenInterface $token): void
     {
         $this->tokenStorage
@@ -227,6 +251,14 @@ private function stubRequestHasTrustedParameter(bool $hasTrustedParam): void
             ->willReturn($hasTrustedParam);
     }
 
+    private function stubRememberMeSetsTrusted(bool $rememberMeSetsTrusted): void
+    {
+        $this->twoFactorFirewallConfig
+            ->expects($this->any())
+            ->method('isRememberMeSetsTrusted')
+            ->willReturn($rememberMeSetsTrusted);
+    }
+
     private function stubHandlersReturnResponse(): Response
     {
         $response = new Response();
@@ -537,15 +569,38 @@ public function authenticate_twoFactorProcessComplete_dispatchCompleteEvent(): v
      */
     public function authenticate_twoFactorProcessCompleteWithTrustedEnabled_setTrustedDevice(): void
     {
-        $authenticatedToken = $this->createMock(TokenInterface::class);
-        $authenticatedToken
-            ->expects($this->any())
-            ->method('getUser')
-            ->willReturn('user');
+        $authenticatedToken = $this->createAuthenticatedToken('user');
 
         $this->stubTokenStorageHasToken($this->createTwoFactorToken());
         $this->stubCsrfTokenIsValid();
         $this->stubRequestHasTrustedParameter(true);
+        $this->stubRememberMeSetsTrusted(false);
+        $this->stubCanSetTrustedDevice(true);
+        $this->stubAuthenticationManagerReturnsToken($authenticatedToken); // Not a TwoFactorToken
+        $this->stubHandlersReturnResponse();
+
+        $this->trustedDeviceManager
+            ->expects($this->once())
+            ->method('addTrustedDevice')
+            ->with('user', 'firewallName');
+
+        $this->listener->authenticate($this->requestEvent);
+    }
+
+    /**
+     * @test
+     */
+    public function authenticate_twoFactorProcessCompleteWithRememberMeSetsTrusted_setTrustedDevice(): void
+    {
+        $authenticatedToken = $this->createAuthenticatedToken('user');
+
+        $rememberMeCookie = new Cookie('remember_me', 'value');
+        $twoFactorToken = $this->createTwoFactorTokenWithRememberMeCookie($rememberMeCookie);
+
+        $this->stubTokenStorageHasToken($twoFactorToken);
+        $this->stubCsrfTokenIsValid();
+        $this->stubRequestHasTrustedParameter(false);
+        $this->stubRememberMeSetsTrusted(true);
         $this->stubCanSetTrustedDevice(true);
         $this->stubAuthenticationManagerReturnsToken($authenticatedToken); // Not a TwoFactorToken
         $this->stubHandlersReturnResponse();
@@ -572,6 +627,7 @@ public function authenticate_twoFactorProcessCompleteTrustedDeviceNotAllowed_not
         $this->stubTokenStorageHasToken($this->createTwoFactorToken());
         $this->stubCsrfTokenIsValid();
         $this->stubRequestHasTrustedParameter(true);
+        $this->stubRememberMeSetsTrusted(false);
         $this->stubCanSetTrustedDevice(false);
         $this->stubAuthenticationManagerReturnsToken($authenticatedToken); // Not a TwoFactorToken
         $this->stubHandlersReturnResponse();
@@ -591,6 +647,8 @@ public function authenticate_twoFactorProcessCompleteWithTrustedDisabled_notSetT
         $this->stubTokenStorageHasToken($this->createTwoFactorToken());
         $this->stubCsrfTokenIsValid();
         $this->stubRequestHasTrustedParameter(false);
+        $this->stubRememberMeSetsTrusted(false);
+        $this->stubCanSetTrustedDevice(true);
         $this->stubAuthenticationManagerReturnsToken($this->createMock(TokenInterface::class)); // Not a TwoFactorToken
         $this->stubHandlersReturnResponse();
 
@@ -613,8 +671,7 @@ public function authenticate_twoFactorProcessCompleteWithRememberMeEnabled_setRe
             ->willReturn('user');
 
         $rememberMeCookie = new Cookie('remember_me', 'value');
-        $attributes = [TwoFactorTokenInterface::ATTRIBUTE_NAME_REMEMBER_ME_COOKIE => [$rememberMeCookie]];
-        $twoFactorToken = $this->createTwoFactorToken(self::FIREWALL_NAME, null, $attributes);
+        $twoFactorToken = $this->createTwoFactorTokenWithRememberMeCookie($rememberMeCookie);
 
         $this->stubTokenStorageHasToken($twoFactorToken);
         $this->stubCsrfTokenIsValid();

tests/Security/TwoFactor/TwoFactorFirewallConfigTest.php

@@ -21,6 +21,7 @@ class TwoFactorFirewallConfigTest extends TestCase
         'multi_factor' => true,
         'auth_code_parameter_name' => 'auth_code_param',
         'trusted_parameter_name' => 'trusted_param',
+        'remember_me_sets_trusted' => true,
         'enable_csrf' => true,
         'csrf_parameter' => 'parameter_name',
         'csrf_token_id' => 'token_id',
@@ -112,6 +113,33 @@ public function getTrustedParameterName_optionSet_returnThatValue(): void
         $this->assertEquals('trusted_param', $returnValue);
     }
 
+    /**
+     * @test
+     */
+    public function isRememberMeSetsTrusted_optionIsNotSet_returnFalse(): void
+    {
+        $returnValue = $this->createConfig([])->isRememberMeSetsTrusted();
+        $this->assertFalse($returnValue);
+    }
+
+    /**
+     * @test
+     */
+    public function isRememberMeSetsTrusted_optionDisabled_returnTrue(): void
+    {
+        $returnValue = $this->createConfig(['remember_me_sets_trusted' => false])->isRememberMeSetsTrusted();
+        $this->assertFalse($returnValue);
+    }
+
+    /**
+     * @test
+     */
+    public function isRememberMeSetsTrusted_optionEnabled_returnTrue(): void
+    {
+        $returnValue = $this->createConfig(self::FULL_OPTIONS)->isRememberMeSetsTrusted();
+        $this->assertTrue($returnValue);
+    }
+
     /**
      * @test
      */