Merge 87c0a92377 into e6f3f56e01

UPGRADE-2.3.md

@@ -44,6 +44,47 @@
      (`identity_generation_preferences` for `PostgreSQLPlatform`) to keep the database schema backward compatible 
      with existing installations.
 
+3. The payment encryption (`sylius_payment.encryption`) got two new options to harden decryption of gateway configs
+   and payment requests. Both are opt-in and keep the previous behavior by default.
+
+   - `strict_mode` (default `false`): when enabled, `Sylius\Component\Payment\Encryption\Encrypter::decrypt()` throws
+     an `EncryptionException` for data that is not encrypted, instead of silently returning it unchanged.
+
+     ```yaml
+     sylius_payment:
+         encryption:
+             strict_mode: true
+     ```
+
+   - `allowed_classes` (default `true`): restricts which PHP classes may be instantiated while decrypting
+     (the `allowed_classes` option of `unserialize()`). Use `true` to allow all classes (previous behavior),
+     `false` to allow none, or a list of class-strings to allow only specific ones. If you store objects in a
+     gateway config or payment request payload (for example a payment plugin deserializing SDK objects such as
+     `Stripe\PaymentIntent`), add those classes to the list when narrowing it down.
+
+     ```yaml
+     sylius_payment:
+         encryption:
+             allowed_classes:
+                 - 'Stripe\PaymentIntent'
+     ```
+
+   The related encrypters gained a matching constructor argument, defaulting to the previous behavior:
+
+   ```diff
+   -public function __construct(private readonly string $encryptionKeyPath)
+   +public function __construct(private readonly string $encryptionKeyPath, private readonly bool $strictDecryption = false)
+   ```
+
+   ```diff
+   -public function __construct(private EncrypterInterface $encrypter)
+   +public function __construct(private EncrypterInterface $encrypter, private array|bool $allowedClasses = true)
+   ```
+
+   Affected classes: `Sylius\Component\Payment\Encryption\Encrypter` (`$strictDecryption`),
+   `Sylius\Component\Payment\Encryption\GatewayConfigEncrypter` and
+   `Sylius\Component\Payment\Encryption\PaymentRequestEncrypter` (`$allowedClasses`).
+
 ## Dependencies
 
 1. The `behat/transliterator` package has been **deprecated** and will be removed in Sylius 3.0.

src/Sylius/Bundle/PaymentBundle/DependencyInjection/Configuration.php

@@ -54,9 +54,18 @@ final class Configuration implements ConfigurationInterface
                     ->addDefaultsIfNotSet()
                     ->children()
                         ->booleanNode('enabled')->defaultTrue()->end()
+                        ->booleanNode('strict_mode')->defaultFalse()->end()
                         ->arrayNode('disabled_for_factories')
                             ->scalarPrototype()->end()
                         ->end()
+                        ->variableNode('allowed_classes')
+                            ->info('Classes allowed during decryption (unserialize). Use "true" to allow all classes, "false" to allow none, or a list of class-strings to allow only specific ones.')
+                            ->defaultTrue()
+                            ->validate()
+                                ->ifTrue(static fn (mixed $value): bool => !is_bool($value) && !is_array($value))
+                                ->thenInvalid('The "allowed_classes" must be a boolean or a list of class-strings, got %s.')
+                            ->end()
+                        ->end()
                     ->end()
                 ->end()
                 ->arrayNode('gateways')

src/Sylius/Bundle/PaymentBundle/DependencyInjection/SyliusPaymentExtension.php

@@ -88,6 +88,8 @@ final class SyliusPaymentExtension extends AbstractResourceExtension
             return;
         }
 
+        $container->setParameter('sylius.encryption.strict_mode', $encryptionConfig['strict_mode']);
+        $container->setParameter('sylius.encryption.allowed_classes', $encryptionConfig['allowed_classes']);
         $container->setParameter('sylius.encryption.disabled_for_factories', $encryptionConfig['disabled_for_factories']);
 
         $loader = new PhpFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config/services/encryption'));

src/Sylius/Bundle/PaymentBundle/Resources/config/services/encryption/encryption.php

