fix: make context commands key protected

lernhart · lernhart · commit e807e4a639c7 · 2025-05-28T11:20:20.000+02:00
diff --git a/src/Gateway/Context/ContextGatewayCommand.php b/src/Gateway/Context/ContextGatewayCommand.php
@@ -11,7 +11,12 @@ abstract class ContextGatewayCommand implements \JsonSerializable
      */
     public array $payload = [];
 
-    public string $keyName;
+    protected string $keyName;
+
+    public function getKey(): string
+    {
+        return $this->keyName;
+    }
 
     public function setPayloadValue(string $key, mixed $value): void
     {
diff --git a/src/Registration/RegistrationService.php b/src/Registration/RegistrationService.php
@@ -19,8 +19,8 @@
 use Shopware\App\SDK\Event\RegistrationCompletedEvent;
 use Shopware\App\SDK\Exception\MissingShopParameterException;
 use Shopware\App\SDK\Exception\ShopNotFoundException;
-use Shopware\App\SDK\Exception\SignatureNotFoundException;
 use Shopware\App\SDK\Exception\SignatureInvalidException;
+use Shopware\App\SDK\Exception\SignatureNotFoundException;
 use Shopware\App\SDK\Shop\ShopInterface;
 use Shopware\App\SDK\Shop\ShopRepositoryInterface;
 
diff --git a/tests/Context/ContextResolverTest.php b/tests/Context/ContextResolverTest.php
@@ -1177,6 +1177,61 @@ public function testAssembleContextGatewayRequest(): void
         static::assertSame(['foo' => 'bar'], $action->data);
     }
 
+    public function testAssembleContextGatewayWithInvalidData(): void
+    {
+        $contextResolver = new ContextResolver($this->createMock(InAppPurchaseProvider::class));
+
+        $body = [
+            'source' => [
+                'url' => 'https://example.com',
+                'appVersion' => 'foo',
+                'inAppPurchases' => 'ey',
+            ],
+            'cart' => [
+                'token' => 'cart-token',
+            ],
+            'salesChannelContext' => [
+                'salesChannel' => [
+                    'id' => 'sales-channel-id'
+                ],
+            ],
+            'data' => 'foo',
+        ];
+
+        $request = new Request('POST', '/', [], json_encode($body, JSON_THROW_ON_ERROR));
+
+        static::expectException(MalformedWebhookBodyException::class);
+
+        $contextResolver->assembleContextGatewayRequest($request, $this->getShop());
+    }
+
+    public function testAssembleContextGatewayWithMissingData(): void
+    {
+        $contextResolver = new ContextResolver($this->createMock(InAppPurchaseProvider::class));
+
+        $body = [
+            'source' => [
+                'url' => 'https://example.com',
+                'appVersion' => 'foo',
+                'inAppPurchases' => 'ey',
+            ],
+            'cart' => [
+                'token' => 'cart-token',
+            ],
+            'salesChannelContext' => [
+                'salesChannel' => [
+                    'id' => 'sales-channel-id'
+                ],
+            ],
+        ];
+
+        $request = new Request('POST', '/', [], json_encode($body, JSON_THROW_ON_ERROR));
+
+        static::expectException(MalformedWebhookBodyException::class);
+
+        $contextResolver->assembleContextGatewayRequest($request, $this->getShop());
+    }
+
     public function testAssembleInAppPurchasesFilterRequest(): void
     {
         $expectedPurchases = new Collection(['identifier-1', 'identifier-2', 'identifier-3']);
diff --git a/tests/Gateway/Context/Command/AddCustomerMessageCommandTest.php b/tests/Gateway/Context/Command/AddCustomerMessageCommandTest.php
@@ -17,7 +17,6 @@ public function testConstruct(): void
 
         static::assertSame('foo', $command->message);
         static::assertSame('foo', $command->getPayloadValue('message'));
-        static::assertSame('context_add-customer-message', $command->keyName);
     }
 
     public function testKey(): void
diff --git a/tests/Gateway/Context/Command/ChangeBillingAddressCommandTest.php b/tests/Gateway/Context/Command/ChangeBillingAddressCommandTest.php
@@ -17,7 +17,6 @@ public function testConstruct(): void
 
         static::assertSame('foo', $command->addressId);
         static::assertSame('foo', $command->getPayloadValue('addressId'));
-        static::assertSame('context_change-billing-address', $command->keyName);
     }
 
     public function testKey(): void
diff --git a/tests/Gateway/Context/Command/ChangeCurrencyCommandTest.php b/tests/Gateway/Context/Command/ChangeCurrencyCommandTest.php
@@ -17,7 +17,6 @@ public function testConstruct(): void
 
         static::assertSame('EUR', $command->iso);
         static::assertSame('EUR', $command->getPayloadValue('iso'));
-        static::assertSame('context_change-currency', $command->keyName);
     }
 
     public function testKey(): void
