From 0f04f5463abd94053f82fe02a9f78c5f3d0e26e5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= <joachim@loevgaard.dk>
Date: Tue, 3 Dec 2024 10:01:35 +0100
Subject: [PATCH] Added security check and a small optimization

---
 src/Controller/RemoveWishlistItemAction.php  | 33 +++++++++++++++-----
 src/Model/Wishlist.php                       | 15 +++++++--
 src/Model/WishlistInterface.php              |  2 +-
 src/Resources/config/services/controller.xml |  2 ++
 4 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/src/Controller/RemoveWishlistItemAction.php b/src/Controller/RemoveWishlistItemAction.php
index 5154dad..d0eac9a 100644
--- a/src/Controller/RemoveWishlistItemAction.php
+++ b/src/Controller/RemoveWishlistItemAction.php
@@ -6,8 +6,11 @@
 
 use Doctrine\Persistence\ManagerRegistry;
 use Setono\Doctrine\ORMTrait;
+use Setono\SyliusWishlistPlugin\Model\UserWishlistInterface;
 use Setono\SyliusWishlistPlugin\Model\WishlistInterface;
 use Setono\SyliusWishlistPlugin\Provider\WishlistProviderInterface;
+use Setono\SyliusWishlistPlugin\Security\Voter\WishlistVoter;
+use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
@@ -19,25 +22,39 @@ final class RemoveWishlistItemAction
     public function __construct(
         private readonly WishlistProviderInterface $wishlistProvider,
         private readonly UrlGeneratorInterface $urlGenerator,
+        private readonly Security $security,
         ManagerRegistry $managerRegistry,
+        /** @var class-string<UserWishlistInterface> $userWishlistClass */
+        private readonly string $userWishlistClass,
     ) {
         $this->managerRegistry = $managerRegistry;
     }
 
     public function __invoke(string $uuid, int $id): RedirectResponse
     {
-        $wishlist = $this->getWishlist($uuid);
+        $manager = $this->getManager($this->userWishlistClass);
 
-        // todo soooo ugly
-        foreach ($wishlist->getItems() as $item) {
-            if ($item->getId() === $id) {
-                $wishlist->removeItem($item);
+        /** @var UserWishlistInterface|null $wishlist */
+        $wishlist = $manager->createQueryBuilder()
+            ->select('o')
+            ->from($this->userWishlistClass, 'o')
+            ->andWhere('o.uuid = :uuid')
+            ->setParameter('uuid', $uuid)
+            ->getQuery()
+            ->getOneOrNullResult()
+        ;
 
-                break;
-            }
+        if (null === $wishlist) {
+            throw new NotFoundHttpException(sprintf('Wishlist with uuid %s not found', $uuid));
+        }
+
+        if (!$this->security->isGranted(WishlistVoter::EDIT, $wishlist)) {
+            throw new NotFoundHttpException(sprintf('Wishlist with uuid %s not found', $uuid));
         }
 
-        $this->getManager($wishlist)->flush();
+        $wishlist->removeItem($id);
+
+        $manager->flush();
 
         return new RedirectResponse($this->urlGenerator->generate('setono_sylius_wishlist_shop_wishlist_show', [
             'uuid' => $uuid,
diff --git a/src/Model/Wishlist.php b/src/Model/Wishlist.php
index 477ea94..83b2640 100644
--- a/src/Model/Wishlist.php
+++ b/src/Model/Wishlist.php
@@ -66,11 +66,22 @@ public function addItem(WishlistItemInterface $item): void
         }
     }
 
-    public function removeItem(WishlistItemInterface $item): void
+    public function removeItem(WishlistItemInterface|int $item): void
     {
-        if ($this->hasItem($item)) {
+        if ($item instanceof WishlistItemInterface && $this->hasItem($item)) {
             $this->items->removeElement($item);
             $item->setWishlist(null);
+
+            return;
+        }
+
+        foreach ($this->items as $wishlistItem) {
+            if ($wishlistItem->getId() === $item) {
+                $this->items->removeElement($wishlistItem);
+                $wishlistItem->setWishlist(null);
+
+                return;
+            }
         }
     }
 
diff --git a/src/Model/WishlistInterface.php b/src/Model/WishlistInterface.php
index 82333e0..000c188 100644
--- a/src/Model/WishlistInterface.php
+++ b/src/Model/WishlistInterface.php
@@ -24,7 +24,7 @@ public function hasItems(): bool;
 
     public function addItem(WishlistItemInterface $item): void;
 
-    public function removeItem(WishlistItemInterface $item): void;
+    public function removeItem(WishlistItemInterface|int $item): void;
 
     public function hasItem(WishlistItemInterface $item): bool;
 
diff --git a/src/Resources/config/services/controller.xml b/src/Resources/config/services/controller.xml
index e3ebddd..8ec4839 100644
--- a/src/Resources/config/services/controller.xml
+++ b/src/Resources/config/services/controller.xml
@@ -31,7 +31,9 @@
         <service id="Setono\SyliusWishlistPlugin\Controller\RemoveWishlistItemAction" public="true">
             <argument type="service" id="Setono\SyliusWishlistPlugin\Provider\WishlistProviderInterface"/>
             <argument type="service" id="router"/>
+            <argument type="service" id="Symfony\Bundle\SecurityBundle\Security"/>
             <argument type="service" id="doctrine"/>
+            <argument>%setono_sylius_wishlist.model.user_wishlist.class%</argument>
         </service>
 
         <service id="Setono\SyliusWishlistPlugin\Controller\AddWishlistToCartAction" public="true">