diff --git a/.github/workflows/coding-standards.yml b/.github/workflows/coding-standards.yml index 2c4b0d5df..f3c90c59e 100644 --- a/.github/workflows/coding-standards.yml +++ b/.github/workflows/coding-standards.yml @@ -13,7 +13,7 @@ permissions: jobs: coding-standards: name: "CS Fixer, PHPStan & TwigCS" - runs-on: "ubuntu-20.04" + runs-on: ubuntu-latest steps: - name: "Checkout" diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 72f48471d..5f3a49f70 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -16,7 +16,7 @@ env: jobs: phpunit: name: "PHP ${{ matrix.php }} using ${{ matrix.database }}" - runs-on: "ubuntu-20.04" + runs-on: ubuntu-latest services: rabbitmq: image: rabbitmq:3-alpine @@ -93,7 +93,7 @@ jobs: phpunit_no_prefix: name: "PHP ${{ matrix.php }} using ${{ matrix.database }} without prefix" - runs-on: "ubuntu-20.04" + runs-on: ubuntu-latest services: rabbitmq: image: rabbitmq:3-alpine diff --git a/.github/workflows/translations.yml b/.github/workflows/translations.yml index 4ebd0516c..5268cb4d6 100644 --- a/.github/workflows/translations.yml +++ b/.github/workflows/translations.yml @@ -13,7 +13,7 @@ permissions: jobs: translations: name: "Translations" - runs-on: "ubuntu-20.04" + runs-on: ubuntu-latest strategy: matrix: diff --git a/CHANGELOG.md b/CHANGELOG.md index cd0ed4c88..45b61efc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ * **[BC BREAK]** Convert 403 errors to 404 errors by @yguedidi in https://github.com/wallabag/wallabag/pull/8075 * `wallassets/` folder renamed to `build/` +## [2.6.11](https://github.com/wallabag/wallabag/tree/2.6.11) +[Full Changelog](https://github.com/wallabag/wallabag/compare/2.6.10...2.6.11) + +### Security fix +* Protect actions with a CSRF token by @yguedidi in https://github.com/wallabag/wallabag/commit/99c8a06594d6ee7480ce4d041ccff3025b353656 + ## [2.6.10](https://github.com/wallabag/wallabag/tree/2.6.10) [Full Changelog](https://github.com/wallabag/wallabag/compare/2.6.9...2.6.10) diff --git a/app/config/config.yml b/app/config/config.yml index bbe36a365..19e139db1 100644 --- a/app/config/config.yml +++ b/app/config/config.yml @@ -29,6 +29,7 @@ framework: handler_id: session.handler.native_file save_path: "%kernel.project_dir%/var/sessions/%kernel.environment%" cookie_secure: auto + cookie_samesite: lax storage_factory_id: session.storage.factory.native fragments: ~ http_method_override: true diff --git a/app/config/wallabag.yml b/app/config/wallabag.yml index 2523aae65..29656236d 100644 --- a/app/config/wallabag.yml +++ b/app/config/wallabag.yml @@ -1,5 +1,5 @@ parameters: - wallabag.version: 2.6.10 + wallabag.version: 2.6.11 wallabag.paypal_url: "https://liberapay.com/wallabag/donate" wallabag.languages: en: 'English' diff --git a/assets/scss/_cards.scss b/assets/scss/_cards.scss index 6ae2b8e9b..f5f7348bf 100644 --- a/assets/scss/_cards.scss +++ b/assets/scss/_cards.scss @@ -179,6 +179,7 @@ a.original:not(.waves-effect) { .card-entry-tags a, .card-entry-labels a, .card-tag-labels a, +.card-tag-labels button, .card-entry-labels-hidden a, #list .chip a { text-decoration: none; diff --git a/assets/scss/_dark_theme.scss b/assets/scss/_dark_theme.scss index aca9049e0..1f8f71814 100644 --- a/assets/scss/_dark_theme.scss +++ b/assets/scss/_dark_theme.scss @@ -63,7 +63,9 @@ .input-field input:focus, .results-item, .sidenav li > a, - .sidenav li > a > i.material-icons { + .sidenav li > a > i.material-icons, + .sidenav li button, + .sidenav li button > i.material-icons { color: #dfdfdf; } @@ -88,6 +90,7 @@ .mass-action-tags .mass-action-tags-input.mass-action-tags-input, .sidenav li:not(.logo) > a:hover, + .sidenav li:not(.logo) button:hover, .sidenav .collapsible-header:hover, .sidenav.sidenav-fixed .collapsible-header:hover { background-color: #1d1d1d; diff --git a/assets/scss/_nav.scss b/assets/scss/_nav.scss index a1db43486..a61657c76 100644 --- a/assets/scss/_nav.scss +++ b/assets/scss/_nav.scss @@ -6,11 +6,32 @@ nav { line-height: initial; } +// adapted from anchor styles from node_modules/@materializecss/materialize/sass/components/_navbar.scss +nav ul button { + transition: background-color .3s; + font-size: 1rem; + color: #fff; + display: block; + padding: 0 15px; + cursor: pointer; + background: none; + border: 0; + + &:focus { + background: none; + } + + &:hover { + background-color: rgba(0 0 0 / 10%); + } +} + nav { input { color: #aaa; } + ul button:hover, ul a:hover { background-color: initial; } @@ -34,6 +55,7 @@ nav { justify-content: space-between; align-items: center; + button, a { padding: 10px 15px; } diff --git a/assets/scss/_sidenav.scss b/assets/scss/_sidenav.scss index d57992063..8f2be7dd3 100644 --- a/assets/scss/_sidenav.scss +++ b/assets/scss/_sidenav.scss @@ -12,6 +12,7 @@ background: initial; } + & button > i.material-icons.theme-toggle-icon, & > a > i.material-icons.theme-toggle-icon { float: none; margin-left: 0; @@ -22,6 +23,7 @@ margin: 0; } + &.sidenav-fixed button, &.sidenav-fixed a { font-size: 13px; line-height: 44px; @@ -41,7 +43,35 @@ } } -.bold > a { +// adapted from anchor styles from node_modules/@materializecss/materialize/sass/components/_sidenav.scss +.sidenav li button { + color: rgba(0 0 0 / 87%); + display: block; + font-size: 14px; + font-weight: 500; + height: 48px; + line-height: 48px; + padding: 0 (16px * 2); + width: 100%; + text-align: left; + + &:hover { + background-color: rgba(0 0 0 / 5%); + } + + & > i, + & > i.material-icons { + float: left; + height: 48px; + line-height: 48px; + margin: 0 (16px * 2) 0 0; + width: 24px; + color: rgba(0 0 0 / 54%); + } +} + +.bold > a, +.bold > button { font-weight: bold; } diff --git a/assets/scss/_various.scss b/assets/scss/_various.scss index ad0703afa..569c46155 100644 --- a/assets/scss/_various.scss +++ b/assets/scss/_various.scss @@ -1,3 +1,5 @@ +@use "variables"; + /* ========================================================================== * Various * ========================================================================== */ @@ -38,3 +40,18 @@ nav .input-field input { .tab { flex: 1; } + +.btn-link { + background: none; + border: 0; + padding: 0; + color: variables.$blue-accent-color; + + &:focus { + background: none; + } +} + +.inline-block { + display: inline-block; +} diff --git a/src/Controller/Api/DeveloperController.php b/src/Controller/Api/DeveloperController.php index 1fd1da554..2eb774f3e 100644 --- a/src/Controller/Api/DeveloperController.php +++ b/src/Controller/Api/DeveloperController.php @@ -6,6 +6,7 @@ use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\Routing\Annotation\Route; use Symfony\Contracts\Translation\TranslatorInterface; use Wallabag\Controller\AbstractController; @@ -73,7 +74,7 @@ class DeveloperController extends AbstractController public function deleteClientAction(Request $request, Client $client, EntityManagerInterface $entityManager, TranslatorInterface $translator) { if (!$this->isCsrfTokenValid('delete-client', $request->request->get('token'))) { - throw $this->createAccessDeniedException('Bad CSRF token.'); + throw new BadRequestHttpException('Bad CSRF token.'); } if (null === $this->getUser() || $client->getUser()->getId() !== $this->getUser()->getId()) { diff --git a/src/Controller/ConfigController.php b/src/Controller/ConfigController.php index 017c84aa8..a5c7bdf1c 100644 --- a/src/Controller/ConfigController.php +++ b/src/Controller/ConfigController.php @@ -17,6 +17,7 @@ use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; +use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\Routing\Annotation\Route; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Validator\Constraints\Locale as LocaleConstraint; @@ -253,7 +254,7 @@ class ConfigController extends AbstractController public function disableOtpEmailAction(Request $request) { if (!$this->isCsrfTokenValid('otp', $request->request->get('token'))) { - throw $this->createAccessDeniedException('Bad CSRF token.'); + throw new BadRequestHttpException('Bad CSRF token.'); } $user = $this->getUser(); @@ -278,7 +279,7 @@ class ConfigController extends AbstractController public function otpEmailAction(Request $request) { if (!$this->isCsrfTokenValid('otp', $request->request->get('token'))) { - throw $this->createAccessDeniedException('Bad CSRF token.'); + throw new BadRequestHttpException('Bad CSRF token.'); } $user = $this->getUser(); @@ -306,7 +307,7 @@ class ConfigController extends AbstractController public function disableOtpAppAction(Request $request) { if (!$this->isCsrfTokenValid('otp', $request->request->get('token'))) { - throw $this->createAccessDeniedException('Bad CSRF token.'); + throw new BadRequestHttpException('Bad CSRF token.'); } $user = $this->getUser(); @@ -333,7 +334,7 @@ class ConfigController extends AbstractController public function otpAppAction(Request $request, GoogleAuthenticatorInterface $googleAuthenticator) { if (!$this->isCsrfTokenValid('otp', $request->request->get('token'))) { - throw $this->createAccessDeniedException('Bad CSRF token.'); + throw new BadRequestHttpException('Bad CSRF token.'); } $user = $this->getUser(); @@ -392,7 +393,7 @@ class ConfigController extends AbstractController public function otpAppCheckAction(Request $request, GoogleAuthenticatorInterface $googleAuthenticator) { if (!$this->isCsrfTokenValid('otp', $request->request->get('token'))) { - throw $this->createAccessDeniedException('Bad CSRF token.'); + throw new BadRequestHttpException('Bad CSRF token.'); } $isValid = $googleAuthenticator->checkCode( @@ -425,20 +426,20 @@ class ConfigController extends AbstractController /** * @return RedirectResponse|JsonResponse */ - #[Route(path: '/generate-token', name: 'generate_token', methods: ['GET'])] + #[Route(path: '/generate-token', name: 'generate_token', methods: ['POST'])] #[IsGranted('EDIT_CONFIG')] public function generateTokenAction(Request $request) { + if (!$this->isCsrfTokenValid('generate-token', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $config = $this->getConfig(); $config->setFeedToken(Utils::generateToken()); $this->entityManager->persist($config); $this->entityManager->flush(); - if ($request->isXmlHttpRequest()) { - return new JsonResponse(['token' => $config->getFeedToken()]); - } - $this->addFlash( 'notice', 'flashes.config.notice.feed_token_updated' @@ -450,20 +451,20 @@ class ConfigController extends AbstractController /** * @return RedirectResponse|JsonResponse */ - #[Route(path: '/revoke-token', name: 'revoke_token', methods: ['GET'])] + #[Route(path: '/revoke-token', name: 'revoke_token', methods: ['POST'])] #[IsGranted('EDIT_CONFIG')] public function revokeTokenAction(Request $request) { + if (!$this->isCsrfTokenValid('revoke-token', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $config = $this->getConfig(); $config->setFeedToken(null); $this->entityManager->persist($config); $this->entityManager->flush(); - if ($request->isXmlHttpRequest()) { - return new JsonResponse(); - } - $this->addFlash( 'notice', 'flashes.config.notice.feed_token_revoked' @@ -477,10 +478,14 @@ class ConfigController extends AbstractController * * @return RedirectResponse */ - #[Route(path: '/tagging-rule/delete/{taggingRule}', name: 'delete_tagging_rule', methods: ['GET'], requirements: ['taggingRule' => '\d+'])] + #[Route(path: '/tagging-rule/delete/{taggingRule}', name: 'delete_tagging_rule', methods: ['POST'], requirements: ['taggingRule' => '\d+'])] #[IsGranted('DELETE', subject: 'taggingRule')] - public function deleteTaggingRuleAction(TaggingRule $taggingRule) + public function deleteTaggingRuleAction(Request $request, TaggingRule $taggingRule) { + if (!$this->isCsrfTokenValid('delete-tagging-rule', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $this->entityManager->remove($taggingRule); $this->entityManager->flush(); @@ -509,10 +514,14 @@ class ConfigController extends AbstractController * * @return RedirectResponse */ - #[Route(path: '/ignore-origin-user-rule/delete/{ignoreOriginUserRule}', name: 'delete_ignore_origin_rule', methods: ['GET'], requirements: ['ignoreOriginUserRule' => '\d+'])] + #[Route(path: '/ignore-origin-user-rule/delete/{ignoreOriginUserRule}', name: 'delete_ignore_origin_rule', methods: ['POST'], requirements: ['ignoreOriginUserRule' => '\d+'])] #[IsGranted('DELETE', subject: 'ignoreOriginUserRule')] - public function deleteIgnoreOriginRuleAction(IgnoreOriginUserRule $ignoreOriginUserRule) + public function deleteIgnoreOriginRuleAction(Request $request, IgnoreOriginUserRule $ignoreOriginUserRule) { + if (!$this->isCsrfTokenValid('delete-ignore-origin-rule', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $this->entityManager->remove($ignoreOriginUserRule); $this->entityManager->flush(); @@ -546,7 +555,7 @@ class ConfigController extends AbstractController public function resetAction(Request $request, string $type, AnnotationRepository $annotationRepository, EntryRepository $entryRepository, TaggingRuleRepository $taggingRuleRepository) { if (!$this->isCsrfTokenValid('reset-area', $request->request->get('token'))) { - throw $this->createAccessDeniedException('Bad CSRF token.'); + throw new BadRequestHttpException('Bad CSRF token.'); } switch ($type) { @@ -602,7 +611,7 @@ class ConfigController extends AbstractController public function deleteAccountAction(Request $request, UserRepository $userRepository, TokenStorageInterface $tokenStorage) { if (!$this->isCsrfTokenValid('delete-account', $request->request->get('token'))) { - throw $this->createAccessDeniedException('Bad CSRF token.'); + throw new BadRequestHttpException('Bad CSRF token.'); } $enabledUsers = $userRepository->getSumEnabledUsers(); @@ -627,10 +636,14 @@ class ConfigController extends AbstractController * * @return RedirectResponse */ - #[Route(path: '/config/view-mode', name: 'switch_view_mode', methods: ['GET'])] + #[Route(path: '/config/view-mode', name: 'switch_view_mode', methods: ['POST'])] #[IsGranted('EDIT_CONFIG')] public function changeViewModeAction(Request $request) { + if (!$this->isCsrfTokenValid('switch-view-mode', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $user = $this->getUser(); $user->getConfig()->setListMode(!$user->getConfig()->getListMode()); @@ -649,10 +662,14 @@ class ConfigController extends AbstractController * * @return RedirectResponse */ - #[Route(path: '/locale/{language}', name: 'changeLocale', methods: ['GET'])] + #[Route(path: '/locale/{language}', name: 'changeLocale', methods: ['POST'])] #[IsGranted('PUBLIC_ACCESS')] public function setLocaleAction(Request $request, ValidatorInterface $validator, $language = null) { + if (!$this->isCsrfTokenValid('change-locale', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $errors = $validator->validate($language, new LocaleConstraint(['canonicalize' => true])); if (0 === \count($errors)) { diff --git a/src/Controller/EntryController.php b/src/Controller/EntryController.php index ebfebe40b..658bf57c5 100644 --- a/src/Controller/EntryController.php +++ b/src/Controller/EntryController.php @@ -14,6 +14,7 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\Routing\Annotation\Route; use Symfony\Component\Security\Core\Security; use Symfony\Contracts\Translation\TranslatorInterface; @@ -52,6 +53,10 @@ class EntryController extends AbstractController #[IsGranted('EDIT_ENTRIES')] public function massAction(Request $request, TagRepository $tagRepository) { + if (!$this->isCsrfTokenValid('mass-action', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $values = $request->request->all(); $tagsToAdd = []; @@ -395,10 +400,14 @@ class EntryController extends AbstractController * * @return RedirectResponse */ - #[Route(path: '/reload/{id}', name: 'reload_entry', methods: ['GET'], requirements: ['id' => '\d+'])] + #[Route(path: '/reload/{id}', name: 'reload_entry', methods: ['POST'], requirements: ['id' => '\d+'])] #[IsGranted('RELOAD', subject: 'entry')] - public function reloadAction(Entry $entry) + public function reloadAction(Request $request, Entry $entry) { + if (!$this->isCsrfTokenValid('reload-entry', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $this->updateEntry($entry, 'entry_reloaded'); // if refreshing entry failed, don't save it @@ -422,10 +431,14 @@ class EntryController extends AbstractController * * @return RedirectResponse */ - #[Route(path: '/archive/{id}', name: 'archive_entry', methods: ['GET'], requirements: ['id' => '\d+'])] + #[Route(path: '/archive/{id}', name: 'archive_entry', methods: ['POST'], requirements: ['id' => '\d+'])] #[IsGranted('ARCHIVE', subject: 'entry')] public function toggleArchiveAction(Request $request, Entry $entry) { + if (!$this->isCsrfTokenValid('archive-entry', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $entry->toggleArchive(); $this->entityManager->flush(); @@ -449,10 +462,14 @@ class EntryController extends AbstractController * * @return RedirectResponse */ - #[Route(path: '/star/{id}', name: 'star_entry', methods: ['GET'], requirements: ['id' => '\d+'])] + #[Route(path: '/star/{id}', name: 'star_entry', methods: ['POST'], requirements: ['id' => '\d+'])] #[IsGranted('STAR', subject: 'entry')] public function toggleStarAction(Request $request, Entry $entry) { + if (!$this->isCsrfTokenValid('star-entry', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $entry->toggleStar(); $entry->updateStar($entry->isStarred()); $this->entityManager->flush(); @@ -477,10 +494,14 @@ class EntryController extends AbstractController * * @return RedirectResponse */ - #[Route(path: '/delete/{id}', name: 'delete_entry', methods: ['GET'], requirements: ['id' => '\d+'])] + #[Route(path: '/delete/{id}', name: 'delete_entry', methods: ['POST'], requirements: ['id' => '\d+'])] #[IsGranted('DELETE', subject: 'entry')] public function deleteEntryAction(Request $request, Entry $entry) { + if (!$this->isCsrfTokenValid('delete-entry', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + // generates the view url for this entry to check for redirection later // to avoid redirecting to the deleted entry. Ugh. $url = $this->generateUrl( @@ -513,10 +534,14 @@ class EntryController extends AbstractController * * @return Response */ - #[Route(path: '/share/{id}', name: 'share', methods: ['GET'], requirements: ['id' => '\d+'])] + #[Route(path: '/share/{id}', name: 'share', methods: ['POST'], requirements: ['id' => '\d+'])] #[IsGranted('SHARE', subject: 'entry')] - 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.'); + } + if (null === $entry->getUid()) { $entry->generateUid(); @@ -534,10 +559,14 @@ class EntryController extends AbstractController * * @return Response */ - #[Route(path: '/share/delete/{id}', name: 'delete_share', methods: ['GET'], requirements: ['id' => '\d+'])] + #[Route(path: '/share/delete/{id}', name: 'delete_share', methods: ['POST'], requirements: ['id' => '\d+'])] #[IsGranted('UNSHARE', subject: 'entry')] - public function deleteShareAction(Entry $entry) + public function deleteShareAction(Request $request, Entry $entry) { + if (!$this->isCsrfTokenValid('delete-share', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $entry->cleanUid(); $this->entityManager->persist($entry); diff --git a/src/Controller/TagController.php b/src/Controller/TagController.php index 38e9173ce..d921ddb4c 100644 --- a/src/Controller/TagController.php +++ b/src/Controller/TagController.php @@ -10,6 +10,7 @@ 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\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\Routing\Annotation\Route; use Symfony\Component\Security\Core\Security; use Symfony\Contracts\Translation\TranslatorInterface; @@ -85,10 +86,14 @@ class TagController extends AbstractController * * @return Response */ - #[Route(path: '/remove-tag/{entry}/{tag}', name: 'remove_tag', methods: ['GET'], requirements: ['entry' => '\d+', 'tag' => '\d+'])] + #[Route(path: '/remove-tag/{entry}/{tag}', name: 'remove_tag', methods: ['POST'], requirements: ['entry' => '\d+', 'tag' => '\d+'])] #[IsGranted('UNTAG', subject: 'entry')] public function removeTagFromEntry(Request $request, Entry $entry, Tag $tag) { + if (!$this->isCsrfTokenValid('remove-tag', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $entry->removeTag($tag); $this->entityManager->flush(); @@ -225,10 +230,14 @@ class TagController extends AbstractController * * @return Response */ - #[Route(path: '/tag/search/{filter}', name: 'tag_this_search', methods: ['GET'])] + #[Route(path: '/tag/search/{filter}', name: 'tag_this_search', methods: ['POST'])] #[IsGranted('CREATE_TAGS')] public function tagThisSearchAction($filter, Request $request, EntryRepository $entryRepository) { + if (!$this->isCsrfTokenValid('tag-this-search', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + $currentRoute = $request->query->has('currentRoute') ? $request->query->get('currentRoute') : ''; /** @var QueryBuilder $qb */ @@ -260,11 +269,15 @@ class TagController extends AbstractController * * @return Response */ - #[Route(path: '/tag/delete/{slug}', name: 'tag_delete', methods: ['GET'])] + #[Route(path: '/tag/delete/{slug}', name: 'tag_delete', methods: ['POST'])] #[ParamConverter('tag', options: ['mapping' => ['slug' => 'slug']])] #[IsGranted('DELETE', subject: 'tag')] public function removeTagAction(Tag $tag, Request $request, EntryRepository $entryRepository) { + if (!$this->isCsrfTokenValid('tag-delete', $request->request->get('token'))) { + throw new BadRequestHttpException('Bad CSRF token.'); + } + foreach ($tag->getEntriesByUserId($this->getUser()->getId()) as $entry) { $entryRepository->removeTag($this->getUser()->getId(), $tag); } diff --git a/templates/Config/index.html.twig b/templates/Config/index.html.twig index 51ec63776..8abeaf15a 100644 --- a/templates/Config/index.html.twig +++ b/templates/Config/index.html.twig @@ -182,48 +182,63 @@
+
+
+ {{ 'config.form_feed.description'|trans }} +
+
+ +
+
+
{{ 'config.form_feed.token_label'|trans }}
+
+ {% if feed.token %} + {{ feed.token }} + {% else %} + {{ 'config.form_feed.no_token'|trans }} + {% endif %} + + {% if feed.token %} + – +
+ + + +
+ – +
+ + + +
+ {% else %} + – +
+ + + +
+ {% endif %} +
+
+
+ {% if feed.token %} +
+
+
{{ 'config.form_feed.feed_links'|trans }}
+ +
+
+ {% endif %} + {{ form_start(form.feed) }} {{ form_errors(form.feed) }} -
-
- {{ 'config.form_feed.description'|trans }} -
-
- -
-
-
{{ 'config.form_feed.token_label'|trans }}
-
- {% if feed.token %} - {{ feed.token }} - {% else %} - {{ 'config.form_feed.no_token'|trans }} - {% endif %} - - {% if feed.token %} - – {{ 'config.form_feed.token_reset'|trans }} - – {{ 'config.form_feed.token_revoke'|trans }} - {% else %} - – {{ 'config.form_feed.token_create'|trans }} - {% endif %} -
-
-
- {% if feed.token %} -
-
-
{{ 'config.form_feed.feed_links'|trans }}
- -
-
- {% endif %} -
{{ form_label(form.feed.feed_limit) }} @@ -384,9 +399,13 @@ mode_edit - - delete - +
+ + + +
{% endfor %} @@ -564,9 +583,13 @@ mode_edit - - delete - +
+ + + +
{% endfor %} diff --git a/templates/Entry/Card/_mass_checkbox.html.twig b/templates/Entry/Card/_mass_checkbox.html.twig index e972f5695..dbd1d80b8 100644 --- a/templates/Entry/Card/_mass_checkbox.html.twig +++ b/templates/Entry/Card/_mass_checkbox.html.twig @@ -1,3 +1,3 @@ diff --git a/templates/Entry/_card_actions.html.twig b/templates/Entry/_card_actions.html.twig index d14572012..0111f9b00 100644 --- a/templates/Entry/_card_actions.html.twig +++ b/templates/Entry/_card_actions.html.twig @@ -17,17 +17,35 @@ {% endif %} {% if is_granted('ARCHIVE', entry) %}
  • - {% if entry.isArchived == 0 %}done{% else %}unarchive{% endif %} +
    + + + +
    • {% endif %} {% if is_granted('STAR', entry) %}
  • - {% if entry.isStarred == 0 %}star_border{% else %}star{% endif %} +
    + + + +
    • {% endif %} {% if is_granted('DELETE', entry) %}
  • - delete +
    + + + +
    • {% endif %} diff --git a/templates/Entry/_card_list.html.twig b/templates/Entry/_card_list.html.twig index 01916607c..2710455f1 100644 --- a/templates/Entry/_card_list.html.twig +++ b/templates/Entry/_card_list.html.twig @@ -22,13 +22,31 @@ language {% endif %} {% if is_granted('ARCHIVE', entry) %} - {% if entry.isArchived == 0 %}done{% else %}unarchive{% endif %} +
    + + + +
    {% endif %} {% if is_granted('STAR', entry) %} - {% if entry.isStarred == 0 %}star_border{% else %}star{% endif %} +
    + + + +
    {% endif %} {% if is_granted('DELETE', entry) %} - delete +
    + + + +
    {% endif %} diff --git a/templates/Entry/_tags.html.twig b/templates/Entry/_tags.html.twig index e0f260016..c23286d77 100644 --- a/templates/Entry/_tags.html.twig +++ b/templates/Entry/_tags.html.twig @@ -5,9 +5,13 @@ {{ tag.label }} {% 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 - +
    + + + +
    {% endif %} {% endfor %} diff --git a/templates/Entry/entries.html.twig b/templates/Entry/entries.html.twig index ac3c580a7..0b7daefaa 100644 --- a/templates/Entry/entries.html.twig +++ b/templates/Entry/entries.html.twig @@ -53,65 +53,80 @@ {% if current_route == 'homepage' %} {% set current_route = 'unread' %} {% endif %} -
    -
    -
    - {{ 'entry.list.number_on_the_page'|trans({'%count%': entries.count}) }} - {% if entries.count > 0 %} - {% if list_mode == 0 %}view_list{% else %}view_module{% endif %} +
    + + + +
    +
    + {{ 'entry.list.number_on_the_page'|trans({'%count%': entries.count}) }} + {% if entries.count > 0 %} +
    + + + +
    + {% endif %} + {% if entries.count > 0 and is_granted('EDIT_ENTRIES') %} + + {% endif %} + {% if app.user.config.feedToken %} + {% include "Entry/_feed_link.html.twig" %} + {% endif %} +
    + {% if current_route == 'search' and is_granted('CREATE_TAGS') %} +
    + + + +
    {% endif %} - {% if entries.count > 0 and is_granted('EDIT_ENTRIES') %} - - {% endif %} - {% if app.user.config.feedToken %} - {% include "Entry/_feed_link.html.twig" %} + {% if entries.getNbPages > 1 %} + {{ pagerfanta(entries, 'default_wallabag') }} {% endif %}
    - {% if current_route == 'search' and is_granted('CREATE_TAGS') %}{% endif %} + + {% if entries.count > 0 %} + +
    +
    + + + + +
    + +
    + + +
    +
    + +
      + + {% for entry in entries %} +
    1. + {% if list_mode == 1 %} + {% include "Entry/_card_list.html.twig" with {'entry': entry, 'currentRoute': current_route, 'routes': entries_with_archived_class_routes} only %} + {% elseif not entry.previewPicture is null and entry.mimetype starts with 'image/' %} + {% include "Entry/_card_full_image.html.twig" with {'entry': entry, 'currentRoute': current_route, 'routes': entries_with_archived_class_routes} only %} + {% else %} + {% include "Entry/_card_preview.html.twig" with {'entry': entry, 'currentRoute': current_route, 'routes': entries_with_archived_class_routes} only %} + {% endif %} +
      2. + {% endfor %} +
    + {% endif %} + {% if entries.getNbPages > 1 %} - {{ pagerfanta(entries, 'default_wallabag') }} +
    + {{ pagerfanta(entries, 'default_wallabag') }} +
    {% endif %}
    - {% if entries.count > 0 %} - -
    -
    - - - - -
    - -
    - - -
    -
    - -
      - - {% for entry in entries %} -
    1. - {% if list_mode == 1 %} - {% include "Entry/_card_list.html.twig" with {'entry': entry, 'currentRoute': current_route, 'routes': entries_with_archived_class_routes} only %} - {% elseif not entry.previewPicture is null and entry.mimetype starts with 'image/' %} - {% include "Entry/_card_full_image.html.twig" with {'entry': entry, 'currentRoute': current_route, 'routes': entries_with_archived_class_routes} only %} - {% else %} - {% include "Entry/_card_preview.html.twig" with {'entry': entry, 'currentRoute': current_route, 'routes': entries_with_archived_class_routes} only %} - {% endif %} -
      2. - {% endfor %} -
    - {% endif %} - - - {% if entries.getNbPages > 1 %} -
    - {{ pagerfanta(entries, 'default_wallabag') }} -
    - {% endif %} - {% if has_exports and is_granted('EXPORT_ENTRIES') %}
    diff --git a/templates/Entry/entry.html.twig b/templates/Entry/entry.html.twig index c702c2351..8faef53a9 100644 --- a/templates/Entry/entry.html.twig +++ b/templates/Entry/entry.html.twig @@ -27,16 +27,24 @@ @@ -61,10 +69,14 @@ {% if is_granted('RELOAD', entry) %}
  • - - refresh - {{ 'entry.view.left_menu.re_fetch_content'|trans }} - +
    + + + +
    • {% endif %} @@ -76,30 +88,42 @@ {% if is_granted('ARCHIVE', entry) %}
  • - - {% if entry.isArchived == 0 %}done{% else %}unarchive{% endif %} - {{ mark_as_read_label|trans }} - +
    + + + +
    • {% endif %} {% if is_granted('STAR', entry) %}
  • - - {% if entry.isStarred == 0 %}star_outline{% else %}star{% endif %} - {{ 'entry.view.left_menu.set_as_starred'|trans }} - +
    + + + +
    • {% endif %} {% if is_granted('DELETE', entry) %}
  • - - delete - {{ 'entry.view.left_menu.delete'|trans }} - +
    + + + +
    • {% endif %} @@ -149,16 +173,24 @@ {% if craue_setting('share_public') %} {% if is_granted('SHARE', entry) %}
  • - - {{ 'entry.view.left_menu.public_link'|trans }} - +
    + + + +
    • {% endif %} {% if is_granted('UNSHARE', entry) %}
  • - - {{ 'entry.view.left_menu.delete_public_link'|trans }} - +
    + + + +
    • {% endif %} {% endif %} @@ -344,13 +376,37 @@
    diff --git a/templates/Tag/tags.html.twig b/templates/Tag/tags.html.twig index 180a910a0..dd9281aee 100644 --- a/templates/Tag/tags.html.twig +++ b/templates/Tag/tags.html.twig @@ -34,9 +34,13 @@ {% endif %} {% if is_granted('DELETE', tag) %} - - delete - +
    + + + +
    {% endif %} {% if app.user.config.feedToken %} rss_feed diff --git a/templates/bundles/FOSUserBundle/layout.html.twig b/templates/bundles/FOSUserBundle/layout.html.twig index c424f42ea..a2d865d9b 100644 --- a/templates/bundles/FOSUserBundle/layout.html.twig +++ b/templates/bundles/FOSUserBundle/layout.html.twig @@ -16,9 +16,23 @@ {% endblock fos_user_content %}
    - Deutsch – - English – - Français +
    + + + +
    + – +
    + + + +
    + – +
    + + + +
    diff --git a/tests/Controller/Api/DeveloperControllerTest.php b/tests/Controller/Api/DeveloperControllerTest.php index ab0b62d7d..9df00a823 100644 --- a/tests/Controller/Api/DeveloperControllerTest.php +++ b/tests/Controller/Api/DeveloperControllerTest.php @@ -105,7 +105,7 @@ class DeveloperControllerTest extends WallabagTestCase $this->logInAs('bob'); $client->request('POST', '/developer/client/delete/' . $adminApiClient->getId()); - $this->assertSame(404, $client->getResponse()->getStatusCode()); + $this->assertSame(400, $client->getResponse()->getStatusCode()); // Try to remove the admin's client with the good user $this->logInAs('admin'); diff --git a/tests/Controller/ConfigControllerTest.php b/tests/Controller/ConfigControllerTest.php index 301ece136..f41f25c13 100644 --- a/tests/Controller/ConfigControllerTest.php +++ b/tests/Controller/ConfigControllerTest.php @@ -328,7 +328,8 @@ class ConfigControllerTest extends WallabagTestCase $this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text'])); $this->assertStringContainsString('config.form_feed.no_token', $body[0]); - $client->request('GET', '/generate-token'); + $client->submit($crawler->selectButton('config.form_feed.token_create')->form()); + $this->assertSame(302, $client->getResponse()->getStatusCode()); $crawler = $client->followRedirect(); @@ -337,38 +338,34 @@ class ConfigControllerTest extends WallabagTestCase $this->assertStringContainsString('config.form_feed.token_reset', $body[0]); } - public function testGenerateTokenAjax() - { - $this->logInAs('admin'); - $client = $this->getTestClient(); - - $client->request( - 'GET', - '/generate-token', - [], - [], - ['HTTP_X-Requested-With' => 'XMLHttpRequest'] - ); - - $this->assertSame(200, $client->getResponse()->getStatusCode()); - $content = json_decode((string) $client->getResponse()->getContent(), true); - $this->assertArrayHasKey('token', $content); - } - public function testRevokeTokenAjax() { $this->logInAs('admin'); $client = $this->getTestClient(); - $client->request( - 'GET', - '/revoke-token', - [], - [], - ['HTTP_X-Requested-With' => 'XMLHttpRequest'] - ); + // set the token + $em = $client->getContainer()->get(EntityManagerInterface::class); + $user = $em + ->getRepository(User::class) + ->findOneByUsername('admin'); - $this->assertSame(200, $client->getResponse()->getStatusCode()); + if (!$user) { + $this->markTestSkipped('No user found in db.'); + } + + $config = $user->getConfig(); + $config->setFeedToken('abcd1234'); + $em->persist($config); + $em->flush(); + + $crawler = $client->request('GET', '/config'); + + $client->submit($crawler->selectButton('config.form_feed.token_revoke')->form()); + + $crawler = $client->followRedirect(); + + $this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text'])); + $this->assertStringContainsString('config.form_feed.token_create', $body[0]); } public function testFeedUpdate() @@ -484,9 +481,8 @@ class ConfigControllerTest extends WallabagTestCase $this->assertStringContainsString('readingTime <= 30', $crawler->filter('body')->extract(['_text'])[0]); - $deleteLink = $crawler->filter('.delete_tagging_rule')->last()->link(); + $crawler = $client->submit($crawler->filter('#set5')->selectButton('delete')->form()); - $crawler = $client->click($deleteLink); $this->assertSame(302, $client->getResponse()->getStatusCode()); $crawler = $client->followRedirect(); @@ -576,7 +572,7 @@ class ConfigControllerTest extends WallabagTestCase ->getRepository(TaggingRule::class) ->findAll()[0]; - $crawler = $client->request('GET', '/tagging-rule/delete/' . $rule->getId()); + $crawler = $client->request('POST', '/tagging-rule/delete/' . $rule->getId()); $this->assertSame(404, $client->getResponse()->getStatusCode()); $this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text'])); @@ -646,9 +642,9 @@ class ConfigControllerTest extends WallabagTestCase $this->assertStringContainsString('host = "example.org"', $crawler->filter('body')->extract(['_text'])[0]); - $deleteLink = $crawler->filter('div[id=set6] a.delete')->last()->link(); + $form = $crawler->filter('#set6')->selectButton('delete')->form(); - $crawler = $client->click($deleteLink); + $crawler = $client->submit($form); $this->assertSame(302, $client->getResponse()->getStatusCode()); $crawler = $client->followRedirect(); @@ -713,7 +709,7 @@ class ConfigControllerTest extends WallabagTestCase ->getRepository(IgnoreOriginUserRule::class) ->findAll()[0]; - $crawler = $client->request('GET', '/ignore-origin-user-rule/edit/' . $rule->getId()); + $crawler = $client->request('POST', '/ignore-origin-user-rule/delete/' . $rule->getId()); $this->assertSame(404, $client->getResponse()->getStatusCode()); $this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text'])); @@ -768,7 +764,7 @@ class ConfigControllerTest extends WallabagTestCase $this->assertStringNotContainsString('config.form_user.delete.button', $body[0]); $client->request('POST', '/account/delete'); - $this->assertSame(404, $client->getResponse()->getStatusCode()); + $this->assertSame(400, $client->getResponse()->getStatusCode()); $user = $em ->getRepository(User::class) @@ -1113,37 +1109,38 @@ class ConfigControllerTest extends WallabagTestCase $this->logInAs('admin'); $client = $this->getTestClient(); - $client->request('GET', '/unread/list'); + $crawler = $client->request('GET', '/unread/list'); $this->assertStringContainsString('row data', $client->getResponse()->getContent()); - $client->request('GET', '/config/view-mode'); - $crawler = $client->followRedirect(); + $form = $crawler->filter('.nb-results')->selectButton('view_list')->form(); - $client->request('GET', '/unread/list'); + $client->submit($form); + + $client->followRedirect(); $this->assertStringContainsString('collection', $client->getResponse()->getContent()); - - $client->request('GET', '/config/view-mode'); } public function testChangeLocaleWithoutReferer() { $client = $this->getTestClient(); - $client->request('GET', '/locale/de'); - $client->followRedirect(); + $crawler = $client->request('POST', '/locale/de'); - $this->assertSame('de', $client->getRequest()->getLocale()); - $this->assertSame('de', $client->getContainer()->get(SessionInterface::class)->get('_locale')); + $this->assertSame(400, $client->getResponse()->getStatusCode()); + $this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text'])); + $this->assertStringContainsString('Bad CSRF token.', $body[0]); } public function testChangeLocaleWithReferer() { $client = $this->getTestClient(); - $client->request('GET', '/login'); - $client->request('GET', '/locale/de'); + $crawler = $client->request('GET', '/login'); + + $client->submit($crawler->selectButton('Deutsch')->form()); + $client->followRedirect(); $this->assertSame('de', $client->getRequest()->getLocale()); @@ -1154,8 +1151,12 @@ class ConfigControllerTest extends WallabagTestCase { $client = $this->getTestClient(); - $client->request('GET', '/login'); - $client->request('GET', '/locale/yuyuyuyu'); + $crawler = $client->request('GET', '/login'); + $token = $crawler->filter('form[action="/locale/de"] input[name=token]')->attr('value'); + + $client->request('POST', '/locale/yuyuyuyu', [ + 'token' => $token, + ]); $client->followRedirect(); $this->assertNotSame('yuyuyuyu', $client->getRequest()->getLocale()); @@ -1376,8 +1377,6 @@ class ConfigControllerTest extends WallabagTestCase $client->request('GET', '/unread/list'); $this->assertStringNotContainsString('class="preview"', $client->getResponse()->getContent()); - - $client->request('GET', '/config/view-mode'); } public function testGeneratedCSS() diff --git a/tests/Controller/EntryControllerTest.php b/tests/Controller/EntryControllerTest.php index b7d020a80..908295694 100644 --- a/tests/Controller/EntryControllerTest.php +++ b/tests/Controller/EntryControllerTest.php @@ -541,7 +541,9 @@ class EntryControllerTest extends WallabagTestCase $client = $this->getTestClient(); - $client->request('GET', '/reload/' . $entry->getId()); + $crawler = $client->request('GET', '/view/' . $entry->getId()); + + $client->submit($crawler->selectButton('entry.view.left_menu.re_fetch_content')->form()); $this->assertSame(302, $client->getResponse()->getStatusCode()); @@ -562,7 +564,9 @@ class EntryControllerTest extends WallabagTestCase $this->getEntityManager()->persist($entry); $this->getEntityManager()->flush(); - $client->request('GET', '/reload/' . $entry->getId()); + $crawler = $client->request('GET', '/view/' . $entry->getId()); + + $client->submit($crawler->selectButton('entry.view.left_menu.re_fetch_content')->form()); $this->assertSame(302, $client->getResponse()->getStatusCode()); @@ -674,7 +678,9 @@ class EntryControllerTest extends WallabagTestCase $client = $this->getTestClient(); - $client->request('GET', '/archive/' . $entry->getId()); + $crawler = $client->request('GET', '/view/' . $entry->getId()); + + $client->submit($crawler->filter('.left-bar')->selectButton('entry.view.left_menu.set_as_read')->form()); $this->assertSame(302, $client->getResponse()->getStatusCode()); @@ -698,7 +704,9 @@ class EntryControllerTest extends WallabagTestCase $client = $this->getTestClient(); - $client->request('GET', '/star/' . $entry->getId()); + $crawler = $client->request('GET', '/view/' . $entry->getId()); + + $client->submit($crawler->filter('.left-bar')->selectButton('entry.view.left_menu.set_as_starred')->form()); $this->assertSame(302, $client->getResponse()->getStatusCode()); @@ -720,13 +728,11 @@ class EntryControllerTest extends WallabagTestCase $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]); } /** @@ -762,10 +768,11 @@ class EntryControllerTest extends WallabagTestCase $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(); @@ -1211,7 +1218,10 @@ class EntryControllerTest extends WallabagTestCase $this->assertSame(404, $client->getResponse()->getStatusCode()); // generating the uid - $client->request('GET', '/share/' . $content->getId()); + $crawler = $client->request('GET', '/view/' . $content->getId()); + + $client->submit($crawler->filter('.left-bar')->selectButton('entry.view.left_menu.public_link')->form()); + $this->assertSame(302, $client->getResponse()->getStatusCode()); $shareUrl = $client->getResponse()->getTargetUrl(); @@ -1238,12 +1248,19 @@ class EntryControllerTest extends WallabagTestCase $this->assertSame(404, $client->getResponse()->getStatusCode()); // removing the share - $client->request('GET', '/share/delete/' . $content->getId()); + $client->getContainer()->get(Config::class)->set('share_public', 1); + $this->logInAs('admin'); + $crawler = $client->request('GET', '/view/' . $content->getId()); + + $client->submit($crawler->filter('.left-bar')->selectButton('entry.view.left_menu.delete_public_link')->form()); + $this->assertSame(302, $client->getResponse()->getStatusCode()); - // share is now disable + // share is now removed $client->request('GET', '/share/' . $content->getUid()); $this->assertSame(404, $client->getResponse()->getStatusCode()); + + $client->getContainer()->get(Config::class)->set('share_public', 0); } /** @@ -1319,7 +1336,9 @@ class EntryControllerTest extends WallabagTestCase ->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()); @@ -1342,8 +1361,9 @@ class EntryControllerTest extends WallabagTestCase $this->getEntityManager()->flush(); - $client->request('GET', '/view/' . $entry->getId()); - $client->request('GET', '/archive/' . $entry->getId()); + $crawler = $client->request('GET', '/view/' . $entry->getId()); + + $client->submit($crawler->filter('.left-bar')->selectButton('entry.view.left_menu.set_as_read')->form()); $this->assertSame(302, $client->getResponse()->getStatusCode()); $this->assertSame('/', $client->getResponse()->headers->get('location')); @@ -1367,8 +1387,7 @@ class EntryControllerTest extends WallabagTestCase $crawler = $client->request('GET', '/view/' . $entry->getId()); - $link = $crawler->filter('a[id="markAsRead"]')->link(); - $client->click($link); + $client->submit($crawler->filter('.left-bar')->selectButton('entry.view.left_menu.set_as_read')->form()); $this->assertSame(302, $client->getResponse()->getStatusCode()); $this->assertStringContainsString('/view/' . $entry->getId(), $client->getResponse()->headers->get('location')); @@ -1504,7 +1523,8 @@ class EntryControllerTest extends WallabagTestCase $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'); @@ -1586,8 +1606,8 @@ class EntryControllerTest extends WallabagTestCase $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); @@ -1760,7 +1780,7 @@ class EntryControllerTest extends WallabagTestCase $this->assertSame('example.com', $content->getDomainName()); } - public function testEntryDeleteTagLink() + public function testEntryDeleteTagForm() { $this->logInAs('admin'); $client = $this->getTestClient(); @@ -1771,10 +1791,7 @@ class EntryControllerTest extends WallabagTestCase $crawler = $client->request('GET', '/view/' . $entry->getId()); - // As long as the deletion link of a tag is following - // a link to the tag view, we take the second one to retrieve - // the deletion link of the first tag - $link = $crawler->filter('body div#article div.tools ul.tags li.chip a')->extract(['href'])[1]; + $link = $crawler->filter('body div#article div.tools ul.tags li.chip form')->extract(['action'])[0]; $this->assertStringStartsWith(\sprintf('/remove-tag/%s/%s', $entry->getId(), $tag->getId()), $link); } @@ -1831,11 +1848,15 @@ class EntryControllerTest extends WallabagTestCase $client = $this->getTestClient(); $entries = []; - $entries[] = $entry1->getId(); - $entries[] = $entry2->getId(); + $entries[] = $entry1Id = $entry1->getId(); + $entries[] = $entry2Id = $entry2->getId(); + + $crawler = $client->request('GET', '/all/list'); + $token = $crawler->filter('#form_mass_action input[name=token]')->attr('value'); // Mass actions : archive $client->request('POST', '/mass', [ + 'token' => $token, 'toggle-archive' => '', 'entry-checkbox' => $entries, ]); @@ -1856,8 +1877,12 @@ class EntryControllerTest extends WallabagTestCase $this->assertSame(1, $res->isArchived()); + $crawler = $client->request('GET', '/all/list'); + $token = $crawler->filter('#form_mass_action input[name=token]')->attr('value'); + // Mass actions : star $client->request('POST', '/mass', [ + 'token' => $token, 'toggle-star' => '', 'entry-checkbox' => $entries, ]); @@ -1878,8 +1903,12 @@ class EntryControllerTest extends WallabagTestCase $this->assertTrue($res->isStarred()); + $crawler = $client->request('GET', '/all/list'); + $token = $crawler->filter('#form_mass_action input[name=token]')->attr('value'); + // Mass actions : tag $client->request('POST', '/mass', [ + 'token' => $token, 'tag' => '', 'tags' => 'foo', 'entry-checkbox' => $entries, @@ -1908,17 +1937,29 @@ class EntryControllerTest extends WallabagTestCase $this->assertNotContains('foo', $res->getTagsLabel()); + $crawler = $client->request('GET', '/all/list'); + $token = $crawler->filter('#form_mass_action input[name=token]')->attr('value'); + // Mass actions : delete $client->request('POST', '/mass', [ + 'token' => $token, 'delete' => '', '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() diff --git a/tests/Controller/TagControllerTest.php b/tests/Controller/TagControllerTest.php index 782cbc127..12db58248 100644 --- a/tests/Controller/TagControllerTest.php +++ b/tests/Controller/TagControllerTest.php @@ -128,8 +128,8 @@ class TagControllerTest extends WallabagTestCase $crawler = $client->request('GET', '/view/' . $entry->getId()); $entryUri = $client->getRequest()->getRequestUri(); - $link = $crawler->filter('a[href^="/remove-tag/' . $entry->getId() . '/' . $tag->getId() . '"]')->link(); - $client->click($link); + $form = $crawler->filter('form[action^="/remove-tag/' . $entry->getId() . '/' . $tag->getId() . '"]')->form(); + $client->submit($form); $this->assertSame(302, $client->getResponse()->getStatusCode()); $this->assertSame($entryUri, $client->getResponse()->getTargetUrl()); @@ -138,9 +138,8 @@ class TagControllerTest extends WallabagTestCase $entry = $this->getEntityManager()->getRepository(Entry::class)->find($entry->getId()); $this->assertNotContains($this->tagName, $entry->getTagsLabel()); - $client->request('GET', '/remove-tag/' . $entry->getId() . '/' . $tag->getId()); - - $this->assertSame(404, $client->getResponse()->getStatusCode()); + $client->request('GET', '/view/' . $entry->getId()); + $this->assertStringNotContainsString('/remove-tag/' . $entry->getId() . '/' . $tag->getId(), $client->getResponse()->getContent()); $tag = $client->getContainer() ->get(EntityManagerInterface::class) @@ -172,8 +171,8 @@ class TagControllerTest extends WallabagTestCase $client = $this->getTestClient(); $crawler = $client->request('GET', '/tag/list'); - $link = $crawler->filter('a[id="delete-' . $tag->getSlug() . '"]')->link(); - $client->click($link); + $form = $crawler->filter('#tag-' . $tag->getId())->selectButton('delete')->form(); + $client->submit($form); $tag = $client->getContainer() ->get(EntityManagerInterface::class) @@ -556,7 +555,7 @@ class TagControllerTest extends WallabagTestCase $crawler = $client->submit($form, $data); - $client->click($crawler->selectLink('entry.list.assign_search_tag')->link()); + $client->submit($crawler->selectButton('entry.list.assign_search_tag')->form()); $client->followRedirect(); $entries = $client->getContainer()