From 2afbbf9a82c0ca1be272be629cf57752efee7adf Mon Sep 17 00:00:00 2001 From: Nicola Saliu Date: Fri, 13 Dec 2024 11:16:07 +0100 Subject: [PATCH] NEXT-39991 - Fix the Shop URL in the registration to create the correct proof if the URL should be sanitized --- src/Registration/RegistrationService.php | 16 +- .../Registration/RegistrationServiceTest.php | 137 ++++++++++++++++-- 2 files changed, 142 insertions(+), 11 deletions(-) diff --git a/src/Registration/RegistrationService.php b/src/Registration/RegistrationService.php index 6c99617..c86bb07 100644 --- a/src/Registration/RegistrationService.php +++ b/src/Registration/RegistrationService.php @@ -58,7 +58,7 @@ public function register(RequestInterface $request): ResponseInterface if ($shop === null) { $shop = $this->shopRepository->createShopStruct( $queries['shop-id'], - $this->sanitizeShopUrl($queries['shop-url']), + $queries['shop-url'], $this->shopSecretGeneratorInterface->generate() ); @@ -66,7 +66,7 @@ public function register(RequestInterface $request): ResponseInterface $this->shopRepository->createShop($shop); } else { - $shop->setShopUrl($this->sanitizeShopUrl($queries['shop-url'])); + $shop->setShopUrl($queries['shop-url']); $this->eventDispatcher?->dispatch(new BeforeRegistrationStartsEvent($request, $shop)); @@ -86,6 +86,8 @@ public function register(RequestInterface $request): ResponseInterface 'secret' => $shop->getShopSecret(), ]; + $this->fixShopUrlInDatabase($shop); + $response = $psrFactory->createResponse(200); return $response @@ -155,4 +157,14 @@ private function sanitizeShopUrl(string $shopUrl): string $normalizedPath ); } + + public function fixShopUrlInDatabase(ShopInterface $shop): void + { + $sanitizedShopUrl = $this->sanitizeShopUrl($shop->getShopUrl()); + + if ($shop->getShopUrl() !== $sanitizedShopUrl) { + $shop->setShopUrl($sanitizedShopUrl); + $this->shopRepository->updateShop($shop); + } + } } diff --git a/tests/Registration/RegistrationServiceTest.php b/tests/Registration/RegistrationServiceTest.php index 8bc1e00..14ecfb8 100644 --- a/tests/Registration/RegistrationServiceTest.php +++ b/tests/Registration/RegistrationServiceTest.php @@ -91,6 +91,11 @@ public function testRegisterCreate(): void ->method('createShopStruct') ->willReturn($shop); + $shopRepository + ->expects(static::never()) + ->method('updateShop') + ->with($shop); + $eventDispatcher ->expects(static::once()) ->method('dispatch'); @@ -137,6 +142,11 @@ public function testRegisterCreateMustNotDispatchBeforeRegistrationStartsEvent() ->method('createShopStruct') ->willReturn($shop); + $shopRepository + ->expects(static::never()) + ->method('updateShop') + ->with($shop); + $response = $registrationService->register( new Request('GET', 'http://localhost?shop-id=123&shop-url=https://my-shop.com×tamp=1234567890') ); @@ -162,9 +172,27 @@ public function testRegisterUpdate(): void $events[] = $event; }); + $shopRepository = $this->createMock(ShopRepositoryInterface::class); + + $shop = new MockShop('123', 'https://my-shop.com', '1234567890'); + + $shopRepository + ->expects(static::once()) + ->method('getShopFromId') + ->willReturn($shop); + + $shopRepository + ->expects(static::never()) + ->method('createShopStruct'); + + $shopRepository + ->expects(static::once()) + ->method('updateShop') + ->with($shop); + $registrationService = new RegistrationService( $this->appConfiguration, - $this->shopRepository, + $shopRepository, $this->createMock(RequestVerifier::class), new ResponseSigner(), new RandomStringShopSecretGenerator(), @@ -176,10 +204,6 @@ public function testRegisterUpdate(): void new Request('GET', 'http://localhost?shop-id=123&shop-url=https://my-shop.com×tamp=1234567890') ); - $shop = $this->shopRepository->getShopFromId('123'); - - $this->shopRepository->updateShop($shop); - static::assertNotNull($shop); static::assertCount(1, $events); @@ -465,18 +489,101 @@ public function testRegisterConfirmMissingShopParameters(array $params): void } #[DataProvider('shopUrlsProvider')] - public function testRegisterShopUrlIsSanitized( + public function testRegisterCreateShopUrlIsSanitized( string $shopUrl, string $expectedUrl, + bool $sanitizeShopUrlInDatabase ): void { + $shopRepository = $this->createMock(ShopRepositoryInterface::class); + + $shop = new MockShop('123', $shopUrl, '1234567890'); + + $shopRepository + ->expects(static::once()) + ->method('getShopFromId') + ->with('123') + ->willReturn(null); + + $shopRepository + ->expects(static::once()) + ->method('createShopStruct') + ->willReturn($shop); + + if ($sanitizeShopUrlInDatabase) { + $shopRepository + ->expects(static::once()) + ->method('updateShop') + ->with($shop); + } + + $registrationService = new RegistrationService( + $this->appConfiguration, + $shopRepository, + $this->createMock(RequestVerifier::class), + new ResponseSigner(), + new RandomStringShopSecretGenerator(), + new NullLogger(), + null + ); + $request = new Request( 'GET', sprintf('http://localhost?shop-id=123&shop-url=%s×tamp=1234567890', $shopUrl) ); - $this->registerService->register($request); + $registrationService->register($request); - $shop = $this->shopRepository->getShopFromId('123'); + static::assertSame($expectedUrl, $shop->getShopUrl()); + } + + #[DataProvider('shopUrlsProvider')] + public function testRegisterUpdateShopUrlIsSanitized( + string $shopUrl, + string $expectedUrl, + bool $sanitizeShopUrlInDatabase + ): void { + $shopRepository = $this->createMock(ShopRepositoryInterface::class); + + $shop = new MockShop('123', $shopUrl, '1234567890'); + + $shopRepository + ->expects(static::once()) + ->method('getShopFromId') + ->willReturn($shop); + + $shopRepository + ->expects(static::never()) + ->method('createShopStruct') + ->willReturn($shop); + + if ($sanitizeShopUrlInDatabase) { + $shopRepository + ->expects(static::exactly(2)) + ->method('updateShop') + ->with($shop); + } else { + $shopRepository + ->expects(static::once()) + ->method('updateShop') + ->with($shop); + } + + $registrationService = new RegistrationService( + $this->appConfiguration, + $shopRepository, + $this->createMock(RequestVerifier::class), + new ResponseSigner(), + new RandomStringShopSecretGenerator(), + new NullLogger(), + null + ); + + $request = new Request( + 'GET', + sprintf('http://localhost?shop-id=123&shop-url=%s×tamp=1234567890', $shopUrl) + ); + + $registrationService->register($request); static::assertSame($expectedUrl, $shop->getShopUrl()); } @@ -505,68 +612,80 @@ public static function missingShopParametersProvider(): iterable } /** - * @return iterable> + * @return iterable> */ public static function shopUrlsProvider(): iterable { yield 'Valid URL with port' => [ 'shopUrl' => 'https://my-shop.com:80', 'expectedUrl' => 'https://my-shop.com:80', + 'sanitizeShopUrlInDatabase' => false, ]; yield 'Valid URL with port and trailing slash' => [ 'shopUrl' => 'https://my-shop.com:8080/', 'expectedUrl' => 'https://my-shop.com:8080', + 'sanitizeShopUrlInDatabase' => true, ]; yield 'Valid URL with port, path and trailing slash' => [ 'shopUrl' => 'https://my-shop.com:8080//test/', 'expectedUrl' => 'https://my-shop.com:8080/test', + 'sanitizeShopUrlInDatabase' => true, ]; yield 'Valid URL without trailing slash' => [ 'shopUrl' => 'https://my-shop.com', 'expectedUrl' => 'https://my-shop.com', + 'sanitizeShopUrlInDatabase' => false, ]; yield 'Valid URL with trailing slash' => [ 'shopUrl' => 'https://my-shop.com/', 'expectedUrl' => 'https://my-shop.com', + 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with trailing slash' => [ 'shopUrl' => 'https://my-shop.com/test/', 'expectedUrl' => 'https://my-shop.com/test', + 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with double slashes' => [ 'shopUrl' => 'https://my-shop.com//test', 'expectedUrl' => 'https://my-shop.com/test', + 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with 2 slashes and trailing slash' => [ 'shopUrl' => 'https://my-shop.com//test/', 'expectedUrl' => 'https://my-shop.com/test', + 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with 3 slashes and trailing slash' => [ 'shopUrl' => 'https://my-shop.com///test/', 'expectedUrl' => 'https://my-shop.com/test', + 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with multiple slashes' => [ 'shopUrl' => 'https://my-shop.com///test/test1//test2', 'expectedUrl' => 'https://my-shop.com/test/test1/test2', + 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with multiple slashes and trailing slash' => [ 'shopUrl' => 'https://my-shop.com///test/test1//test2/', 'expectedUrl' => 'https://my-shop.com/test/test1/test2', + 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with multiple slashes and multplie trailing slash' => [ 'shopUrl' => 'https://my-shop.com///test/test1//test2//', 'expectedUrl' => 'https://my-shop.com/test/test1/test2', + 'sanitizeShopUrlInDatabase' => true, ]; } }