@@ -31,17 +31,29 @@ return static function (ContainerConfigurator $container) {
     ;
     $services->alias(GatewayConfigEncryptionCheckerInterface::class, 'sylius.checker.gateway_config_encryption');
 
-    $services->set('sylius.encrypter', Encrypter::class)->args(['%env(resolve:SYLIUS_PAYMENT_ENCRYPTION_KEY_PATH)%']);
+    $services
+        ->set('sylius.encrypter', Encrypter::class)
+        ->args([
+            '%env(resolve:SYLIUS_PAYMENT_ENCRYPTION_KEY_PATH)%',
+            '%sylius.encryption.strict_mode%',
+        ])
+    ;
     $services->alias(EncrypterInterface::class, 'sylius.encrypter');
 
     $services
         ->set('sylius.encrypter.payment_request', PaymentRequestEncrypter::class)
-        ->args([service('sylius.encrypter')])
+        ->args([
+            service('sylius.encrypter'),
+            '%sylius.encryption.allowed_classes%',
+        ])
     ;
 
     $services
         ->set('sylius.encrypter.gateway_config', GatewayConfigEncrypter::class)
-        ->args([service('sylius.encrypter')])
+        ->args([
+            service('sylius.encrypter'),
+            '%sylius.encryption.allowed_classes%',
+        ])
     ;
 
     $services

src/Sylius/Bundle/PaymentBundle/tests/DependencyInjection/ConfigurationTest.php

@@ -62,6 +62,55 @@ final class ConfigurationTest extends TestCase
         );
     }
 
