refactor #11964 [API][AddressBook] Protect addresses with new firewal…

…l (AdamKasp)

This PR was merged into the 1.9-dev branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | master
| Bug fix?        | no
| New feature?    | yes
| BC breaks?      | no
| License         | MIT

<!--
 - Bug fixes must be submitted against the 1.7 or 1.8 branch (the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

977e073 [API][AddressBook] protect addressbook with new firewall
df3ad6e [Api][Address] fix addresItemProvider and and missing scenarios
787d7e4 [Api][Address] Add missing behats scenario

UPGRADE-1.9.md

@@ -0,0 +1,16 @@
+# UPGRADE FROM `v1.8.X` TO `v1.9.0`
+
+1. Add new parameters, new access control configuration and reorder it:
+
+    ```diff
+        parameters:
+    +       sylius.security.new_api_user_account_route: "%sylius.security.new_api_shop_route%/account"
+    +       sylius.security.new_api_user_account_regex: "^%sylius.security.new_api_user_account_route%"
+
+        security:
+            access_control:
+    +           - { path: "%sylius.security.new_api_user_account_regex%/.*", role: ROLE_USER }
+    -           - { path: "%sylius.security.new_api_shop_regex%/.*", role: IS_AUTHENTICATED_ANONYMOUSLY }
+                - { path: "%sylius.security.new_api_route%/shop/authentication-token", role: IS_AUTHENTICATED_ANONYMOUSLY }
+    +           - { path: "%sylius.security.new_api_shop_regex%/.*", role: IS_AUTHENTICATED_ANONYMOUSLY }
+        ```

config/packages/security.yaml

@@ -8,6 +8,8 @@ parameters:
     sylius.security.new_api_admin_regex: "^%sylius.security.new_api_admin_route%"
     sylius.security.new_api_shop_route: "%sylius.security.new_api_route%/shop"
     sylius.security.new_api_shop_regex: "^%sylius.security.new_api_shop_route%"
+    sylius.security.new_api_user_account_route: "%sylius.security.new_api_shop_route%/account"
+    sylius.security.new_api_user_account_regex: "^%sylius.security.new_api_user_account_route%"
 
 security:
     always_authenticate_before_granting: true
@@ -145,5 +147,6 @@ security:
 
         - { path: "%sylius.security.new_api_admin_regex%/.*", role: ROLE_API_ACCESS }
         - { path: "%sylius.security.new_api_route%/admin/authentication-token", role: IS_AUTHENTICATED_ANONYMOUSLY }
-        - { path: "%sylius.security.new_api_shop_regex%/.*", role: IS_AUTHENTICATED_ANONYMOUSLY }
+        - { path: "%sylius.security.new_api_user_account_regex%/.*", role: ROLE_USER }
         - { path: "%sylius.security.new_api_route%/shop/authentication-token", role: IS_AUTHENTICATED_ANONYMOUSLY }
+        - { path: "%sylius.security.new_api_shop_regex%/.*", role: IS_AUTHENTICATED_ANONYMOUSLY }

features/account/customer_account/address_book/preventing_not_logged_user_from_operations_on_address_book.feature

@@ -0,0 +1,27 @@
+@address_book
+Feature: Preventing not logged user from operations on the address book
+    In order to protect address book from unauthorised operation
+    As a Visitor
+    I want not to be able to operate on address book
+
+    Background:
+        Given the store operates on a single channel in "United States"
+        And there is a customer "John Doe" identified by an email "doe@example.com" and a password "banana"
+        And this customer has an address "John Doe", "Banana Street", "90232", "New York", "United States", "Kansas" in their address book
+
+    @api
+    Scenario: Trying to add new address as a Visitor
+        When I want to add a new address to my address book
+        And I specify the address as "Lucifer Morningstar", "Seaside Fwy", "90802", "Los Angeles", "United States", "Arkansas"
+        And I try to add it
+        Then I should not be able to add it
+
+    @api
+    Scenario: Trying to view address as a Visitor
+        When I try to view details of address belongs to "John Doe"
+        Then I should not see any details of address
+
+    @api
+    Scenario: Trying to delete address as a Visitor
+        When I try to delete address belongs to "John Doe"
+        Then I should not be able to delete it

features/account/customer_account/address_book/viewing_addresses.feature

@@ -6,25 +6,19 @@ Feature: Viewing my address book
 
     Background:
         Given the store operates on a single channel in "United States"
+        And there is a customer "John Doe" identified by an email "doe@example.com" and a password "banana"
+        And this customer has an address "John Doe", "Banana Street", "90232", "New York", "United States", "Kansas" in their address book
         And I am a logged in customer with name "Lucifer Morningstar"
+        And I have an address "Lucifer Morningstar", "Seaside Fwy", "90802", "Los Angeles", "United States", "Arkansas" in my address book
 
     @ui @api
     Scenario: Viewing all addresses
-        Given I have an address "Lucifer Morningstar", "Seaside Fwy", "90802", "Los Angeles", "United States", "Arkansas" in my address book
         When I browse my address book
         Then I should have a single address in my address book
 
     @ui @api
-    Scenario: Inability to view the addresses of other customers
-        Given there is a customer "John Doe" identified by an email "doe@example.com" and a password "banana"
-        And this customer has an address "John Doe", "Banana Street", "90232", "New York", "United States", "Kansas" in their address book
-        And I have an address "Lucifer Morningstar", "Seaside Fwy", "90802", "Los Angeles", "United States", "Arkansas" in my address book
+    Scenario: Inability to view the addresses of other customers in address book
         When I browse my address book
         Then I should have a single address in my address book
         And this address should be assigned to "Lucifer Morningstar"
         And I should not see the address assigned to "John Doe"
-
-    @ui @api
-    Scenario: Viewing an empty address book
-        When I browse my address book
-        Then there should be no addresses

features/account/customer_account/address_book/viewing_empty_address_book.feature

@@ -0,0 +1,14 @@
+@address_book
+Feature: Viewing empty address book
+    In order to see only added addresses
+    As a Customer
+    I want to be able to see empty address book
+
+    Background:
+        Given the store operates on a single channel in "United States"
+        And I am a logged in customer with name "Lucifer Morningstar"
+
+    @ui @api
+    Scenario: Viewing an empty address book
+        When I browse my address book
+        Then there should be no addresses

src/Sylius/Behat/Context/Api/Shop/AddressContext.php

@@ -13,6 +13,7 @@
 
 namespace Sylius\Behat\Context\Api\Shop;
 
+use ApiPlatform\Core\Api\IriConverterInterface;
 use Behat\Behat\Context\Context;
 use Sylius\Behat\Client\ApiClientInterface;
 use Sylius\Behat\Client\ResponseCheckerInterface;
@@ -37,6 +38,9 @@ final class AddressContext implements Context
     /** @var AdminToShopIriConverterInterface */
     private $adminToShopIriConverter;
 
+    /** @var IriConverterInterface */
+    private $iriConverter;
+
     /** @var SharedStorageInterface */
     private $sharedStorage;
 
@@ -45,12 +49,14 @@ public function __construct(
         ApiClientInterface $customerClient,
         ResponseCheckerInterface $responseChecker,
         AdminToShopIriConverterInterface $adminToShopIriConverter,
+        IriConverterInterface $iriConverter,
         SharedStorageInterface $sharedStorage
     ) {
         $this->addressClient = $addressClient;
         $this->customerClient = $customerClient;
         $this->responseChecker = $responseChecker;
         $this->adminToShopIriConverter = $adminToShopIriConverter;
+        $this->iriConverter = $iriConverter;
         $this->sharedStorage = $sharedStorage;
     }
 
@@ -88,6 +94,7 @@ public function iSpecifyTheAddressAs(AddressInterface $address): void
 
     /**
      * @When I add it
+     * @When I try to add it
      */
     public function iAddIt(): void
     {
@@ -153,6 +160,14 @@ public function iDeleteTheAddress(string $fullName): void
         $this->addressClient->delete($id);
     }
 
+    /**
+     * @When /^I try to delete (address belongs to "([^"]+)")$/
+     */
+    public function iDeleteTheAddressBelongsTo(AddressInterface $address): void
+    {
+        $this->addressClient->delete((string) $address->getId());
+    }
+
     /**
      * @When I set the address of :fullName as default
      */
@@ -274,6 +289,14 @@ public function iShouldBeUnableToEditTheirAddress(): void
         Assert::false($this->responseChecker->isUpdateSuccessful($this->addressClient->update()));
     }
 
