Fix several trusted device issues

core-bundle/src/Controller/FrontendModule/TwoFactorController.php

@@ -137,7 +137,6 @@ protected function getResponse(Template $template, ModuleModel $model, Request $
         $template->href = $this->page->getAbsoluteUrl().'?2fa=enable';
         $template->backupCodes = json_decode((string) $user->backupCodes, true) ?? [];
         $template->trustedDevices = $this->get('contao.security.two_factor.trusted_device_manager')->getTrustedDevices($user);
-        $template->currentDevice = $request->cookies->get($this->getParameter('scheb_two_factor.trusted_device.cookie_name'));
 
         return new Response($template->parse());
     }

core-bundle/src/Entity/TrustedDevice.php

@@ -51,20 +51,6 @@ class TrustedDevice
      */
     protected $userId;
 
-    /**
-     * @var string
-     *
-     * @ORM\Column(type="text", name="cookie_value")
-     */
-    protected $cookieValue;
-
-    /**
-     * @var int
-     *
-     * @ORM\Column(type="integer")
-     */
-    protected $version;
-
     /**
      * @var string|null
      *
@@ -93,10 +79,9 @@ class TrustedDevice
      */
     protected $deviceFamily;
 
-    public function __construct(User $user, int $version)
+    public function __construct(User $user)
     {
         $this->userId = (int) $user->id;
-        $this->version = $version;
         $this->created = new \DateTime();
         $this->userClass = \get_class($user);
     }
@@ -130,23 +115,6 @@ public function getUserId(): int
         return $this->userId;
     }
 
-    public function getCookieValue(): string
-    {
-        return $this->cookieValue;
-    }
-
-    public function setCookieValue(string $cookieValue): self
-    {
-        $this->cookieValue = $cookieValue;
-
-        return $this;
-    }
-
-    public function getVersion(): int
-    {
-        return $this->version;
-    }
-
     public function setUserAgent(?string $userAgent): self
     {
         $this->userAgent = $userAgent;

core-bundle/src/Resources/contao/languages/en/default.xlf

@@ -1988,9 +1988,6 @@
       <trans-unit id="MSC.device">
         <source>Device</source>
       </trans-unit>
-      <trans-unit id="MSC.thisDevice">
-        <source>this device</source>
-      </trans-unit>
       <trans-unit id="MSC.browser">
         <source>Browser</source>
       </trans-unit>

core-bundle/src/Resources/contao/modules/ModuleTwoFactor.php

@@ -93,13 +93,9 @@ protected function compile()
 			$container->get('contao.security.two_factor.trusted_device_manager')->clearTrustedDevices($user);
 		}
 
-		/** @var Request $request */
-		$request = $container->get('request_stack')->getMasterRequest();
-
 		$this->Template->isEnabled = (bool) $user->useTwoFactor;
 		$this->Template->backupCodes = json_decode((string) $user->backupCodes, true) ?? array();
 		$this->Template->trustedDevices = $container->get('contao.security.two_factor.trusted_device_manager')->getTrustedDevices($user);
-		$this->Template->currentDevice = $request->cookies->get($container->getParameter('scheb_two_factor.trusted_device.cookie_name'));
 	}
 
 	/**

core-bundle/src/Resources/contao/templates/backend/be_two_factor.html5

@@ -88,7 +88,7 @@
             </tr>
             <?php foreach ($this->trustedDevices as $index => $trustedDevice): ?>
               <tr class="<?= $index % 2 ? 'odd' : 'even' ?> hover-row">
-                <td class="tl_file_list"><?= $trustedDevice->getDeviceFamily() ?><?php if ($this->currentDevice === $trustedDevice->getCookieValue()): ?> (<?= $this->trans('MSC.thisDevice') ?>)<?php endif; ?></td>
+                <td class="tl_file_list"><?= $trustedDevice->getDeviceFamily() ?></td>
                 <td class="tl_file_list"><?= $trustedDevice->getUaFamily() ?></td>
                 <td class="tl_file_list"><?= $trustedDevice->getOsFamily() ?></td>
                 <td class="tl_file_list"><?= $trustedDevice->getCreated()->format(Contao\Config::get('datimFormat')) ?></td>

core-bundle/src/Resources/contao/templates/modules/mod_two_factor.html5

@@ -92,7 +92,7 @@
             </tr>
             <?php foreach ($this->trustedDevices as $trustedDevice): ?>
               <tr>
-                <td><?= $trustedDevice->getDeviceFamily() ?><?php if ($this->currentDevice === $trustedDevice->getCookieValue()): ?> (<?= $this->trans('MSC.thisDevice') ?>)<?php endif; ?></td>
+                <td><?= $trustedDevice->getDeviceFamily() ?></td>
                 <td><?= $trustedDevice->getUaFamily() ?></td>
                 <td><?= $trustedDevice->getOsFamily() ?></td>
                 <td><?= $trustedDevice->getCreated()->format(Contao\Config::get('datimFormat')) ?></td>

core-bundle/src/Security/Authentication/AuthenticationSuccessHandler.php

@@ -113,7 +113,9 @@ public function onAuthenticationSuccess(Request $request, TokenInterface $token)
             /** @var FirewallConfig $firewallConfig */
             $firewallConfig = $this->firewallMap->getFirewallConfig($request);
 
-            $this->trustedDeviceManager->addTrustedDevice($token->getUser(), $firewallConfig->getName());
+            if (!$this->trustedDeviceManager->isTrustedDevice($user, $firewallConfig->getName())) {
+                $this->trustedDeviceManager->addTrustedDevice($token->getUser(), $firewallConfig->getName());
+            }
         }
 
         $response = new RedirectResponse($this->determineTargetUrl($request));

