Add IsGranted to EntryRestController

src/Controller/Api/EntryRestController.php

@@ -8,6 +8,7 @@ use Nelmio\ApiDocBundle\Annotation\Operation;
 use OpenApi\Annotations as OA;
 use Pagerfanta\Pagerfanta;
 use Psr\Log\LoggerInterface;
+use Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
@@ -85,13 +86,12 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/exists.{_format}", name="api_get_entries_exists", methods={"GET"}, defaults={"_format": "json"})
+     * @IsGranted("LIST_ENTRIES")
      *
      * @return JsonResponse
      */
     public function getEntriesExistsAction(Request $request, EntryRepository $entryRepository)
     {
-        $this->validateAuthentication();
-
         $returnId = (null === $request->query->get('return_id')) ? false : (bool) $request->query->get('return_id');
 
         $hashedUrls = $request->query->all('hashed_urls');
@@ -300,13 +300,12 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries.{_format}", name="api_get_entries", methods={"GET"}, defaults={"_format": "json"})
+     * @IsGranted("LIST_ENTRIES")
      *
      * @return JsonResponse
      */
     public function getEntriesAction(Request $request, EntryRepository $entryRepository)
     {
-        $this->validateAuthentication();
-
         $isArchived = (null === $request->query->get('archive')) ? null : (bool) $request->query->get('archive');
         $isStarred = (null === $request->query->get('starred')) ? null : (bool) $request->query->get('starred');
         $isPublic = (null === $request->query->get('public')) ? null : (bool) $request->query->get('public');
@@ -392,14 +391,12 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/{entry}.{_format}", name="api_get_entry", methods={"GET"}, defaults={"_format": "json"})
+     * @IsGranted("VIEW", subject="entry")
      *
      * @return JsonResponse
      */
     public function getEntryAction(Entry $entry)
     {
-        $this->validateAuthentication();
-        $this->validateUserAccess($entry->getUser()->getId());
-
         return $this->sendResponse($entry);
     }
 
@@ -436,14 +433,12 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/{entry}/export.{_format}", name="api_get_entry_export", methods={"GET"}, defaults={"_format": "json"})
+     * @IsGranted("VIEW", subject="entry")
      *
      * @return Response
      */
     public function getEntryExportAction(Entry $entry, Request $request, EntriesExport $entriesExport)
     {
-        $this->validateAuthentication();
-        $this->validateUserAccess($entry->getUser()->getId());
-
         return $entriesExport
             ->setEntries($entry)
             ->updateTitle('entry')
@@ -471,13 +466,12 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/list.{_format}", name="api_delete_entries_list", methods={"DELETE"}, defaults={"_format": "json"})
+     * @IsGranted("DELETE_ENTRIES")
      *
      * @return JsonResponse
      */
     public function deleteEntriesListAction(Request $request, EntryRepository $entryRepository, EventDispatcherInterface $eventDispatcher)
     {
-        $this->validateAuthentication();
-
         $urls = json_decode($request->query->get('urls', '[]'));
 
         if (empty($urls)) {
@@ -495,7 +489,7 @@ class EntryRestController extends WallabagRestController
 
             $results[$key]['url'] = $url;
 
-            if (false !== $entry) {
+            if (false !== $entry && $this->authorizationChecker->isGranted('DELETE', $entry)) {
                 // entry deleted, dispatch event about it!
                 $eventDispatcher->dispatch(new EntryDeletedEvent($entry), EntryDeletedEvent::NAME);
 
@@ -529,6 +523,7 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/lists.{_format}", name="api_post_entries_list", methods={"POST"}, defaults={"_format": "json"})
+     * @IsGranted("CREATE_ENTRIES")
      *
      * @throws HttpException When limit is reached
      *
@@ -536,8 +531,6 @@ class EntryRestController extends WallabagRestController
      */
     public function postEntriesListAction(Request $request, EntryRepository $entryRepository, EventDispatcherInterface $eventDispatcher, ContentProxy $contentProxy)
     {
-        $this->validateAuthentication();
-
         $urls = json_decode($request->query->get('urls', '[]'));
 
         $limit = $this->getParameter('wallabag.api_limit_mass_actions');
@@ -714,6 +707,7 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries.{_format}", name="api_post_entries", methods={"POST"}, defaults={"_format": "json"})
+     * @IsGranted("CREATE_ENTRIES")
      *
      * @return JsonResponse
      */
@@ -726,8 +720,6 @@ class EntryRestController extends WallabagRestController
         EventDispatcherInterface $eventDispatcher,
         ValidatorInterface $validator,
     ) {
-        $this->validateAuthentication();
-
         $url = $request->request->get('url');
 
         $entry = $entryRepository->findByUrlAndUserId(
@@ -939,14 +931,12 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/{entry}.{_format}", name="api_patch_entries", methods={"PATCH"}, defaults={"_format": "json"})
+     * @IsGranted("EDIT", subject="entry")
      *
      * @return JsonResponse
      */
     public function patchEntriesAction(Entry $entry, Request $request, ContentProxy $contentProxy, LoggerInterface $logger, TagsAssigner $tagsAssigner, EventDispatcherInterface $eventDispatcher)
     {
-        $this->validateAuthentication();
-        $this->validateUserAccess($entry->getUser()->getId());
-
         $data = $this->retrieveValueFromRequest($request);
 
         // this is a special case where user want to manually update the entry content
@@ -1056,14 +1046,12 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/{entry}/reload.{_format}", name="api_patch_entries_reload", methods={"PATCH"}, defaults={"_format": "json"})
+     * @IsGranted("RELOAD", subject="entry")
      *
      * @return JsonResponse
      */
     public function patchEntriesReloadAction(Entry $entry, ContentProxy $contentProxy, LoggerInterface $logger, EventDispatcherInterface $eventDispatcher)
     {
-        $this->validateAuthentication();
-        $this->validateUserAccess($entry->getUser()->getId());
-
         try {
             $contentProxy->updateEntry($entry, $entry->getUrl());
         } catch (\Exception $e) {
@@ -1113,6 +1101,7 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/{entry}.{_format}", name="api_delete_entries", methods={"DELETE"}, defaults={"_format": "json"})
+     * @IsGranted("DELETE", subject="entry")
      *
      * @return JsonResponse
      */
@@ -1122,8 +1111,6 @@ class EntryRestController extends WallabagRestController
         if (!\in_array($expect, ['id', 'entry'], true)) {
             throw new BadRequestHttpException(\sprintf("expect: 'id' or 'entry' expected, %s given", $expect));
         }
-        $this->validateAuthentication();
-        $this->validateUserAccess($entry->getUser()->getId());
 
         $response = $this->sendResponse([
             'id' => $entry->getId(),
@@ -1166,14 +1153,12 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/{entry}/tags.{_format}", name="api_get_entries_tags", methods={"GET"}, defaults={"_format": "json"})
+     * @IsGranted("LIST_TAGS", subject="entry")
      *
      * @return JsonResponse
      */
     public function getEntriesTagsAction(Entry $entry)
     {
-        $this->validateAuthentication();
-        $this->validateUserAccess($entry->getUser()->getId());
-
         return $this->sendResponse($entry->getTags());
     }
 
@@ -1210,14 +1195,12 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/{entry}/tags.{_format}", name="api_post_entries_tags", methods={"POST"}, defaults={"_format": "json"})
+     * @IsGranted("TAG", subject="entry")
      *
      * @return JsonResponse
      */
     public function postEntriesTagsAction(Request $request, Entry $entry, TagsAssigner $tagsAssigner)
     {
-        $this->validateAuthentication();
-        $this->validateUserAccess($entry->getUser()->getId());
-
         $tags = $request->request->get('tags', '');
         if (!empty($tags)) {
             $tagsAssigner->assignTagsToEntry($entry, $tags);
@@ -1262,14 +1245,12 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/{entry}/tags/{tag}.{_format}", name="api_delete_entries_tags", methods={"DELETE"}, defaults={"_format": "json"})
+     * @IsGranted("UNTAG", subject="entry")
      *
      * @return JsonResponse
      */
     public function deleteEntriesTagsAction(Entry $entry, Tag $tag)
     {
-        $this->validateAuthentication();
-        $this->validateUserAccess($entry->getUser()->getId());
-
         $entry->removeTag($tag);
 
         $this->entityManager->persist($entry);
@@ -1298,13 +1279,12 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/tags/list.{_format}", name="api_delete_entries_tags_list", methods={"DELETE"}, defaults={"_format": "json"})
+     * @IsGranted("DELETE_TAGS")
      *
      * @return JsonResponse
      */
     public function deleteEntriesTagsListAction(Request $request, TagRepository $tagRepository, EntryRepository $entryRepository)
     {
-        $this->validateAuthentication();
-
         $list = json_decode($request->query->get('list', '[]'));
 
         if (empty($list)) {
@@ -1325,7 +1305,7 @@ class EntryRestController extends WallabagRestController
 
             $tags = $element->tags;
 
-            if (false !== $entry && !(empty($tags))) {
+            if (false !== $entry && !(empty($tags)) && $this->authorizationChecker->isGranted('UNTAG', $entry)) {
                 $tags = explode(',', $tags);
                 foreach ($tags as $label) {
                     $label = trim($label);
@@ -1365,13 +1345,12 @@ class EntryRestController extends WallabagRestController
      * )
      *
      * @Route("/api/entries/tags/lists.{_format}", name="api_post_entries_tags_list", methods={"POST"}, defaults={"_format": "json"})
+     * @IsGranted("CREATE_TAGS")
      *
      * @return JsonResponse
      */
     public function postEntriesTagsListAction(Request $request, EntryRepository $entryRepository, TagsAssigner $tagsAssigner)
     {
-        $this->validateAuthentication();
-
         $list = json_decode($request->query->get('list', '[]'));
 
         if (empty($list)) {
@@ -1392,7 +1371,7 @@ class EntryRestController extends WallabagRestController
 
             $tags = $element->tags;
 
-            if (false !== $entry && !(empty($tags))) {
+            if (false !== $entry && !(empty($tags)) && $this->authorizationChecker->isGranted('TAG', $entry)) {
                 $tagsAssigner->assignTagsToEntry($entry, $tags);
 
                 $this->entityManager->persist($entry);

src/Controller/Api/WallabagRestController.php

@@ -101,22 +101,6 @@ class WallabagRestController extends AbstractFOSRestController
         }
     }
 
-    /**
-     * Validate that the first id is equal to the second one.
-     * If not, throw exception. It means a user try to access information from an other user.
-     *
-     * @param int $requestUserId User id from the requested source
-     */
-    protected function validateUserAccess($requestUserId)
-    {
-        $user = $this->tokenStorage->getToken()->getUser();
-        \assert($user instanceof User);
-
-        if ($requestUserId !== $user->getId()) {
-            throw $this->createAccessDeniedException('Access forbidden. Entry user id: ' . $requestUserId . ', logged user id: ' . $user->getId());
-        }
-    }
-
     /**
      * Shortcut to send data serialized in json.
      *

src/Security/Voter/EntryVoter.php

@@ -20,6 +20,7 @@ class EntryVoter extends Voter
     public const DELETE = 'DELETE';
     public const LIST_ANNOTATIONS = 'LIST_ANNOTATIONS';
     public const CREATE_ANNOTATIONS = 'CREATE_ANNOTATIONS';
+    public const LIST_TAGS = 'LIST_TAGS';
     public const TAG = 'TAG';
     public const UNTAG = 'UNTAG';
 
@@ -29,7 +30,7 @@ class EntryVoter extends Voter
             return false;
         }
 
-        if (!\in_array($attribute, [self::VIEW, self::EDIT, self::RELOAD, self::STAR, self::ARCHIVE, self::SHARE, self::UNSHARE, self::EXPORT, self::DELETE, self::LIST_ANNOTATIONS, self::CREATE_ANNOTATIONS, self::TAG, self::UNTAG], true)) {
+        if (!\in_array($attribute, [self::VIEW, self::EDIT, self::RELOAD, self::STAR, self::ARCHIVE, self::SHARE, self::UNSHARE, self::EXPORT, self::DELETE, self::LIST_ANNOTATIONS, self::CREATE_ANNOTATIONS, self::LIST_TAGS, self::TAG, self::UNTAG], true)) {
             return false;
         }
 
@@ -58,6 +59,7 @@ class EntryVoter extends Voter
             case self::DELETE:
             case self::LIST_ANNOTATIONS:
             case self::CREATE_ANNOTATIONS:
+            case self::LIST_TAGS:
             case self::TAG:
             case self::UNTAG:
                 return $user === $subject->getUser();

src/Security/Voter/MainVoter.php

@@ -13,8 +13,10 @@ class MainVoter extends Voter
     public const EDIT_ENTRIES = 'EDIT_ENTRIES';
     public const EXPORT_ENTRIES = 'EXPORT_ENTRIES';
     public const IMPORT_ENTRIES = 'IMPORT_ENTRIES';
+    public const DELETE_ENTRIES = 'DELETE_ENTRIES';
     public const LIST_TAGS = 'LIST_TAGS';
     public const CREATE_TAGS = 'CREATE_TAGS';
+    public const DELETE_TAGS = 'DELETE_TAGS';
     public const LIST_SITE_CREDENTIALS = 'LIST_SITE_CREDENTIALS';
     public const CREATE_SITE_CREDENTIALS = 'CREATE_SITE_CREDENTIALS';
     public const EDIT_CONFIG = 'EDIT_CONFIG';
@@ -32,7 +34,7 @@ class MainVoter extends Voter
             return false;
         }
 
-        if (!\in_array($attribute, [self::LIST_ENTRIES, self::CREATE_ENTRIES, self::EDIT_ENTRIES, self::EXPORT_ENTRIES, self::IMPORT_ENTRIES, self::LIST_TAGS, self::CREATE_TAGS, self::LIST_SITE_CREDENTIALS, self::CREATE_SITE_CREDENTIALS, self::EDIT_CONFIG], true)) {
+        if (!\in_array($attribute, [self::LIST_ENTRIES, self::CREATE_ENTRIES, self::EDIT_ENTRIES, self::EXPORT_ENTRIES, self::IMPORT_ENTRIES, self::DELETE_ENTRIES, self::LIST_TAGS, self::CREATE_TAGS, self::DELETE_TAGS, self::LIST_SITE_CREDENTIALS, self::CREATE_SITE_CREDENTIALS, self::EDIT_CONFIG], true)) {
             return false;
         }
 
@@ -47,8 +49,10 @@ class MainVoter extends Voter
             case self::EDIT_ENTRIES:
             case self::EXPORT_ENTRIES:
             case self::IMPORT_ENTRIES:
+            case self::DELETE_ENTRIES:
             case self::LIST_TAGS:
             case self::CREATE_TAGS:
+            case self::DELETE_TAGS:
             case self::LIST_SITE_CREDENTIALS:
             case self::CREATE_SITE_CREDENTIALS:
             case self::EDIT_CONFIG:

tests/Controller/Api/EntryRestControllerTest.php

@@ -6,7 +6,6 @@ use Doctrine\ORM\EntityManagerInterface;
 use Symfony\Component\DependencyInjection\Container;
 use Wallabag\Entity\Entry;
 use Wallabag\Entity\Tag;
-use Wallabag\Entity\User;
 use Wallabag\Helper\ContentProxy;
 
 class EntryRestControllerTest extends WallabagApiTestCase
@@ -535,7 +534,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
     public function testDeleteEntry()
     {
         $em = $this->client->getContainer()->get(EntityManagerInterface::class);
-        $entry = new Entry($em->getReference(User::class, 1));
+        $entry = new Entry($this->user);
         $entry->setUrl('http://0.0.0.0/test-delete-entry');
         $entry->setTitle('Test delete entry');
         $em->persist($entry);
@@ -569,7 +568,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
     public function testDeleteEntryExpectId()
     {
         $em = $this->client->getContainer()->get(EntityManagerInterface::class);
-        $entry = new Entry($em->getReference(User::class, 1));
+        $entry = new Entry($this->user);
         $entry->setUrl('http://0.0.0.0/test-delete-entry-id');
         $em->persist($entry);
         $em->flush();
@@ -659,7 +658,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
     public function testPostSameEntry()
     {
         $em = $this->client->getContainer()->get(EntityManagerInterface::class);
-        $entry = new Entry($em->getReference(User::class, $this->getUserId()));
+        $entry = new Entry($this->user);
         $entry->setUrl('https://www.20minutes.fr/sport/jo_2024/4095122-20240712-jo-paris-2024-saut-ange-bombe-comment-anne-hidalgo-va-plonger-seine-si-fait-vraiment');
         $entry->setArchived(true);
         $entry->addTag((new Tag())->setLabel('google'));
@@ -1355,7 +1354,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
     public function testDeleteEntriesTagsListAction()
     {
         $em = $this->client->getContainer()->get(EntityManagerInterface::class);
-        $entry = new Entry($em->getReference(User::class, $this->getUserId()));
+        $entry = new Entry($this->user);
         $entry->setUrl('http://0.0.0.0/test-entry');
         $entry->addTag((new Tag())->setLabel('foo-tag'));
         $entry->addTag((new Tag())->setLabel('bar-tag'));
@@ -1423,7 +1422,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
     public function testDeleteEntriesListAction()
     {
         $em = $this->client->getContainer()->get(EntityManagerInterface::class);
-        $em->persist((new Entry($em->getReference(User::class, $this->getUserId())))->setUrl('http://0.0.0.0/test-entry1'));
+        $em->persist((new Entry($this->user))->setUrl('http://0.0.0.0/test-entry1'));
 
         $em->flush();
 
@@ -1483,7 +1482,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
     public function testRePostEntryAndReUsePublishedAt()
     {
         $em = $this->client->getContainer()->get(EntityManagerInterface::class);
-        $entry = new Entry($em->getReference(User::class, $this->getUserId()));
+        $entry = new Entry($this->user);
         $entry->setTitle('Antoine de Caunes : « Je veux avoir le droit de tâtonner »');
         $entry->setContent('hihi');
         $entry->setUrl('https://www.lemonde.fr/m-perso/article/2017/06/25/antoine-de-caunes-je-veux-avoir-le-droit-de-tatonner_5150728_4497916.html');

tests/Security/Voter/EntryVoterTest.php

@@ -189,6 +189,20 @@ class EntryVoterTest extends TestCase
         $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->entryVoter->vote($this->token, $this->entry, [EntryVoter::CREATE_ANNOTATIONS]));
     }
 
+    public function testVoteReturnsDeniedForNonEntryUserListTags(): void
+    {
+        $this->token->method('getUser')->willReturn(new User());
+
+        $this->assertSame(VoterInterface::ACCESS_DENIED, $this->entryVoter->vote($this->token, $this->entry, [EntryVoter::LIST_TAGS]));
+    }
+
+    public function testVoteReturnsGrantedForEntryUserListTags(): void
+    {
+        $this->token->method('getUser')->willReturn($this->user);
+
+        $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->entryVoter->vote($this->token, $this->entry, [EntryVoter::LIST_TAGS]));
+    }
+
     public function testVoteReturnsDeniedForNonEntryUserTag(): void
     {
         $this->token->method('getUser')->willReturn(new User());

tests/Security/Voter/MainVoterTest.php

@@ -112,6 +112,20 @@ class MainVoterTest extends TestCase
         $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->mainVoter->vote($this->token, null, [MainVoter::IMPORT_ENTRIES]));
     }
 
+    public function testVoteReturnsDeniedForNonUserDeleteEntries(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_USER')->willReturn(false);
+
+        $this->assertSame(VoterInterface::ACCESS_DENIED, $this->mainVoter->vote($this->token, null, [MainVoter::DELETE_ENTRIES]));
+    }
+
+    public function testVoteReturnsGrantedForUserDeleteEntries(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_USER')->willReturn(true);
+
+        $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->mainVoter->vote($this->token, null, [MainVoter::DELETE_ENTRIES]));
+    }
+
     public function testVoteReturnsDeniedForNonUserListTags(): void
     {
         $this->security->method('isGranted')->with('ROLE_USER')->willReturn(false);
@@ -140,6 +154,20 @@ class MainVoterTest extends TestCase
         $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->mainVoter->vote($this->token, null, [MainVoter::CREATE_TAGS]));
     }
 
+    public function testVoteReturnsDeniedForNonUserDeleteTags(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_USER')->willReturn(false);
+
+        $this->assertSame(VoterInterface::ACCESS_DENIED, $this->mainVoter->vote($this->token, null, [MainVoter::DELETE_TAGS]));
+    }
+
+    public function testVoteReturnsGrantedForUserDeleteTags(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_USER')->willReturn(true);
+
+        $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->mainVoter->vote($this->token, null, [MainVoter::DELETE_TAGS]));
+    }
+
     public function testVoteReturnsDeniedForNonUserListSiteCredentials(): void
     {
         $this->security->method('isGranted')->with('ROLE_USER')->willReturn(false);