Reset prepared provider on each login, to fix preparation issue when …

…`invalidate_session` option is false #296
scheb · Aug 13, 2020 · 5c2cbc4 · 5c2cbc4
1 parent 0f757ff
commit 5c2cbc4
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 14 deletions.
diff --git a/Security/TwoFactor/Provider/TwoFactorProviderPreparationListener.php b/Security/TwoFactor/Provider/TwoFactorProviderPreparationListener.php
@@ -68,6 +68,12 @@ public function __construct(
     public function onLogin(AuthenticationEvent $event): void
     {
         $token = $event->getAuthenticationToken();
+
+        if ($token instanceof TwoFactorTokenInterface) {
+            $firewallName = $token->getProviderKey();
+            $this->preparationRecorder->startRecording($firewallName);
+        }
+
         if ($this->prepareOnLogin && $this->supports($token)) {
             /** @var TwoFactorTokenInterface $token */
             // After login, when the token is a TwoFactorTokenInterface, execute preparation

diff --git a/Security/TwoFactor/Provider/TwoFactorProviderPreparationRecorder.php b/Security/TwoFactor/Provider/TwoFactorProviderPreparationRecorder.php
@@ -28,6 +28,13 @@ public function isProviderPrepared(string $firewallName, string $providerName):
         return \in_array($providerName, $firewallCalledProviders, true);
     }
 
+    public function startRecording(string $firewallName): void
+    {
+        $calledProviders = $this->session->get(self::CALLED_PROVIDERS_SESSION_KEY, []);
+        $calledProviders[$firewallName] = [];
+        $this->session->set(self::CALLED_PROVIDERS_SESSION_KEY, $calledProviders);
+    }
+
     public function recordProviderIsPrepared(string $firewallName, string $providerName): void
     {
         $calledProviders = $this->session->get(self::CALLED_PROVIDERS_SESSION_KEY, []);

diff --git a/Tests/Security/TwoFactor/Provider/TwoFactorProviderPreparationListenerTest.php b/Tests/Security/TwoFactor/Provider/TwoFactorProviderPreparationListenerTest.php
@@ -14,9 +14,9 @@
 use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderRegistry;
 use Scheb\TwoFactorBundle\Tests\TestCase;
 use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpFoundation\Session\SessionInterface;
 use Symfony\Component\HttpKernel\Event\FinishRequestEvent;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Event\AuthenticationEvent;
 
 class TwoFactorProviderPreparationListenerTest extends TestCase
@@ -35,17 +35,17 @@ class TwoFactorProviderPreparationListenerTest extends TestCase
     private $request;
 
     /**
-     * @var MockObject|SessionInterface
+     * @var MockObject|TwoFactorProviderPreparationRecorder
      */
     private $preparationRecorder;
 
     /**
      * @var MockObject|TwoFactorToken
      */
-    private $token;
+    private $twoFactorToken;
 
     /**
-     * @var
+     * @var \stdClass
      */
     private $user;
 
@@ -58,16 +58,16 @@ protected function setUp(): void
     {
         $this->request = $this->createMock(Request::class);
         $this->user = new \stdClass();
-        $this->token = $this->createMock(TwoFactorToken::class);
-        $this->token
+        $this->twoFactorToken = $this->createMock(TwoFactorToken::class);
+        $this->twoFactorToken
             ->expects($this->any())
             ->method('getProviderKey')
             ->willReturn(self::FIREWALL_NAME);
-        $this->token
+        $this->twoFactorToken
             ->expects($this->any())
             ->method('getCurrentTwoFactorProvider')
             ->willReturn(self::CURRENT_PROVIDER_NAME);
-        $this->token
+        $this->twoFactorToken
             ->expects($this->any())
             ->method('getUser')
             ->willReturn($this->user);
@@ -91,12 +91,12 @@ private function initTwoFactorProviderPreparationListener($prepareOnLogin, $prep
 
     private function createTwoFactorAuthenticationEvent(): TwoFactorAuthenticationEvent
     {
-        return new TwoFactorAuthenticationEvent($this->request, $this->token);
+        return new TwoFactorAuthenticationEvent($this->request, $this->twoFactorToken);
     }
 
-    private function createAuthenticationEvent(): AuthenticationEvent
+    private function createAuthenticationEvent(TokenInterface $token): AuthenticationEvent
     {
-        return new AuthenticationEvent($this->token);
+        return new AuthenticationEvent($token);
     }
 
     private function createFinishRequestEvent(): FinishRequestEvent
@@ -141,7 +141,7 @@ private function expectNotPrepareCurrentProvider(): void
     {
         $this->preparationRecorder
             ->expects($this->never())
-            ->method($this->anything());
+            ->method('recordProviderIsPrepared');
 
         $this->providerRegistry
             ->expects($this->never())
@@ -154,7 +154,7 @@ private function expectNotPrepareCurrentProvider(): void
     public function onLogin_optionPrepareOnLoginTrue_twoFactorProviderIsPrepared(): void
     {
         $this->initTwoFactorProviderPreparationListener(true, false);
-        $event = $this->createAuthenticationEvent();
+        $event = $this->createAuthenticationEvent($this->twoFactorToken);
 
         $this->expectPrepareCurrentProvider();
 
@@ -168,14 +168,46 @@ public function onLogin_optionPrepareOnLoginTrue_twoFactorProviderIsPrepared():
     public function onLogin_optionPrepareOnLoginFalse_twoFactorProviderIsNotPrepared(): void
     {
         $this->initTwoFactorProviderPreparationListener(false, false);
-        $event = $this->createAuthenticationEvent();
+        $event = $this->createAuthenticationEvent($this->twoFactorToken);
 
         $this->expectNotPrepareCurrentProvider();
 
         $this->listener->onLogin($event);
         $this->listener->onKernelFinishRequest($this->createFinishRequestEvent());
     }
 
+    /**
+     * @test
+     */
+    public function onLogin_twoFactorToken_startRecording(): void
+    {
+        $this->initTwoFactorProviderPreparationListener(false, false);
+        $event = $this->createAuthenticationEvent($this->twoFactorToken);
+
+        $this->preparationRecorder
+            ->expects($this->once())
+            ->method('startRecording')
+            ->with(self::FIREWALL_NAME);
+
+        $this->listener->onLogin($event);
+    }
+
+    /**
+     * @test
+     */
+    public function onLogin_otherToken_doNothing(): void
+    {
+        $this->initTwoFactorProviderPreparationListener(false, false);
+        $token = $this->createMock(TokenInterface::class);
+        $event = $this->createAuthenticationEvent($token);
+
+        $this->preparationRecorder
+            ->expects($this->never())
+            ->method('startRecording');
+
+        $this->listener->onLogin($event);
+    }
+
     /**
      * @test
      */

diff --git a/Tests/Security/TwoFactor/Provider/TwoFactorProviderPreparationRecorderTest.php b/Tests/Security/TwoFactor/Provider/TwoFactorProviderPreparationRecorderTest.php
@@ -30,6 +30,31 @@ protected function setUp(): void
         $this->recorder = new TwoFactorProviderPreparationRecorder($this->session);
     }
 
+    /**
+     * @test
+     */
+    public function startRecording_wasPreparedBefore_noLongerPrepared(): void
+    {
+        $this->session
+            ->expects($this->any())
+            ->method('get')
+            ->with('2fa_called_providers')
+            ->willReturn([
+                'otherFirewallName' => [self::CURRENT_PROVIDER_NAME],
+                self::FIREWALL_NAME => [self::CURRENT_PROVIDER_NAME],
+            ]);
+
+        $this->session
+            ->expects($this->once())
+            ->method('set')
+            ->with('2fa_called_providers', [
+                'otherFirewallName' => [self::CURRENT_PROVIDER_NAME],
+                self::FIREWALL_NAME => [],
+            ]);
+
+        $this->recorder->startRecording(self::FIREWALL_NAME);
+    }
+
     /**
      * @test
      */