From 687d22c6d6218340e77bdcd007e42837da99b503 Mon Sep 17 00:00:00 2001 From: Jesse Rushlow Date: Fri, 26 Apr 2024 06:41:13 -0400 Subject: [PATCH 1/6] make classes final --- src/Generator/ResetPasswordRandomGenerator.php | 4 +--- src/Generator/ResetPasswordTokenGenerator.php | 4 +--- src/Model/ResetPasswordTokenComponents.php | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/Generator/ResetPasswordRandomGenerator.php b/src/Generator/ResetPasswordRandomGenerator.php index 29421d07..cdddecc6 100644 --- a/src/Generator/ResetPasswordRandomGenerator.php +++ b/src/Generator/ResetPasswordRandomGenerator.php @@ -14,10 +14,8 @@ * @author Ryan Weaver * * @internal - * - * @final */ -class ResetPasswordRandomGenerator +final class ResetPasswordRandomGenerator { /** * Original credit to Laravel's Str::random() method. diff --git a/src/Generator/ResetPasswordTokenGenerator.php b/src/Generator/ResetPasswordTokenGenerator.php index 8e5ce354..c2649a0d 100644 --- a/src/Generator/ResetPasswordTokenGenerator.php +++ b/src/Generator/ResetPasswordTokenGenerator.php @@ -17,10 +17,8 @@ * @author Ryan Weaver * * @internal - * - * @final */ -class ResetPasswordTokenGenerator +final class ResetPasswordTokenGenerator { /** * @param string $signingKey Unique, random, cryptographically secure string diff --git a/src/Model/ResetPasswordTokenComponents.php b/src/Model/ResetPasswordTokenComponents.php index 4d382982..676b6f20 100644 --- a/src/Model/ResetPasswordTokenComponents.php +++ b/src/Model/ResetPasswordTokenComponents.php @@ -14,10 +14,8 @@ * @author Ryan Weaver * * @internal - * - * @final */ -class ResetPasswordTokenComponents +final class ResetPasswordTokenComponents { public function __construct( #[\SensitiveParameter] From b89600515f2158a9ed13c8fe76a0ce841f44f690 Mon Sep 17 00:00:00 2001 From: Jesse Rushlow Date: Thu, 16 May 2024 18:35:59 -0400 Subject: [PATCH 2/6] refactor token generator unit test --- .../ResetPasswordTokenGeneratorTest.php | 124 +++++++----------- 1 file changed, 51 insertions(+), 73 deletions(-) diff --git a/tests/Unit/Generator/ResetPasswordTokenGeneratorTest.php b/tests/Unit/Generator/ResetPasswordTokenGeneratorTest.php index 462d4838..95de1fab 100644 --- a/tests/Unit/Generator/ResetPasswordTokenGeneratorTest.php +++ b/tests/Unit/Generator/ResetPasswordTokenGeneratorTest.php @@ -20,93 +20,71 @@ */ class ResetPasswordTokenGeneratorTest extends TestCase { - /** - * @var MockObject|ResetPasswordRandomGenerator - */ - private $mockRandomGenerator; - - /** - * @var MockObject|\DateTimeImmutable - */ - private $mockExpiresAt; + private MockObject&\DateTimeImmutable $mockExpiresAt; + private ResetPasswordTokenGenerator $tokenGenerator; protected function setUp(): void { - $this->mockRandomGenerator = $this->createMock(ResetPasswordRandomGenerator::class); $this->mockExpiresAt = $this->createMock(\DateTimeImmutable::class); + $this->tokenGenerator = new ResetPasswordTokenGenerator('secret-key', new ResetPasswordRandomGenerator()); } - public function testSelectorGeneratedByRandomGenerator(): void + public function testCreateTokenReturnsValidHashedTokenComponents(): void { - $this->mockRandomGenerator - ->expects($this->exactly(2)) - ->method('getRandomAlphaNumStr') - ; + $result = $this->tokenGenerator->createToken($this->mockExpiresAt, 'userId'); - $generator = $this->getTokenGenerator(); - $generator->createToken($this->mockExpiresAt, 'userId'); - } + // The public token = "selector token" + "verifier token" + self::assertSame(20, \strlen($result->getSelector())); + self::assertSame(40, \strlen($result->getPublicToken())); - public function testHashedTokenIsCreatedWithExpectedParams(): void - { - $this->mockRandomGenerator - ->expects($this->exactly(2)) - ->method('getRandomAlphaNumStr') - ->willReturnOnConsecutiveCalls('verifier', 'selector') - ; - - $this->mockExpiresAt - ->expects($this->once()) - ->method('getTimestamp') - ->willReturn(2020) - ; - - $expected = hash_hmac( - 'sha256', - json_encode(['verifier', 'user1234', 2020]), - 'key', - true - ); - - $generator = $this->getTokenGenerator(); - $result = $generator->createToken($this->mockExpiresAt, 'user1234'); - - self::assertSame(base64_encode($expected), $result->getHashedToken()); + $verifier = substr($result->getPublicToken(), 20, 20); + + $expectedHash = base64_encode(hash_hmac( + algo: 'sha256', + data: json_encode([$verifier, 'userId', $this->mockExpiresAt->getTimestamp()]), + key: 'secret-key', + binary: true + )); + + self::assertSame($expectedHash, $result->getHashedToken()); } - public function testHashedTokenIsCreatedUsingOptionVerifierParam(): void + public function testCreateTokenUsesProvidedVerifierToken(): void { - $date = 2020; - $userId = 'user1234'; - $knownVerifier = 'verified'; - - $this->mockRandomGenerator - ->expects($this->once()) - ->method('getRandomAlphaNumStr') - ->willReturnOnConsecutiveCalls('un-used-verifier', 'selector') - ; - - $this->mockExpiresAt - ->expects($this->once()) - ->method('getTimestamp') - ->willReturn($date) - ; - - $knownToken = hash_hmac( - 'sha256', - json_encode([$knownVerifier, $userId, $date]), - 'key', - true - ); - - $generator = $this->getTokenGenerator(); - $result = $generator->createToken($this->mockExpiresAt, $userId, $knownVerifier); - - self::assertSame(base64_encode($knownToken), $result->getHashedToken()); + $result = $this->tokenGenerator->createToken($this->mockExpiresAt, 'userId', '1234'); + + $expectedHash = base64_encode(hash_hmac( + algo: 'sha256', + data: json_encode(['1234', 'userId', $this->mockExpiresAt->getTimestamp()]), + key: 'secret-key', + binary: true + )); + + self::assertSame($expectedHash, $result->getHashedToken()); } - private function getTokenGenerator(): ResetPasswordTokenGenerator + public function testCreateTokenUsesProvidedParams(): void { - return new ResetPasswordTokenGenerator('key', $this->mockRandomGenerator); + $result = $this->tokenGenerator->createToken($this->mockExpiresAt, 'userId', '1234'); + + $expectedHash = base64_encode(hash_hmac( + algo: 'sha256', + data: json_encode(['1234', 'userId', '0123456789']), + key: 'secret-key', + binary: true + )); + + // We used a "fake" timestamp in our expectedHash + self::assertNotSame($expectedHash, $result->getHashedToken()); + + $expectedHash = base64_encode(hash_hmac( + algo: 'sha256', + data: json_encode(['1234', 'bad-user-id', $this->mockExpiresAt->getTimestamp()]), + key: 'secret-key', + binary: true + )); + + // We used a "fake" user id in our expectedHash + self::assertNotSame($expectedHash, $result->getHashedToken()); } } From e732eb25b46444688697a50a7737e8c480e92d2d Mon Sep 17 00:00:00 2001 From: Jesse Rushlow Date: Fri, 17 May 2024 02:38:22 -0400 Subject: [PATCH 3/6] wip - refactor helper unit test --- tests/Unit/ResetPasswordHelperTest.php | 46 ++++++++------------------ 1 file changed, 14 insertions(+), 32 deletions(-) diff --git a/tests/Unit/ResetPasswordHelperTest.php b/tests/Unit/ResetPasswordHelperTest.php index 83b991ee..15ba31a9 100644 --- a/tests/Unit/ResetPasswordHelperTest.php +++ b/tests/Unit/ResetPasswordHelperTest.php @@ -14,6 +14,7 @@ use SymfonyCasts\Bundle\ResetPassword\Exception\ExpiredResetPasswordTokenException; use SymfonyCasts\Bundle\ResetPassword\Exception\InvalidResetPasswordTokenException; use SymfonyCasts\Bundle\ResetPassword\Exception\TooManyPasswordRequestsException; +use SymfonyCasts\Bundle\ResetPassword\Generator\ResetPasswordRandomGenerator; use SymfonyCasts\Bundle\ResetPassword\Generator\ResetPasswordTokenGenerator; use SymfonyCasts\Bundle\ResetPassword\Model\ResetPasswordRequestInterface; use SymfonyCasts\Bundle\ResetPassword\Persistence\ResetPasswordRequestRepositoryInterface; @@ -27,35 +28,16 @@ */ class ResetPasswordHelperTest extends TestCase { - /** - * @var MockObject|ResetPasswordRequestRepositoryInterface - */ - private $mockRepo; - - /** - * @var MockObject|ResetPasswordTokenGenerator - */ - private $mockTokenGenerator; - - /** - * @var MockObject|ResetPasswordRequestInterface - */ - private $mockResetRequest; - - /** - * @var MockObject|ResetPasswordCleaner - */ - private $mockCleaner; - - /** - * @var string - */ - private $randomToken; + private MockObject&ResetPasswordRequestRepositoryInterface $mockRepo; + private ResetPasswordTokenGenerator $tokenGenerator; + private MockObject&ResetPasswordRequestInterface $mockResetRequest; + private MockObject|ResetPasswordCleaner $mockCleaner; + private string $randomToken; protected function setUp(): void { $this->mockRepo = $this->createMock(ResetPasswordRequestRepositoryInterface::class); - $this->mockTokenGenerator = $this->createMock(ResetPasswordTokenGenerator::class); + $this->tokenGenerator = new ResetPasswordTokenGenerator('secret-key', new ResetPasswordRandomGenerator()); $this->mockCleaner = $this->createMock(ResetPasswordCleaner::class); $this->mockResetRequest = $this->createMock(ResetPasswordRequestInterface::class); $this->randomToken = bin2hex(random_bytes(20)); @@ -112,7 +94,7 @@ public function testHasUserThrottlingReturnsNullIfNotBeforeThrottleTime(): void ; $helper = new ResetPasswordHelper( - $this->mockTokenGenerator, + $this->tokenGenerator, $this->mockCleaner, $this->mockRepo, 99999999, @@ -131,7 +113,7 @@ public function testExceptionThrownIfRequestBeforeThrottleLimit(): void ; $helper = new ResetPasswordHelper( - $this->mockTokenGenerator, + $this->tokenGenerator, $this->mockCleaner, $this->mockRepo, 99999999, @@ -332,7 +314,7 @@ public function testExpiresAtUsesCurrentTimeZone(): void public function testExpiresAtUsingDefault(): void { $helper = new ResetPasswordHelper( - $this->mockTokenGenerator, + $this->tokenGenerator, $this->mockCleaner, $this->mockRepo, 60, @@ -349,7 +331,7 @@ public function testExpiresAtUsingDefault(): void public function testExpiresAtUsingOverride(): void { $helper = new ResetPasswordHelper( - $this->mockTokenGenerator, + $this->tokenGenerator, $this->mockCleaner, $this->mockRepo, 60, @@ -366,7 +348,7 @@ public function testExpiresAtUsingOverride(): void public function testFakeTokenExpiresAtUsingDefault(): void { $helper = new ResetPasswordHelper( - $this->mockTokenGenerator, + $this->tokenGenerator, $this->mockCleaner, $this->mockRepo, 60, @@ -383,7 +365,7 @@ public function testFakeTokenExpiresAtUsingDefault(): void public function testFakeTokenExpiresAtUsingOverride(): void { $helper = new ResetPasswordHelper( - $this->mockTokenGenerator, + $this->tokenGenerator, $this->mockCleaner, $this->mockRepo, 60, @@ -400,7 +382,7 @@ public function testFakeTokenExpiresAtUsingOverride(): void private function getPasswordResetHelper(): ResetPasswordHelper { return new ResetPasswordHelper( - $this->mockTokenGenerator, + $this->tokenGenerator, $this->mockCleaner, $this->mockRepo, 99999999, From 88b60c2af4ab1ed5f5560bb1c1b518f8fd012056 Mon Sep 17 00:00:00 2001 From: Jesse Rushlow Date: Fri, 17 May 2024 13:46:51 -0400 Subject: [PATCH 4/6] this shouldnt be a union --- tests/Unit/ResetPasswordHelperTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/ResetPasswordHelperTest.php b/tests/Unit/ResetPasswordHelperTest.php index 15ba31a9..8670f0ee 100644 --- a/tests/Unit/ResetPasswordHelperTest.php +++ b/tests/Unit/ResetPasswordHelperTest.php @@ -31,7 +31,7 @@ class ResetPasswordHelperTest extends TestCase private MockObject&ResetPasswordRequestRepositoryInterface $mockRepo; private ResetPasswordTokenGenerator $tokenGenerator; private MockObject&ResetPasswordRequestInterface $mockResetRequest; - private MockObject|ResetPasswordCleaner $mockCleaner; + private MockObject&ResetPasswordCleaner $mockCleaner; private string $randomToken; protected function setUp(): void From c656d65a9b557d656432ddc1784e5e657f094196 Mon Sep 17 00:00:00 2001 From: Jesse Rushlow Date: Fri, 17 May 2024 16:07:35 -0400 Subject: [PATCH 5/6] refactor helper test w/ new interfaces --- src/Generator/ResetPasswordTokenGenerator.php | 2 +- .../ResetPasswordTokenGeneratorInterface.php | 20 ++ src/ResetPasswordHelper.php | 8 +- src/Util/ResetPasswordCleaner.php | 2 +- src/Util/ResetPasswordCleanerInterface.php | 18 ++ tests/Unit/ResetPasswordHelperTest.php | 277 +++++++++++------- 6 files changed, 210 insertions(+), 117 deletions(-) create mode 100644 src/Generator/ResetPasswordTokenGeneratorInterface.php create mode 100644 src/Util/ResetPasswordCleanerInterface.php diff --git a/src/Generator/ResetPasswordTokenGenerator.php b/src/Generator/ResetPasswordTokenGenerator.php index c2649a0d..620bf693 100644 --- a/src/Generator/ResetPasswordTokenGenerator.php +++ b/src/Generator/ResetPasswordTokenGenerator.php @@ -18,7 +18,7 @@ * * @internal */ -final class ResetPasswordTokenGenerator +final class ResetPasswordTokenGenerator implements ResetPasswordTokenGeneratorInterface { /** * @param string $signingKey Unique, random, cryptographically secure string diff --git a/src/Generator/ResetPasswordTokenGeneratorInterface.php b/src/Generator/ResetPasswordTokenGeneratorInterface.php new file mode 100644 index 00000000..9128f15b --- /dev/null +++ b/src/Generator/ResetPasswordTokenGeneratorInterface.php @@ -0,0 +1,20 @@ + + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace SymfonyCasts\Bundle\ResetPassword\Generator; + +use SymfonyCasts\Bundle\ResetPassword\Model\ResetPasswordTokenComponents; + +/** + * @author Jesse Rushlow + */ +interface ResetPasswordTokenGeneratorInterface +{ + public function createToken(\DateTimeInterface $expiresAt, int|string $userId, ?string $verifier = null): ResetPasswordTokenComponents; +} diff --git a/src/ResetPasswordHelper.php b/src/ResetPasswordHelper.php index bd286dd5..e79ce5ca 100644 --- a/src/ResetPasswordHelper.php +++ b/src/ResetPasswordHelper.php @@ -12,11 +12,11 @@ use SymfonyCasts\Bundle\ResetPassword\Exception\ExpiredResetPasswordTokenException; use SymfonyCasts\Bundle\ResetPassword\Exception\InvalidResetPasswordTokenException; use SymfonyCasts\Bundle\ResetPassword\Exception\TooManyPasswordRequestsException; -use SymfonyCasts\Bundle\ResetPassword\Generator\ResetPasswordTokenGenerator; +use SymfonyCasts\Bundle\ResetPassword\Generator\ResetPasswordTokenGeneratorInterface; use SymfonyCasts\Bundle\ResetPassword\Model\ResetPasswordRequestInterface; use SymfonyCasts\Bundle\ResetPassword\Model\ResetPasswordToken; use SymfonyCasts\Bundle\ResetPassword\Persistence\ResetPasswordRequestRepositoryInterface; -use SymfonyCasts\Bundle\ResetPassword\Util\ResetPasswordCleaner; +use SymfonyCasts\Bundle\ResetPassword\Util\ResetPasswordCleanerInterface; /** * @author Jesse Rushlow @@ -34,8 +34,8 @@ final class ResetPasswordHelper implements ResetPasswordHelperInterface * @param int $requestThrottleTime Another password reset cannot be made faster than this throttle time in seconds */ public function __construct( - private ResetPasswordTokenGenerator $generator, - private ResetPasswordCleaner $cleaner, + private ResetPasswordTokenGeneratorInterface $generator, + private ResetPasswordCleanerInterface $cleaner, private ResetPasswordRequestRepositoryInterface $repository, private int $resetRequestLifetime, private int $requestThrottleTime, diff --git a/src/Util/ResetPasswordCleaner.php b/src/Util/ResetPasswordCleaner.php index f81bb661..077fdd19 100644 --- a/src/Util/ResetPasswordCleaner.php +++ b/src/Util/ResetPasswordCleaner.php @@ -19,7 +19,7 @@ * * @final */ -class ResetPasswordCleaner +class ResetPasswordCleaner implements ResetPasswordCleanerInterface { /** * @param bool $enabled Enable/disable garbage collection diff --git a/src/Util/ResetPasswordCleanerInterface.php b/src/Util/ResetPasswordCleanerInterface.php new file mode 100644 index 00000000..3e481c1d --- /dev/null +++ b/src/Util/ResetPasswordCleanerInterface.php @@ -0,0 +1,18 @@ + + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace SymfonyCasts\Bundle\ResetPassword\Util; + +/** + * @author Jesse Rushlow + */ +interface ResetPasswordCleanerInterface +{ + public function handleGarbageCollection(bool $force = false): int; +} diff --git a/tests/Unit/ResetPasswordHelperTest.php b/tests/Unit/ResetPasswordHelperTest.php index 8670f0ee..d0fe87b4 100644 --- a/tests/Unit/ResetPasswordHelperTest.php +++ b/tests/Unit/ResetPasswordHelperTest.php @@ -14,13 +14,13 @@ use SymfonyCasts\Bundle\ResetPassword\Exception\ExpiredResetPasswordTokenException; use SymfonyCasts\Bundle\ResetPassword\Exception\InvalidResetPasswordTokenException; use SymfonyCasts\Bundle\ResetPassword\Exception\TooManyPasswordRequestsException; -use SymfonyCasts\Bundle\ResetPassword\Generator\ResetPasswordRandomGenerator; -use SymfonyCasts\Bundle\ResetPassword\Generator\ResetPasswordTokenGenerator; +use SymfonyCasts\Bundle\ResetPassword\Generator\ResetPasswordTokenGeneratorInterface; use SymfonyCasts\Bundle\ResetPassword\Model\ResetPasswordRequestInterface; +use SymfonyCasts\Bundle\ResetPassword\Model\ResetPasswordTokenComponents; use SymfonyCasts\Bundle\ResetPassword\Persistence\ResetPasswordRequestRepositoryInterface; use SymfonyCasts\Bundle\ResetPassword\ResetPasswordHelper; use SymfonyCasts\Bundle\ResetPassword\Tests\Fixtures\Entity\ResetPasswordTestFixtureRequest; -use SymfonyCasts\Bundle\ResetPassword\Util\ResetPasswordCleaner; +use SymfonyCasts\Bundle\ResetPassword\Util\ResetPasswordCleanerInterface; /** * @author Jesse Rushlow @@ -29,62 +29,96 @@ class ResetPasswordHelperTest extends TestCase { private MockObject&ResetPasswordRequestRepositoryInterface $mockRepo; - private ResetPasswordTokenGenerator $tokenGenerator; + private MockObject&ResetPasswordTokenGeneratorInterface $tokenGenerator; private MockObject&ResetPasswordRequestInterface $mockResetRequest; - private MockObject&ResetPasswordCleaner $mockCleaner; + private MockObject&ResetPasswordCleanerInterface $mockCleaner; private string $randomToken; + private int $requestLifetime = 99999999; + private int $requestThrottleTime = 99999999; protected function setUp(): void { $this->mockRepo = $this->createMock(ResetPasswordRequestRepositoryInterface::class); - $this->tokenGenerator = new ResetPasswordTokenGenerator('secret-key', new ResetPasswordRandomGenerator()); - $this->mockCleaner = $this->createMock(ResetPasswordCleaner::class); + $this->tokenGenerator = $this->createMock(ResetPasswordTokenGeneratorInterface::class); + $this->mockCleaner = $this->createMock(ResetPasswordCleanerInterface::class); $this->mockResetRequest = $this->createMock(ResetPasswordRequestInterface::class); $this->randomToken = bin2hex(random_bytes(20)); + $this->requestLifetime = 99999999; + $this->requestThrottleTime = 99999999; } - /** - * @covers \SymfonyCasts\Bundle\ResetPassword\ResetPasswordHelper::hasUserHitThrottling - */ - public function testHasUserThrottlingReturnsFalseWithNoLastRequestDate(): void + public function testGenerateResetTokenCallsGarbageCollector(): void { - $this->mockRepo + $this->mockCleaner ->expects($this->once()) - ->method('getUserIdentifier') - ->willReturn('1234') + ->method('handleGarbageCollection') + ; + + // We don't care about the mock configuration below, we're only testing if garbage collection is called. + $this->tokenGenerator + ->expects(self::once()) + ->method('createToken') + ->willReturn(new ResetPasswordTokenComponents('', '', '')) ; + $this->getPasswordResetHelper()->generateResetToken(new \stdClass()); + } + + public function testHasUserThrottlingReturnsNullWithNoLastRequestDate(): void + { + $user = new \stdClass(); + $this->mockRepo ->expects($this->once()) ->method('getMostRecentNonExpiredRequestDate') + ->with($user) ->willReturn(null) ; + // We don't care about the mock configuration below, we're only testing the helpers hasUserItThrottling method. + $this->mockRepo + ->expects($this->once()) + ->method('getUserIdentifier') + ->willReturn('1234') + ; + + $this->tokenGenerator + ->expects(self::once()) + ->method('createToken') + ->willReturn(new ResetPasswordTokenComponents('', '', '')) + ; + $this->mockRepo ->expects($this->once()) ->method('createResetPasswordRequest') ->willReturn(new ResetPasswordTestFixtureRequest()) ; - $helper = $this->getPasswordResetHelper(); - $helper->generateResetToken(new \stdClass()); + $this->getPasswordResetHelper()->generateResetToken(new \stdClass()); } - /** - * @covers \SymfonyCasts\Bundle\ResetPassword\ResetPasswordHelper::hasUserHitThrottling - */ public function testHasUserThrottlingReturnsNullIfNotBeforeThrottleTime(): void { + $user = new \stdClass(); + $this->mockRepo ->expects($this->once()) - ->method('getUserIdentifier') - ->willReturn('1234') + ->method('getMostRecentNonExpiredRequestDate') + ->with($user) + ->willReturn(new \DateTime('-3 hours')) ; + // We don't care about the mock configuration below, we're only testing the helpers hasUserItThrottling method. $this->mockRepo ->expects($this->once()) - ->method('getMostRecentNonExpiredRequestDate') - ->willReturn(new \DateTime('-3 hours')) + ->method('getUserIdentifier') + ->willReturn('1234') + ; + + $this->tokenGenerator + ->expects(self::once()) + ->method('createToken') + ->willReturn(new ResetPasswordTokenComponents('', '', '')) ; $this->mockRepo @@ -93,35 +127,25 @@ public function testHasUserThrottlingReturnsNullIfNotBeforeThrottleTime(): void ->willReturn(new ResetPasswordTestFixtureRequest()) ; - $helper = new ResetPasswordHelper( - $this->tokenGenerator, - $this->mockCleaner, - $this->mockRepo, - 99999999, - 7200 // 2 hours - ); - - $helper->generateResetToken(new \stdClass()); + $this->requestThrottleTime = 7200; // 2 hours + $this->getPasswordResetHelper()->generateResetToken(new \stdClass()); } public function testExceptionThrownIfRequestBeforeThrottleLimit(): void { + $user = new \stdClass(); + $this->mockRepo ->expects($this->once()) ->method('getMostRecentNonExpiredRequestDate') + ->with($user) ->willReturn(new \DateTime('-1 hour')) ; - $helper = new ResetPasswordHelper( - $this->tokenGenerator, - $this->mockCleaner, - $this->mockRepo, - 99999999, - 7200 // 2 hours - ); + $this->requestThrottleTime = 7200; // 2 hours try { - $helper->generateResetToken(new \stdClass()); + $this->getPasswordResetHelper()->generateResetToken($user); } catch (TooManyPasswordRequestsException $exception) { // account for time changes during test self::assertGreaterThanOrEqual(3599, $exception->getRetryAfter()); @@ -133,43 +157,55 @@ public function testExceptionThrownIfRequestBeforeThrottleLimit(): void $this->fail('Exception was not thrown.'); } - public function testRemoveResetRequestThrowsExceptionWithEmptyToken(): void + public function testExpiresAtUsesCurrentTimeZone(): void { - $this->expectException(InvalidResetPasswordTokenException::class); + // We don't care about the mock configuration below, we're only testing if the correct timezone is used. + $this->tokenGenerator + ->expects(self::once()) + ->method('createToken') + ->willReturn(new ResetPasswordTokenComponents('', '', '')) + ; - $helper = $this->getPasswordResetHelper(); - $helper->removeResetRequest(''); + $token = $this->getPasswordResetHelper()->generateResetToken(new \stdClass()); + + $expiresAt = $token->getExpiresAt(); + self::assertSame(date_default_timezone_get(), $expiresAt->getTimezone()->getName()); } - public function testRemoveResetRequestRetrievesTokenFromRepository(): void + public function testExpiresAtUsingDefaultLifetime(): void { - $this->mockRepo - ->expects($this->once()) - ->method('findResetPasswordRequest') - ->with(substr($this->randomToken, 0, 20)) - ->willReturn($this->mockResetRequest) + // We don't care about the mock configuration below, we're only testing . + $this->tokenGenerator + ->expects(self::once()) + ->method('createToken') + ->willReturn(new ResetPasswordTokenComponents('', '', '')) ; - $helper = $this->getPasswordResetHelper(); - $helper->removeResetRequest($this->randomToken); + $this->requestLifetime = 60; + + $token = $this->getPasswordResetHelper()->generateResetToken(new \stdClass()); + $expiresAt = $token->getExpiresAt(); + + self::assertGreaterThan(new \DateTimeImmutable('+55 seconds'), $expiresAt); + self::assertLessThan(new \DateTimeImmutable('+65 seconds'), $expiresAt); } - public function testRemoveResetRequestCallsRepositoryToRemoveResetRequestObject(): void + public function testExpiresAtUsingOverrideLifetime(): void { - $this->mockRepo - ->expects($this->once()) - ->method('findResetPasswordRequest') - ->willReturn($this->mockResetRequest) + // We don't care about the mock configuration below, we're only testing . + $this->tokenGenerator + ->expects(self::once()) + ->method('createToken') + ->willReturn(new ResetPasswordTokenComponents('', '', '')) ; - $this->mockRepo - ->expects($this->once()) - ->method('removeResetPasswordRequest') - ->with($this->mockResetRequest) - ; + $this->requestLifetime = 60; - $helper = $this->getPasswordResetHelper(); - $helper->removeResetRequest('1234'); + $token = $this->getPasswordResetHelper()->generateResetToken(new \stdClass(), 30); + $expiresAt = $token->getExpiresAt(); + + self::assertGreaterThan(new \DateTimeImmutable('+25 seconds'), $expiresAt); + self::assertLessThan(new \DateTimeImmutable('+35 seconds'), $expiresAt); } public function testExceptionThrownIfTokenLengthIsNotOfCorrectSize(): void @@ -217,6 +253,9 @@ public function testValidateTokenThrowsExceptionOnExpiredResetRequest(): void public function testValidateTokenFetchesUserIfTokenNotExpired(): void { + $user = new \stdClass(); + $expiresAt = new \DateTimeImmutable(); + $this->mockResetRequest ->expects($this->once()) ->method('isExpired') @@ -226,13 +265,13 @@ public function testValidateTokenFetchesUserIfTokenNotExpired(): void $this->mockResetRequest ->expects($this->once()) ->method('getUser') - ->willReturn(new \stdClass()) + ->willReturn($user) ; $this->mockResetRequest ->expects($this->once()) ->method('getExpiresAt') - ->willReturn(new \DateTimeImmutable()) + ->willReturn($expiresAt) ; $this->mockRepo @@ -242,22 +281,39 @@ public function testValidateTokenFetchesUserIfTokenNotExpired(): void ->willReturn($this->mockResetRequest) ; + $this->mockRepo + ->expects(self::once()) + ->method('getUserIdentifier') + ->with($user) + ->willReturn('1234') + ; + + $this->tokenGenerator + ->expects(self::once()) + ->method('createToken') + ->with($expiresAt, '1234', substr($this->randomToken, 20, 20)) + ->willReturn(new ResetPasswordTokenComponents('', '', '')) + ; + $helper = $this->getPasswordResetHelper(); $helper->validateTokenAndFetchUser($this->randomToken); } public function testValidateTokenThrowsExceptionIfTokenAndVerifierDoNotMatch(): void { + $user = new \stdClass(); + $expiresAt = new \DateTimeImmutable(); + $this->mockResetRequest ->expects($this->once()) ->method('getExpiresAt') - ->willReturn(new \DateTimeImmutable()) + ->willReturn($expiresAt) ; $this->mockResetRequest ->expects($this->once()) ->method('getUser') - ->willReturn(new \stdClass()) + ->willReturn($user) ; $this->mockResetRequest @@ -272,21 +328,24 @@ public function testValidateTokenThrowsExceptionIfTokenAndVerifierDoNotMatch(): ->willReturn($this->mockResetRequest) ; - $this->expectException(InvalidResetPasswordTokenException::class); - - $helper = $this->getPasswordResetHelper(); - $helper->validateTokenAndFetchUser($this->randomToken); - } + $this->mockRepo + ->expects(self::once()) + ->method('getUserIdentifier') + ->with($user) + ->willReturn('1234') + ; - public function testGenerateResetTokenCallsGarbageCollector(): void - { - $this->mockCleaner - ->expects($this->once()) - ->method('handleGarbageCollection') + $this->tokenGenerator + ->expects(self::once()) + ->method('createToken') + ->with($expiresAt, '1234', substr($this->randomToken, 20, 20)) + ->willReturn(new ResetPasswordTokenComponents('', '', '')) ; + $this->expectException(InvalidResetPasswordTokenException::class); + $helper = $this->getPasswordResetHelper(); - $helper->generateResetToken(new \stdClass()); + $helper->validateTokenAndFetchUser($this->randomToken); } public function testGarbageCollectorCalledDuringValidation(): void @@ -302,47 +361,43 @@ public function testGarbageCollectorCalledDuringValidation(): void $helper->validateTokenAndFetchUser($this->randomToken); } - public function testExpiresAtUsesCurrentTimeZone(): void + public function testRemoveResetRequestThrowsExceptionWithEmptyToken(): void { - $helper = $this->getPasswordResetHelper(); - $token = $helper->generateResetToken(new \stdClass()); + $this->expectException(InvalidResetPasswordTokenException::class); - $expiresAt = $token->getExpiresAt(); - self::assertSame(date_default_timezone_get(), $expiresAt->getTimezone()->getName()); + $helper = $this->getPasswordResetHelper(); + $helper->removeResetRequest(''); } - public function testExpiresAtUsingDefault(): void + public function testRemoveResetRequestRetrievesTokenFromRepository(): void { - $helper = new ResetPasswordHelper( - $this->tokenGenerator, - $this->mockCleaner, - $this->mockRepo, - 60, - 99999999 - ); - - $token = $helper->generateResetToken(new \stdClass()); - $expiresAt = $token->getExpiresAt(); + $this->mockRepo + ->expects($this->once()) + ->method('findResetPasswordRequest') + ->with(substr($this->randomToken, 0, 20)) + ->willReturn($this->mockResetRequest) + ; - self::assertGreaterThan(new \DateTimeImmutable('+55 seconds'), $expiresAt); - self::assertLessThan(new \DateTimeImmutable('+65 seconds'), $expiresAt); + $helper = $this->getPasswordResetHelper(); + $helper->removeResetRequest($this->randomToken); } - public function testExpiresAtUsingOverride(): void + public function testRemoveResetRequestCallsRepositoryToRemoveResetRequestObject(): void { - $helper = new ResetPasswordHelper( - $this->tokenGenerator, - $this->mockCleaner, - $this->mockRepo, - 60, - 99999999 - ); + $this->mockRepo + ->expects($this->once()) + ->method('findResetPasswordRequest') + ->willReturn($this->mockResetRequest) + ; - $token = $helper->generateResetToken(new \stdClass(), 30); - $expiresAt = $token->getExpiresAt(); + $this->mockRepo + ->expects($this->once()) + ->method('removeResetPasswordRequest') + ->with($this->mockResetRequest) + ; - self::assertGreaterThan(new \DateTimeImmutable('+25 seconds'), $expiresAt); - self::assertLessThan(new \DateTimeImmutable('+35 seconds'), $expiresAt); + $helper = $this->getPasswordResetHelper(); + $helper->removeResetRequest('1234'); } public function testFakeTokenExpiresAtUsingDefault(): void @@ -385,8 +440,8 @@ private function getPasswordResetHelper(): ResetPasswordHelper $this->tokenGenerator, $this->mockCleaner, $this->mockRepo, - 99999999, - 99999999 + $this->requestLifetime, + $this->requestThrottleTime ); } } From bb102433932389c0fa58df93ee52778891e9451a Mon Sep 17 00:00:00 2001 From: Jesse Rushlow Date: Thu, 6 Jun 2024 10:23:07 -0400 Subject: [PATCH 6/6] allow for a custom implementation of the token generator ResetPasswordTokenComponents is internal, consumers now can create their own implementation of the ResetPasswordTokenGenerator without having to use our internal token components class. --- src/Generator/ResetPasswordTokenGenerator.php | 3 +- .../ResetPasswordTokenGeneratorInterface.php | 4 +-- src/Model/ResetPasswordTokenComponents.php | 2 +- .../ResetPasswordTokenComponentsInterface.php | 31 +++++++++++++++++++ 4 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 src/Model/ResetPasswordTokenComponentsInterface.php diff --git a/src/Generator/ResetPasswordTokenGenerator.php b/src/Generator/ResetPasswordTokenGenerator.php index 620bf693..4ba98f5c 100644 --- a/src/Generator/ResetPasswordTokenGenerator.php +++ b/src/Generator/ResetPasswordTokenGenerator.php @@ -11,6 +11,7 @@ use SymfonyCasts\Bundle\ResetPassword\Exception\ResetPasswordRuntimeException; use SymfonyCasts\Bundle\ResetPassword\Model\ResetPasswordTokenComponents; +use SymfonyCasts\Bundle\ResetPassword\Model\ResetPasswordTokenComponentsInterface; /** * @author Jesse Rushlow @@ -38,7 +39,7 @@ public function __construct( * * @throws ResetPasswordRuntimeException */ - public function createToken(\DateTimeInterface $expiresAt, int|string $userId, ?string $verifier = null): ResetPasswordTokenComponents + public function createToken(\DateTimeInterface $expiresAt, int|string $userId, ?string $verifier = null): ResetPasswordTokenComponentsInterface { if (null === $verifier) { $verifier = $this->generator->getRandomAlphaNumStr(); diff --git a/src/Generator/ResetPasswordTokenGeneratorInterface.php b/src/Generator/ResetPasswordTokenGeneratorInterface.php index 9128f15b..9edbe7c8 100644 --- a/src/Generator/ResetPasswordTokenGeneratorInterface.php +++ b/src/Generator/ResetPasswordTokenGeneratorInterface.php @@ -9,12 +9,12 @@ namespace SymfonyCasts\Bundle\ResetPassword\Generator; -use SymfonyCasts\Bundle\ResetPassword\Model\ResetPasswordTokenComponents; +use SymfonyCasts\Bundle\ResetPassword\Model\ResetPasswordTokenComponentsInterface; /** * @author Jesse Rushlow */ interface ResetPasswordTokenGeneratorInterface { - public function createToken(\DateTimeInterface $expiresAt, int|string $userId, ?string $verifier = null): ResetPasswordTokenComponents; + public function createToken(\DateTimeInterface $expiresAt, int|string $userId, ?string $verifier = null): ResetPasswordTokenComponentsInterface; } diff --git a/src/Model/ResetPasswordTokenComponents.php b/src/Model/ResetPasswordTokenComponents.php index 676b6f20..81ea6c78 100644 --- a/src/Model/ResetPasswordTokenComponents.php +++ b/src/Model/ResetPasswordTokenComponents.php @@ -15,7 +15,7 @@ * * @internal */ -final class ResetPasswordTokenComponents +final class ResetPasswordTokenComponents implements ResetPasswordTokenComponentsInterface { public function __construct( #[\SensitiveParameter] diff --git a/src/Model/ResetPasswordTokenComponentsInterface.php b/src/Model/ResetPasswordTokenComponentsInterface.php new file mode 100644 index 00000000..b8ec7b31 --- /dev/null +++ b/src/Model/ResetPasswordTokenComponentsInterface.php @@ -0,0 +1,31 @@ + + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace SymfonyCasts\Bundle\ResetPassword\Model; + +/** + * @author Jesse Rushlow + */ +interface ResetPasswordTokenComponentsInterface +{ + /** + * @return string Non-hashed random string used to fetch request objects from persistence + */ + public function getSelector(): string; + + /** + * @return string The hashed non-public token used to validate reset password requests + */ + public function getHashedToken(): string; + + /** + * The public token consists of a concatenated random non-hashed selector string and random non-hashed verifier string. + */ + public function getPublicToken(): string; +}