+    #[Test]
+    public function it_disables_strict_decryption_mode_by_default(): void
+    {
+        $this->assertProcessedConfigurationEquals(
+            [[]],
+            ['encryption' => ['strict_mode' => false]],
+            'encryption.strict_mode',
+        );
+    }
+
+    #[Test]
+    public function its_strict_decryption_mode_can_be_enabled(): void
+    {
+        $this->assertProcessedConfigurationEquals(
+            [['encryption' => ['strict_mode' => true]]],
+            ['encryption' => ['strict_mode' => true]],
+            'encryption.strict_mode',
+        );
+    }
+
+    #[Test]
+    public function it_allows_all_classes_during_decryption_by_default(): void
+    {
+        $this->assertProcessedConfigurationEquals(
+            [[]],
+            ['encryption' => ['allowed_classes' => true]],
+            'encryption.allowed_classes',
+        );
+    }
+
+    #[Test]
+    public function it_can_configure_an_explicit_list_of_allowed_classes(): void
+    {
+        $this->assertProcessedConfigurationEquals(
+            [['encryption' => ['allowed_classes' => [\stdClass::class]]]],
+            ['encryption' => ['allowed_classes' => [\stdClass::class]]],
+            'encryption.allowed_classes',
+        );
+    }
+
+    #[Test]
+    public function it_does_not_allow_a_scalar_value_for_allowed_classes(): void
+    {
+        $this->assertPartialConfigurationIsInvalid(
+            [['encryption' => ['allowed_classes' => 'stdClass']]],
+            'encryption.allowed_classes',
+        );
+    }
+
     protected function getConfiguration(): Configuration
     {
         return new Configuration();

src/Sylius/Component/Payment/Encryption/Encrypter.php

@@ -29,6 +29,7 @@ final class Encrypter implements EncrypterInterface
 
     public function __construct(
         private readonly string $encryptionKeyPath,
+        private readonly bool $strictDecryption = false,
     ) {
     }
 
@@ -44,6 +45,12 @@ final class Encrypter implements EncrypterInterface
     public function decrypt(string $data): string
     {
         if (!str_ends_with($data, self::ENCRYPTION_SUFFIX)) {
+            if ($this->strictDecryption) {
+                throw EncryptionException::cannotDecrypt(
+                    new \RuntimeException('Data is not encrypted.'),
+                );
+            }
+
             return $data;
         }
 

src/Sylius/Component/Payment/Encryption/GatewayConfigEncrypter.php

@@ -14,6 +14,7 @@ declare(strict_types=1);
 namespace Sylius\Component\Payment\Encryption;
 
 use Sylius\Component\Payment\Model\GatewayConfigInterface;
+use Webmozart\Assert\Assert;
 
 /**
  * @implements EntityEncrypterInterface<GatewayConfigInterface>
@@ -24,9 +25,15 @@ final readonly class GatewayConfigEncrypter implements EntityEncrypterInterface
 {
     use EncryptionCheckTrait;
 
+    /** @param bool|list<class-string> $allowedClasses */
     public function __construct(
         private EncrypterInterface $encrypter,
+        private array|bool $allowedClasses = true,
     ) {
+        if (is_array($this->allowedClasses)) {
+            Assert::allStringNotEmpty($allowedClasses, 'Each allowed class must be a non-empty string. Got: %s');
+            Assert::allClassExists($allowedClasses, 'Allowed class %s does not exist.');
+        }
     }
 
     public function encrypt(EncryptionAwareInterface $resource): void
@@ -41,15 +48,30 @@ final readonly class GatewayConfigEncrypter implements EntityEncrypterInterface
 
     public function decrypt(EncryptionAwareInterface $resource): void
     {
-        if (!$this->isEncrypted(current($resource->getConfig()))) {
+        $config = $resource->getConfig();
+        if (!$this->isFullyEncrypted($config)) {
             return;
         }
 
         $decryptedConfig = [];
-        foreach ($resource->getConfig() as $key => $value) {
-            $decryptedConfig[$key] = unserialize($this->encrypter->decrypt($value));
+        foreach ($config as $key => $value) {
+            $decryptedConfig[$key] = unserialize($this->encrypter->decrypt($value), ['allowed_classes' => $this->allowedClasses]);
         }
 
-        $resource->setConfig($decryptedConfig);
+        if ([] !== $decryptedConfig) {
+            $resource->setConfig($decryptedConfig);
+        }
+    }
+
+    /** @param array<array-key, mixed> $values */
+    private function isFullyEncrypted(array $values): bool
+    {
+        foreach ($values as $value) {
+            if (!$this->isEncrypted($value)) {
+                return false;
+            }
+        }
+
+        return true;
     }
 }

src/Sylius/Component/Payment/Encryption/PaymentRequestEncrypter.php

@@ -14,6 +14,7 @@ declare(strict_types=1);
 namespace Sylius\Component\Payment\Encryption;
 
 use Sylius\Component\Payment\Model\PaymentRequestInterface;
+use Webmozart\Assert\Assert;
 
 /**
  * @implements EntityEncrypterInterface<PaymentRequestInterface>
@@ -24,9 +25,15 @@ final readonly class PaymentRequestEncrypter implements EntityEncrypterInterface
 {
     use EncryptionCheckTrait;
 
+    /** @param bool|list<class-string> $allowedClasses */
     public function __construct(
         private EncrypterInterface $encrypter,
+        private array|bool $allowedClasses = true,
     ) {
+        if (is_array($this->allowedClasses)) {
+            Assert::allStringNotEmpty($allowedClasses, 'Each allowed class must be a non-empty string. Got: %s');
+            Assert::allClassExists($allowedClasses, 'Allowed class %s does not exist.');
+        }
     }
 
     public function encrypt(EncryptionAwareInterface $resource): void
@@ -46,18 +53,33 @@ final readonly class PaymentRequestEncrypter implements EntityEncrypterInterface
     public function decrypt(EncryptionAwareInterface $resource): void
     {
         if (null !== $resource->getPayload() && $this->isEncrypted($resource->getPayload())) {
-            $resource->setPayload(unserialize($this->encrypter->decrypt($resource->getPayload())));
+            $resource->setPayload(unserialize($this->encrypter->decrypt($resource->getPayload()), ['allowed_classes' => $this->allowedClasses]));
         }
 
-        if (!$this->isEncrypted(current($resource->getResponseData()))) {
+        $responseData = $resource->getResponseData();
+        if (!$this->isFullyEncrypted($responseData)) {
             return;
         }
 
         $decryptedRequestData = [];
-        foreach ($resource->getResponseData() as $key => $value) {
-            $decryptedRequestData[$key] = unserialize($this->encrypter->decrypt($value));
+        foreach ($responseData as $key => $value) {
+            $decryptedRequestData[$key] = unserialize($this->encrypter->decrypt($value), ['allowed_classes' => $this->allowedClasses]);
         }
 
-        $resource->setResponseData($decryptedRequestData);
+        if ([] !== $decryptedRequestData) {
+            $resource->setResponseData($decryptedRequestData);
+        }
+    }
+
+    /** @param array<array-key, mixed> $values */
+    private function isFullyEncrypted(array $values): bool
+    {
+        foreach ($values as $value) {
+            if (!$this->isEncrypted($value)) {
+                return false;
+            }
+        }
+
+        return true;
     }
 }

src/Sylius/Component/Payment/tests/Encryption/EncrypterTest.php

@@ -84,4 +84,12 @@ final class EncrypterTest extends TestCase
     {
         $this->assertSame('data', $this->encrypter->decrypt('data'));
     }
+
+    public function testThrowsAnExceptionWhenStrictDecryptionIsEnabledAndDataIsNotEncrypted(): void
+    {
+        $encrypter = new Encrypter('', true);
+
+        $this->expectException(EncryptionException::class);
+        $encrypter->decrypt('data');
+    }
 }

src/Sylius/Component/Payment/tests/Encryption/GatewayConfigEncrypterTest.php

@@ -173,4 +173,83 @@ final class GatewayConfigEncrypterTest extends TestCase
 
         $this->gatewayConfigEncrypter->decrypt($this->gatewayConfig);
     }
+
+    public function testDoesNotDecryptConfigWhenOnlySomeElementsAreEncrypted(): void
+    {
+        $this->gatewayConfig
+            ->expects($this->once())
+            ->method('getConfig')
+            ->willReturn([
+                'key' => 'encrypted_value#ENCRYPTED',
+                'key-two' => 'not_encrypted_value',
+            ]);
+        $this->encrypter
+            ->expects($this->never())
+            ->method('decrypt');
+        $this->gatewayConfig
+            ->expects($this->never())
+            ->method('setConfig');
+
+        $this->gatewayConfigEncrypter->decrypt($this->gatewayConfig);
+    }
+
+    public function testDecryptsWithExplicitAllowedClasses(): void
+    {
+        $encrypter = $this->createMock(EncrypterInterface::class);
+        $gatewayConfigEncrypter = new GatewayConfigEncrypter($encrypter, [\stdClass::class]);
+
+        $object = new \stdClass();
+        $object->foo = 'bar';
+
+        $gatewayConfig = $this->createMock(GatewayConfigInterface::class);
+        $gatewayConfig
+            ->expects($this->atLeastOnce())
+            ->method('getConfig')
+            ->willReturn(['key' => 'encrypted_value#ENCRYPTED']);
+        $encrypter
+            ->expects($this->once())
+            ->method('decrypt')
+            ->with('encrypted_value#ENCRYPTED')
+            ->willReturn(serialize($object));
+        $gatewayConfig
+            ->expects($this->once())
+            ->method('setConfig')
+            ->with($this->callback(function (array $config): bool {
+                return
+                    isset($config['key']) &&
+                    $config['key'] instanceof \stdClass &&
+                    $config['key']->foo === 'bar'
+                ;
+            }));
+
+        $gatewayConfigEncrypter->decrypt($gatewayConfig);
+    }
+
+    public function testDecryptsWithNoAllowedClasses(): void
+    {
+        $encrypter = $this->createMock(EncrypterInterface::class);
+        $gatewayConfigEncrypter = new GatewayConfigEncrypter($encrypter, false);
+
+        $gatewayConfig = $this->createMock(GatewayConfigInterface::class);
+        $object = new \stdClass();
+        $object->foo = 'bar';
+
+        $gatewayConfig
+            ->expects($this->atLeastOnce())
+            ->method('getConfig')
+            ->willReturn(['key' => 'encrypted_value#ENCRYPTED']);
+        $encrypter
+            ->expects($this->once())
+            ->method('decrypt')
+            ->with('encrypted_value#ENCRYPTED')
+            ->willReturn(serialize($object));
+        $gatewayConfig
+            ->expects($this->once())
+            ->method('setConfig')
+            ->with($this->callback(function (array $config): bool {
+                return isset($config['key']) && $config['key'] instanceof \__PHP_Incomplete_Class;
+            }));
+
+        $gatewayConfigEncrypter->decrypt($gatewayConfig);
+    }
 }

src/Sylius/Component/Payment/tests/Encryption/PaymentRequestEncrypterTest.php

@@ -445,4 +445,133 @@ final class PaymentRequestEncrypterTest extends TestCase
 
         $this->paymentRequestEncrypter->decrypt($this->paymentRequest);
     }