diff --git a/tests/Gateway/Context/Command/ChangeLanguageCommandTest.php b/tests/Gateway/Context/Command/ChangeLanguageCommandTest.php
@@ -17,7 +17,6 @@ public function testConstruct(): void
 
         static::assertSame('de-DE', $command->iso);
         static::assertSame('de-DE', $command->getPayloadValue('iso'));
-        static::assertSame('context_change-language', $command->keyName);
     }
 
     public function testKey(): void
diff --git a/tests/Gateway/Context/Command/ChangePaymentMethodCommandTest.php b/tests/Gateway/Context/Command/ChangePaymentMethodCommandTest.php
@@ -17,7 +17,6 @@ public function testConstruct(): void
 
         static::assertSame('payment_technical-name', $command->technicalName);
         static::assertSame('payment_technical-name', $command->getPayloadValue('technicalName'));
-        static::assertSame('context_change-payment-method', $command->keyName);
     }
 
     public function testKey(): void
diff --git a/tests/Gateway/Context/Command/ChangeShippingAddressCommandTest.php b/tests/Gateway/Context/Command/ChangeShippingAddressCommandTest.php
@@ -17,7 +17,6 @@ public function testConstruct(): void
 
         static::assertSame('foo', $command->addressId);
         static::assertSame('foo', $command->getPayloadValue('addressId'));
-        static::assertSame('context_change-shipping-address', $command->keyName);
     }
 
     public function testKey(): void
diff --git a/tests/Gateway/Context/Command/ChangeShippingLocationCommandTest.php b/tests/Gateway/Context/Command/ChangeShippingLocationCommandTest.php
@@ -19,7 +19,6 @@ public function testConstruct(): void
         static::assertSame('DE-BY', $command->countryStateIso);
         static::assertSame('DE', $command->getPayloadValue('countryIso'));
         static::assertSame('DE-BY', $command->getPayloadValue('countryStateIso'));
-        static::assertSame('context_change-shipping-location', $command->keyName);
     }
 
     public function testConstructDefaults(): void
@@ -30,7 +29,6 @@ public function testConstructDefaults(): void
         static::assertSame(null, $command->countryStateIso);
         static::assertFalse($command->hasPayloadValue('countryIso'));
         static::assertFalse($command->hasPayloadValue('countryStateIso'));
-        static::assertSame('context_change-shipping-location', $command->keyName);
     }
 
     public function testKey(): void
diff --git a/tests/Gateway/Context/Command/ChangeShippingMethodCommandTest.php b/tests/Gateway/Context/Command/ChangeShippingMethodCommandTest.php
@@ -17,7 +17,6 @@ public function testConstruct(): void
 
         static::assertSame('shipping_technical-name', $command->technicalName);
         static::assertSame('shipping_technical-name', $command->getPayloadValue('technicalName'));
-        static::assertSame('context_change-shipping-method', $command->keyName);
     }
 
     public function testKey(): void
diff --git a/tests/Gateway/Context/Command/LoginCustomerCommandTest.php b/tests/Gateway/Context/Command/LoginCustomerCommandTest.php
@@ -17,7 +17,6 @@ public function testConstruct(): void
 
         static::assertSame('foo@bar.com', $command->customerEmail);
         static::assertSame('foo@bar.com', $command->getPayloadValue('customerEmail'));
-        static::assertSame('context_login-customer', $command->keyName);
     }
 
     public function testKey(): void
diff --git a/tests/Gateway/Context/Command/RegisterCustomerCommandTest.php b/tests/Gateway/Context/Command/RegisterCustomerCommandTest.php
@@ -23,7 +23,6 @@ public function testConstruct(): void
 
         static::assertSame($data, $command->data);
         static::assertSame($data, $command->getPayloadValue('data'));
-        static::assertSame('context_register-customer', $command->keyName);
     }
 
     public function testKey(): void
diff --git a/tests/Gateway/Context/ContextGatewayCommandTest.php b/tests/Gateway/Context/ContextGatewayCommandTest.php
@@ -16,7 +16,7 @@ public function testSetPayloadValue(): void
         $command = $this->getCommand();
         $command->setPayloadValue('key', 'value');
 
-        static::assertEquals('value', $command->getPayloadValue('key'));
+        static::assertSame('value', $command->getPayloadValue('key'));
     }
 
     public function testHasPayloadValue(): void
@@ -40,7 +40,7 @@ public function testGetPayloadValue(): void
         $command = $this->getCommand();
         $command->setPayloadValue('key', 'value');
 
-        static::assertEquals('value', $command->getPayloadValue('key'));
+        static::assertSame('value', $command->getPayloadValue('key'));
     }
 
     public function testGetNonExistentPayloadValue(): void
@@ -53,16 +53,20 @@ public function testGetNonExistentPayloadValue(): void
 
     public function testJsonSerialize(): void
     {
-        $command = $this->getCommand();
-        $command->keyName = 'key';
+        $command = $this->getCommand('foo');
         $command->setPayloadValue('key', 'value');
 
-        static::assertEquals(['command' => 'key', 'payload' => ['key' => 'value']], $command->jsonSerialize());
+        static::assertSame(['command' => 'foo', 'payload' => ['key' => 'value']], $command->jsonSerialize());
+        static::assertSame('foo', $command->getKey());
     }
 