+    /**
+     * @When /^I try to view details of (address belongs to "([^"]+)")$/
+     */
+    public function iTryToViewDetailsOfAddressBelongingTo(AddressInterface $address): void
+    {
+        $this->addressClient->showByIri($this->iriConverter->getIriFromItem($address));
+    }
+
     /**
      * @Then /^I should(?:| still) have a single address in my address book$/
      * @Then /^I should(?:| still) have (\d+) addresses in my address book$/
@@ -417,6 +440,30 @@ public function iShouldBeNotifiedThatAddressHasBeenSetAsDefault(): void
         Assert::true($this->responseChecker->isUpdateSuccessful($this->addressClient->getLastResponse()));
     }
 
+    /**
+     * @Then I should not see any details of address
+     */
+    public function iShouldNotSeeAnyDetailsOfAddress(): void
+    {
+        Assert::true($this->responseChecker->hasAccessDenied($this->addressClient->getLastResponse()));
+    }
+
+    /**
+     * @Then I should not be able to add it
+     */
+    public function iShouldNotBeAbleToDoIt(): void
+    {
+        Assert::true($this->responseChecker->hasAccessDenied($this->addressClient->getLastResponse()));
+    }
+
+    /**
+     * @Then I should not be able to delete it
+     */
+    public function iShouldNotBeAbleToDeleteIt(): void
+    {
+        Assert::true($this->responseChecker->hasAccessDenied($this->addressClient->getLastResponse()));
+    }
+
     private function addressBookHasAddress(array $addressBook, AddressInterface $addressToCompare): bool
     {
         foreach ($addressBook as $address) {

src/Sylius/Behat/Context/Transform/AddressContext.php

@@ -150,6 +150,7 @@ public function getByStreet($street)
 
     /**
      * @Transform /^address of "([^"]+)"$/
+     * @Transform /^address belongs to "([^"]+)"$/
      */
     public function getByFullName(string $fullName): AddressInterface
     {

src/Sylius/Behat/Resources/config/services/api.xml

@@ -20,7 +20,7 @@
 
         <service id="sylius.behat.api_platform_client.shop.address" class="Sylius\Behat\Client\ApiPlatformClient" parent="sylius.behat.api_platform_client">
             <argument>addresses</argument>
-            <argument>shop</argument>
+            <argument>shop/account</argument>
         </service>
 
         <service id="sylius.behat.api_platform_client.administrator" class="Sylius\Behat\Client\ApiPlatformClient" parent="sylius.behat.api_platform_client">

src/Sylius/Behat/Resources/config/services/contexts/api/shop.xml

@@ -91,6 +91,7 @@
             <argument type="service" id="sylius.behat.api_platform_client.shop.account" />
             <argument type="service" id="Sylius\Behat\Client\ResponseCheckerInterface" />
             <argument type="service" id="sylius.admin_to_shop_iri_converter" />
+            <argument type="service" id="api_platform.iri_converter" />
             <argument type="service" id="sylius.behat.shared_storage" />
         </service>
     </services>

src/Sylius/Bundle/ApiBundle/DataProvider/AddressItemDataProvider.php

@@ -54,7 +54,7 @@ public function getItem(string $resourceClass, $id, string $operationName = null
             in_array('ROLE_USER', $user->getRoles(), true) &&
             $customer !== null
         ) {
-            return $this->addressRepository->findOneByCustomer($id, $customer);
+            return $this->addressRepository->findOneByCustomer((string) $id, $customer);
         }
 
         if ($user instanceof AdminUserInterface && in_array('ROLE_API_ACCESS', $user->getRoles(), true)) {

src/Sylius/Bundle/ApiBundle/Resources/config/api_resources/Address.xml

@@ -12,7 +12,7 @@
            xsi:schemaLocation="https://api-platform.com/schema/metadata https://api-platform.com/schema/metadata/metadata-2.0.xsd"
 >
     <resource class="%sylius.model.address.class%" shortName="Address">
-        <attribute name="route_prefix">shop</attribute>
+        <attribute name="route_prefix">shop/account</attribute>
 
         <attribute name="normalization_context">
             <attribute name="groups">

src/Sylius/Bundle/ApiBundle/Resources/config/services/data_providers.xml

@@ -25,7 +25,7 @@
         <service id="sylius.api.item_data_provider.address" class="Sylius\Bundle\ApiBundle\DataProvider\AddressItemDataProvider">
             <argument type="service" id="sylius.repository.address" />
             <argument type="service" id="sylius.api.context.user" />
-            <tag name="api_platform.collection_data_provider" priority="10" />
+            <tag name="api_platform.item_data_provider" priority="10" />
         </service>
 
         <service id="sylius.api.collection_data_provider.country" class="Sylius\Bundle\ApiBundle\DataProvider\CountryCollectionDataProvider">