umperge conflicts - fixes

popatkaran · Feb 24, 2022 · 973c182 · 973c182
2 parents 5396c9b + 0a77a6d
commit 973c182
Show file tree

Hide file tree

Showing 28 changed files with 776 additions and 9 deletions.
diff --git a/UPGRADE-1.10.md b/UPGRADE-1.10.md
@@ -1,3 +1,11 @@
+# UPGRADE FROM `v1.10.8` TO `v1.10.10`
+
+1. Field `createdByGuest` has been added to `Sylius\Component\Core\Model\Order`, this change will allow us to distinguish carts 
+between guests and logged in customers.
+
+2. Not passing `createdByGuestFlagResolver` through constructor in `Sylius\Component\Core\Cart\Context\ShopBasedCartContext` 
+is deprecated in Sylius 1.10.9 and it will be prohibited in Sylius 2.0.
+
 # UPGRADE FROM `v1.10.x` TO `v1.10.8`
 
 1. Update `payum/payum` to `^1.7` and execute Doctrine Migrations

diff --git a/adr/2022_02_21_using_cart_by_guest_and_logged_in_customer.md b/adr/2022_02_21_using_cart_by_guest_and_logged_in_customer.md
@@ -0,0 +1,60 @@
+# Using cart by guest and logged in customer
+
+* Status: accepted
+* Date: 2022-02-22
+
+## Context and Problem Statement
+
+Cart and its processing is one of the key aspects of Sylius. It turned out that it has a vulnerability and there is
+possible for an anonymous user to override the cart of logged in customer by using only its email. This is because 
+when entering an email, during addressing step, the customer with this email is assigned to the cart and from then, 
+there is no simple way to distinguish between the carts created by the guest and the logged in user. The question is 
+how should we distinguish the carts to solve the vulnerability.
+
+## Decision Drivers
+
+Provided solution should:
+* solve the initial problem with overriding cart of logged in customer by anonymous user
+* not break a backward compatibility, both code and business behaviour
+
+## Considered Options
+
+### Forcing logging in during checkout
+
+* Good, because it solves the initial problem
+* Bad, because it changes the current expected behaviour in checkout
+* Bad, because it breaks a backward compatibility from the business perspective
+
+### Changing priority of cart contexts
+
+* Good, because it solves the initial problem
+* Bad, because it does not solve the case when a logged in customer does not have a cart
+* Bad, because it changes the current expected behaviour with keeping cart after logging in
+* Bad, because it breaks a backward compatibility from the business perspective
+
+### Introducing flag on order entity to mark order created by guest
+
+Adding a flag to order entity allows us to distinguish carts created by guest or by logged in customer. To mark cart 
+as soon as possible, to solve all cases of the initial problem, the flag needs to be set in Sylius\Component\Core\Cart\Context\ShopBasedCartContext, 
+where the customer of logged in user is also set on cart. This context is definitely not a proper service to do such operations, 
+however it is consistent with current approach in Sylius. Setting this value only after logging in would not be enough, 
+as it doesn't resolve the situation when the already logged in user creates a cart. It is similar with setting flag after
+the addressing step, it doesn't resolve the problem as the cart of logged in user is marked too late and before addressing
+the cart couldn't be distinguished to whom it belongs.
+
+* Good, because it solves the initial problem
+* Good, because it does not break a backward compatibility
+* Good, because it can be used to additional features in the future
+* Bad, because it requires to set the flag in Sylius\Component\Core\Cart\Context\ShopBasedCartContext 
+
+## Decision Outcome
+
+Chosen option: **"Introducing flag on order entity to mark order created by guest"**
+
+For now, it is the most straightforward solution, that resolves the initial problem, does not introduce any BC break
+and it is the solution that could be used to additional features in the future.
+
+## References
+
+* [Approach with changing priorities](https://github.com/Sylius/Sylius/pull/13603)
+* [Chosen approach with introducing flag](https://github.com/Sylius/Sylius/pull/13676)
diff --git a/behat.yml.dist b/behat.yml.dist
@@ -33,6 +33,10 @@ default:
                     chrome:
                         api_url: http://127.0.0.1:9222
                         validate_certificate: false
+                chrome_headless_second_session:
+                    chrome:
+                        api_url: http://127.0.0.1:9222
+                        validate_certificate: false
                 chrome:
                     selenium2:
                         browser: chrome

diff --git a/features/checkout/preventing_from_claiming_cart_of_wrong_user.feature b/features/checkout/preventing_from_claiming_cart_of_wrong_user.feature
@@ -0,0 +1,72 @@
+@checkout
+Feature: Preventing from claiming cart of a wrong user
+    In order to make the checkout cart available only for user who owns the cart
+    As a Customer
+    I want to be able to checkout with my previous cart when someone used my email in checkout
+
+    Background:
+        Given the store operates on a single channel in "United States"
+        And the store has a product "PHP T-Shirt" priced at "$20.00"
+        And the store has a product "Kotlin T-Shirt" priced at "$30.00"
+        And the store has a product "Symfony T-Shirt" priced at "$100.00"
+        And the store has a product "Sylius T-Shirt" priced at "$150.00"
+        And the store ships everywhere for free
+        And the store allows paying offline
+        And there is a user "robb@stark.com" identified by "KingInTheNorth"
+
+    @ui @no-api
+    Scenario: Preventing anonymous user from claiming cart of logged in user
+        Given I am logged in as "robb@stark.com"
+        And I have product "PHP T-Shirt" in the cart
+        When an anonymous user in another browser adds products "PHP T-Shirt" and "Kotlin T-Shirt" to the cart
+        And they complete addressing step with email "robb@stark.com" and "United States" based billing address
+        And they add product "Symfony T-Shirt" to the cart
+        Then their cart total should be "$150.00"
+
+    @ui @no-api
+    Scenario: Preventing anonymous user from claiming cart of logged in user
+        Given I am logged in as "robb@stark.com"
+        And I have product "PHP T-Shirt" in the cart
+        When an anonymous user in another browser adds products "PHP T-Shirt" and "Kotlin T-Shirt" to the cart
+        And they complete addressing step with email "robb@stark.com" and "United States" based billing address
+        And they add product "Symfony T-Shirt" to the cart
+        And I view my cart in the previous session
+        Then there should be one item in my cart
+        And my cart's total should be "$20.00"
+
+    @ui @no-api
+    Scenario: Preventing anonymous user from claiming cart of logged in user
+        Given I have product "PHP T-Shirt" in the cart
+        When I sign in with email "robb@stark.com" and password "KingInTheNorth"
+        And I log out
+        And an anonymous user in another browser adds products "PHP T-Shirt" and "Kotlin T-Shirt" to the cart
+        And they complete addressing step with email "robb@stark.com" and "United States" based billing address
+        And they add product "Symfony T-Shirt" to the cart
+        And I sign in again with email "robb@stark.com" and password "KingInTheNorth" in the previous session
+        And I see the summary of my previous cart
+        Then there should be one item in my cart
+        And my cart's total should be "$20.00"
+
+    @ui @no-api
+    Scenario: Preventing anonymous user from claiming cart of logged in user
+        Given on this channel account verification is not required
+        And I have product "PHP T-Shirt" in the cart
+        When I register with email "eddard@stark.com" and password "handOfTheKing"
+        And I log out
+        And an anonymous user in another browser adds products "PHP T-Shirt" and "Kotlin T-Shirt" to the cart
+        And they complete addressing step with email "robb@stark.com" and "United States" based billing address
+        And they add product "Symfony T-Shirt" to the cart
+        And I sign in again with email "eddard@stark.com" and password "handOfTheKing" in the previous session
+        And I see the summary of my previous cart
+        Then there should be one item in my cart
+        And my cart's total should be "$20.00"
+
+    @ui @no-api
+    Scenario: Preventing logged in user from claiming cart of anonymous user
+        Given an anonymous user added product "Kotlin T-Shirt" to the cart
+        And they have completed addressing step with email "robb@stark.com" and "United States" based billing address
+        When I log in as "robb@stark.com"
+        And I add product "Sylius T-Shirt" to the cart
+        And I view my cart in the previous session
+        Then there should be one item in my cart
+        And my cart's total should be "$150.00"
diff --git a/src/Sylius/Behat/Context/Setup/ShopSecurityContext.php b/src/Sylius/Behat/Context/Setup/ShopSecurityContext.php
@@ -44,6 +44,7 @@ public function __construct(
 
     /**
      * @Given I am logged in as :email
+     * @When I log in as :email
      */
     public function iAmLoggedInAs(string $email): void
     {

diff --git a/src/Sylius/Behat/Context/Ui/Shop/AuthorizationContext.php b/src/Sylius/Behat/Context/Ui/Shop/AuthorizationContext.php
@@ -0,0 +1,64 @@
+<?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.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Behat\Context\Ui\Shop;
+
+use Behat\Behat\Context\Context;
+use Sylius\Behat\Element\Shop\Account\RegisterElementInterface;
+use Sylius\Behat\Page\Shop\Account\LoginPageInterface;
+use Sylius\Behat\Page\Shop\Account\RegisterPageInterface;
+
+final class AuthorizationContext implements Context
+{
+    private LoginPageInterface $loginPage;
+
+    private RegisterPageInterface $registerPage;
+
+    private RegisterElementInterface $registerElement;
+
+    public function __construct(
+        LoginPageInterface $loginPage,
+        RegisterPageInterface $registerPage,
+        RegisterElementInterface $registerElement
+    ) {
+        $this->loginPage = $loginPage;
+        $this->registerPage = $registerPage;
+        $this->registerElement = $registerElement;
+    }
+
+    /**
+     * @When I sign in with email :email and password :password
+     * @When I sign in again with email :email and password :password in the previous session
+     */
+    public function iSignInWithEmailAndPassword(string $email, string $password): void
+    {
+        $this->loginPage->open();
+        $this->loginPage->specifyUsername($email);
+        $this->loginPage->specifyPassword($password);
+        $this->loginPage->logIn();
+    }
+
+    /**
+     * @When I register with email :email and password :password
+     */
+    public function iRegisterWithEmailAndPassword(string $email, string $password): void
+    {
+        $this->registerPage->open();
+        $this->registerElement->specifyEmail($email);
+        $this->registerElement->specifyPassword($password);
+        $this->registerElement->verifyPassword($password);
+        $this->registerElement->specifyFirstName('Carrot');
+        $this->registerElement->specifyLastName('Ironfoundersson');
+        $this->registerElement->register();
+    }
+}
diff --git a/src/Sylius/Behat/Context/Ui/Shop/CartContext.php b/src/Sylius/Behat/Context/Ui/Shop/CartContext.php
@@ -19,6 +19,7 @@
 use Sylius\Behat\Page\Shop\Cart\SummaryPageInterface;
 use Sylius\Behat\Page\Shop\Product\ShowPageInterface;
 use Sylius\Behat\Service\NotificationCheckerInterface;
+use Sylius\Behat\Service\SessionManagerInterface;
 use Sylius\Behat\Service\SharedStorageInterface;
 use Sylius\Component\Product\Model\ProductInterface;
 use Sylius\Component\Product\Model\ProductOptionInterface;
@@ -34,16 +35,20 @@ final class CartContext implements Context
 
     private NotificationCheckerInterface $notificationChecker;
 
+    private SessionManagerInterface $sessionManager;
+
     public function __construct(
         SharedStorageInterface $sharedStorage,
         SummaryPageInterface $summaryPage,
         ShowPageInterface $productShowPage,
-        NotificationCheckerInterface $notificationChecker
+        NotificationCheckerInterface $notificationChecker,
+        SessionManagerInterface $sessionManager
     ) {
         $this->sharedStorage = $sharedStorage;
         $this->summaryPage = $summaryPage;
         $this->productShowPage = $productShowPage;
         $this->notificationChecker = $notificationChecker;
+        $this->sessionManager = $sessionManager;
     }
 
     /**
@@ -99,6 +104,7 @@ public function iChangeQuantityTo($productName, $quantity)
      * @Then the grand total value should be :total
      * @Then my cart total should be :total
      * @Then the cart total should be :total
+     * @Then their cart total should be :total
      */
     public function myCartTotalShouldBe($total)
     {
@@ -246,13 +252,16 @@ public function productPriceShouldNotBeDecreased(ProductInterface $product)
     }
 
     /**
+     * @Given /^an anonymous user added (product "([^"]+)") to the cart$/
      * @Given /^I (?:add|added) (this product) to the cart$/
      * @Given /^I have (product "[^"]+") added to the cart$/
      * @Given I added product :product to the cart
+     * @Given he added product :product to the cart
      * @Given /^I (?:have|had) (product "[^"]+") in the cart$/
      * @Given /^the customer (?:added|adds) ("[^"]+" product) to the cart$/
      * @Given /^I (?:add|added) ("[^"]+" product) to the (cart)$/
      * @When I add product :product to the cart
+     * @When they add product :product to the cart
      */
     public function iAddProductToTheCart(ProductInterface $product): void
     {
@@ -273,6 +282,18 @@ public function iAddMultipleProductsToTheCart(array $products)
         }
     }
 
+    /**
+     * @When /^an anonymous user in another browser adds (products "([^"]+)" and "([^"]+)") to the cart$/
+     */
+    public function anonymousUserAddsMultipleProductsToTheCart(array $products): void
+    {
+        $this->sessionManager->changeSession();
+
+        foreach ($products as $product) {
+            $this->iAddProductToTheCart($product);
+        }
+    }
+
     /**
      * @When I add :variantName variant of product :product to the cart
      * @When /^I add "([^"]+)" variant of (this product) to the cart$/
@@ -366,6 +387,16 @@ public function thisItemShouldHaveCode($variantCode)
         Assert::true($this->summaryPage->hasItemWithCode($variantCode));
     }
 
+    /**
+     * @When I view my cart in the previous session
+     */
+    public function iViewMyCartInPreviousSession(): void
+    {
+        $this->sessionManager->restorePreviousSession();
+
+        $this->summaryPage->open();
+    }
+
     /**
      * @Given I have :product with :productOption :productOptionValue in the cart
      * @Given I have product :product with product option :productOption :productOptionValue in the cart

diff --git a/src/Sylius/Behat/Context/Ui/Shop/Checkout/CheckoutAddressingContext.php b/src/Sylius/Behat/Context/Ui/Shop/Checkout/CheckoutAddressingContext.php
@@ -74,7 +74,9 @@ public function iAmAtTheCheckoutAddressingStep(): void
 
     /**
      * @Given /^I have completed addressing step with email "([^"]+)" and ("[^"]+" based billing address)$/
+     * @Given /^they have completed addressing step with email "([^"]+)" and ("[^"]+" based billing address)$/
      * @When /^I complete addressing step with email "([^"]+)" and ("[^"]+" based billing address)$/
+     * @When /^they complete addressing step with email "([^"]+)" and ("[^"]+" based billing address)$/
      */
     public function iCompleteAddressingStepWithEmail(string $email, AddressInterface $address): void
     {

diff --git a/src/Sylius/Behat/Resources/config/services.xml b/src/Sylius/Behat/Resources/config/services.xml
@@ -48,6 +48,12 @@
             <argument type="string">shop</argument>
         </service>
 
+        <service id="Sylius\Behat\Service\SessionManagerInterface" class="Sylius\Behat\Service\SessionManager" public="false">
+            <argument type="service" id="behat.mink" />
+            <argument type="service" id="sylius.behat.shared_storage" />
+            <argument type="service" id="sylius.behat.shop_security" />
+        </service>
+
         <service id="sylius.behat.shared_security" class="Sylius\Behat\Service\SharedSecurityService" public="false">
             <argument type="service" id="sylius.behat.admin_security" />
         </service>

diff --git a/src/Sylius/Behat/Resources/config/services/contexts/ui.xml b/src/Sylius/Behat/Resources/config/services/contexts/ui.xml
@@ -433,11 +433,18 @@
             <argument type="service" id="sylius.behat.notification_checker" />
         </service>
 
+        <service id="sylius.behat.context.ui.shop.authorization" class="Sylius\Behat\Context\Ui\Shop\AuthorizationContext">
+            <argument type="service" id="sylius.behat.page.shop.account.login" />
+            <argument type="service" id="sylius.behat.page.shop.account.register" />
+            <argument type="service" id="sylius.behat.element.shop.account.register" />
+        </service>
+
         <service id="sylius.behat.context.ui.shop.cart" class="Sylius\Behat\Context\Ui\Shop\CartContext">
             <argument type="service" id="sylius.behat.shared_storage" />
             <argument type="service" id="sylius.behat.page.shop.cart_summary" />
             <argument type="service" id="sylius.behat.page.shop.product.show" />
             <argument type="service" id="sylius.behat.notification_checker" />
+            <argument type="service" id="Sylius\Behat\Service\SessionManagerInterface"/>
         </service>
 
         <service id="sylius.behat.context.ui.shop.contact" class="Sylius\Behat\Context\Ui\Shop\ContactContext">

diff --git a/src/Sylius/Behat/Resources/config/suites/ui/checkout/checkout.yml b/src/Sylius/Behat/Resources/config/suites/ui/checkout/checkout.yml
@@ -52,6 +52,7 @@ default:
                 - sylius.behat.context.ui.channel
                 - sylius.behat.context.ui.email
                 - sylius.behat.context.ui.shop.address_book
+                - sylius.behat.context.ui.shop.authorization
                 - sylius.behat.context.ui.shop.cart
                 - sylius.behat.context.ui.shop.checkout
                 - sylius.behat.context.ui.shop.checkout.addressing
@@ -63,6 +64,7 @@ default:
                 - sylius.behat.context.ui.shop.currency
                 - sylius.behat.context.ui.shop.homepage
                 - sylius.behat.context.ui.shop.locale
+                - sylius.behat.context.ui.user
 
             filters:
                 tags: "@checkout&&@ui"
-Original file line number
+Diff line change
@@ Expand Up / @@ -44,6 +44,7 @@ public function __construct( @@
         /**
          * @Given I am logged in as :email
+         * @When I log in as :email
          */
         public function iAmLoggedInAs(string $email): void
         {
@@ Expand Down @@