This commit is contained in:
Yassine Guedidi 2025-03-13 01:34:39 +01:00 committed by GitHub
commit ec57536233
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 128 additions and 70 deletions

View file

@ -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
*/

View file

@ -0,0 +1,29 @@
<?php
namespace Wallabag\Event\Subscriber;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\ExceptionEvent;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\HttpKernel\KernelEvents;
class AccessDeniedToNotFoundSubscriber implements EventSubscriberInterface
{
public static function getSubscribedEvents(): array
{
return [
KernelEvents::EXCEPTION => 'onKernelException',
];
}
public function onKernelException(ExceptionEvent $event): void
{
$exception = $event->getThrowable();
if ($exception instanceof AccessDeniedHttpException) {
$notFoundException = new NotFoundHttpException('', $exception);
$event->setThrowable($notFoundException);
}
}
}

View file

@ -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';
protected function supports(string $attribute, $subject): bool
@ -24,7 +25,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], true)) {
if (!\in_array($attribute, [self::VIEW, self::EDIT, self::RELOAD, self::STAR, self::ARCHIVE, self::SHARE, self::UNSHARE, self::EXPORT, self::DELETE], true)) {
return false;
}
@ -49,6 +50,7 @@ class EntryVoter extends Voter
case self::ARCHIVE:
case self::SHARE:
case self::UNSHARE:
case self::EXPORT:
case self::DELETE:
return $user === $subject->getUser();
}

View file

@ -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');

View file