+
+    public function testDoesNotDecryptResponseDataWhenOnlySomeElementsAreEncrypted(): void
+    {
+        $this->paymentRequest
+            ->expects($this->once())
+            ->method('getPayload')
+            ->willReturn(null);
+        $this->paymentRequest
+            ->expects($this->once())
+            ->method('getResponseData')
+            ->willReturn([
+                'key' => 'encrypted_value#ENCRYPTED',
+                'key-two' => 'not_encrypted_value',
+            ]);
+        $this->encrypter
+            ->expects($this->never())
+            ->method('decrypt');
+        $this->paymentRequest
+            ->expects($this->never())
+            ->method('setResponseData');
+
+        $this->paymentRequestEncrypter->decrypt($this->paymentRequest);
+    }
+
+    public function testDecryptsPayloadWithExplicitAllowedClasses(): void
+    {
+        $encrypter = $this->createMock(EncrypterInterface::class);
+        $paymentRequestEncrypter = new PaymentRequestEncrypter($encrypter, [\stdClass::class]);
+
+        $object = new \stdClass();
+        $object->foo = 'bar';
+
+        $paymentRequest = $this->createMock(PaymentRequestInterface::class);
+        $paymentRequest
+            ->expects($this->atLeastOnce())
+            ->method('getPayload')
+            ->willReturn('encrypted_payload#ENCRYPTED');
+        $paymentRequest
+            ->expects($this->once())
+            ->method('getResponseData')
+            ->willReturn([]);
+        $encrypter
+            ->expects($this->once())
+            ->method('decrypt')
+            ->with('encrypted_payload#ENCRYPTED')
+            ->willReturn(serialize($object));
+        $paymentRequest
+            ->expects($this->once())
+            ->method('setPayload')
+            ->with($this->callback(function (mixed $payload): bool {
+                return $payload instanceof \stdClass && $payload->foo === 'bar';
+            }));
+        $paymentRequest
+            ->expects($this->never())
+            ->method('setResponseData');
+
+        $paymentRequestEncrypter->decrypt($paymentRequest);
+    }
+
+    public function testDecryptsWithNoAllowedClasses(): void
+    {
+        $encrypter = $this->createMock(EncrypterInterface::class);
+        $paymentRequestEncrypter = new PaymentRequestEncrypter($encrypter, false);
+
+        $paymentRequest = $this->createMock(PaymentRequestInterface::class);
+        $object = new \stdClass();
+        $object->foo = 'bar';
+
+        $paymentRequest
+            ->expects($this->atLeastOnce())
+            ->method('getPayload')
+            ->willReturn('encrypted_payload#ENCRYPTED');
+        $paymentRequest
+            ->expects($this->once())
+            ->method('getResponseData')
+            ->willReturn([]);
+        $encrypter
+            ->expects($this->once())
+            ->method('decrypt')
+            ->with('encrypted_payload#ENCRYPTED')
+            ->willReturn(serialize($object));
+        $paymentRequest
+            ->expects($this->once())
+            ->method('setPayload')
+            ->with($this->callback(function (mixed $payload): bool {
+                return $payload instanceof \__PHP_Incomplete_Class;
+            }));
+        $paymentRequest
+            ->expects($this->never())
+            ->method('setResponseData');
+
+        $paymentRequestEncrypter->decrypt($paymentRequest);
+    }
+
+    public function testDecryptsResponseDataWithExplicitAllowedClasses(): void
+    {
+        $encrypter = $this->createMock(EncrypterInterface::class);
+        $paymentRequestEncrypter = new PaymentRequestEncrypter($encrypter, [\stdClass::class]);
+
+        $object = new \stdClass();
+        $object->foo = 'bar';
+
+        $paymentRequest = $this->createMock(PaymentRequestInterface::class);
+        $paymentRequest
+            ->expects($this->atLeastOnce())
+            ->method('getPayload')
+            ->willReturn(null);
+        $paymentRequest
+            ->expects($this->atLeastOnce())
+            ->method('getResponseData')
+            ->willReturn(['key' => 'encrypted_value#ENCRYPTED']);
+        $encrypter
+            ->expects($this->once())
+            ->method('decrypt')
+            ->with('encrypted_value#ENCRYPTED')
+            ->willReturn(serialize($object));
+        $paymentRequest
+            ->expects($this->once())
+            ->method('setResponseData')
+            ->with($this->callback(function (array $data): bool {
+                return
+                    isset($data['key']) &&
+                    $data['key'] instanceof \stdClass &&
+                    $data['key']->foo === 'bar'
+                ;
+            }));
+
+        $paymentRequestEncrypter->decrypt($paymentRequest);
+    }
 }