adding an extra security mechanism to remove previous user tokens

weaverryan · weaverryan · commit 77b4c8aecd59 · 2020-09-16T06:35:16.000-04:00
This is to imitate what Auth0 does, which only honors the "latest"
magic link request - any previous do not work, even if they
have not yet been used and should not have expired.
diff --git a/src/Symfony/Bridge/Doctrine/Security/MagicLink/MagicLoginLinkTokenRepositoryTrait.php b/src/Symfony/Bridge/Doctrine/Security/MagicLink/MagicLoginLinkTokenRepositoryTrait.php
@@ -24,7 +24,7 @@
  *
  *          private function createMagicToken(string $selector, string $hashedVerifier, UserInterface $user, \DateTimeInterface $expiresAt): StoredMagicLinkTokenInterface
  *
- *      B) Your entity needs "selector" and "expiresAtTimestamp" properties.
+ *      B) Your entity needs "selector", "expiresAtTimestamp" and "user" properties.
  *
  * @author Ryan Weaver <ryan@symfonycasts.com>
  */
@@ -57,6 +57,16 @@ public function invalidateToken(string $selector): void
             ->execute();
     }
 
+    public function invalidateAllUserTokens(UserInterface $user): void
+    {
+        $this->createQueryBuilder('mlt')
+            ->delete()
+            ->where('mlt.user = :user')
+            ->setParameter('user', $user)
+            ->getQuery()
+            ->execute();
+    }
+
     public function removeExpiredTokens(): void
     {
         // keep very-recently expired tokens so that you
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/MagicLinkLoginTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/MagicLinkLoginTest.php
@@ -11,7 +11,6 @@
 
 namespace Symfony\Bundle\SecurityBundle\Tests\Functional;
 
-use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Security\Core\User\User;
diff --git a/src/Symfony/Component/Security/Http/Authenticator/MagicLinkAuthenticator.php b/src/Symfony/Component/Security/Http/Authenticator/MagicLinkAuthenticator.php
@@ -11,7 +11,6 @@
 
 namespace Symfony\Component\Security\Http\Authenticator;
 
-use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
diff --git a/src/Symfony/Component/Security/Http/MagicLink/MagicLinkTokenStorageInterface.php b/src/Symfony/Component/Security/Http/MagicLink/MagicLinkTokenStorageInterface.php
@@ -38,6 +38,15 @@ public function findToken(string $selector): ?StoredMagicLinkTokenInterface;
      */
     public function invalidateToken(string $selector): void;
 
+    /**
+     * Invalidate all tokens for the given user.
+     *
+     * This is called whenever a new token is generated to guarantee
+     * that only one valid token is ever active. This is an extra
+     * security mechanism.
+     */
+    public function invalidateAllUserTokens(UserInterface $user): void;
+
     /**
      * Remove all tokens that have expired from storage.
      *
diff --git a/src/Symfony/Component/Security/Http/MagicLink/MagicLoginLinker.php b/src/Symfony/Component/Security/Http/MagicLink/MagicLoginLinker.php
@@ -44,6 +44,8 @@ public function createMagicLink(UserInterface $user): MagicLoginLinkDetails
     {
         $this->removeExpiredLinks();
 
+        $this->storage->invalidateAllUserTokens($user);
+
         // length will be 20
         $selector = $this->generateRandomString(15);
         $verifier = $this->generateRandomString(18);

Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@`
`24`	`24`	`*`
`25`	`25`	`* private function createMagicToken(string $selector, string $hashedVerifier, UserInterface $user, \DateTimeInterface $expiresAt): StoredMagicLinkTokenInterface`
`26`	`26`	`*`
`27`		`- * B) Your entity needs "selector" and "expiresAtTimestamp" properties.`
	`27`	`+ * B) Your entity needs "selector", "expiresAtTimestamp" and "user" properties.`
`28`	`28`	`*`
`29`	`29`	`* @author Ryan Weaver <ryan@symfonycasts.com>`
`30`	`30`	`*/`
`@@ -57,6 +57,16 @@ public function invalidateToken(string $selector): void`
`57`	`57`	`->execute();`
`58`	`58`	`}`
`59`	`59`
	`60`	`+ public function invalidateAllUserTokens(UserInterface $user): void`
	`61`	`+ {`
	`62`	`+ $this->createQueryBuilder('mlt')`
	`63`	`+ ->delete()`
	`64`	`+ ->where('mlt.user = :user')`
	`65`	`+ ->setParameter('user', $user)`
	`66`	`+ ->getQuery()`
	`67`	`+ ->execute();`
	`68`	`+ }`
	`69`	`+`
`60`	`70`	`public function removeExpiredTokens(): void`
`61`	`71`	`{`
`62`	`72`	`// keep very-recently expired tokens so that you`
Original file line number	Diff line number	Diff line change
`@@ -44,6 +44,8 @@ public function createMagicLink(UserInterface $user): MagicLoginLinkDetails`
`44`	`44`	`{`
`45`	`45`	`$this->removeExpiredLinks();`
`46`	`46`
	`47`	`+ $this->storage->invalidateAllUserTokens($user);`
	`48`	`+`
`47`	`49`	`// length will be 20`
`48`	`50`	`$selector = $this->generateRandomString(15);`
`49`	`51`	`$verifier = $this->generateRandomString(18);`