diff --git a/src/Controller/AbstractController.php b/src/Controller/AbstractController.php index 0b47544c..9fd50f65 100644 --- a/src/Controller/AbstractController.php +++ b/src/Controller/AbstractController.php @@ -207,6 +207,17 @@ abstract class AbstractController extends BaseAbstractController implements Serv throw new \InvalidArgumentException('handleSearchForm() requires an instanceof BaseQuery as form data'); } + $actions = ['resetSearchFilter', 'removeDefaultQuery', 'setDefaultQuery']; + foreach ($actions as $action) { + if ($request->query->has($action)) { + if (!$this->isCsrfTokenValid('search', $request->query->get('_token'))) { + $this->flashError('action.csrf.error'); + + return false; + } + } + } + if ($request->query->has('resetSearchFilter')) { $data->resetFilter(); $this->removeLastSearch($data); diff --git a/src/Controller/InvoiceController.php b/src/Controller/InvoiceController.php index 6fe96be2..790ecc7e 100644 --- a/src/Controller/InvoiceController.php +++ b/src/Controller/InvoiceController.php @@ -76,6 +76,13 @@ final class InvoiceController extends AbstractController } $query = $this->getDefaultQuery(); + + $token = null; + if ($request->query->has('token')) { + $token = $request->query->get('token'); + $request->query->remove('token'); + } + $form = $this->getToolbarForm($query, $configuration->find('invoice.simple_form')); if ($this->handleSearch($form, $request)) { return $this->redirectToRoute('invoice'); @@ -87,6 +94,12 @@ final class InvoiceController extends AbstractController if ($form->isValid() && $this->isGranted('create_invoice')) { if ($request->query->has('createInvoice')) { + if (!$this->isCsrfTokenValid('invoice.create', $token)) { + $this->flashError('action.csrf.error'); + + return $this->redirectToRoute('invoice'); + } + try { return $this->renderInvoice($query, $request); } catch (Exception $ex) { @@ -160,6 +173,18 @@ final class InvoiceController extends AbstractController return $this->redirectToRoute('invoice'); } + $token = null; + if ($request->query->has('token')) { + $token = $request->query->get('token'); + $request->query->remove('token'); + } + + if (!$this->isCsrfTokenValid('invoice.create', $token)) { + $this->flashError('action.csrf.error'); + + return $this->redirectToRoute('invoice'); + } + $query = $this->getDefaultQuery(); $form = $this->getToolbarForm($query, $configuration->find('invoice.simple_form')); $form->submit($request->query->all(), false); @@ -177,14 +202,22 @@ final class InvoiceController extends AbstractController } /** - * @Route(path="/change-status/{id}/{status}", name="admin_invoice_status", methods={"GET", "POST"}) + * @Route(path="/change-status/{id}/{status}/{token}", name="admin_invoice_status", methods={"GET", "POST"}) * @Security("is_granted('access', invoice.getCustomer())") * @Security("is_granted('create_invoice')") */ - public function changeStatusAction(Invoice $invoice, string $status, Request $request): Response + public function changeStatusAction(Invoice $invoice, string $status, string $token, Request $request, CsrfTokenManagerInterface $csrfTokenManager): Response { + if (!$csrfTokenManager->isTokenValid(new CsrfToken('invoice.status', $token))) { + $this->flashError('action.csrf.error'); + + return $this->redirectToRoute('admin_invoice_list'); + } + + $token = $csrfTokenManager->refreshToken('invoice.status'); + if ($status === Invoice::STATUS_PAID) { - $form = $this->createPaymentDateForm($invoice, $status); + $form = $this->createPaymentDateForm($invoice, $status, $token->getValue()); $form->handleRequest($request); if (!$form->isSubmitted() || !$form->isValid()) { @@ -212,13 +245,13 @@ final class InvoiceController extends AbstractController */ public function deleteInvoiceAction(Invoice $invoice, string $token, CsrfTokenManagerInterface $csrfTokenManager): Response { - if (!$csrfTokenManager->isTokenValid(new CsrfToken('invoice.delete', $token))) { + if (!$csrfTokenManager->isTokenValid(new CsrfToken('invoice.status', $token))) { $this->flashError('action.csrf.error'); return $this->redirectToRoute('admin_invoice_list'); } - $csrfTokenManager->refreshToken('invoice.delete'); + $csrfTokenManager->refreshToken('invoice.status'); try { $this->service->deleteInvoice($invoice); @@ -624,13 +657,13 @@ final class InvoiceController extends AbstractController ]); } - private function createPaymentDateForm(Invoice $invoice, string $status): FormInterface + private function createPaymentDateForm(Invoice $invoice, string $status, string $token): FormInterface { if (null === $invoice->getPaymentDate()) { $invoice->setPaymentDate($this->getDateTimeFactory()->createDateTime()); } - $url = $this->generateUrl('admin_invoice_status', ['id' => $invoice->getId(), 'status' => $status]); + $url = $this->generateUrl('admin_invoice_status', ['id' => $invoice->getId(), 'status' => $status, 'token' => $token]); return $this->createForm(InvoicePaymentDateForm::class, $invoice, [ 'action' => $url, diff --git a/src/EventSubscriber/Actions/InvoiceSubscriber.php b/src/EventSubscriber/Actions/InvoiceSubscriber.php index d277ce0d..8dfa02f3 100644 --- a/src/EventSubscriber/Actions/InvoiceSubscriber.php +++ b/src/EventSubscriber/Actions/InvoiceSubscriber.php @@ -31,15 +31,15 @@ class InvoiceSubscriber extends AbstractActionsSubscriber } if (!$invoice->isPending()) { - $event->addAction('invoice.pending', ['url' => $this->path('admin_invoice_status', ['id' => $invoice->getId(), 'status' => 'pending'])]); + $event->addAction('invoice.pending', ['url' => $this->path('admin_invoice_status', ['id' => $invoice->getId(), 'status' => 'pending', 'token' => $payload['token']])]); } else { - $event->addAction('invoice.paid', ['url' => $this->path('admin_invoice_status', ['id' => $invoice->getId(), 'status' => 'paid']), 'class' => 'modal-ajax-form']); + $event->addAction('invoice.paid', ['url' => $this->path('admin_invoice_status', ['id' => $invoice->getId(), 'status' => 'paid', 'token' => $payload['token']]), 'class' => 'modal-ajax-form']); } $allowDelete = $this->isGranted('delete_invoice'); if (!$invoice->isCanceled()) { $id = $allowDelete ? 'invoice.cancel' : 'trash'; - $event->addAction($id, ['url' => $this->path('admin_invoice_status', ['id' => $invoice->getId(), 'status' => 'canceled']), 'title' => 'invoice.cancel', 'translation_domain' => 'actions']); + $event->addAction($id, ['url' => $this->path('admin_invoice_status', ['id' => $invoice->getId(), 'status' => 'canceled', 'token' => $payload['token']]), 'title' => 'invoice.cancel', 'translation_domain' => 'actions']); } $event->addDivider(); diff --git a/templates/invoice/actions.html.twig b/templates/invoice/actions.html.twig index d65e29e0..672c211c 100644 --- a/templates/invoice/actions.html.twig +++ b/templates/invoice/actions.html.twig @@ -6,7 +6,7 @@ {% macro invoice(invoice, view) %} {% import "macros/widgets.html.twig" as widgets %} - {% set event = actions(app.user, 'invoice', view, {'invoice': invoice, 'token': csrf_token('invoice.delete')}) %} + {% set event = actions(app.user, 'invoice', view, {'invoice': invoice, 'token': csrf_token('invoice.status')}) %} {{ widgets.table_actions(event.actions) }} {% endmacro %} diff --git a/templates/invoice/index.html.twig b/templates/invoice/index.html.twig index 53ac2019..816bb93b 100644 --- a/templates/invoice/index.html.twig +++ b/templates/invoice/index.html.twig @@ -218,7 +218,7 @@ const overwrites = {'customers[]': link.dataset['customer'], 'template': link.dataset['template']}; const uri = formPlugin.convertFormDataToQueryString(document.getElementById('{{ formId }}'), overwrites); - link.href = link.dataset['href'] + '?' + uri; + link.href = link.dataset['href'] + '?token={{ csrf_token('invoice.create') }}&' + uri; return true; } @@ -228,7 +228,7 @@ const formPlugin = kimai.getPlugin('form'); const uri = formPlugin.convertFormDataToQueryString(document.getElementById('{{ formId }}')); - link.href = '{{ path('invoice') }}?createInvoice=true&' + uri; + link.href = '{{ path('invoice') }}?token={{ csrf_token('invoice.create') }}&createInvoice=true&' + uri; return true; } diff --git a/templates/macros/search.html.twig b/templates/macros/search.html.twig index 520250c5..f616f1a8 100644 --- a/templates/macros/search.html.twig +++ b/templates/macros/search.html.twig @@ -40,6 +40,7 @@ {% endmacro %} {% macro searchButton(form) %} +
@@ -56,7 +57,7 @@ {% else %} diff --git a/tests/Controller/InvoiceControllerTest.php b/tests/Controller/InvoiceControllerTest.php index cf913b4b..80f8d156 100644 --- a/tests/Controller/InvoiceControllerTest.php +++ b/tests/Controller/InvoiceControllerTest.php @@ -190,7 +190,9 @@ class InvoiceControllerTest extends ControllerBaseTest 'markAsExported' => 1, ]; - $action = '/invoice/save-invoice/1/' . $template->getId() . '?' . http_build_query($urlParams); + $token = self::$container->get('security.csrf.token_manager')->getToken('invoice.create'); + + $action = '/invoice/save-invoice/1/' . $template->getId() . '?token=' . $token->getValue() . '&' . http_build_query($urlParams); $this->request($client, $action); $this->assertIsRedirect($client); $this->assertRedirectUrl($client, '/invoice/show?id=', false); @@ -286,9 +288,11 @@ class InvoiceControllerTest extends ControllerBaseTest // but the datatable with all timesheets $this->assertDataTableRowCount($client, 'datatable_invoice', 20); + $token = self::$container->get('security.csrf.token_manager')->getToken('invoice.create'); + $form = $client->getCrawler()->filter('#invoice-print-form')->form(); $node = $form->getFormNode(); - $node->setAttribute('action', $this->createUrl('/invoice/?createInvoice=true')); + $node->setAttribute('action', $this->createUrl('/invoice/?createInvoice=true&token=' . $token->getValue())); $node->setAttribute('method', 'GET'); $client->submit($form, [ 'template' => $template->getId(), @@ -317,17 +321,20 @@ class InvoiceControllerTest extends ControllerBaseTest self::assertInstanceOf(BinaryFileResponse::class, $response); self::assertFileExists($response->getFile()); - $this->request($client, '/invoice/change-status/' . $id . '/pending'); + $token = self::$container->get('security.csrf.token_manager')->getToken('invoice.status'); + $this->request($client, '/invoice/change-status/' . $id . '/pending/' . $token->getValue()); $this->assertIsRedirect($client, '/invoice/show'); $client->followRedirect(); $this->assertTrue($client->getResponse()->isSuccessful()); - $this->request($client, '/invoice/change-status/' . $id . '/paid'); + $token = self::$container->get('security.csrf.token_manager')->getToken('invoice.status'); + $this->request($client, '/invoice/change-status/' . $id . '/paid/' . $token->getValue()); $this->assertTrue($client->getResponse()->isSuccessful()); + $token = self::$container->get('security.csrf.token_manager')->getToken('invoice.status'); $this->assertHasValidationError( $client, - '/invoice/change-status/' . $id . '/paid', + '/invoice/change-status/' . $id . '/paid/' . $token->getValue(), 'form[name=invoice_payment_date_form]', [ 'invoice_payment_date_form' => [ @@ -350,7 +357,8 @@ class InvoiceControllerTest extends ControllerBaseTest $client->followRedirect(); $this->assertTrue($client->getResponse()->isSuccessful()); - $this->request($client, '/invoice/change-status/' . $id . '/new'); + $token = self::$container->get('security.csrf.token_manager')->getToken('invoice.status'); + $this->request($client, '/invoice/change-status/' . $id . '/new/' . $token->getValue()); $this->assertIsRedirect($client, '/invoice/show'); $client->followRedirect(); $this->assertTrue($client->getResponse()->isSuccessful());