From 82430b50c6bae28cea6dd68cc98c85928f802cb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20L=C5=93uillet?= Date: Mon, 18 Nov 2024 15:24:29 +0100 Subject: [PATCH 001/402] Fix redirection after action in search results --- .../views/Entry/_card_actions.html.twig | 2 +- .../views/Entry/_card_list.html.twig | 2 +- .../Resources/views/Entry/entries.html.twig | 2 +- .../Controller/EntryControllerTest.php | 32 +++++++++++++++++++ 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig index d77a48ec0..8102409ee 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig @@ -7,7 +7,7 @@ - {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} + {% set current_path = app.request.requesturi %} diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig index 4aaa006bf..c5f1600de 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig @@ -104,10 +104,14 @@
  • - - delete - {{ 'entry.view.left_menu.delete'|trans }} - +
    + + + +
    • @@ -338,7 +342,15 @@ -
  • delete
    • +
  • +
    + + + +
    +
    • diff --git a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php index 5d1c74b46..e3ffbafd1 100644 --- a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php +++ b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php @@ -694,13 +694,11 @@ class EntryControllerTest extends WallabagCoreTestCase $this->getEntityManager()->persist($entry); $this->getEntityManager()->flush(); - $client->request('GET', '/delete/' . $entry->getId()); + $crawler = $client->request('POST', '/delete/' . $entry->getId()); - $this->assertSame(302, $client->getResponse()->getStatusCode()); - - $client->request('GET', '/delete/' . $entry->getId()); - - $this->assertSame(404, $client->getResponse()->getStatusCode()); + $this->assertSame(400, $client->getResponse()->getStatusCode()); + $this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text'])); + $this->assertStringContainsString('Bad CSRF token.', $body[0]); } /** @@ -736,10 +734,11 @@ class EntryControllerTest extends WallabagCoreTestCase $em->persist($content); $em->flush(); - $client->request('GET', '/view/' . $content->getId()); + $crawler = $client->request('GET', '/view/' . $content->getId()); $this->assertSame(200, $client->getResponse()->getStatusCode()); - $client->request('GET', '/delete/' . $content->getId()); + $client->submit($crawler->filter('.left-bar')->selectButton('entry.view.left_menu.delete')->form()); + $this->assertSame(302, $client->getResponse()->getStatusCode()); $client->followRedirect(); @@ -1264,7 +1263,9 @@ class EntryControllerTest extends WallabagCoreTestCase ->getRepository(Entry::class) ->findByUrlAndUserId($url, $this->getLoggedInUserId()); - $client->request('GET', '/delete/' . $content->getId()); + $crawler = $client->request('GET', '/view/' . $content->getId()); + + $client->submit($crawler->filter('.left-bar')->selectButton('entry.view.left_menu.delete')->form()); $this->assertSame(302, $client->getResponse()->getStatusCode()); @@ -1437,7 +1438,8 @@ class EntryControllerTest extends WallabagCoreTestCase $crawler = $client->submit($form, $data); $this->assertCount(1, $crawler->filter($this->entryDataTestAttribute)); - $client->request('GET', '/delete/' . $entry->getId()); + + $client->submit($crawler->filter('.tools, .tools-list')->selectButton('delete')->form()); // test on list of all articles $crawler = $client->request('GET', '/all/list'); @@ -1508,8 +1510,8 @@ class EntryControllerTest extends WallabagCoreTestCase $crawler = $client->submit($form, $data); $currentUrl = $client->getRequest()->getUri(); - $element = $crawler->filter('a[data-action="delete"]')->link(); - $client->click($element); + $form = $crawler->filter('.tools, .tools-list')->selectButton('delete')->form(); + $client->submit($form); $client->followRedirect(); $nextUrl = $client->getRequest()->getUri(); $this->assertSame($currentUrl, $nextUrl); @@ -1752,8 +1754,8 @@ class EntryControllerTest extends WallabagCoreTestCase $this->getEntityManager()->clear(); $entries = []; - $entries[] = $entry1->getId(); - $entries[] = $entry2->getId(); + $entries[] = $entry1Id = $entry1->getId(); + $entries[] = $entry2Id = $entry2->getId(); // Mass actions : archive $client->request('POST', '/mass', [ @@ -1835,11 +1837,19 @@ class EntryControllerTest extends WallabagCoreTestCase 'entry-checkbox' => $entries, ]); - $client->request('GET', '/delete/' . $entry1->getId()); - $this->assertSame(404, $client->getResponse()->getStatusCode()); + $res = $client->getContainer() + ->get(EntityManagerInterface::class) + ->getRepository(Entry::class) + ->find($entry1Id); - $client->request('GET', '/delete/' . $entry2->getId()); - $this->assertSame(404, $client->getResponse()->getStatusCode()); + $this->assertNull($res); + + $res = $client->getContainer() + ->get(EntityManagerInterface::class) + ->getRepository(Entry::class) + ->find($entry2Id); + + $this->assertNull($res); } public function testGetSameDomainEntries() From 0d8429dfc77b84f50060b253fd84f1c09b892226 Mon Sep 17 00:00:00 2001 From: Yassine Guedidi Date: Sun, 23 Mar 2025 12:52:16 +0100 Subject: [PATCH 187/402] Protect share with a CSRF token --- .../static/themes/material/css/dark_theme.scss | 1 + src/Wallabag/CoreBundle/Controller/EntryController.php | 10 +++++++--- .../CoreBundle/Resources/views/Entry/entry.html.twig | 10 +++++++--- .../CoreBundle/Controller/EntryControllerTest.php | 5 ++++- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/app/Resources/static/themes/material/css/dark_theme.scss b/app/Resources/static/themes/material/css/dark_theme.scss index 9e4aec806..532442360 100644 --- a/app/Resources/static/themes/material/css/dark_theme.scss +++ b/app/Resources/static/themes/material/css/dark_theme.scss @@ -89,6 +89,7 @@ .mass-action-tags .mass-action-tags-input.mass-action-tags-input, .side-nav li:not(.logo) > a:hover, + .side-nav li:not(.logo) button:hover, .side-nav .collapsible-header:hover, .side-nav.fixed .collapsible-header:hover { background-color: #1d1d1d; diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php index 1f82cf9f0..1efc81991 100644 --- a/src/Wallabag/CoreBundle/Controller/EntryController.php +++ b/src/Wallabag/CoreBundle/Controller/EntryController.php @@ -543,12 +543,16 @@ class EntryController extends AbstractController /** * Get public URL for entry (and generate it if necessary). * - * @Route("/share/{id}", requirements={"id" = "\d+"}, name="share") + * @Route("/share/{id}", name="share", methods={"POST"}, requirements={"id" = "\d+"}) * * @return Response */ - public function shareAction(Entry $entry) + public function shareAction(Request $request, Entry $entry) { + if (!$this->isCsrfTokenValid('share-entry', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $this->checkUserAction($entry); if (null === $entry->getUid()) { @@ -587,7 +591,7 @@ class EntryController extends AbstractController /** * Ability to view a content publicly. * - * @Route("/share/{uid}", requirements={"uid" = ".+"}, name="share_entry") + * @Route("/share/{uid}", name="share_entry", methods={"GET"}, requirements={"uid" = ".+"}) * @Cache(maxage="25200", smaxage="25200", public=true) * * @return Response diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig index c5f1600de..9f2154766 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig @@ -159,9 +159,13 @@ -