From 7bf496a9a601a18ae257ff78950f9c542e625383 Mon Sep 17 00:00:00 2001 From: arnorietdijk <0964045@hr.nl> Date: Mon, 23 Nov 2020 13:39:27 +0100 Subject: [PATCH 1/5] add downloaded field to recoverycode --- src/Entity/RecoveryCode.php | 15 +++++ src/Handler/DownloadedRecoveryCodeHandler.php | 58 +++++++++++++++++++ src/Message/DownloadedRecoveryCode.php | 40 +++++++++++++ 3 files changed, 113 insertions(+) create mode 100644 src/Handler/DownloadedRecoveryCodeHandler.php create mode 100644 src/Message/DownloadedRecoveryCode.php diff --git a/src/Entity/RecoveryCode.php b/src/Entity/RecoveryCode.php index e1785d1..49af57e 100644 --- a/src/Entity/RecoveryCode.php +++ b/src/Entity/RecoveryCode.php @@ -37,6 +37,11 @@ class RecoveryCode */ private string $secret; + /** + * @ORM\Column(type="boolean") + */ + private bool $downloaded = false; + public function getId(): int { return $this->id; @@ -61,4 +66,14 @@ public function setSecret(string $secret): void { $this->secret = $secret; } + + public function getDownloaded(): bool + { + return $this->downloaded; + } + + public function setDownloaded(bool $downloaded): void + { + $this->downloaded = $downloaded; + } } diff --git a/src/Handler/DownloadedRecoveryCodeHandler.php b/src/Handler/DownloadedRecoveryCodeHandler.php new file mode 100644 index 0000000..eeb0466 --- /dev/null +++ b/src/Handler/DownloadedRecoveryCodeHandler.php @@ -0,0 +1,58 @@ +doctrine = $doctrine; + $this->tokenStorage = $tokenStorage; + } + + /** + * Invalidate codes for logged on user and create new codes. + */ + public function __invoke(DownloadedRecoveryCode $recoveryCode): void + { + $user = $this->doctrine->getRepository(RecoveryCodeEntity::class)->findBy(['secret' => $recoveryCode->getValue()]); + + if ($user instanceof TwoFactorUserInterface) { + $this->setDownloaded($user->getGoogleAuthenticatorSecret()); + } + + throw new \RuntimeException('Unable to create recovery codes for non-2fa users'); + } + + /** + * Set downloaded to true on recovery code(s) of current User. + */ + private function setDownloaded(string $secret): void + { + $currentCodes = $this->doctrine->getRepository(RecoveryCodeEntity::class)->findBy(['secret' => $secret]); + foreach($currentCodes as $code) { + $code->setDownloaded(true); + } + $manager = $this->doctrine->getManagerForClass(RecoveryCodeEntity::class); + $manager->flush(); + } +} diff --git a/src/Message/DownloadedRecoveryCode.php b/src/Message/DownloadedRecoveryCode.php new file mode 100644 index 0000000..873cb83 --- /dev/null +++ b/src/Message/DownloadedRecoveryCode.php @@ -0,0 +1,40 @@ +value = $value; + } + + public function getValue(): bool + { + return $this->value; + } +} From 1dda616b646d71383ee93baf27b36a0824412789 Mon Sep 17 00:00:00 2001 From: arnorietdijk <0964045@hr.nl> Date: Mon, 23 Nov 2020 15:09:07 +0100 Subject: [PATCH 2/5] made test for new handler --- src/Handler/DownloadedRecoveryCodeHandler.php | 18 +++--- .../DownloadedRecoveryCodeHandlerTest.php | 60 +++++++++++++++++++ 2 files changed, 69 insertions(+), 9 deletions(-) create mode 100644 tests/Handler/DownloadedRecoveryCodeHandlerTest.php diff --git a/src/Handler/DownloadedRecoveryCodeHandler.php b/src/Handler/DownloadedRecoveryCodeHandler.php index eeb0466..2c94982 100644 --- a/src/Handler/DownloadedRecoveryCodeHandler.php +++ b/src/Handler/DownloadedRecoveryCodeHandler.php @@ -30,29 +30,29 @@ public function __construct(ManagerRegistry $doctrine, TokenStorageInterface $to } /** - * Invalidate codes for logged on user and create new codes. + * Set downloaded to true on recovery code(s) for logged on user. */ public function __invoke(DownloadedRecoveryCode $recoveryCode): void { - $user = $this->doctrine->getRepository(RecoveryCodeEntity::class)->findBy(['secret' => $recoveryCode->getValue()]); + $user = $this->tokenStorage->getToken()->getUser(); if ($user instanceof TwoFactorUserInterface) { - $this->setDownloaded($user->getGoogleAuthenticatorSecret()); + $this->setDownloaded($user->getGoogleAuthenticatorSecret(), $recoveryCode->getValue()); + } else { + throw new \RuntimeException('Unable to set downloaded on recovery codes for non-2fa users'); } - - throw new \RuntimeException('Unable to create recovery codes for non-2fa users'); } /** * Set downloaded to true on recovery code(s) of current User. */ - private function setDownloaded(string $secret): void + private function setDownloaded(string $secret, bool $value): void { $currentCodes = $this->doctrine->getRepository(RecoveryCodeEntity::class)->findBy(['secret' => $secret]); - foreach($currentCodes as $code) { - $code->setDownloaded(true); - } $manager = $this->doctrine->getManagerForClass(RecoveryCodeEntity::class); + + array_walk($currentCodes, fn (RecoveryCodeEntity $recoveryCode) => $recoveryCode->setDownloaded($value)); + $manager->flush(); } } diff --git a/tests/Handler/DownloadedRecoveryCodeHandlerTest.php b/tests/Handler/DownloadedRecoveryCodeHandlerTest.php new file mode 100644 index 0000000..bf77c06 --- /dev/null +++ b/tests/Handler/DownloadedRecoveryCodeHandlerTest.php @@ -0,0 +1,60 @@ +createMock(ManagerRegistry::class); + $manager = $this->createMock(EntityManager::class); + $repository = $this->createMock(EntityRepository::class); + $tokenStorage = new TokenStorage(); + $user = new User(); + + $user->setGoogleAuthenticatorSecret('verysecret!'); + $tokenStorage->setToken(new JWTUserToken([], $user)); + + $repository + ->method('findBy') + ->willReturn( + [ + new RecoveryCodeEntity(), + new RecoveryCodeEntity(), + new RecoveryCodeEntity(), + ] + ); + + $manager + ->expects($this->exactly(1)) + ->method('flush'); + + $doctrine + ->method('getManagerForClass') + ->willReturn($manager); + + $doctrine + ->method('getRepository') + ->willReturn($repository); + + $handler = new DownloadedRecoveryCodeHandler($doctrine, $tokenStorage); + $handler(new DownloadedRecoveryCode(true)); + } +} From 712ae40984d1bde55a1048009c87b1d83f8cf0ed Mon Sep 17 00:00:00 2001 From: arnorietdijk <0964045@hr.nl> Date: Tue, 24 Nov 2020 08:42:44 +0100 Subject: [PATCH 3/5] process PR feedback --- src/Entity/RecoveryCode.php | 2 +- src/Handler/DownloadedRecoveryCodeHandler.php | 10 +++++----- tests/Handler/DownloadedRecoveryCodeHandlerTest.php | 8 +++++--- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/Entity/RecoveryCode.php b/src/Entity/RecoveryCode.php index 49af57e..76db90e 100644 --- a/src/Entity/RecoveryCode.php +++ b/src/Entity/RecoveryCode.php @@ -67,7 +67,7 @@ public function setSecret(string $secret): void $this->secret = $secret; } - public function getDownloaded(): bool + public function isDownloaded(): bool { return $this->downloaded; } diff --git a/src/Handler/DownloadedRecoveryCodeHandler.php b/src/Handler/DownloadedRecoveryCodeHandler.php index 2c94982..20cc782 100644 --- a/src/Handler/DownloadedRecoveryCodeHandler.php +++ b/src/Handler/DownloadedRecoveryCodeHandler.php @@ -7,7 +7,7 @@ namespace ConnectHolland\SecureJWTBundle\Handler; -use ConnectHolland\SecureJWTBundle\Entity\RecoveryCode as RecoveryCodeEntity; +use ConnectHolland\SecureJWTBundle\Entity\RecoveryCode; use ConnectHolland\SecureJWTBundle\Entity\TwoFactorUserInterface; use ConnectHolland\SecureJWTBundle\Message\DownloadedRecoveryCode; use Doctrine\Common\Persistence\ManagerRegistry; @@ -44,14 +44,14 @@ public function __invoke(DownloadedRecoveryCode $recoveryCode): void } /** - * Set downloaded to true on recovery code(s) of current User. + * Set downloaded to true on recovery code(s) for the given User. */ private function setDownloaded(string $secret, bool $value): void { - $currentCodes = $this->doctrine->getRepository(RecoveryCodeEntity::class)->findBy(['secret' => $secret]); - $manager = $this->doctrine->getManagerForClass(RecoveryCodeEntity::class); + $currentCodes = $this->doctrine->getRepository(RecoveryCode::class)->findBy(['secret' => $secret]); + $manager = $this->doctrine->getManagerForClass(RecoveryCode::class); - array_walk($currentCodes, fn (RecoveryCodeEntity $recoveryCode) => $recoveryCode->setDownloaded($value)); + array_walk($currentCodes, fn (RecoveryCode $recoveryCode) => $recoveryCode->setDownloaded($value)); $manager->flush(); } diff --git a/tests/Handler/DownloadedRecoveryCodeHandlerTest.php b/tests/Handler/DownloadedRecoveryCodeHandlerTest.php index bf77c06..d27bb19 100644 --- a/tests/Handler/DownloadedRecoveryCodeHandlerTest.php +++ b/tests/Handler/DownloadedRecoveryCodeHandlerTest.php @@ -28,6 +28,7 @@ public function testSetCodesDownloadedSuccessful(): void $repository = $this->createMock(EntityRepository::class); $tokenStorage = new TokenStorage(); $user = new User(); + $recoveryCode = new RecoveryCodeEntity(); $user->setGoogleAuthenticatorSecret('verysecret!'); $tokenStorage->setToken(new JWTUserToken([], $user)); @@ -36,9 +37,9 @@ public function testSetCodesDownloadedSuccessful(): void ->method('findBy') ->willReturn( [ - new RecoveryCodeEntity(), - new RecoveryCodeEntity(), - new RecoveryCodeEntity(), + $recoveryCode, + $recoveryCode, + $recoveryCode, ] ); @@ -56,5 +57,6 @@ public function testSetCodesDownloadedSuccessful(): void $handler = new DownloadedRecoveryCodeHandler($doctrine, $tokenStorage); $handler(new DownloadedRecoveryCode(true)); + $this->assertSame(true, $recoveryCode->isDownloaded()); } } From e5769fe7449a13d1ebfea8b290659a5bebd36875 Mon Sep 17 00:00:00 2001 From: arnorietdijk <0964045@hr.nl> Date: Tue, 24 Nov 2020 08:55:37 +0100 Subject: [PATCH 4/5] codestyle fixes --- src/Handler/DownloadedRecoveryCodeHandler.php | 4 ++-- src/Message/DownloadedRecoveryCode.php | 2 -- tests/Handler/DownloadedRecoveryCodeHandlerTest.php | 9 ++++----- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Handler/DownloadedRecoveryCodeHandler.php b/src/Handler/DownloadedRecoveryCodeHandler.php index 20cc782..82e0a8f 100644 --- a/src/Handler/DownloadedRecoveryCodeHandler.php +++ b/src/Handler/DownloadedRecoveryCodeHandler.php @@ -25,7 +25,7 @@ class DownloadedRecoveryCodeHandler implements MessageHandlerInterface */ public function __construct(ManagerRegistry $doctrine, TokenStorageInterface $tokenStorage) { - $this->doctrine = $doctrine; + $this->doctrine = $doctrine; $this->tokenStorage = $tokenStorage; } @@ -49,7 +49,7 @@ public function __invoke(DownloadedRecoveryCode $recoveryCode): void private function setDownloaded(string $secret, bool $value): void { $currentCodes = $this->doctrine->getRepository(RecoveryCode::class)->findBy(['secret' => $secret]); - $manager = $this->doctrine->getManagerForClass(RecoveryCode::class); + $manager = $this->doctrine->getManagerForClass(RecoveryCode::class); array_walk($currentCodes, fn (RecoveryCode $recoveryCode) => $recoveryCode->setDownloaded($value)); diff --git a/src/Message/DownloadedRecoveryCode.php b/src/Message/DownloadedRecoveryCode.php index 873cb83..9a1c74d 100644 --- a/src/Message/DownloadedRecoveryCode.php +++ b/src/Message/DownloadedRecoveryCode.php @@ -8,7 +8,6 @@ namespace ConnectHolland\SecureJWTBundle\Message; use ApiPlatform\Core\Annotation\ApiResource; -use ConnectHolland\SecureJWTBundle\DTO\GeneratedCodes; /** * Create recovery codes for the current user. Will invalidate any existing codes for that user. @@ -23,7 +22,6 @@ * * @codeCoverageIgnore Trivial class with only a getter */ - class DownloadedRecoveryCode { private bool $value; diff --git a/tests/Handler/DownloadedRecoveryCodeHandlerTest.php b/tests/Handler/DownloadedRecoveryCodeHandlerTest.php index d27bb19..68cfb4e 100644 --- a/tests/Handler/DownloadedRecoveryCodeHandlerTest.php +++ b/tests/Handler/DownloadedRecoveryCodeHandlerTest.php @@ -17,17 +17,16 @@ use Lexik\Bundle\JWTAuthenticationBundle\Security\Authentication\Token\JWTUserToken; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; -use Symfony\Component\Security\Core\Exception\RuntimeException; class DownloadedRecoveryCodeHandlerTest extends TestCase { public function testSetCodesDownloadedSuccessful(): void { - $doctrine = $this->createMock(ManagerRegistry::class); - $manager = $this->createMock(EntityManager::class); - $repository = $this->createMock(EntityRepository::class); + $doctrine = $this->createMock(ManagerRegistry::class); + $manager = $this->createMock(EntityManager::class); + $repository = $this->createMock(EntityRepository::class); $tokenStorage = new TokenStorage(); - $user = new User(); + $user = new User(); $recoveryCode = new RecoveryCodeEntity(); $user->setGoogleAuthenticatorSecret('verysecret!'); From 0f27ba73965a9ef496a4256c4fe403fba984e30c Mon Sep 17 00:00:00 2001 From: arnorietdijk <0964045@hr.nl> Date: Tue, 24 Nov 2020 13:24:20 +0100 Subject: [PATCH 5/5] refactor test logic --- .../DownloadedRecoveryCodeHandlerTest.php | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/Handler/DownloadedRecoveryCodeHandlerTest.php b/tests/Handler/DownloadedRecoveryCodeHandlerTest.php index 68cfb4e..2860cca 100644 --- a/tests/Handler/DownloadedRecoveryCodeHandlerTest.php +++ b/tests/Handler/DownloadedRecoveryCodeHandlerTest.php @@ -22,25 +22,23 @@ class DownloadedRecoveryCodeHandlerTest extends TestCase { public function testSetCodesDownloadedSuccessful(): void { - $doctrine = $this->createMock(ManagerRegistry::class); - $manager = $this->createMock(EntityManager::class); - $repository = $this->createMock(EntityRepository::class); - $tokenStorage = new TokenStorage(); - $user = new User(); - $recoveryCode = new RecoveryCodeEntity(); + $doctrine = $this->createMock(ManagerRegistry::class); + $manager = $this->createMock(EntityManager::class); + $repository = $this->createMock(EntityRepository::class); + $tokenStorage = new TokenStorage(); + $user = new User(); + $recoveryCodes = [ + new RecoveryCodeEntity(), + new RecoveryCodeEntity(), + new RecoveryCodeEntity() + ]; $user->setGoogleAuthenticatorSecret('verysecret!'); $tokenStorage->setToken(new JWTUserToken([], $user)); $repository ->method('findBy') - ->willReturn( - [ - $recoveryCode, - $recoveryCode, - $recoveryCode, - ] - ); + ->willReturn($recoveryCodes); $manager ->expects($this->exactly(1)) @@ -56,6 +54,8 @@ public function testSetCodesDownloadedSuccessful(): void $handler = new DownloadedRecoveryCodeHandler($doctrine, $tokenStorage); $handler(new DownloadedRecoveryCode(true)); - $this->assertSame(true, $recoveryCode->isDownloaded()); + foreach ($recoveryCodes as $code) { + $this->assertSame(true, $code->isDownloaded()); + } } }