diff --git a/src/Controller/ExportController.php b/src/Controller/ExportController.php index d48b54894..1a65c7a9e 100644 --- a/src/Controller/ExportController.php +++ b/src/Controller/ExportController.php @@ -2,10 +2,12 @@ namespace Wallabag\Controller; +use Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\Routing\Annotation\Route; +use Wallabag\Entity\Entry; use Wallabag\Helper\EntriesExport; use Wallabag\Repository\EntryRepository; use Wallabag\Repository\TagRepository; @@ -19,27 +21,17 @@ class ExportController extends AbstractController /** * Gets one entry content. * - * @Route("/export/{id}.{format}", name="export_entry", methods={"GET"}, requirements={ + * @Route("/export/{entry}.{format}", name="export_entry", methods={"GET"}, requirements={ * "format": "epub|pdf|json|xml|txt|csv|md", - * "id": "\d+" + * "entry": "\d+" * }) + * @IsGranted("EXPORT", subject="entry") * * @return Response */ - public function downloadEntryAction(Request $request, EntryRepository $entryRepository, EntriesExport $entriesExport, string $format, int $id) + public function downloadEntryAction(Request $request, EntryRepository $entryRepository, EntriesExport $entriesExport, string $format, Entry $entry) { try { - $entry = $entryRepository->find($id); - - /* - * We duplicate EntryController::checkUserAction here as a quick fix for an improper authorization vulnerability - * - * This should be eventually rewritten - */ - if (null === $entry || null === $this->getUser() || $this->getUser()->getId() !== $entry->getUser()->getId()) { - throw new NotFoundHttpException(); - } - return $entriesExport ->setEntries($entry) ->updateTitle('entry') @@ -57,6 +49,7 @@ class ExportController extends AbstractController * "format": "epub|pdf|json|xml|txt|csv|md", * "category": "all|unread|starred|archive|tag_entries|untagged|search|annotated|same_domain" * }) + * @IsGranted("EXPORT_ENTRIES") * * @return Response */ diff --git a/src/Security/Voter/EntryVoter.php b/src/Security/Voter/EntryVoter.php index baeca0408..11806ea19 100644 --- a/src/Security/Voter/EntryVoter.php +++ b/src/Security/Voter/EntryVoter.php @@ -16,6 +16,7 @@ class EntryVoter extends Voter public const ARCHIVE = 'ARCHIVE'; public const SHARE = 'SHARE'; public const UNSHARE = 'UNSHARE'; + public const EXPORT = 'EXPORT'; public const DELETE = 'DELETE'; public const LIST_ANNOTATIONS = 'LIST_ANNOTATIONS'; public const CREATE_ANNOTATIONS = 'CREATE_ANNOTATIONS'; @@ -26,7 +27,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::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], true)) { return false; } @@ -51,6 +52,7 @@ class EntryVoter extends Voter case self::ARCHIVE: case self::SHARE: case self::UNSHARE: + case self::EXPORT: case self::DELETE: case self::LIST_ANNOTATIONS: case self::CREATE_ANNOTATIONS: diff --git a/src/Security/Voter/MainVoter.php b/src/Security/Voter/MainVoter.php index b036cc0cb..e9b22ea90 100644 --- a/src/Security/Voter/MainVoter.php +++ b/src/Security/Voter/MainVoter.php @@ -11,6 +11,7 @@ class MainVoter extends Voter public const LIST_ENTRIES = 'LIST_ENTRIES'; public const CREATE_ENTRIES = 'CREATE_ENTRIES'; public const EDIT_ENTRIES = 'EDIT_ENTRIES'; + public const EXPORT_ENTRIES = 'EXPORT_ENTRIES'; public const LIST_SITE_CREDENTIALS = 'LIST_SITE_CREDENTIALS'; public const CREATE_SITE_CREDENTIALS = 'CREATE_SITE_CREDENTIALS'; @@ -27,7 +28,7 @@ class MainVoter extends Voter return false; } - if (!\in_array($attribute, [self::LIST_ENTRIES, self::CREATE_ENTRIES, self::EDIT_ENTRIES, self::LIST_SITE_CREDENTIALS, self::CREATE_SITE_CREDENTIALS], true)) { + if (!\in_array($attribute, [self::LIST_ENTRIES, self::CREATE_ENTRIES, self::EDIT_ENTRIES, self::EXPORT_ENTRIES, self::LIST_SITE_CREDENTIALS, self::CREATE_SITE_CREDENTIALS], true)) { return false; } @@ -40,6 +41,7 @@ class MainVoter extends Voter case self::LIST_ENTRIES: case self::CREATE_ENTRIES: case self::EDIT_ENTRIES: + case self::EXPORT_ENTRIES: case self::LIST_SITE_CREDENTIALS: case self::CREATE_SITE_CREDENTIALS: return $this->security->isGranted('ROLE_USER'); diff --git a/templates/Entry/entries.html.twig b/templates/Entry/entries.html.twig index 22b77a0c3..bb1653fec 100644 --- a/templates/Entry/entries.html.twig +++ b/templates/Entry/entries.html.twig @@ -86,28 +86,30 @@ {% endif %} -
- {% set current_tag = null %} - {% if tag is defined %} - {% set current_tag = tag.slug %} + {% if is_granted('EXPORT_ENTRIES') %} +
+ {% set current_tag = null %} + {% if tag is defined %} + {% set current_tag = tag.slug %} + {% endif %} + {% set export_search_term = null %} + {% if searchTerm is defined %} + {% set export_search_term = searchTerm %} + {% endif %} + {% set entry = app.request.attributes.get('entry') %} + {% set previous_route = app.request.attributes.get('currentRoute') %} +

{{ 'entry.list.export_title'|trans }}

+ +
{% endif %} - {% set export_search_term = null %} - {% if searchTerm is defined %} - {% set export_search_term = searchTerm %} - {% endif %} - {% set entry = app.request.attributes.get('id') %} - {% set previous_route = app.request.attributes.get('currentRoute') %} -

{{ 'entry.list.export_title'|trans }}

- -
{% if form is not null and is_granted('LIST_ENTRIES') %} diff --git a/templates/Entry/entry.html.twig b/templates/Entry/entry.html.twig index 13bb249bc..12325c430 100644 --- a/templates/Entry/entry.html.twig +++ b/templates/Entry/entry.html.twig @@ -229,23 +229,25 @@ {% endif %} -
  • - - file_download - {{ 'entry.view.left_menu.export'|trans }} - -
    - -
    -
    • + {% if is_granted('EXPORT', entry) %} +
  • + + file_download + {{ 'entry.view.left_menu.export'|trans }} + +
    + +
    +
    • + {% endif %}
  • diff --git a/tests/Security/Voter/EntryVoterTest.php b/tests/Security/Voter/EntryVoterTest.php index 04e066880..1d6e0bbce 100644 --- a/tests/Security/Voter/EntryVoterTest.php +++ b/tests/Security/Voter/EntryVoterTest.php @@ -133,6 +133,20 @@ class EntryVoterTest extends TestCase $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->entryVoter->vote($this->token, $this->entry, [EntryVoter::UNSHARE])); } + public function testVoteReturnsDeniedForNonEntryUserExport(): void + { + $this->token->method('getUser')->willReturn(new User()); + + $this->assertSame(VoterInterface::ACCESS_DENIED, $this->entryVoter->vote($this->token, $this->entry, [EntryVoter::EXPORT])); + } + + public function testVoteReturnsGrantedForEntryUserExport(): void + { + $this->token->method('getUser')->willReturn($this->user); + + $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->entryVoter->vote($this->token, $this->entry, [EntryVoter::EXPORT])); + } + public function testVoteReturnsDeniedForNonEntryUserDelete(): void { $this->token->method('getUser')->willReturn(new User()); diff --git a/tests/Security/Voter/MainVoterTest.php b/tests/Security/Voter/MainVoterTest.php index eec969c3c..2530c0336 100644 --- a/tests/Security/Voter/MainVoterTest.php +++ b/tests/Security/Voter/MainVoterTest.php @@ -84,6 +84,20 @@ class MainVoterTest extends TestCase $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->mainVoter->vote($this->token, null, [MainVoter::EDIT_ENTRIES])); } + public function testVoteReturnsDeniedForNonUserExportEntries(): void + { + $this->security->method('isGranted')->with('ROLE_USER')->willReturn(false); + + $this->assertSame(VoterInterface::ACCESS_DENIED, $this->mainVoter->vote($this->token, null, [MainVoter::EXPORT_ENTRIES])); + } + + public function testVoteReturnsGrantedForUserExportEntries(): void + { + $this->security->method('isGranted')->with('ROLE_USER')->willReturn(true); + + $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->mainVoter->vote($this->token, null, [MainVoter::EXPORT_ENTRIES])); + } + public function testVoteReturnsDeniedForNonUserListSiteCredentials(): void { $this->security->method('isGranted')->with('ROLE_USER')->willReturn(false);