Merge pull request #40 from bartoszpietrzak1994/fix-security-issue

Fix security issue

features/being_unable_to_reorder_the_order_placed_by_another_customer.feature

@@ -0,0 +1,21 @@
+@reordering
+Feature: Being unable to reorder the order placed by another customer
+    In order to maintain shop security
+    As a Store Owner
+    I want Customer to be the only person allowed to reorder their previously placed order
+
+    Background:
+        Given the store operates on a single channel in "United States"
+        And the store has a product "Angel T-Shirt"
+        And the store ships everywhere for free
+        And the store allows paying with "Cash on Delivery"
+        And there is a customer "Rick Sanchez" identified by an email "rick.sanchez@wubba-lubba-dub-dub.com" and a password "Morty"
+        And there is a customer "Morty Smith" identified by an email "morty.smith@wubba-lubba-dub-dub.com" and a password "Rick"
+        And a customer "Morty Smith" placed an order "#00000666"
+        And the customer bought a single "Angel T-Shirt"
+        And the customer chose "Free" shipping method to "United States" with "Cash on Delivery" payment
+
+    @application
+    Scenario: Being unable to reorder the order placed by another customer
+        When the customer "rick.sanchez@wubba-lubba-dub-dub.com" tries to reorder the order "#00000666"
+        Then the order "#00000666" should not be reordered

spec/Checker/OrderCustomerRelationCheckerSpec.php

@@ -0,0 +1,53 @@
+<?php
+
+declare(strict_types=1);
+
+namespace spec\Sylius\CustomerReorderPlugin\Checker;
+
+use PhpSpec\ObjectBehavior;
+use Sylius\Component\Core\Model\CustomerInterface;
+use Sylius\Component\Core\Model\OrderInterface;
+use Sylius\CustomerReorderPlugin\Checker\OrderCustomerRelationCheckerInterface;
+
+final class OrderCustomerRelationCheckerSpec extends ObjectBehavior
+{
+   function it_implements_order_customer_relation_checker_interface(): void
+   {
+       $this->shouldImplement(OrderCustomerRelationCheckerInterface::class);
+   }
+
+   function it_returns_true_when_order_was_placed_by_customer(
+       CustomerInterface $orderCustomer,
+       CustomerInterface $customer,
+       OrderInterface $order
+   ): void {
+        $orderCustomer->getId()->willReturn(1);
+        $customer->getId()->willReturn(1);
+
+        $order->getCustomer()->willReturn($orderCustomer);
+
+        $this->wasOrderPlacedByCustomer($order, $customer)->shouldReturn(true);
+   }
+
+   function it_returns_false_when_order_was_not_placed_by_customer(
+       CustomerInterface $orderCustomer,
+       CustomerInterface $customer,
+       OrderInterface $order
+   ): void {
+        $orderCustomer->getId()->willReturn(1);
+        $customer->getId()->willReturn(2);
+
+        $order->getCustomer()->willReturn($orderCustomer);
+
+        $this->wasOrderPlacedByCustomer($order, $customer)->shouldReturn(false);
+   }
+
+   function it_returns_false_when_order_has_no_customer_assigned(
+       CustomerInterface $customer,
+       OrderInterface $order
+   ): void {
+       $order->getCustomer()->willReturn(null);
+
+       $this->wasOrderPlacedByCustomer($order, $customer)->shouldReturn(false);
+   }
+}

spec/Reorder/ReordererSpec.php

@@ -6,14 +6,17 @@
 
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\ORM\EntityManagerInterface;
+use Nette\InvalidStateException;
 use PhpSpec\ObjectBehavior;
 use Sylius\Bundle\MoneyBundle\Formatter\MoneyFormatterInterface;
 use Sylius\Component\Core\Model\ChannelInterface;
+use Sylius\Component\Core\Model\CustomerInterface;
 use Sylius\Component\Core\Model\OrderInterface;
 use Sylius\Component\Core\Model\OrderItemInterface;
 use Sylius\Component\Core\Model\ProductVariantInterface;
 use Sylius\Component\Core\Model\PromotionInterface;
 use Sylius\Component\Order\Processor\OrderProcessorInterface;
