From cb6b75fd0f79a744aad9e91965da4ad90357a37e Mon Sep 17 00:00:00 2001 From: Oleg Andreyev Date: Mon, 11 Nov 2019 22:26:43 +0200 Subject: [PATCH] checking csrf_protection in delete method. (#5712) --- src/Controller/CRUDController.php | 4 + tests/Controller/CRUDControllerTest.php | 188 ++++++++++++++++++++++++ 2 files changed, 192 insertions(+) diff --git a/src/Controller/CRUDController.php b/src/Controller/CRUDController.php index 2dc258de0e..880f53a31b 100644 --- a/src/Controller/CRUDController.php +++ b/src/Controller/CRUDController.php @@ -1415,6 +1415,10 @@ protected function getAclRoles() */ protected function validateCsrfToken($intention) { + if (false === $this->admin->getFormBuilder()->getOption('csrf_protection')) { + return; + } + $request = $this->getRequest(); $token = $request->get('_sonata_csrf_token'); diff --git a/tests/Controller/CRUDControllerTest.php b/tests/Controller/CRUDControllerTest.php index e33ba08602..54bcd00a8f 100644 --- a/tests/Controller/CRUDControllerTest.php +++ b/tests/Controller/CRUDControllerTest.php @@ -44,7 +44,9 @@ use Symfony\Bundle\FrameworkBundle\Templating\DelegatingEngine; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\Form\Form; +use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\Form\FormError; +use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\FormRenderer; use Symfony\Component\Form\FormView; use Symfony\Component\HttpFoundation\JsonResponse; @@ -148,6 +150,11 @@ class CRUDControllerTest extends TestCase */ private $translator; + /** + * @var FormBuilderInterface + */ + private $formBuilder; + /** * {@inheritdoc} */ @@ -166,6 +173,11 @@ protected function setUp(): void $this->parameters = []; $this->template = ''; + $this->formBuilder = $this->createMock(FormBuilderInterface::class); + $this->admin->expects($this->any()) + ->method('getFormBuilder') + ->willReturn($this->formBuilder); + $this->templateRegistry = $this->prophesize(TemplateRegistryInterface::class); $templating = $this->getMockBuilder(DelegatingEngine::class) @@ -1173,6 +1185,11 @@ public function testDeleteActionAjaxSuccess1(): void ->with($this->equalTo('delete')) ->willReturn(true); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $this->request->setMethod('DELETE'); $this->request->headers->set('X-Requested-With', 'XMLHttpRequest'); @@ -1198,6 +1215,11 @@ public function testDeleteActionAjaxSuccess2(): void ->with($this->equalTo('delete')) ->willReturn(true); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $this->request->setMethod('POST'); $this->request->request->set('_method', 'DELETE'); $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.delete'); @@ -1228,6 +1250,11 @@ public function testDeleteActionAjaxError(): void ->method('getClass') ->willReturn('stdClass'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $this->assertLoggerLogsModelManagerException($this->admin, 'delete'); $this->request->setMethod('DELETE'); @@ -1257,6 +1284,11 @@ public function testDeleteActionWithModelManagerExceptionInDebugMode(): void ->with($this->equalTo('delete')) ->willReturn(true); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $this->admin->expects($this->once()) ->method('delete') ->willReturnCallback(static function (): void { @@ -1296,6 +1328,11 @@ public function testDeleteActionSuccess1($expectedToStringValue, $toStringValue) ->with($this->equalTo('delete')) ->willReturn(true); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $this->request->setMethod('DELETE'); $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.delete'); @@ -1335,6 +1372,11 @@ public function testDeleteActionSuccess2($expectedToStringValue, $toStringValue) $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.delete'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $response = $this->controller->deleteAction(1, $this->request); $this->assertInstanceOf(RedirectResponse::class, $response); @@ -1370,6 +1412,11 @@ public function testDeleteActionSuccessNoCsrfTokenProvider($expectedToStringValu $this->request->setMethod('POST'); $this->request->request->set('_method', 'DELETE'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $response = $this->controller->deleteAction(1, $this->request); $this->assertInstanceOf(RedirectResponse::class, $response); @@ -1435,6 +1482,11 @@ public function testDeleteActionError($expectedToStringValue, $toStringValue): v $this->request->setMethod('DELETE'); $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.delete'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $response = $this->controller->deleteAction(1, $this->request); $this->assertInstanceOf(RedirectResponse::class, $response); @@ -1459,6 +1511,11 @@ public function testDeleteActionInvalidCsrfToken(): void $this->request->request->set('_method', 'DELETE'); $this->request->request->set('_sonata_csrf_token', 'CSRF-INVALID'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + try { $this->controller->deleteAction(1, $this->request); } catch (HttpException $e) { @@ -1467,6 +1524,39 @@ public function testDeleteActionInvalidCsrfToken(): void } } + public function testDeleteActionWithDisabledCsrfProtection(): void + { + $object = new \stdClass(); + + $this->admin->expects($this->once()) + ->method('getObject') + ->willReturn($object); + + $this->admin->expects($this->once()) + ->method('checkAccess') + ->with($this->equalTo('delete')) + ->willReturn(true); + + $this->request->setMethod('POST'); + $this->request->request->set('_method', 'DELETE'); + + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(false); + + $this->admin->expects($this->once()) + ->method('toString') + ->with($object) + ->willReturn(\stdClass::class); + + $this->admin->expects($this->once()) + ->method('delete') + ->with($object); + + $this->controller->deleteAction(1, $this->request); + } + public function testEditActionNotFoundException(): void { $this->expectException(NotFoundHttpException::class); @@ -3579,6 +3669,11 @@ public function testBatchActionActionNotDefined(): void $this->request->request->set('data', json_encode(['action' => 'foo', 'idx' => ['123', '456'], 'all_elements' => false])); $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.batch'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $this->controller->batchAction($this->request); } @@ -3588,6 +3683,11 @@ public function testBatchActionActionInvalidCsrfToken(): void $this->request->request->set('data', json_encode(['action' => 'foo', 'idx' => ['123', '456'], 'all_elements' => false])); $this->request->request->set('_sonata_csrf_token', 'CSRF-INVALID'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + try { $this->controller->batchAction($this->request); } catch (HttpException $e) { @@ -3596,6 +3696,44 @@ public function testBatchActionActionInvalidCsrfToken(): void } } + public function testBatchActionActionWithDisabledCsrfProtection(): void + { + $this->request->setMethod('POST'); + $this->request->request->set('data', json_encode(['action' => 'foo', 'idx' => ['123', '456'], 'all_elements' => false])); + + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(false); + + $this->admin->expects($this->once()) + ->method('getBatchActions') + ->willReturn(['foo' => ['label' => 'foo']]); + + $datagrid = $this->createMock(DatagridInterface::class); + + $this->admin->expects($this->once()) + ->method('getDatagrid') + ->willReturn($datagrid); + + $datagrid->expects($this->once()) + ->method('buildPager'); + + $form = $this->createMock(FormInterface::class); + + $datagrid->expects($this->once()) + ->method('getForm') + ->willReturn($form); + + $formView = $this->createMock(FormView::class); + + $form->expects($this->once()) + ->method('createView') + ->willReturn($formView); + + $this->controller->batchAction($this->request); + } + /** * NEXT_MAJOR: Remove this legacy group. * @@ -3620,6 +3758,11 @@ public function testBatchActionMethodNotExist(): void $this->request->request->set('data', json_encode(['action' => 'foo', 'idx' => ['123', '456'], 'all_elements' => false])); $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.batch'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $this->controller->batchAction($this->request); } @@ -3673,6 +3816,11 @@ public function testBatchActionWithoutConfirmation(): void $this->request->request->set('data', json_encode(['action' => 'delete', 'idx' => ['123', '456'], 'all_elements' => false])); $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.batch'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $result = $this->controller->batchAction($this->request); $this->assertInstanceOf(RedirectResponse::class, $result); @@ -3731,6 +3879,11 @@ public function testBatchActionWithoutConfirmation2(): void $this->request->request->set('idx', ['123', '456']); $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.batch'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $result = $this->controller->batchAction($this->request); $this->assertInstanceOf(RedirectResponse::class, $result); @@ -3775,6 +3928,11 @@ public function testBatchActionWithConfirmation(): void ->method('getForm') ->willReturn($form); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $this->assertInstanceOf(Response::class, $this->controller->batchAction($this->request)); $this->assertSame($this->admin, $this->parameters['admin']); @@ -3821,6 +3979,11 @@ public function testBatchActionNonRelevantAction(): void $this->request->request->set('idx', ['789']); $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.batch'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $result = $controller->batchAction($this->request); $this->assertInstanceOf(RedirectResponse::class, $result); @@ -3858,6 +4021,11 @@ public function testBatchActionWithCustomConfirmationTemplate(): void ->method('getForm') ->willReturn($form); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $this->controller->batchAction($this->request); $this->assertSame('custom_template.html.twig', $this->template); @@ -3892,6 +4060,11 @@ public function testBatchActionNonRelevantAction2(): void $this->request->request->set('idx', ['999']); $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.batch'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $result = $controller->batchAction($this->request); $this->assertInstanceOf(RedirectResponse::class, $result); @@ -3925,6 +4098,11 @@ public function testBatchActionNoItems(): void $this->request->request->set('idx', []); $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.batch'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $result = $this->controller->batchAction($this->request); $this->assertInstanceOf(RedirectResponse::class, $result); @@ -3969,6 +4147,11 @@ public function testBatchActionNoItemsEmptyQuery(): void ->method('getClass') ->willReturn('Foo'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $this->request->setMethod('POST'); $this->request->request->set('action', 'bar'); $this->request->request->set('idx', []); @@ -4032,6 +4215,11 @@ public function testBatchActionWithRequesData(): void $this->request->request->set('foo', 'bar'); $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.batch'); + $this->formBuilder->expects($this->once()) + ->method('getOption') + ->with('csrf_protection') + ->willReturn(true); + $result = $this->controller->batchAction($this->request); $this->assertInstanceOf(RedirectResponse::class, $result);