diff --git a/src/Controller/TagController.php b/src/Controller/TagController.php index 1c38fdc73..125cfdbba 100644 --- a/src/Controller/TagController.php +++ b/src/Controller/TagController.php @@ -6,10 +6,12 @@ use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\QueryBuilder; use Pagerfanta\Adapter\ArrayAdapter; use Pagerfanta\Exception\OutOfRangeCurrentPageException; +use Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted; use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; +use Symfony\Component\Security\Core\Security; use Symfony\Contracts\Translation\TranslatorInterface; use Wallabag\Entity\Entry; use Wallabag\Entity\Tag; @@ -26,16 +28,19 @@ class TagController extends AbstractController private EntityManagerInterface $entityManager; private TagsAssigner $tagsAssigner; private Redirect $redirectHelper; + private Security $security; - public function __construct(EntityManagerInterface $entityManager, TagsAssigner $tagsAssigner, Redirect $redirectHelper) + public function __construct(EntityManagerInterface $entityManager, TagsAssigner $tagsAssigner, Redirect $redirectHelper, Security $security) { $this->entityManager = $entityManager; $this->tagsAssigner = $tagsAssigner; $this->redirectHelper = $redirectHelper; + $this->security = $security; } /** * @Route("/new-tag/{entry}", name="new_tag", methods={"POST"}, requirements={"entry" = "\d+"}) + * @IsGranted("TAG", subject="entry") * * @return Response */ @@ -59,8 +64,6 @@ class TagController extends AbstractController } if ($form->isSubmitted() && $form->isValid()) { - $this->checkUserAction($entry); - $this->tagsAssigner->assignTagsToEntry( $entry, $form->get('label')->getData() @@ -87,18 +90,17 @@ class TagController extends AbstractController * Removes tag from entry. * * @Route("/remove-tag/{entry}/{tag}", name="remove_tag", methods={"GET"}, requirements={"entry" = "\d+", "tag" = "\d+"}) + * @IsGranted("UNTAG", subject="entry") * * @return Response */ public function removeTagFromEntry(Request $request, Entry $entry, Tag $tag) { - $this->checkUserAction($entry); - $entry->removeTag($tag); $this->entityManager->flush(); // remove orphan tag in case no entries are associated to it - if (0 === \count($tag->getEntries())) { + if (0 === \count($tag->getEntries()) && $this->security->isGranted('DELETE', $tag)) { $this->entityManager->remove($tag); $this->entityManager->flush(); } @@ -112,21 +114,22 @@ class TagController extends AbstractController * Shows tags for current user. * * @Route("/tag/list", name="tag", methods={"GET"}) + * @IsGranted("LIST_TAGS") * * @return Response */ public function showTagAction(TagRepository $tagRepository, EntryRepository $entryRepository) { - $tags = $tagRepository->findAllFlatTagsWithNbEntries($this->getUser()->getId()); + $allTagsWithNbEntries = $tagRepository->findAllTagsWithNbEntries($this->getUser()->getId()); $nbEntriesUntagged = $entryRepository->countUntaggedEntriesByUser($this->getUser()->getId()); $renameForms = []; - foreach ($tags as $tag) { - $renameForms[$tag['id']] = $this->createForm(RenameTagType::class, new Tag())->createView(); + foreach ($allTagsWithNbEntries as $tagWithNbEntries) { + $renameForms[$tagWithNbEntries['tag']->getId()] = $this->createForm(RenameTagType::class, new Tag())->createView(); } return $this->render('Tag/tags.html.twig', [ - 'tags' => $tags, + 'allTagsWithNbEntries' => $allTagsWithNbEntries, 'renameForms' => $renameForms, 'nbEntriesUntagged' => $nbEntriesUntagged, ]); @@ -137,6 +140,8 @@ class TagController extends AbstractController * * @Route("/tag/list/{slug}/{page}", name="tag_entries", methods={"GET"}, defaults={"page" = "1"}) * @ParamConverter("tag", options={"mapping": {"slug": "slug"}}) + * @IsGranted("LIST_ENTRIES") + * @IsGranted("VIEW", subject="tag") * * @return Response */ @@ -176,6 +181,7 @@ class TagController extends AbstractController * * @Route("/tag/rename/{slug}", name="tag_rename", methods={"POST"}) * @ParamConverter("tag", options={"mapping": {"slug": "slug"}}) + * @IsGranted("EDIT", subject="tag") * * @return Response */ @@ -228,6 +234,7 @@ class TagController extends AbstractController * Tag search results with the current search term. * * @Route("/tag/search/{filter}", name="tag_this_search", methods={"GET"}) + * @IsGranted("CREATE_TAGS") * * @return Response */ @@ -264,6 +271,7 @@ class TagController extends AbstractController * * @Route("/tag/delete/{slug}", name="tag_delete", methods={"GET"}) * @ParamConverter("tag", options={"mapping": {"slug": "slug"}}) + * @IsGranted("DELETE", subject="tag") * * @return Response */ @@ -282,14 +290,4 @@ class TagController extends AbstractController return $this->redirect($redirectUrl); } - - /** - * Check if the logged user can manage the given entry. - */ - private function checkUserAction(Entry $entry) - { - if (null === $this->getUser() || $this->getUser()->getId() !== $entry->getUser()->getId()) { - throw $this->createAccessDeniedException('You can not access this entry.'); - } - } } diff --git a/src/Repository/TagRepository.php b/src/Repository/TagRepository.php index 1eefcf307..3fe8a2e88 100644 --- a/src/Repository/TagRepository.php +++ b/src/Repository/TagRepository.php @@ -3,6 +3,7 @@ namespace Wallabag\Repository; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; +use Doctrine\ORM\Query\ResultSetMappingBuilder; use Doctrine\ORM\QueryBuilder; use Doctrine\Persistence\ManagerRegistry; use Wallabag\Entity\Tag; @@ -13,8 +14,10 @@ use Wallabag\Entity\Tag; */ class TagRepository extends ServiceEntityRepository { - public function __construct(ManagerRegistry $registry) - { + public function __construct( + ManagerRegistry $registry, + private string $tablePrefix, + ) { parent::__construct($registry, Tag::class); } @@ -85,6 +88,36 @@ class TagRepository extends ServiceEntityRepository ->getArrayResult(); } + /** + * Find all tags per user with nb entries. + * + * @param int $userId + * + * @return array + */ + public function findAllTagsWithNbEntries($userId) + { + $rsm = new ResultSetMappingBuilder($this->getEntityManager()); + $rsm->addRootEntityFromClassMetadata(Tag::class, 't'); + $rsm->addEntityResult(Tag::class, 't', 'tag'); + $rsm->addScalarResult('nbEntries', 'nbEntries', 'integer'); + + $sql = <<generateSelectClause()}, count(e.id) as nbEntries + FROM {$this->tablePrefix}tag t + LEFT JOIN {$this->tablePrefix}entry_tag et ON et.tag_id = t.id + JOIN {$this->tablePrefix}entry e ON e.id = et.entry_id + WHERE e.user_id = :userId + GROUP BY t.id + ORDER BY t.label + SQL; + + $query = $this->getEntityManager()->createNativeQuery($sql, $rsm); + $query->setParameter('userId', $userId); + + return $query->getResult(); + } + public function findByLabelsAndUser($labels, $userId) { $qb = $this->getQueryBuilderByUser($userId) diff --git a/src/Security/Voter/EntryVoter.php b/src/Security/Voter/EntryVoter.php index 11806ea19..e2c845648 100644 --- a/src/Security/Voter/EntryVoter.php +++ b/src/Security/Voter/EntryVoter.php @@ -20,6 +20,8 @@ class EntryVoter extends Voter public const DELETE = 'DELETE'; public const LIST_ANNOTATIONS = 'LIST_ANNOTATIONS'; public const CREATE_ANNOTATIONS = 'CREATE_ANNOTATIONS'; + public const TAG = 'TAG'; + public const UNTAG = 'UNTAG'; protected function supports(string $attribute, $subject): bool { @@ -27,7 +29,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], 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::TAG, self::UNTAG], true)) { return false; } @@ -56,6 +58,8 @@ class EntryVoter extends Voter case self::DELETE: case self::LIST_ANNOTATIONS: case self::CREATE_ANNOTATIONS: + case self::TAG: + case self::UNTAG: return $user === $subject->getUser(); } diff --git a/src/Security/Voter/MainVoter.php b/src/Security/Voter/MainVoter.php index 6d1f0afeb..63db306ee 100644 --- a/src/Security/Voter/MainVoter.php +++ b/src/Security/Voter/MainVoter.php @@ -13,6 +13,8 @@ class MainVoter extends Voter public const EDIT_ENTRIES = 'EDIT_ENTRIES'; public const EXPORT_ENTRIES = 'EXPORT_ENTRIES'; public const IMPORT_ENTRIES = 'IMPORT_ENTRIES'; + public const LIST_TAGS = 'LIST_TAGS'; + public const CREATE_TAGS = 'CREATE_TAGS'; public const LIST_SITE_CREDENTIALS = 'LIST_SITE_CREDENTIALS'; public const CREATE_SITE_CREDENTIALS = 'CREATE_SITE_CREDENTIALS'; public const EDIT_CONFIG = 'EDIT_CONFIG'; @@ -30,7 +32,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_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::LIST_TAGS, self::CREATE_TAGS, self::LIST_SITE_CREDENTIALS, self::CREATE_SITE_CREDENTIALS, self::EDIT_CONFIG], true)) { return false; } @@ -45,6 +47,8 @@ class MainVoter extends Voter case self::EDIT_ENTRIES: case self::EXPORT_ENTRIES: case self::IMPORT_ENTRIES: + case self::LIST_TAGS: + case self::CREATE_TAGS: case self::LIST_SITE_CREDENTIALS: case self::CREATE_SITE_CREDENTIALS: case self::EDIT_CONFIG: diff --git a/src/Security/Voter/TagVoter.php b/src/Security/Voter/TagVoter.php new file mode 100644 index 000000000..e81b1944d --- /dev/null +++ b/src/Security/Voter/TagVoter.php @@ -0,0 +1,54 @@ +getUser(); + + if (!$user instanceof User) { + return false; + } + + switch ($attribute) { + case self::VIEW: + case self::EDIT: + case self::DELETE: + return $this->security->isGranted('ROLE_USER'); + } + + return false; + } +} diff --git a/templates/Entry/_tags.html.twig b/templates/Entry/_tags.html.twig index 2ab67e1c8..e0f260016 100644 --- a/templates/Entry/_tags.html.twig +++ b/templates/Entry/_tags.html.twig @@ -3,7 +3,7 @@ {% for tag in tags %}
  • {{ tag.label }} - {% if withRemove is defined and withRemove == true %} + {% if withRemove is defined and withRemove == true and is_granted('DELETE', tag) %} {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} delete diff --git a/templates/Entry/entries.html.twig b/templates/Entry/entries.html.twig index bb1653fec..d437b1afd 100644 --- a/templates/Entry/entries.html.twig +++ b/templates/Entry/entries.html.twig @@ -40,7 +40,7 @@ {% include "Entry/_feed_link.html.twig" %} {% endif %} - {% if current_route == 'search' %}
    {{ 'entry.list.assign_search_tag'|trans }}
    {% endif %} + {% if current_route == 'search' and is_granted('CREATE_TAGS') %}
    {{ 'entry.list.assign_search_tag'|trans }}
    {% endif %} {% if entries.getNbPages > 1 %} {{ pagerfanta(entries, 'default_wallabag') }} {% endif %} diff --git a/templates/Entry/entry.html.twig b/templates/Entry/entry.html.twig index 12325c430..101e9aabc 100644 --- a/templates/Entry/entry.html.twig +++ b/templates/Entry/entry.html.twig @@ -316,12 +316,14 @@
  • {% endif %} - {% include "Entry/_tags.html.twig" with {'tags': entry.tags, 'entryId': entry.id, 'withRemove': true} only %} + {% include "Entry/_tags.html.twig" with {'tags': entry.tags, 'entryId': entry.id, 'withRemove': is_granted('UNTAG', entry)} only %} - + {% if is_granted('TAG', entry) %} + + {% endif %} diff --git a/templates/Tag/tags.html.twig b/templates/Tag/tags.html.twig index fb3b3ed2d..882fe0e9b 100644 --- a/templates/Tag/tags.html.twig +++ b/templates/Tag/tags.html.twig @@ -6,7 +6,7 @@ {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %}
    - {{ 'tag.list.number_on_the_page'|trans({'%count%': tags|length}) }} + {{ 'tag.list.number_on_the_page'|trans({'%count%': allTagsWithNbEntries|length}) }}
    @@ -16,12 +16,14 @@ {{ 'tag.list.untagged'|trans }} ({{ nbEntriesUntagged }}) {% endif %} - {% for tag in tags %} -
  • + {% for tagWithNbEntries in allTagsWithNbEntries %} + {% set tag = tagWithNbEntries.tag %} + {% set nbEntries = tagWithNbEntries.nbEntries %} +
  • - {{ tag.label }} ({{ tag.nbEntries }}) + {{ tag.label }} ({{ nbEntries }}) - {% if renameForms is defined and renameForms[tag.id] is defined %} + {% if renameForms is defined and renameForms[tag.id] is defined and is_granted('EDIT', tag) %}
  • {% endif %} -
  • - {{ 'menu.left.tags'|trans }} {{ count_tags() }} -
  • + {% if is_granted('LIST_TAGS') %} +
  • + {{ 'menu.left.tags'|trans }} {{ count_tags() }} +
  • + {% endif %}