+use Sylius\CustomerReorderPlugin\Checker\OrderCustomerRelationCheckerInterface;
 use Sylius\CustomerReorderPlugin\Factory\OrderFactoryInterface;
 use Sylius\CustomerReorderPlugin\Reorder\Reorderer;
 use Sylius\CustomerReorderPlugin\Reorder\ReordererInterface;
@@ -32,7 +35,8 @@ function let(
         MoneyFormatterInterface $moneyFormatter,
         Session $session,
         ReorderEligibilityChecker $reorderEligibilityChecker,
-        ReorderEligibilityCheckerResponseProcessorInterface $reorderEligibilityCheckerResponseProcessor
+        ReorderEligibilityCheckerResponseProcessorInterface $reorderEligibilityCheckerResponseProcessor,
+        OrderCustomerRelationCheckerInterface $orderCustomerRelationChecker
     ): void {
         $this->beConstructedWith(
             $orderFactory,
@@ -41,7 +45,8 @@ function let(
             $moneyFormatter,
             $session,
             $reorderEligibilityChecker,
-            $reorderEligibilityCheckerResponseProcessor
+            $reorderEligibilityCheckerResponseProcessor,
+            $orderCustomerRelationChecker
         );
     }
 
@@ -59,7 +64,9 @@ function it_creates_and_persists_reorder_from_existing_order(
         OrderFactoryInterface $orderFactory,
         EntityManagerInterface $entityManager,
         ReorderEligibilityChecker $reorderEligibilityChecker,
+        OrderCustomerRelationCheckerInterface $orderCustomerRelationChecker,
         ChannelInterface $channel,
+        CustomerInterface $customer,
         OrderInterface $order,
         OrderInterface $reorder,
         OrderItemInterface $firstOrderItem,
@@ -68,6 +75,8 @@ function it_creates_and_persists_reorder_from_existing_order(
         $order->getTotal()->willReturn(100);
         $order->getCurrencyCode()->willReturn('USD');
 
+        $orderCustomerRelationChecker->wasOrderPlacedByCustomer($order, $customer)->willReturn(true);
+
         $reorder->getTotal()->willReturn(100);
 
         $orderFactory->createFromExistingOrder($order, $channel)->willReturn($reorder);
@@ -81,14 +90,16 @@ function it_creates_and_persists_reorder_from_existing_order(
             $secondOrderItem->getWrappedObject()
         ]));
 
-        $this->reorder($order, $channel);
+        $this->reorder($order, $channel, $customer);
     }
 
     function it_checks_if_orders_totals_differ(
         OrderFactoryInterface $orderFactory,
         EntityManagerInterface $entityManager,
         ReorderEligibilityChecker $reorderEligibilityChecker,
+        OrderCustomerRelationCheckerInterface $orderCustomerRelationChecker,
         ChannelInterface $channel,
+        CustomerInterface $customer,
         OrderInterface $order,
         OrderInterface $reorder,
         MoneyFormatterInterface $moneyFormatter,
@@ -101,6 +112,8 @@ function it_checks_if_orders_totals_differ(
         $order->getCurrencyCode()->willReturn('USD');
         $order->getPromotions()->willReturn($promotions);
 
+        $orderCustomerRelationChecker->wasOrderPlacedByCustomer($order, $customer)->willReturn(true);
+
         $reorder->getTotal()->willReturn(150);
         $reorder->getPromotions()->willReturn($promotions);
 
@@ -122,14 +135,16 @@ function it_checks_if_orders_totals_differ(
         $entityManager->persist($reorder)->shouldBeCalled();
         $entityManager->flush()->shouldBeCalled();
 
-        $this->reorder($order, $channel);
+        $this->reorder($order, $channel, $customer);
     }
 
     function it_checks_if_promotion_is_no_longer_available(
         OrderFactoryInterface $orderFactory,
         EntityManagerInterface $entityManager,
         ReorderEligibilityChecker $reorderEligibilityChecker,
+        OrderCustomerRelationCheckerInterface $orderCustomerRelationChecker,
         ChannelInterface $channel,
+        CustomerInterface $customer,
         OrderInterface $order,
         OrderInterface $reorder,
         MoneyFormatterInterface $moneyFormatter,
@@ -144,6 +159,8 @@ function it_checks_if_promotion_is_no_longer_available(
             $secondPromotion->getWrappedObject()
         ]));
 
+        $orderCustomerRelationChecker->wasOrderPlacedByCustomer($order, $customer)->willReturn(true);
+
         $firstPromotion->getName()->willReturn('test_promotion_01');
         $secondPromotion->getName()->willReturn('test_promotion_02');
 
@@ -169,14 +186,16 @@ function it_checks_if_promotion_is_no_longer_available(
         $entityManager->persist($reorder)->shouldBeCalled();
         $entityManager->flush()->shouldBeCalled();
 
-        $this->reorder($order, $channel);
+        $this->reorder($order, $channel, $customer);
     }
 
     function it_checks_if_price_of_any_item_has_changed(
         OrderFactoryInterface $orderFactory,
         EntityManagerInterface $entityManager,
         ReorderEligibilityChecker $reorderEligibilityChecker,
+        OrderCustomerRelationCheckerInterface $orderCustomerRelationChecker,
         ChannelInterface $channel,
+        CustomerInterface $customer,
         OrderInterface $order,
         OrderInterface $reorder,
         OrderItemInterface $firstOrderItem,
@@ -194,6 +213,8 @@ function it_checks_if_price_of_any_item_has_changed(
             $secondOrderItem->getWrappedObject()
         ]));
 
