Merge pull request #7900 from NeverResponse/random-token-generator

[Security] Improved randomness of tokens generation
Sylius · Apr 10, 2017 · 29eead7 · 29eead7
2 parents 3542cf6 + 3c9bd2f
commit 29eead7
Show file tree

Hide file tree

Showing 17 changed files with 330 additions and 152 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -165,6 +165,8 @@
 * Services tagged with `sylius.promotion_action` and `sylius.promotion_rule_checker` must include `form-type` parameter
   being the FQCN of configuration type.
 
+* Removed class `Sylius\Component\Core\TokenAssigner\UniqueTokenGenerator`, use `Sylius\Component\Resource\Generator\RandomnessGenerator` instead.
+
 ### Currency / CurrencyBundle
 
 * The following classes were removed due to being no longer used in current implementation:

diff --git a/src/Sylius/Bundle/CoreBundle/Resources/config/services.xml b/src/Sylius/Bundle/CoreBundle/Resources/config/services.xml
@@ -108,7 +108,9 @@
             <tag name="twig.extension" />
         </service>
 
-        <service id="sylius.unique_id_based_order_token_assigner" class="Sylius\Component\Core\TokenAssigner\UniqueIdBasedOrderTokenAssigner" />
+        <service id="sylius.unique_id_based_order_token_assigner" class="Sylius\Component\Core\TokenAssigner\UniqueIdBasedOrderTokenAssigner">
+            <argument type="service" id="sylius.random_generator" />
+        </service>
 
         <service id="sylius.customer_unique_address_adder" class="Sylius\Component\Core\Customer\CustomerUniqueAddressAdder">
             <argument type="service" id="sylius.address_comparator" />

diff --git a/src/Sylius/Bundle/ResourceBundle/Resources/config/services.xml b/src/Sylius/Bundle/ResourceBundle/Resources/config/services.xml
@@ -25,6 +25,7 @@
     </parameters>
 
     <services>
+        <service id="sylius.random_generator" class="Sylius\Component\Resource\Generator\RandomnessGenerator" />
         <service id="sylius.form.type_extension.form.request_handler"
                  class="Sylius\Bundle\ResourceBundle\Form\Extension\HttpFoundation\HttpFoundationRequestHandler"
                  decorates="form.type_extension.form.request_handler" decoration-priority="256" public="false" />