core-bundle/src/Security/Authentication/Provider/AuthenticationProvider.php

@@ -123,9 +123,6 @@ public function authenticate(TokenInterface $token): TokenInterface
 
         // Skip two-factor authentication on trusted devices
         if ($this->trustedDeviceManager->isTrustedDevice($user, $firewallName)) {
-            // Renew the token
-            $this->trustedDeviceManager->addTrustedDevice($user, $firewallName);
-
             return $context->getToken();
         }
 

core-bundle/src/Security/TwoFactor/TrustedDeviceManager.php

@@ -15,7 +15,6 @@
 use Contao\CoreBundle\Entity\TrustedDevice;
 use Contao\User;
 use Doctrine\ORM\EntityManagerInterface;
-use Doctrine\ORM\NonUniqueResultException;
 use Scheb\TwoFactorBundle\Security\TwoFactor\Trusted\TrustedDeviceManagerInterface;
 use Scheb\TwoFactorBundle\Security\TwoFactor\Trusted\TrustedDeviceTokenStorage;
 use Symfony\Component\HttpFoundation\RequestStack;
@@ -51,25 +50,16 @@ public function addTrustedDevice($user, string $firewallName): void
             return;
         }
 
-        $version = (int) $user->trustedTokenVersion;
-        $oldCookieValue = $this->trustedTokenStorage->getCookieValue();
         $userAgent = $this->requestStack->getMasterRequest()->headers->get('User-Agent');
 
         $parser = Parser::create();
         $parsedUserAgent = $parser->parse($userAgent);
 
-        $this->trustedTokenStorage->addTrustedToken($user->username, $firewallName, $version);
-
-        // Check if already an earlier version of the trusted device exists
-        try {
-            $trustedDevice = $this->findExistingTrustedDevice((int) $user->id, $oldCookieValue, $version) ?? new TrustedDevice($user, $version);
-        } catch (NonUniqueResultException $exception) {
-            $trustedDevice = new TrustedDevice($user, $version);
-        }
+        $this->trustedTokenStorage->addTrustedToken((string) $user->id, $firewallName, (int) $user->trustedTokenVersion);
 
+        $trustedDevice = new TrustedDevice($user);
         $trustedDevice
             ->setCreated(new \DateTime())
-            ->setCookieValue($this->trustedTokenStorage->getCookieValue())
             ->setUserAgent($userAgent)
             ->setUaFamily($parsedUserAgent->ua->family)
             ->setOsFamily($parsedUserAgent->os->family)
@@ -86,10 +76,7 @@ public function isTrustedDevice($user, string $firewallName): bool
             return false;
         }
 
-        $username = $user->username;
-        $version = (int) $user->trustedTokenVersion;
-
-        return $this->trustedTokenStorage->hasTrustedToken($username, $firewallName, $version);
+        return $this->trustedTokenStorage->hasTrustedToken((string) $user->id, $firewallName, (int) $user->trustedTokenVersion);
     }
 
     public function clearTrustedDevices(User $user): void
@@ -120,21 +107,4 @@ public function getTrustedDevices(User $user)
             ->execute()
         ;
     }
-
-    public function findExistingTrustedDevice(int $userId, string $cookieValue, int $version)
-    {
-        return $this->entityManager
-            ->createQueryBuilder()
-            ->select('td')
-            ->from(TrustedDevice::class, 'td')
-            ->andWhere('td.userId = :userId')
-            ->andWhere('td.cookieValue = :cookieValue')
-            ->andWhere('td.version = :version')
-            ->setParameter('userId', $userId)
-            ->setParameter('cookieValue', $cookieValue)
-            ->setParameter('version', $version)
-            ->getQuery()
-            ->getOneOrNullResult()
-        ;
-    }
 }

core-bundle/tests/Security/Authentication/Provider/AuthenticationProviderTest.php

@@ -334,12 +334,6 @@ public function testSkipsTwoFactorAuthenticationForTrustedDevices(): void
             ->willReturn(true)
         ;
 
-        $trustedDeviceManager
-            ->expects($this->once())
-            ->method('addTrustedDevice')
-            ->with($user, 'contao_frontend')
-        ;
-
         $provider = $this->createUsernamePasswordProvider(null, $twoFactorHandler, $trustedDeviceManager);
         $provider->authenticate($token);
     }

core-bundle/tests/Security/TwoFactor/TrustedDeviceManagerTest.php

@@ -28,13 +28,13 @@ public function testIsTrustedDevice(): void
         $tokenStorage
             ->expects($this->once())
             ->method('hasTrustedToken')
-            ->with('foo', 'contao_backend', 1)
+            ->with('1', 'contao_backend', 1)
             ->willReturn(true)
         ;
 
         /** @var BackendUser $user */
         $user = $this->mockClassWithProperties(BackendUser::class);
-        $user->username = 'foo';
+        $user->id = 1;
         $user->trustedTokenVersion = 1;
 
         $manager = new TrustedDeviceManager(

manager-bundle/src/Resources/skeleton/config/config.yml

@@ -126,3 +126,8 @@ fos_http_cache:
         annotations:
             enabled: false
         max_header_value_length: 4096
+
+# TwoFactor configuration
+scheb_two_factor:
+    trusted_device:
+        lifetime: 15768000 # 15768000 seconds corresponds to six months