From b33526fd0b3d90fdcb937462f50f34a07a85e0f1 Mon Sep 17 00:00:00 2001 From: Moritz Krafeld Date: Thu, 16 Jan 2025 15:14:32 +0100 Subject: [PATCH] fix: use sanitized shop url in before registration starts event --- src/Registration/RegistrationService.php | 52 +++--- .../Registration/RegistrationServiceTest.php | 154 ++++++------------ 2 files changed, 79 insertions(+), 127 deletions(-) diff --git a/src/Registration/RegistrationService.php b/src/Registration/RegistrationService.php index deb6024..6148b4f 100644 --- a/src/Registration/RegistrationService.php +++ b/src/Registration/RegistrationService.php @@ -5,6 +5,7 @@ namespace Shopware\App\SDK\Registration; use Http\Discovery\Psr17Factory; +use Nyholm\Psr7\Uri; use Psr\EventDispatcher\EventDispatcherInterface; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; @@ -62,32 +63,33 @@ public function register(RequestInterface $request): ResponseInterface $this->shopSecretGeneratorInterface->generate() ); - $this->eventDispatcher?->dispatch(new BeforeRegistrationStartsEvent($request, $shop)); + $sanitizedShop = $this->getSanitizedShop($shop); + $this->eventDispatcher?->dispatch(new BeforeRegistrationStartsEvent($request, $sanitizedShop)); - $this->shopRepository->createShop($shop); + $this->shopRepository->createShop($sanitizedShop); } else { $shop->setShopUrl($queries['shop-url']); - $this->eventDispatcher?->dispatch(new BeforeRegistrationStartsEvent($request, $shop)); + $sanitizedShop = $this->getSanitizedShop($shop); + $this->eventDispatcher?->dispatch(new BeforeRegistrationStartsEvent($request, $sanitizedShop)); - $this->shopRepository->updateShop($shop); + $this->shopRepository->updateShop($sanitizedShop); } $this->logger->info('Shop registration request received', [ - 'shop-id' => $shop->getShopId(), - 'shop-url' => $shop->getShopUrl(), + 'shop-id' => $sanitizedShop->getShopId(), + 'shop-url' => $sanitizedShop->getShopUrl(), ]); $psrFactory = new Psr17Factory(); $data = [ + // old shop is needed because the shop url is not sanitized 'proof' => $this->responseSigner->getRegistrationSignature($this->appConfiguration, $shop), 'confirmation_url' => $this->appConfiguration->getRegistrationConfirmUrl(), 'secret' => $shop->getShopSecret(), ]; - $this->fixShopUrlInDatabase($shop); - $response = $psrFactory->createResponse(200); return $response @@ -145,34 +147,30 @@ public function registerConfirm(RequestInterface $request): ResponseInterface private function sanitizeShopUrl(string $shopUrl): string { - $parsedUrl = parse_url($shopUrl); + $uri = new Uri($shopUrl); - $protocol = $parsedUrl['scheme'] ?? ''; - $host = $parsedUrl['host'] ?? ''; - $path = $parsedUrl['path'] ?? ''; - $port = $parsedUrl['port'] ?? ''; + $protocol = $uri->getScheme(); + $host = $uri->getHost(); + $path = $uri->getPath(); + $port = $uri->getPort(); /** @var string $normalizedPath */ $normalizedPath = preg_replace('#/{2,}#', '/', $path); $normalizedPath = rtrim($normalizedPath, '/'); - return sprintf( - '%s://%s%s%s', - $protocol, - $host, - $port ? ':' . $port : null, - $normalizedPath - ); + $url = $protocol . '://' . $host; + if ($port) { + $url .= ':' . $port; + } + $url .= $normalizedPath; + + return $url; } - private function fixShopUrlInDatabase(ShopInterface $shop): void + private function getSanitizedShop(ShopInterface $shop): ShopInterface { - $sanitizedShopUrl = $this->sanitizeShopUrl($shop->getShopUrl()); + $sanitizedShop = clone $shop; - - if ($shop->getShopUrl() !== $sanitizedShopUrl) { - $shop->setShopUrl($sanitizedShopUrl); - $this->shopRepository->updateShop($shop); - } + return $sanitizedShop->setShopUrl($this->sanitizeShopUrl($shop->getShopUrl())); } } diff --git a/tests/Registration/RegistrationServiceTest.php b/tests/Registration/RegistrationServiceTest.php index f85cc54..dcbe69c 100644 --- a/tests/Registration/RegistrationServiceTest.php +++ b/tests/Registration/RegistrationServiceTest.php @@ -107,7 +107,7 @@ public function testRegisterCreate(): void static::assertSame(200, $response->getStatusCode()); static::assertSame('https://my-shop.com', $shop->getShopUrl()); - $json = json_decode((string) $response->getBody()->getContents(), true); + $json = json_decode((string)$response->getBody()->getContents(), true); static::assertCount(1, $events); static::assertInstanceOf(BeforeRegistrationStartsEvent::class, $events[0]); @@ -156,7 +156,7 @@ public function testRegisterCreateMustNotDispatchBeforeRegistrationStartsEvent() static::assertSame(200, $response->getStatusCode()); static::assertSame('https://my-shop.com', $shop->getShopUrl()); - $json = json_decode((string) $response->getBody()->getContents(), true); + $json = json_decode((string)$response->getBody()->getContents(), true); static::assertIsArray($json); static::assertArrayHasKey('proof', $json); @@ -496,13 +496,12 @@ public function testRegisterConfirmMissingShopParameters(array $params): void #[DataProvider('shopUrlsProviderForCreation')] public function testRegisterCreateShopUrlIsSanitized( - string $shopUrl, + string $unsanitizedShopUrl, string $expectedUrl, - bool $sanitizeShopUrlInDatabase ): void { $shopRepository = $this->createMock(ShopRepositoryInterface::class); - $shop = new MockShop('123', $shopUrl, '1234567890'); + $expectedShop = new MockShop('123', $expectedUrl, '1234567890'); $shopRepository ->expects(static::once()) @@ -513,14 +512,7 @@ public function testRegisterCreateShopUrlIsSanitized( $shopRepository ->expects(static::once()) ->method('createShopStruct') - ->willReturn($shop); - - if ($sanitizeShopUrlInDatabase) { - $shopRepository - ->expects(static::once()) - ->method('updateShop') - ->with($shop); - } + ->willReturn($expectedShop); $registrationService = new RegistrationService( $this->appConfiguration, @@ -534,25 +526,22 @@ public function testRegisterCreateShopUrlIsSanitized( $request = new Request( 'GET', - sprintf('http://localhost?shop-id=123&shop-url=%s×tamp=1234567890', $shopUrl) + sprintf('http://localhost?shop-id=123&shop-url=%s×tamp=1234567890', $unsanitizedShopUrl) ); $registrationService->register($request); - - static::assertSame($expectedUrl, $shop->getShopUrl()); } #[DataProvider('shopUrlsProviderForUpdate')] public function testRegisterUpdateShopUrlIsSanitized( - string $shopUrl, - string $newShopUrl, + string $oldShopUrl, + string $newUnsanitizedShopUrl, string $expectedUrl, - bool $sanitizeShopUrlInDatabase ): void { $shopRepository = $this->createMock(ShopRepositoryInterface::class); - $shop = new MockShop('123', $shopUrl, '1234567890'); + $shop = new MockShop('123', $oldShopUrl, '1234567890'); $shopRepository ->expects(static::once()) @@ -563,31 +552,12 @@ public function testRegisterUpdateShopUrlIsSanitized( ->expects(static::never()) ->method('createShopStruct'); - $expectedUrls = [ - $newShopUrl, - $expectedUrl, - ]; - - $callCount = 0; - - if ($sanitizeShopUrlInDatabase) { - $shopRepository - ->expects(static::exactly(2)) - ->method('updateShop') - ->with($this->callback(function (MockShop $shop) use (&$callCount, $expectedUrls) { - $expectedUrl = $expectedUrls[$callCount] ?? null; - $result = $shop->getShopUrl() === $expectedUrl; - $callCount++; - return $result; - })); - } else { - $shopRepository - ->expects(static::once()) - ->method('updateShop') - ->with($this->callback(function (MockShop $shop) use ($newShopUrl) { - return $shop->getShopUrl() === $newShopUrl; - })); - } + $shopRepository + ->expects(static::once()) + ->method('updateShop') + ->with($this->callback(function (MockShop $shop) use ($expectedUrl) { + return $shop->getShopUrl() === $expectedUrl; + })); $registrationService = new RegistrationService( $this->appConfiguration, @@ -601,12 +571,10 @@ public function testRegisterUpdateShopUrlIsSanitized( $request = new Request( 'GET', - sprintf('http://localhost?shop-id=123&shop-url=%s×tamp=1234567890', $newShopUrl) + sprintf('http://localhost?shop-id=123&shop-url=%s×tamp=1234567890', $newUnsanitizedShopUrl) ); $registrationService->register($request); - - static::assertSame($expectedUrl, $shop->getShopUrl()); } /** @@ -659,75 +627,63 @@ public static function missingRegisterConfirmShopParametersProvider(): iterable public function shopUrlsProviderForCreation(): iterable { yield 'Valid URL with port' => [ - 'shopUrl' => 'https://my-shop.com:80', + 'unsanitizedShopUrl' => '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/', + 'unsanitizedShopUrl' => '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/', + 'unsanitizedShopUrl' => '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', + 'unsanitizedShopUrl' => 'https://my-shop.com', 'expectedUrl' => 'https://my-shop.com', - 'sanitizeShopUrlInDatabase' => false, ]; yield 'Valid URL with trailing slash' => [ - 'shopUrl' => 'https://my-shop.com/', + 'unsanitizedShopUrl' => 'https://my-shop.com/', 'expectedUrl' => 'https://my-shop.com', - 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with trailing slash' => [ - 'shopUrl' => 'https://my-shop.com/test/', + 'unsanitizedShopUrl' => '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', + 'unsanitizedShopUrl' => '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/', + 'unsanitizedShopUrl' => '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/', + 'unsanitizedShopUrl' => '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', + 'unsanitizedShopUrl' => '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/', + 'unsanitizedShopUrl' => 'https://my-shop.com///test/test1//test2/', 'expectedUrl' => 'https://my-shop.com/test/test1/test2', - 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with multiple slashes and multiple trailing slash' => [ - 'shopUrl' => 'https://my-shop.com///test/test1//test2//', + 'unsanitizedShopUrl' => 'https://my-shop.com///test/test1//test2//', 'expectedUrl' => 'https://my-shop.com/test/test1/test2', - 'sanitizeShopUrlInDatabase' => true, ]; } @@ -737,85 +693,83 @@ public function shopUrlsProviderForCreation(): iterable public static function shopUrlsProviderForUpdate(): iterable { yield 'Valid URL with port' => [ - 'shopUrl' => 'https://my-shop.com:80', - 'newShopUrl' => 'https://my-changed-shop.de:80', - 'expectedUrl' => 'https://my-changed-shop.de:80', - 'sanitizeShopUrlInDatabase' => false, + 'oldShopUrl' => 'https://my-shop.com:80', + 'newUnsanitizedShopUrl' => 'https://my-changed-shop.de:80', + 'expectedUrl' => 'https://my-changed-shop.de:80', ]; yield 'Valid URL with port and trailing slash' => [ - 'shopUrl' => 'https://my-shop.com:8080/', - 'newShopUrl' => 'https://my-changed-shop.com:8080/', + 'oldShopUrl' => 'https://my-shop.com:8080/', + 'newUnsanitizedShopUrl' => 'https://my-changed-shop.com:8080/', 'expectedUrl' => 'https://my-changed-shop.com:8080', - 'sanitizeShopUrlInDatabase' => true, ]; - + // yield 'Valid URL with port, path and trailing slash' => [ - 'shopUrl' => 'https://my-shop.com:8080//test/', - 'newShopUrl' => 'https://my-changed-shop.com:8080//test/', + 'oldShopUrl' => 'https://my-shop.com:8080//test/', + 'newUnsanitizedShopUrl' => 'https://my-changed-shop.com:8080//test/', 'expectedUrl' => 'https://my-changed-shop.com:8080/test', 'sanitizeShopUrlInDatabase' => true, ]; yield 'Valid URL without trailing slash' => [ - 'shopUrl' => 'https://my-shop.com', - 'newShopUrl' => 'https://my-changed-shop.com', + 'oldShopUrl' => 'https://my-shop.com', + 'newUnsanitizedShopUrl' => 'https://my-changed-shop.com', 'expectedUrl' => 'https://my-changed-shop.com', 'sanitizeShopUrlInDatabase' => false, ]; yield 'Valid URL with trailing slash' => [ - 'shopUrl' => 'https://my-shop.com/', - 'newShopUrl' => 'https://my-changed-shop.com/', + 'oldShopUrl' => 'https://my-shop.com/', + 'newUnsanitizedShopUrl' => 'https://my-changed-shop.com/', 'expectedUrl' => 'https://my-changed-shop.com', 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with trailing slash' => [ - 'shopUrl' => 'https://my-shop.com/test/', - 'newShopUrl' => 'https://my-changed-shop.com/test/', + 'oldShopUrl' => 'https://my-shop.com/test/', + 'newUnsanitizedShopUrl' => 'https://my-changed-shop.com/test/', 'expectedUrl' => 'https://my-changed-shop.com/test', 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with double slashes' => [ - 'shopUrl' => 'https://my-shop.com//test', - 'newShopUrl' => 'https://my-changed-shop.com//test', + 'oldShopUrl' => 'https://my-shop.com//test', + 'newUnsanitizedShopUrl' => 'https://my-changed-shop.com//test', 'expectedUrl' => 'https://my-changed-shop.com/test', 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with 2 slashes and trailing slash' => [ - 'shopUrl' => 'https://my-shop.com//test/', - 'newShopUrl' => 'https://my-changed-shop.com//test/', + 'oldShopUrl' => 'https://my-shop.com//test/', + 'newUnsanitizedShopUrl' => 'https://my-changed-shop.com//test/', 'expectedUrl' => 'https://my-changed-shop.com/test', 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with 3 slashes and trailing slash' => [ - 'shopUrl' => 'https://my-shop.com///test/', - 'newShopUrl' => 'https://my-changed-shop.com///test/', + 'oldShopUrl' => 'https://my-shop.com///test/', + 'newUnsanitizedShopUrl' => 'https://my-changed-shop.com///test/', 'expectedUrl' => 'https://my-changed-shop.com/test', 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with multiple slashes' => [ - 'shopUrl' => 'https://my-shop.com///test/test1//test2', - 'newShopUrl' => 'https://my-changed-shop.com///test/test1//test2', + 'oldShopUrl' => 'https://my-shop.com///test/test1//test2', + 'newUnsanitizedShopUrl' => 'https://my-changed-shop.com///test/test1//test2', 'expectedUrl' => 'https://my-changed-shop.com/test/test1/test2', 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with multiple slashes and trailing slash' => [ - 'shopUrl' => 'https://my-shop.com///test/test1//test2/', - 'newShopUrl' => 'https://my-changed-shop.com///test/test1//test2/', + 'oldShopUrl' => 'https://my-shop.com///test/test1//test2/', + 'newUnsanitizedShopUrl' => 'https://my-changed-shop.com///test/test1//test2/', 'expectedUrl' => 'https://my-changed-shop.com/test/test1/test2', 'sanitizeShopUrlInDatabase' => true, ]; yield 'Invalid URL with multiple slashes and multiple trailing slash' => [ - 'shopUrl' => 'https://my-shop.com///test/test1//test2//', - 'newShopUrl' => 'https://my-changed-shop.com///test/test1//test2//', + 'oldShopUrl' => 'https://my-shop.com///test/test1//test2//', + 'newUnsanitizedShopUrl' => 'https://my-changed-shop.com///test/test1//test2//', 'expectedUrl' => 'https://my-changed-shop.com/test/test1/test2', 'sanitizeShopUrlInDatabase' => true, ];