@ -86,28 +86,30 @@
{% endif %}
<!-- Export -->
<div id="export" class="sidenav">
{% set current_tag = null %}
{% if tag is defined %}
{% set current_tag = tag.slug %}
{% if is_granted('EXPORT_ENTRIES') %}
<div id="export" class="sidenav">
{% 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') %}
<h4 class="center">{{ 'entry.list.export_title'|trans }}</h4>
<ul>
{% if craue_setting('export_epub') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'epub', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">EPUB</a></li>{% endif %}
{% if craue_setting('export_pdf') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'pdf', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">PDF</a></li>{% endif %}
{% if craue_setting('export_json') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'json', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">JSON</a></li>{% endif %}
{% if craue_setting('export_csv') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'csv', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">CSV</a></li>{% endif %}
{% if craue_setting('export_txt') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'txt', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">TXT</a></li>{% endif %}
{% if craue_setting('export_xml') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'xml', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">XML</a></li>{% endif %}
{% if craue_setting('export_md') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'md', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">Markdown</a></li>{% endif %}
</ul>
</div>
{% 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') %}
<h4 class="center">{{ 'entry.list.export_title'|trans }}</h4>
<ul>
{% if craue_setting('export_epub') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'epub', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">EPUB</a></li>{% endif %}
{% if craue_setting('export_pdf') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'pdf', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">PDF</a></li>{% endif %}
{% if craue_setting('export_json') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'json', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">JSON</a></li>{% endif %}
{% if craue_setting('export_csv') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'csv', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">CSV</a></li>{% endif %}
{% if craue_setting('export_txt') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'txt', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">TXT</a></li>{% endif %}
{% if craue_setting('export_xml') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'xml', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">XML</a></li>{% endif %}
{% if craue_setting('export_md') %}<li class="bold"><a class="waves-effect" href="{{ path('export_entries', {'category': current_route, 'format': 'md', 'tag': current_tag, 'search_entry[term]': export_search_term, 'currentRoute': previous_route, 'entry': entry}) }}">Markdown</a></li>{% endif %}
</ul>
</div>
<!-- Filters -->
{% if form is not null and is_granted('LIST_ENTRIES') %}

View file

@ -229,23 +229,25 @@
</li>
{% endif %}
<li class="bold">
<a class="waves-effect collapsible-header">
<i class="material-icons small">file_download</i>
<span>{{ 'entry.view.left_menu.export'|trans }}</span>
</a>
<div class="collapsible-body">
<ul>
{% if craue_setting('export_epub') %}<li><a href="{{ path('export_entry', {'id': entry.id, 'format': 'epub'}) }}" title="Generate ePub file">EPUB</a></li>{% endif %}
{% if craue_setting('export_pdf') %}<li><a href="{{ path('export_entry', {'id': entry.id, 'format': 'pdf'}) }}" title="Generate PDF file">PDF</a></li>{% endif %}
{% if craue_setting('export_csv') %}<li><a href="{{ path('export_entry', {'id': entry.id, 'format': 'csv'}) }}" title="Generate CSV file">CSV</a></li>{% endif %}
{% if craue_setting('export_json') %}<li><a href="{{ path('export_entry', {'id': entry.id, 'format': 'json'}) }}" title="Generate JSON file">JSON</a></li>{% endif %}
{% if craue_setting('export_txt') %}<li><a href="{{ path('export_entry', {'id': entry.id, 'format': 'txt'}) }}" title="Generate TXT file">TXT</a></li>{% endif %}
{% if craue_setting('export_xml') %}<li><a href="{{ path('export_entry', {'id': entry.id, 'format': 'xml'}) }}" title="Generate XML file">XML</a></li>{% endif %}
{% if craue_setting('export_md') %}<li><a href="{{ path('export_entry', {'id': entry.id, 'format': 'md'}) }}" title="Generate MD file">Markdown</a></li>{% endif %}
</ul>
</div>
</li>
{% if is_granted('EXPORT', entry) %}
<li class="bold">
<a class="waves-effect collapsible-header">
<i class="material-icons small">file_download</i>
<span>{{ 'entry.view.left_menu.export'|trans }}</span>
</a>
<div class="collapsible-body">
<ul>
{% if craue_setting('export_epub') %}<li><a href="{{ path('export_entry', {entry: entry.id, 'format': 'epub'}) }}" title="Generate ePub file">EPUB</a></li>{% endif %}
{% if craue_setting('export_pdf') %}<li><a href="{{ path('export_entry', {entry: entry.id, 'format': 'pdf'}) }}" title="Generate PDF file">PDF</a></li>{% endif %}
{% if craue_setting('export_csv') %}<li><a href="{{ path('export_entry', {entry: entry.id, 'format': 'csv'}) }}" title="Generate CSV file">CSV</a></li>{% endif %}
{% if craue_setting('export_json') %}<li><a href="{{ path('export_entry', {entry: entry.id, 'format': 'json'}) }}" title="Generate JSON file">JSON</a></li>{% endif %}
{% if craue_setting('export_txt') %}<li><a href="{{ path('export_entry', {entry: entry.id, 'format': 'txt'}) }}" title="Generate TXT file">TXT</a></li>{% endif %}
{% if craue_setting('export_xml') %}<li><a href="{{ path('export_entry', {entry: entry.id, 'format': 'xml'}) }}" title="Generate XML file">XML</a></li>{% endif %}
{% if craue_setting('export_md') %}<li><a href="{{ path('export_entry', {entry: entry.id, 'format': 'md'}) }}" title="Generate MD file">Markdown</a></li>{% endif %}
</ul>
</div>
</li>
{% endif %}
<li class="bold">
<a class="waves-effect collapsible-header" href="mailto:siteconfig@wallabag.org?subject=Wrong%20display%20in%20wallabag&amp;body={{ entry.url|url_encode }}" title="{{ 'entry.view.left_menu.problem.description'|trans }}">

View file

@ -105,7 +105,7 @@ class DeveloperControllerTest extends WallabagTestCase
$this->logInAs('bob');
$client->request('POST', '/developer/client/delete/' . $adminApiClient->getId());
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
// Try to remove the admin's client with the good user
$this->logInAs('admin');

View file

@ -110,7 +110,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->client->request('GET', '/api/entries/' . $entry->getId() . '.json');
$this->assertSame(403, $this->client->getResponse()->getStatusCode());
$this->assertSame(404, $this->client->getResponse()->getStatusCode());
}
public function testGetEntries()
@ -1260,14 +1260,14 @@ class EntryRestControllerTest extends WallabagApiTestCase
{
$this->client->request('GET', '/api/entries/exists?url=');
$this->assertSame(403, $this->client->getResponse()->getStatusCode());
$this->assertSame(404, $this->client->getResponse()->getStatusCode());
}
public function testGetEntriesExistsWithNoHashedUrl()
{
$this->client->request('GET', '/api/entries/exists?hashed_url=');
$this->assertSame(403, $this->client->getResponse()->getStatusCode());
$this->assertSame(404, $this->client->getResponse()->getStatusCode());
}
public function testReloadEntryErrorWhileFetching()

View file

@ -577,9 +577,9 @@ class ConfigControllerTest extends WallabagTestCase
$crawler = $client->request('GET', '/tagging-rule/delete/' . $rule->getId());
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
$this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text']));
$this->assertStringContainsString('You can not access this rule', $body[0]);
$this->assertStringContainsString('404: Not Found', $body[0]);
}
public function testEditingTaggingRuleFromAnOtherUser()
@ -593,9 +593,9 @@ class ConfigControllerTest extends WallabagTestCase
$crawler = $client->request('GET', '/tagging-rule/edit/' . $rule->getId());
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
$this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text']));
$this->assertStringContainsString('You can not access this rule', $body[0]);
$this->assertStringContainsString('404: Not Found', $body[0]);
}
public function testIgnoreOriginRuleCreation()
@ -714,9 +714,9 @@ class ConfigControllerTest extends WallabagTestCase
$crawler = $client->request('GET', '/ignore-origin-user-rule/edit/' . $rule->getId());
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
$this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text']));
$this->assertStringContainsString('You can not access this rule', $body[0]);
$this->assertStringContainsString('404: Not Found', $body[0]);
}
public function testEditingIgnoreOriginRuleFromAnOtherUser()
@ -730,9 +730,9 @@ class ConfigControllerTest extends WallabagTestCase
$crawler = $client->request('GET', '/ignore-origin-user-rule/edit/' . $rule->getId());
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
$this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text']));
$this->assertStringContainsString('You can not access this rule', $body[0]);
$this->assertStringContainsString('404: Not Found', $body[0]);
}
public function testDeleteUserButtonVisibility()
@ -767,7 +767,7 @@ class ConfigControllerTest extends WallabagTestCase
$this->assertStringNotContainsString('config.form_user.delete.button', $body[0]);
$client->request('POST', '/account/delete');
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
$user = $em
->getRepository(User::class)

View file

@ -781,7 +781,7 @@ class EntryControllerTest extends WallabagTestCase
$client->request('GET', '/view/' . $content->getId());
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
}
public function testFilterOnReadingTime()

View file

@ -27,6 +27,6 @@ class SettingsControllerTest extends WallabagTestCase
$crawler = $client->request('GET', '/settings');
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
}
}

View file

@ -114,7 +114,7 @@ class SiteCredentialControllerTest extends WallabagTestCase
$client->request('GET', '/site-credentials/' . $credential->getId() . '/edit');
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
}
public function testDeleteSiteCredential()

View file

@ -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());

View file

@ -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);