-    private function getCommand(): ContextGatewayCommand
+    private function getCommand(string $key = 'key'): ContextGatewayCommand
     {
-        return new class () extends ContextGatewayCommand {
+        return new class ($key) extends ContextGatewayCommand {
+            public function __construct(string $key)
+            {
+                $this->keyName = $key;
+            }
         };
     }
 }
diff --git a/tests/Registration/RegistrationServiceTest.php b/tests/Registration/RegistrationServiceTest.php
@@ -7,6 +7,7 @@
 use Nyholm\Psr7\Request;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
 use Psr\EventDispatcher\EventDispatcherInterface;
 use Psr\Http\Message\StreamInterface;
 use Psr\Log\LoggerInterface;
@@ -21,7 +22,6 @@
 use Shopware\App\SDK\Exception\ShopNotFoundException;
 use Shopware\App\SDK\Registration\RandomStringShopSecretGenerator;
 use Shopware\App\SDK\Registration\RegistrationService;
-use PHPUnit\Framework\TestCase;
 use Shopware\App\SDK\Shop\ShopRepositoryInterface;
 use Shopware\App\SDK\Test\MockShop;
 use Shopware\App\SDK\Test\MockShopRepository;
@@ -445,10 +445,46 @@ public function testBodyRewindIsCalled(): void
         $this->registerService->registerConfirm($request);
     }
 
+    public function testGetSanitizedShopIsCloned(): void
+    {
+        $shop = new MockShop('123', 'https://foo.com', '1234567890');
+
+        $shopRepo = $this->createMock(ShopRepositoryInterface::class);
+        $shopRepo
+            ->expects(static::once())
+            ->method('getShopFromId')
+            ->with('123')
+            ->willReturn(null);
+
+        $shopRepo
+            ->expects(static::once())
+            ->method('createShopStruct')
+            ->willReturn($shop);
+
+        $shopRepo
+            ->expects(static::once())
+            ->method('createShop')
+            ->with(static::callback(function (MockShop $clonedShop) use ($shop): bool {
+                static::assertEquals($shop, $clonedShop);
+                static::assertNotSame($shop, $clonedShop, 'The shop should be cloned during registration.');
+
+                return true;
+            }));
+
+        $service = new RegistrationService(
+            $this->appConfiguration,
+            $shopRepo,
+            $this->createMock(RequestVerifier::class),
+        );
+
+        $request = new Request('GET', 'http://localhost?shop-id=123&shop-url=https://foo.com');
+        $service->register($request);
+    }
+
     /**
-     * @dataProvider missingRegisterShopParametersProvider
      * @param array<string, mixed> $params
      */
+    #[DataProvider('missingRegisterShopParametersProvider')]
     public function testRegisterMissingShopParameters(array $params): void
     {
         // Skip test, provider is for another test
@@ -475,9 +511,9 @@ public function testRegisterMissingShopParameters(array $params): void
     }
 
     /**
-     * @dataProvider missingRegisterConfirmShopParametersProvider
      * @param array<string, mixed> $params
      */
+    #[DataProvider('missingRegisterConfirmShopParametersProvider')]
     public function testRegisterConfirmMissingShopParameters(array $params): void
     {
         $request = new Request('POST', '/', [], \json_encode($params, \JSON_THROW_ON_ERROR));

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,6 @@ public function testConstruct(): void`
`17`	`17`
`18`	`18`	`static::assertSame('foo', $command->message);`
`19`	`19`	`static::assertSame('foo', $command->getPayloadValue('message'));`
`20`		`- static::assertSame('context_add-customer-message', $command->keyName);`
`21`	`20`	`}`
`22`	`21`
`23`	`22`	`public function testKey(): void`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,6 @@ public function testConstruct(): void`
`19`	`19`	`static::assertSame('DE-BY', $command->countryStateIso);`
`20`	`20`	`static::assertSame('DE', $command->getPayloadValue('countryIso'));`
`21`	`21`	`static::assertSame('DE-BY', $command->getPayloadValue('countryStateIso'));`
`22`		`- static::assertSame('context_change-shipping-location', $command->keyName);`
`23`	`22`	`}`
`24`	`23`
`25`	`24`	`public function testConstructDefaults(): void`
`@@ -30,7 +29,6 @@ public function testConstructDefaults(): void`
`30`	`29`	`static::assertSame(null, $command->countryStateIso);`
`31`	`30`	`static::assertFalse($command->hasPayloadValue('countryIso'));`
`32`	`31`	`static::assertFalse($command->hasPayloadValue('countryStateIso'));`
`33`		`- static::assertSame('context_change-shipping-location', $command->keyName);`
`34`	`32`	`}`
`35`	`33`
`36`	`34`	`public function testKey(): void`