diff --git a/app/Resources/static/themes/material/js/shortcuts/entry.js b/app/Resources/static/themes/material/js/shortcuts/entry.js index 3c84e4c6b..1c916b200 100644 --- a/app/Resources/static/themes/material/js/shortcuts/entry.js +++ b/app/Resources/static/themes/material/js/shortcuts/entry.js @@ -20,7 +20,7 @@ $(document).ready(() => { /* delete */ Mousetrap.bind('del', () => { - $('ul.side-nav a.delete i')[0].click(); + $('ul.side-nav button.delete i')[0].click(); }); } }); diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php index 4032c5c73..1f82cf9f0 100644 --- a/src/Wallabag/CoreBundle/Controller/EntryController.php +++ b/src/Wallabag/CoreBundle/Controller/EntryController.php @@ -501,12 +501,16 @@ class EntryController extends AbstractController /** * Deletes entry and redirect to the homepage or the last viewed page. * - * @Route("/delete/{id}", requirements={"id" = "\d+"}, name="delete_entry") + * @Route("/delete/{id}", name="delete_entry", methods={"POST"}, requirements={"id" = "\d+"}) * * @return RedirectResponse */ public function deleteEntryAction(Request $request, Entry $entry) { + if (!$this->isCsrfTokenValid('delete-entry', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $this->checkUserAction($entry); // generates the view url for this entry to check for redirection later 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 bd6f7f2b9..140c283df 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig @@ -32,7 +32,13 @@
  • - delete +
    + + + +
  • diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig index cd1ff7df6..41877a0f5 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig @@ -29,7 +29,13 @@ {% if entry.isStarred == 0 %}star_border{% else %}star{% endif %} - delete +
    + + + +
    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()