diff --git a/src/Sylius/Bundle/UserBundle/DependencyInjection/SyliusUserExtension.php b/src/Sylius/Bundle/UserBundle/DependencyInjection/SyliusUserExtension.php
@@ -105,6 +105,7 @@ private function createTokenGenerators($userType, array $config, ContainerBuilde
             $this->createTokenGeneratorDefinition(
                 UniqueTokenGenerator::class,
                 [
+                    new Reference('sylius.random_generator'),
                     new Reference(sprintf('sylius.%s_user.token_uniqueness_checker.password_reset', $userType)),
                     $config['resetting']['token']['length']
                 ]
@@ -116,6 +117,7 @@ private function createTokenGenerators($userType, array $config, ContainerBuilde
             $this->createTokenGeneratorDefinition(
                 UniquePinGenerator::class,
                 [
+                    new Reference('sylius.random_generator'),
                     new Reference(sprintf('sylius.%s_user.pin_uniqueness_checker.password_reset', $userType)),
                     $config['resetting']['pin']['length']
                 ]
@@ -127,6 +129,7 @@ private function createTokenGenerators($userType, array $config, ContainerBuilde
             $this->createTokenGeneratorDefinition(
                 UniqueTokenGenerator::class,
                 [
+                    new Reference('sylius.random_generator'),
                     new Reference(sprintf('sylius.%s_user.token_uniqueness_checker.email_verification', $userType)),
                     $config['verification']['token']['length']
                 ]

diff --git a/src/Sylius/Component/Core/TokenAssigner/UniqueIdBasedOrderTokenAssigner.php b/src/Sylius/Component/Core/TokenAssigner/UniqueIdBasedOrderTokenAssigner.php
@@ -12,27 +12,31 @@
 namespace Sylius\Component\Core\TokenAssigner;
 
 use Sylius\Component\Core\Model\OrderInterface;
+use Sylius\Component\Resource\Generator\RandomnessGeneratorInterface;
 
 /**
  * @author Arkadiusz Krakowiak <arkadiusz.krakowiak@lakion.com>
  */
 final class UniqueIdBasedOrderTokenAssigner implements OrderTokenAssignerInterface
 {
     /**
-     * @var UniqueTokenGenerator
+     * @var RandomnessGeneratorInterface
      */
-    private $tokenGenerator;
+    private $generator;
 
-    public function __construct()
+    /**
+     * @param RandomnessGeneratorInterface $generator
+     */
+    public function __construct(RandomnessGeneratorInterface $generator)
     {
-        $this->tokenGenerator = new UniqueTokenGenerator();
+        $this->generator = $generator;
     }
 
     /**
      * {@inheritdoc}
      */
     public function assignTokenValue(OrderInterface $order)
     {
-        $order->setTokenValue($this->tokenGenerator->generate(10));
+        $order->setTokenValue($this->generator->generateUriSafeString(10));
     }
 }
diff --git a/src/Sylius/Component/Core/TokenAssigner/UniqueTokenGenerator.php b/src/Sylius/Component/Core/TokenAssigner/UniqueTokenGenerator.php
diff --git a/src/Sylius/Component/Core/spec/TokenAssigner/UniqueIdBasedOrderTokenAssignerSpec.php b/src/Sylius/Component/Core/spec/TokenAssigner/UniqueIdBasedOrderTokenAssignerSpec.php
@@ -12,16 +12,21 @@
 namespace spec\Sylius\Component\Core\TokenAssigner;
 
 use PhpSpec\ObjectBehavior;
-use Prophecy\Argument;
 use Sylius\Component\Core\Model\OrderInterface;
 use Sylius\Component\Core\TokenAssigner\OrderTokenAssignerInterface;
 use Sylius\Component\Core\TokenAssigner\UniqueIdBasedOrderTokenAssigner;
+use Sylius\Component\Resource\Generator\RandomnessGeneratorInterface;
 
 /**
  * @author Arkadiusz Krakowiak <arkadiusz.krakowiak@lakion.com>
  */
 final class UniqueIdBasedOrderTokenAssignerSpec extends ObjectBehavior
 {
+    public function let(RandomnessGeneratorInterface $generator)
+    {
+        $this->beConstructedWith($generator);
+    }
+
     function it_is_initializable()
     {
         $this->shouldHaveType(UniqueIdBasedOrderTokenAssigner::class);
@@ -32,9 +37,10 @@ function it_is_an_order_token_assigner()
         $this->shouldImplement(OrderTokenAssignerInterface::class);
     }
 
-    function it_assigns_a_token_value_for_order(OrderInterface $order)
+    function it_assigns_a_token_value_for_order(RandomnessGeneratorInterface $generator, OrderInterface $order)
     {
-        $order->setTokenValue(Argument::type('string'))->shouldBeCalled();
+        $generator->generateUriSafeString(10)->willReturn('yahboiiiii');
+        $order->setTokenValue('yahboiiiii')->shouldBeCalled();
 
         $this->assignTokenValue($order);
     }

diff --git a/src/Sylius/Component/Product/Generator/SlugGenerator.php b/src/Sylius/Component/Product/Generator/SlugGenerator.php
@@ -23,6 +23,9 @@ final class SlugGenerator implements SlugGeneratorInterface
      */
     public function generate($name)
     {
+        // Manually replacing apostrophes since Transliterator started removing them at v1.2.
+        $name = str_replace('\'', '-', $name);
+
         return Transliterator::transliterate($name);
     }
 }
diff --git a/src/Sylius/Component/Resource/Generator/RandomnessGenerator.php b/src/Sylius/Component/Resource/Generator/RandomnessGenerator.php
@@ -0,0 +1,83 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Sylius\Component\Resource\Generator;
+
+/**
+ * @author Jan Góralski <jan.goralski@lakion.com>
+ */
+final class RandomnessGenerator implements RandomnessGeneratorInterface
+{
+    /**
+     * @var string
+     */
+    private $uriSafeAlphabet;
+
+    /**
+     * @var string
+     */
+    private $digits;
+
+    public function __construct()
+    {
+        $this->digits = implode(range(0, 9));
+
+        $this->uriSafeAlphabet =
+            implode(range(0, 9))
+            .implode(range('a', 'z'))
+            .implode(range('A', 'Z'))
+            .implode(['-', '_', '~', '.'])
+        ;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function generateUriSafeString($length)
+    {
+        return $this->generateStringOfLength($length, $this->uriSafeAlphabet);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function generateNumeric($length)
+    {
+        return $this->generateStringOfLength($length, $this->digits);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function generateInt($min, $max)
+    {
+        return random_int($min, $max);
+    }
+
+    /**
+     * @param int $length
+     * @param string $alphabet
+     *
+     * @return string
+     */
+    private function generateStringOfLength($length, $alphabet)
+    {
+        $alphabetMaxIndex = strlen($alphabet) - 1;
+        $randomString = '';
+
+        for ($i = 0; $i < $length; ++$i) {
+            $index = random_int(0, $alphabetMaxIndex);
+            $randomString .= $alphabet[$index];
+        }
+
+        return $randomString;
+    }
+}
diff --git a/src/Sylius/Component/Resource/Generator/RandomnessGeneratorInterface.php b/src/Sylius/Component/Resource/Generator/RandomnessGeneratorInterface.php
@@ -0,0 +1,40 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Sylius\Component\Resource\Generator;
+
+/**
+ * @author Jan Góralski <jan.goralski@lakion.com>
+ */
+interface RandomnessGeneratorInterface
+{
+    /**
+     * @param int $length
+     *
+     * @return string
+     */
+    public function generateUriSafeString($length);
+
+    /**
+     * @param int $length
+     *
+     * @return string
+     */
+    public function generateNumeric($length);
+
+    /**
+     * @param int $min
+     * @param int $max
+     *
+     * @return int
+     */
+    public function generateInt($min, $max);
+}
diff --git a/src/Sylius/Component/Resource/composer.json b/src/Sylius/Component/Resource/composer.json
@@ -25,6 +25,7 @@
         "doctrine/common": "^2.6",
         "gedmo/doctrine-extensions": "^2.4",
         "symfony/event-dispatcher": "^3.2",
+        "symfony/polyfill-php70": "~1.0",
         "symfony/property-access": "^3.2",
         "winzou/state-machine": "^0.3",
         "pagerfanta/pagerfanta": "^1.0"