From 677b2986bc78df4c7ecfed87a24593fa0553fd3c Mon Sep 17 00:00:00 2001 From: Yassine Guedidi Date: Sun, 23 Mar 2025 22:12:08 +0100 Subject: [PATCH] Use 400 Bad Request errors for invalid CSRF everywhere --- .../ApiBundle/Controller/DeveloperController.php | 3 ++- .../CoreBundle/Controller/ConfigController.php | 14 +++++++------- .../Controller/DeveloperControllerTest.php | 2 +- .../CoreBundle/Controller/ConfigControllerTest.php | 2 +- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/Wallabag/ApiBundle/Controller/DeveloperController.php b/src/Wallabag/ApiBundle/Controller/DeveloperController.php index 0c08968e2..5b2d6accc 100644 --- a/src/Wallabag/ApiBundle/Controller/DeveloperController.php +++ b/src/Wallabag/ApiBundle/Controller/DeveloperController.php @@ -7,6 +7,7 @@ use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; 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\ApiBundle\Entity\Client; @@ -76,7 +77,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/Wallabag/CoreBundle/Controller/ConfigController.php b/src/Wallabag/CoreBundle/Controller/ConfigController.php index 21f1f990d..1ba7faaed 100644 --- a/src/Wallabag/CoreBundle/Controller/ConfigController.php +++ b/src/Wallabag/CoreBundle/Controller/ConfigController.php @@ -263,7 +263,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(); @@ -287,7 +287,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(); @@ -314,7 +314,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(); @@ -340,7 +340,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(); @@ -399,7 +399,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( @@ -569,7 +569,7 @@ class ConfigController extends AbstractController public function resetAction(Request $request, string $type, AnnotationRepository $annotationRepository, EntryRepository $entryRepository) { if (!$this->isCsrfTokenValid('reset-area', $request->request->get('token'))) { - throw $this->createAccessDeniedException('Bad CSRF token.'); + throw new BadRequestHttpException('Bad CSRF token.'); } switch ($type) { @@ -623,7 +623,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(); diff --git a/tests/Wallabag/ApiBundle/Controller/DeveloperControllerTest.php b/tests/Wallabag/ApiBundle/Controller/DeveloperControllerTest.php index 35573809a..31218a057 100644 --- a/tests/Wallabag/ApiBundle/Controller/DeveloperControllerTest.php +++ b/tests/Wallabag/ApiBundle/Controller/DeveloperControllerTest.php @@ -105,7 +105,7 @@ class DeveloperControllerTest extends WallabagCoreTestCase $this->logInAs('bob'); $client->request('POST', '/developer/client/delete/' . $adminApiClient->getId()); - $this->assertSame(403, $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/Wallabag/CoreBundle/Controller/ConfigControllerTest.php b/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php index 9c6965ff6..c4bcf6194 100644 --- a/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php +++ b/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php @@ -794,7 +794,7 @@ class ConfigControllerTest extends WallabagCoreTestCase $this->assertStringNotContainsString('config.form_user.delete.button', $body[0]); $client->request('POST', '/account/delete'); - $this->assertSame(403, $client->getResponse()->getStatusCode()); + $this->assertSame(400, $client->getResponse()->getStatusCode()); $user = $em ->getRepository(User::class)