From 800e08e79a0417cb4fe6f63498486d718119de82 Mon Sep 17 00:00:00 2001 From: Nicola Saliu Date: Tue, 12 Nov 2024 15:32:56 +0100 Subject: [PATCH 1/3] NEXT-39504 - Sanitize the Shop URL during the registration process --- src/Registration/RegistrationService.php | 22 ++++- .../Registration/RegistrationServiceTest.php | 87 +++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/src/Registration/RegistrationService.php b/src/Registration/RegistrationService.php index abc4502..86aa284 100644 --- a/src/Registration/RegistrationService.php +++ b/src/Registration/RegistrationService.php @@ -57,7 +57,7 @@ public function register(RequestInterface $request): ResponseInterface if ($shop === null) { $shop = $this->shopRepository->createShopStruct( $queries['shop-id'], - $queries['shop-url'], + $this->sanitizeShopUrl($queries['shop-url']), $this->shopSecretGeneratorInterface->generate() ); @@ -127,4 +127,24 @@ public function registerConfirm(RequestInterface $request): ResponseInterface return (new Psr17Factory())->createResponse(204); } + + private function sanitizeShopUrl(string $shopUrl): string + { + $parsedUrl = parse_url($shopUrl); + + $protocol = $parsedUrl['scheme'] ?? ''; + $host = $parsedUrl['host'] ?? ''; + $path = $parsedUrl['path'] ?? ''; + + /** @var string $normalizedPath */ + $normalizedPath = preg_replace('#/{2,}#', '/', $path); + $normalizedPath = rtrim($normalizedPath, '/'); + + return sprintf( + '%s://%s%s', + $protocol, + $host, + $normalizedPath + ); + } } diff --git a/tests/Registration/RegistrationServiceTest.php b/tests/Registration/RegistrationServiceTest.php index 03b3023..ff5485d 100644 --- a/tests/Registration/RegistrationServiceTest.php +++ b/tests/Registration/RegistrationServiceTest.php @@ -6,6 +6,7 @@ use Nyholm\Psr7\Request; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use Psr\EventDispatcher\EventDispatcherInterface; use Psr\Http\Message\StreamInterface; use Psr\Log\LoggerInterface; @@ -337,6 +338,23 @@ public function testRegisterConfirmMissingShopParameters(array $params): void $registrationService->registerConfirm($request); } + #[DataProvider('shopUrlsProvider')] + public function testRegisterShopUrlIsSanitized( + string $shopUrl, + string $expectedUrl, + ): void { + $request = new Request( + 'GET', + sprintf('http://localhost?shop-id=123&shop-url=%s×tamp=1234567890', $shopUrl) + ); + + $this->registerService->register($request); + + $shop = $this->shopRepository->getShopFromId('123'); + + $this->assertSame($expectedUrl, $shop->getShopUrl()); + } + /** * @return iterable>> */ @@ -359,4 +377,73 @@ public static function missingShopParametersProvider(): iterable yield [['shop-id' => '123', 'apiKey' => '123']]; yield [['shop-id' => '123', 'apiKey' => '123', 'secretKey' => 123]]; } + + /** + * @return iterable> + */ + public static function shopUrlsProvider(): iterable + { + yield 'Valid URL without trailing slash' => [ + 'shopUrl' => 'https://my-shop.com', + 'expectedUrl' => 'https://my-shop.com', + 'expectedException' => null, + 'expectedExceptionMessage' => null, + ]; + + yield 'Valid URL with trailing slash' => [ + 'shopUrl' => 'https://my-shop.com/', + 'expectedUrl' => 'https://my-shop.com', + 'expectedException' => null, + 'expectedExceptionMessage' => null, + ]; + + yield 'Invalid URL with trailing slash' => [ + 'shopUrl' => 'https://my-shop.com/test/', + 'expectedUrl' => 'https://my-shop.com/test', + 'expectedException' => null, + 'expectedExceptionMessage' => null, + ]; + + yield 'Invalid URL with double slashes' => [ + 'shopUrl' => 'https://my-shop.com//test', + 'expectedUrl' => 'https://my-shop.com/test', + 'expectedException' => null, + 'expectedExceptionMessage' => null, + ]; + + yield 'Invalid URL with 2 slashes and trailing slash' => [ + 'shopUrl' => 'https://my-shop.com//test/', + 'expectedUrl' => 'https://my-shop.com/test', + 'expectedException' => null, + 'expectedExceptionMessage' => null, + ]; + + yield 'Invalid URL with 3 slashes and trailing slash' => [ + 'shopUrl' => 'https://my-shop.com///test/', + 'expectedUrl' => 'https://my-shop.com/test', + 'expectedException' => null, + 'expectedExceptionMessage' => null, + ]; + + yield 'Invalid URL with multiple slashes' => [ + 'shopUrl' => 'https://my-shop.com///test/test1//test2', + 'expectedUrl' => 'https://my-shop.com/test/test1/test2', + 'expectedException' => null, + 'expectedExceptionMessage' => null, + ]; + + 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', + 'expectedException' => null, + 'expectedExceptionMessage' => null, + ]; + + 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', + 'expectedException' => null, + 'expectedExceptionMessage' => null, + ]; + } } From 556da469d817489affe94208202841d5cf6643ce Mon Sep 17 00:00:00 2001 From: Nicola Saliu Date: Tue, 12 Nov 2024 15:39:18 +0100 Subject: [PATCH 2/3] NEXT-39504 - Fix PR issue --- .../Registration/RegistrationServiceTest.php | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/tests/Registration/RegistrationServiceTest.php b/tests/Registration/RegistrationServiceTest.php index ff5485d..6ecc237 100644 --- a/tests/Registration/RegistrationServiceTest.php +++ b/tests/Registration/RegistrationServiceTest.php @@ -352,7 +352,7 @@ public function testRegisterShopUrlIsSanitized( $shop = $this->shopRepository->getShopFromId('123'); - $this->assertSame($expectedUrl, $shop->getShopUrl()); + static::assertSame($expectedUrl, $shop->getShopUrl()); } /** @@ -386,64 +386,47 @@ public static function shopUrlsProvider(): iterable yield 'Valid URL without trailing slash' => [ 'shopUrl' => 'https://my-shop.com', 'expectedUrl' => 'https://my-shop.com', - 'expectedException' => null, - 'expectedExceptionMessage' => null, ]; yield 'Valid URL with trailing slash' => [ 'shopUrl' => 'https://my-shop.com/', 'expectedUrl' => 'https://my-shop.com', - 'expectedException' => null, - 'expectedExceptionMessage' => null, + ]; yield 'Invalid URL with trailing slash' => [ 'shopUrl' => 'https://my-shop.com/test/', 'expectedUrl' => 'https://my-shop.com/test', - 'expectedException' => null, - 'expectedExceptionMessage' => null, ]; yield 'Invalid URL with double slashes' => [ 'shopUrl' => 'https://my-shop.com//test', 'expectedUrl' => 'https://my-shop.com/test', - 'expectedException' => null, - 'expectedExceptionMessage' => null, ]; yield 'Invalid URL with 2 slashes and trailing slash' => [ 'shopUrl' => 'https://my-shop.com//test/', 'expectedUrl' => 'https://my-shop.com/test', - 'expectedException' => null, - 'expectedExceptionMessage' => null, ]; yield 'Invalid URL with 3 slashes and trailing slash' => [ 'shopUrl' => 'https://my-shop.com///test/', 'expectedUrl' => 'https://my-shop.com/test', - 'expectedException' => null, - 'expectedExceptionMessage' => null, ]; yield 'Invalid URL with multiple slashes' => [ 'shopUrl' => 'https://my-shop.com///test/test1//test2', 'expectedUrl' => 'https://my-shop.com/test/test1/test2', - 'expectedException' => null, - 'expectedExceptionMessage' => null, ]; 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', - 'expectedException' => null, - 'expectedExceptionMessage' => null, ]; 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', - 'expectedException' => null, - 'expectedExceptionMessage' => null, ]; } } From fd15027759abcb99b63d35e24185c31969e29e49 Mon Sep 17 00:00:00 2001 From: Nicola Saliu Date: Tue, 12 Nov 2024 18:36:19 +0100 Subject: [PATCH 3/3] NEXT-39504 - Consider also the port when building the Shop URL --- src/Registration/RegistrationService.php | 4 +++- tests/Registration/RegistrationServiceTest.php | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/Registration/RegistrationService.php b/src/Registration/RegistrationService.php index 86aa284..4e6a4de 100644 --- a/src/Registration/RegistrationService.php +++ b/src/Registration/RegistrationService.php @@ -135,15 +135,17 @@ private function sanitizeShopUrl(string $shopUrl): string $protocol = $parsedUrl['scheme'] ?? ''; $host = $parsedUrl['host'] ?? ''; $path = $parsedUrl['path'] ?? ''; + $port = $parsedUrl['port'] ?? ''; /** @var string $normalizedPath */ $normalizedPath = preg_replace('#/{2,}#', '/', $path); $normalizedPath = rtrim($normalizedPath, '/'); return sprintf( - '%s://%s%s', + '%s://%s%s%s', $protocol, $host, + $port ? ':' . $port : null, $normalizedPath ); } diff --git a/tests/Registration/RegistrationServiceTest.php b/tests/Registration/RegistrationServiceTest.php index 6ecc237..45929cf 100644 --- a/tests/Registration/RegistrationServiceTest.php +++ b/tests/Registration/RegistrationServiceTest.php @@ -383,6 +383,21 @@ public static function missingShopParametersProvider(): iterable */ public static function shopUrlsProvider(): iterable { + yield 'Valid URL with port' => [ + 'shopUrl' => 'https://my-shop.com:80', + 'expectedUrl' => 'https://my-shop.com:80', + ]; + + yield 'Valid URL with port and trailing slash' => [ + 'shopUrl' => 'https://my-shop.com:8080/', + 'expectedUrl' => 'https://my-shop.com:8080', + ]; + + yield 'Valid URL with port, path and trailing slash' => [ + 'shopUrl' => 'https://my-shop.com:8080//test/', + 'expectedUrl' => 'https://my-shop.com:8080/test', + ]; + yield 'Valid URL without trailing slash' => [ 'shopUrl' => 'https://my-shop.com', 'expectedUrl' => 'https://my-shop.com', @@ -391,7 +406,6 @@ public static function shopUrlsProvider(): iterable yield 'Valid URL with trailing slash' => [ 'shopUrl' => 'https://my-shop.com/', 'expectedUrl' => 'https://my-shop.com', - ]; yield 'Invalid URL with trailing slash' => [