+        $orderCustomerRelationChecker->wasOrderPlacedByCustomer($order, $customer)->willReturn(true);
+
         $reorder->getItems()->willReturn(new ArrayCollection([
             $firstOrderItem->getWrappedObject(),
             $secondOrderItem->getWrappedObject()
@@ -213,14 +234,16 @@ function it_checks_if_price_of_any_item_has_changed(
         $entityManager->persist($reorder)->shouldBeCalled();
         $entityManager->flush()->shouldBeCalled();
 
-        $this->reorder($order, $channel);
+        $this->reorder($order, $channel, $customer);
     }
 
     function it_checks_if_any_item_is_out_of_stock(
         OrderFactoryInterface $orderFactory,
         EntityManagerInterface $entityManager,
         ReorderEligibilityChecker $reorderEligibilityChecker,
+        OrderCustomerRelationCheckerInterface $orderCustomerRelationChecker,
         ChannelInterface $channel,
+        CustomerInterface $customer,
         OrderInterface $order,
         OrderInterface $reorder,
         OrderItemInterface $firstOrderItem,
@@ -244,6 +267,8 @@ function it_checks_if_any_item_is_out_of_stock(
             $secondOrderItem->getWrappedObject()
         ]));
 
+        $orderCustomerRelationChecker->wasOrderPlacedByCustomer($order, $customer)->willReturn(true);
+
         $reorder->getItems()->willReturn(new ArrayCollection([
             $firstOrderItem->getWrappedObject()
         ]));
@@ -261,6 +286,26 @@ function it_checks_if_any_item_is_out_of_stock(
         $entityManager->persist($reorder)->shouldBeCalled();
         $entityManager->flush()->shouldBeCalled();
 
-        $this->reorder($order, $channel);
+        $this->reorder($order, $channel, $customer);
+    }
+
+    function it_does_not_create_reorder_when_order_does_not_belong_to_given_customer(
+        OrderInterface $order,
+        ChannelInterface $channel,
+        CustomerInterface $firstCustomer,
+        CustomerInterface $secondCustomer,
+        OrderCustomerRelationCheckerInterface $orderCustomerRelationChecker
+    ): void {
+        $firstCustomer->getId()->willReturn('1');
+        $secondCustomer->getId()->willReturn('2');
+
+        $order->getCustomer()->willReturn($firstCustomer);
+
+        $orderCustomerRelationChecker->wasOrderPlacedByCustomer($order, $secondCustomer)->shouldBeCalled();
+
+        $this
+            ->shouldThrow(InvalidStateException::class)
+            ->during('reorder', [$order, $channel, $secondCustomer])
+        ;
     }
 }

src/Checker/OrderCustomerRelationChecker.php

@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sylius\CustomerReorderPlugin\Checker;
+
+use Sylius\Component\Core\Model\CustomerInterface;
+use Sylius\Component\Core\Model\OrderInterface;
+
+final class OrderCustomerRelationChecker implements OrderCustomerRelationCheckerInterface
+{
+    public function wasOrderPlacedByCustomer(OrderInterface $order, CustomerInterface $customer): bool
+    {
+        /** @var CustomerInterface|null $orderCustomer */
+        $orderCustomer = $order->getCustomer();
+
+        return
+            null !== $orderCustomer &&
+            $orderCustomer->getId() === $customer->getId()
+        ;
+    }
+}

src/Checker/OrderCustomerRelationCheckerInterface.php

@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sylius\CustomerReorderPlugin\Checker;
+
+use Sylius\Component\Core\Model\CustomerInterface;
+use Sylius\Component\Core\Model\OrderInterface;
+
+interface OrderCustomerRelationCheckerInterface
+{
+    public function wasOrderPlacedByCustomer(OrderInterface $order, CustomerInterface $customer): bool;
+}

src/Controller/CustomerReorderAction.php

@@ -8,8 +8,10 @@
 use Sylius\Bundle\CoreBundle\Storage\CartSessionStorage;
 use Sylius\Component\Channel\Context\ChannelContextInterface;
 use Sylius\Component\Core\Model\ChannelInterface;
+use Sylius\Component\Core\Model\CustomerInterface;
 use Sylius\Component\Core\Model\OrderInterface;
 use Sylius\Component\Core\Repository\OrderRepositoryInterface;
+use Sylius\Component\Customer\Context\CustomerContextInterface;
 use Sylius\Component\Order\Context\CartContextInterface;
 use Sylius\CustomerReorderPlugin\Reorder\ReordererInterface;
 use Symfony\Component\HttpFoundation\RedirectResponse;
@@ -29,6 +31,9 @@ final class CustomerReorderAction
     /** @var CartContextInterface */
     private $cartContext;
 
+    /** @var CustomerContextInterface */
+    private $customerContext;
+
     /** @var OrderRepositoryInterface */
     private $orderRepository;
 
@@ -45,6 +50,7 @@ public function __construct(
         CartSessionStorage $cartSessionStorage,
         ChannelContextInterface $channelContext,
         CartContextInterface $cartContext,
+        CustomerContextInterface $customerContext,
         OrderRepositoryInterface $orderRepository,
         ReordererInterface $reorderService,
         UrlGeneratorInterface $urlGenerator,
@@ -53,6 +59,7 @@ public function __construct(
         $this->cartSessionStorage = $cartSessionStorage;
         $this->channelContext = $channelContext;
         $this->cartContext = $cartContext;
+        $this->customerContext = $customerContext;
         $this->orderRepository = $orderRepository;
         $this->reorderer = $reorderService;
         $this->urlGenerator = $urlGenerator;
@@ -67,10 +74,13 @@ public function __invoke(Request $request): Response
         $channel = $this->channelContext->getChannel();
         assert($channel instanceof ChannelInterface);
 
+        /** @var CustomerInterface $customer */
+        $customer = $this->customerContext->getCustomer();
+
         $reorder = null;
 
         try {
-            $reorder = $this->reorderer->reorder($order, $channel);
+            $reorder = $this->reorderer->reorder($order, $channel, $customer);
         } catch (InvalidStateException $exception) {
             $this->session->getFlashBag()->add('info', $exception->getMessage());