Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #3002 from acrobat/add-csrf-protection
[AllBundles] Add csrf protection to multiple routes
  • Loading branch information
acrobat committed Oct 26, 2021
2 parents e0d68ec + 33c341d commit 4f5612a
Show file tree
Hide file tree
Showing 22 changed files with 311 additions and 35 deletions.
31 changes: 31 additions & 0 deletions UPGRADE-5.10.md
@@ -1,6 +1,37 @@
UPGRADE FROM 5.9 to 5.10
========================

General
-------

### CSRF protection

CSRF protection was added to multiple routes in the cms. No passing a csrf token to those routes
is deprecated and will be required in 6.0. Below is a list of controller actions that will require
a csrf token. Check the specific twig templates or the deprecation messages for the specific csrf token id that needs to
be used.

* `Kunstmaan\AdminListBundle\Controller\AdminListController::doDeleteAction`
* `Kunstmaan\AdminBundle\Controller\ExceptionController::resolveAllAction`
* `Kunstmaan\AdminBundle\Controller\ExceptionController::toggleResolveAction`
* `Kunstmaan\MediaBundle\Controller\FolderController::deleteAction`
* `Kunstmaan\MediaBundle\Controller\MediaController::deleteAction`
* `Kunstmaan\FormBundle\Controller\FormSubmissionsController::deleteAction`
* `Kunstmaan\UserManagementBundle\Controller\GroupsController::deleteAction`
* `Kunstmaan\NodeBundle\Controller\NodeAdminController::recopyFromOtherLanguageAction`
* `Kunstmaan\NodeBundle\Controller\NodeAdminController::deleteAction`
* `Kunstmaan\NodeBundle\Controller\NodeAdminController::duplicateAction`
* `Kunstmaan\NodeBundle\Controller\NodeAdminController::duplicateWithChildrenAction`
* `Kunstmaan\UserManagementBundle\Controller\RolesController::deleteAction`
* `Kunstmaan\TranslatorBundle\Controller\TranslatorController::deleteAction`

Together with the CSRF token some of those routes will only be available to post requests in 6.0

* `Kunstmaan\AdminBundle\Controller\ExceptionController::resolveAllAction`
* `Kunstmaan\AdminBundle\Controller\ExceptionController::toggleResolveAction`
* `Kunstmaan\MediaBundle\Controller\FolderController::deleteAction`
* `Kunstmaan\MediaBundle\Controller\MediaController::deleteAction`

AdminBundle
------------

Expand Down
38 changes: 35 additions & 3 deletions src/Kunstmaan/AdminBundle/Controller/ExceptionController.php
Expand Up @@ -36,16 +36,32 @@ public function indexAction(Request $request)
}

/**
* @Route("/resolve_all", name="kunstmaanadminbundle_admin_exception_resolve_all")
* @Route("/resolve_all", name="kunstmaanadminbundle_admin_exception_resolve_all", methods={"GET", "POST"})
*
* @return RedirectResponse
*
* @throws \Doctrine\ORM\NonUniqueResultException
* @throws \Doctrine\ORM\NoResultException
* @throws \InvalidArgumentException
*/
public function resolveAllAction()
public function resolveAllAction(Request $request)
{
// NEXT_MAJOR: remove check and change methods property in route annotation
if ($request->isMethod(Request::METHOD_GET)) {
@trigger_error(sprintf('Calling the action "%s" with a GET request is deprecated since KunstmaanAdminBundle 5.10 and will only allow a POST request in KunstmaanAdminBundle 6.0.', __METHOD__), E_USER_DEPRECATED);
}

$csrfId = 'exception-resolve_all';
$hasToken = $request->request->has('token');
// NEXT_MAJOR remove hasToken check and make csrf token required
if (!$hasToken) {
@trigger_error(sprintf('Not passing as csrf token with id "%s" in field "token" is deprecated in KunstmaanAdminBundle 5.10 and will be required in KunstmaanAdminBundle 6.0. If you override the adminlist delete action template make sure to post a csrf token.', $csrfId), E_USER_DEPRECATED);
}

if ($hasToken && !$this->isCsrfTokenValid($csrfId, $request->request->get('token'))) {
return new RedirectResponse($this->generateUrl('kunstmaanadminbundle_admin_exception'));
}

$this->getEntityManager()->getRepository(Exception::class)->markAllAsResolved();

$indexUrl = $this->getAdminListConfigurator()->getIndexUrl();
Expand All @@ -59,7 +75,7 @@ public function resolveAllAction()
}

/**
* @Route("/toggle_resolve/{id}", name="kunstmaanadminbundle_admin_exception_toggle_resolve")
* @Route("/toggle_resolve/{id}", name="kunstmaanadminbundle_admin_exception_toggle_resolve", methods={"GET", "POST"})
*
* @return RedirectResponse
*
Expand All @@ -69,6 +85,22 @@ public function resolveAllAction()
*/
public function toggleResolveAction(Request $request, Exception $model)
{
// NEXT_MAJOR: remove check and change methods property in route annotation
if ($request->isMethod(Request::METHOD_GET)) {
@trigger_error(sprintf('Calling the action "%s" with a GET request is deprecated since KunstmaanAdminBundle 5.10 and will only allow a POST request in KunstmaanAdminBundle 6.0.', __METHOD__), E_USER_DEPRECATED);
}

$csrfId = 'exception-resolve-item';
$hasToken = $request->request->has('token');
// NEXT_MAJOR remove hasToken check and make csrf token required
if (!$hasToken) {
@trigger_error(sprintf('Not passing as csrf token with id "%s" in field "token" is deprecated in KunstmaanAdminBundle 5.10 and will be required in KunstmaanAdminBundle 6.0. If you override the adminlist delete action template make sure to post a csrf token.', $csrfId), E_USER_DEPRECATED);
}

if ($hasToken && !$this->isCsrfTokenValid($csrfId, $request->request->get('token'))) {
return new RedirectResponse($this->generateUrl('kunstmaanadminbundle_admin_exception'));
}

/* @var EntityManager $em */
$em = $this->getEntityManager();

Expand Down
@@ -1,8 +1,12 @@
{% set action = itemAction.getUrlFor(item) %}
<a href="{{ path(action['path'], action['params']) }}">
{% if item.isResolved() %}
<i class="fa fa-toggle-on" aria-hidden="true"></i> {{ "settings.exceptions.unresolved" | trans }}
{% else %}
<i class="fa fa-toggle-off" aria-hidden="true"></i> {{ "settings.exceptions.resolved" | trans }}
{% endif %}
</a>
<form method="post" action="{{ path(action['path'], action['params']) }}">
<input type="hidden" name="token" value="{{ csrf_token('exception-resolve-item') }}"/>

<button type="submit" class="btn-link">
{% if item.isResolved() %}
<i class="fa fa-toggle-on" aria-hidden="true"></i> {{ "settings.exceptions.unresolved" | trans }}
{% else %}
<i class="fa fa-toggle-off" aria-hidden="true"></i> {{ "settings.exceptions.resolved" | trans }}
{% endif %}
</button>
</form>
@@ -1,3 +1,7 @@
<a href="{{ path(action.getUrl()["path"], action.getUrl()["params"]) }}" class="btn btn-default btn--raise-on-hover">
{{ action.getLabel() | trans }}
</a>
<form method="post" action="{{ path(action.getUrl()["path"], action.getUrl()["params"]) }}">
<input type="hidden" name="token" value="{{ csrf_token('exception-resolve_all') }}"/>

<button type="submit" class="btn btn-default btn--raise-on-hover">
{{ action.getLabel() | trans }}
</button>
</form>
Expand Up @@ -20,6 +20,7 @@
use Kunstmaan\AdminListBundle\Service\ExportService;
use Kunstmaan\NodeBundle\Entity\HasNodeInterface;
use Kunstmaan\NodeBundle\Entity\NodeTranslation;
use Kunstmaan\UtilitiesBundle\Helper\SlugifierInterface;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\LegacyEventDispatcherProxy;
Expand Down Expand Up @@ -353,6 +354,22 @@ protected function doViewAction(AbstractAdminListConfigurator $configurator, $en
*/
protected function doDeleteAction(AbstractAdminListConfigurator $configurator, $entityId, Request $request)
{
/** @var SlugifierInterface $slugifier */
$slugifier = $this->container->get('kunstmaan_utilities.slugifier');
$csrfId = 'delete-' . $slugifier->slugify($configurator->getEntityName());

$hasToken = $request->request->has('token');
// NEXT_MAJOR remove hasToken check and make csrf token required
if (!$hasToken) {
@trigger_error(sprintf('Not passing as csrf token with id "%s" in field "token" is deprecated in KunstmaanAdminListBundle 5.10 and will be required in KunstmaanAdminListBundle 6.0. If you override the adminlist delete action template make sure to post a csrf token.', $csrfId), E_USER_DEPRECATED);
}

if ($hasToken && !$this->isCsrfTokenValid($csrfId, $request->request->get('token'))) {
$indexUrl = $configurator->getIndexUrl();

return new RedirectResponse($this->generateUrl($indexUrl['path'], $indexUrl['params'] ?? []));
}

/* @var $em EntityManager */
$em = $this->getEntityManager();
$helper = $em->getRepository($configurator->getRepositoryName())->findOneById($entityId);
Expand Down Expand Up @@ -583,6 +600,7 @@ public static function getSubscribedServices(): array
'kunstmaan_entity.admin_entity.entity_version_lock_service' => EntityVersionLockService::class,
'translator' => interface_exists(TranslatorInterface::class) ? TranslatorInterface::class : LegacyTranslatorInterface::class,
'event_dispatcher' => EventDispatcherInterface::class,
'kunstmaan_utilities.slugifier' => SlugifierInterface::class,
]);
}
}
16 changes: 16 additions & 0 deletions src/Kunstmaan/AdminListBundle/Controller/AdminListController.php
Expand Up @@ -17,6 +17,7 @@
use Kunstmaan\AdminListBundle\Service\EntityVersionLockService;
use Kunstmaan\NodeBundle\Entity\HasNodeInterface;
use Kunstmaan\NodeBundle\Entity\NodeTranslation;
use Kunstmaan\UtilitiesBundle\Helper\SlugifierInterface;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\EventDispatcher\LegacyEventDispatcherProxy;
use Symfony\Component\HttpFoundation\RedirectResponse;
Expand Down Expand Up @@ -350,6 +351,21 @@ protected function doViewAction(AbstractAdminListConfigurator $configurator, $en
*/
protected function doDeleteAction(AbstractAdminListConfigurator $configurator, $entityId, Request $request)
{
/** @var SlugifierInterface $slugifier */
$slugifier = $this->container->get('kunstmaan_utilities.slugifier');
$csrfId = 'delete-' . $slugifier->slugify($configurator->getEntityName());

$hasToken = $request->request->has('token');
if (!$hasToken) {
@trigger_error(sprintf('Not passing as csrf token with id "%s" in field "token" is deprecated in KunstmaanAdminListBundle 5.10 and will be required in KunstmaanAdminListBundle 6.0. If you override the adminlist delete action template make sure to post a csrf token.', $csrfId), E_USER_DEPRECATED);
}

if ($hasToken && !$this->isCsrfTokenValid($csrfId, $request->request->get('token'))) {
$indexUrl = $configurator->getIndexUrl();

return new RedirectResponse($this->generateUrl($indexUrl['path'], $indexUrl['params'] ?? []));
}

/* @var $em EntityManager */
$em = $this->getEntityManager();
$helper = $em->getRepository($configurator->getRepositoryName())->findOneById($entityId);
Expand Down
Expand Up @@ -12,6 +12,8 @@
</h3>
</div>
<form action="{{ path(adminlist.getDeleteUrlFor(item)["path"], adminlist.getDeleteUrlFor(item)[("params")] ) }}" method="POST">
<input type="hidden" name="token" value="{{ csrf_token('delete-'~adminlist.configurator.entityName|slugify) }}"/>

<!-- Body -->
<div class="modal-body">
{{ 'form.deletesure' | trans }}
Expand Down
1 change: 1 addition & 0 deletions src/Kunstmaan/AdminListBundle/composer.json
Expand Up @@ -16,6 +16,7 @@
"php": "^7.2|^8.0",
"box/spout": "^2.5",
"kunstmaan/admin-bundle": "^5.9|^6.0",
"kunstmaan/utilities-bundle": "^5.10|^6.0",
"pagerfanta/core": "^2.4",
"pagerfanta/doctrine-orm-adapter": "^2.4",
"pagerfanta/twig": "^2.4"
Expand Down
20 changes: 18 additions & 2 deletions src/Kunstmaan/FormBundle/Controller/FormSubmissionsController.php
Expand Up @@ -14,6 +14,7 @@
use Kunstmaan\FormBundle\Entity\FormSubmissionField;
use Kunstmaan\NodeBundle\Entity\Node;
use Kunstmaan\NodeBundle\Entity\NodeTranslation;
use Kunstmaan\UtilitiesBundle\Helper\SlugifierInterface;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\RedirectResponse;
Expand Down Expand Up @@ -145,8 +146,6 @@ public function exportAction($nodeTranslationId, $_format)
* @param int $id
*
* @return RedirectResponse
*
* @throws AccessDeniedException
*/
public function deleteAction(Request $request, $id)
{
Expand All @@ -156,6 +155,23 @@ public function deleteAction(Request $request, $id)
$node = $em->getRepository(Node::class)->find($submission->getNode());
$nt = $node->getNodeTranslation($request->getLocale());

$configurator = new FormSubmissionAdminListConfigurator($em, $nt, $this->getParameter('kunstmaan_form.deletable_formsubmissions'));

/** @var SlugifierInterface $slugifier */
$slugifier = $this->container->get('kunstmaan_utilities.slugifier');
$csrfId = 'delete-' . $slugifier->slugify($configurator->getEntityName());

$hasToken = $request->request->has('token');
if (!$hasToken) {
@trigger_error(sprintf('Not passing as csrf token with id "%s" in field "token" is deprecated in KunstmaanFormBundle 5.10 and will be required in KunstmaanFormBundle 6.0. If you override the adminlist delete action template make sure to post a csrf token.', $csrfId), E_USER_DEPRECATED);
}

if ($hasToken && !$this->isCsrfTokenValid($csrfId, $request->request->get('token'))) {
$indexUrl = $configurator->getIndexUrl();

return new RedirectResponse($this->generateUrl($indexUrl['path'], $indexUrl['params'] ?? []));
}

$this->denyAccessUnlessGranted(PermissionMap::PERMISSION_DELETE, $node);

$url = $this->get('router')->generate(
Expand Down
18 changes: 17 additions & 1 deletion src/Kunstmaan/MediaBundle/Controller/FolderController.php
Expand Up @@ -98,12 +98,28 @@ public function showAction(Request $request, $folderId)
/**
* @param int $folderId
*
* @Route("/delete/{folderId}", requirements={"folderId" = "\d+"}, name="KunstmaanMediaBundle_folder_delete")
* @Route("/delete/{folderId}", requirements={"folderId" = "\d+"}, name="KunstmaanMediaBundle_folder_delete", methods={"GET", "POST"})
*
* @return RedirectResponse
*/
public function deleteAction(Request $request, $folderId)
{
// NEXT_MAJOR: remove check and change methods property in route annotation
if ($request->isMethod(Request::METHOD_GET)) {
@trigger_error(sprintf('Calling the action "%s" with a GET request is deprecated since KunstmaanMediaBundle 5.10 and will only allow a POST request in KunstmaanMediaBundle 6.0.', __METHOD__), E_USER_DEPRECATED);
}

$csrfId = 'media-folder-delete';
$hasToken = $request->request->has('token');
// NEXT_MAJOR remove hasToken check and make csrf token required
if (!$hasToken) {
@trigger_error(sprintf('Not passing as csrf token with id "%s" in field "token" is deprecated in KunstmaanMediaBundle 5.10 and will be required in KunstmaanMediaBundle 6.0. If you override the adminlist delete action template make sure to post a csrf token.', $csrfId), E_USER_DEPRECATED);
}

if ($hasToken && !$this->isCsrfTokenValid($csrfId, $request->request->get('token'))) {
return new RedirectResponse($this->generateUrl(''));
}

/** @var EntityManager $em */
$em = $this->getDoctrine()->getManager();

Expand Down
18 changes: 17 additions & 1 deletion src/Kunstmaan/MediaBundle/Controller/MediaController.php
Expand Up @@ -77,12 +77,28 @@ public function showAction(Request $request, $mediaId)
/**
* @param int $mediaId
*
* @Route("/delete/{mediaId}", requirements={"mediaId" = "\d+"}, name="KunstmaanMediaBundle_media_delete")
* @Route("/delete/{mediaId}", requirements={"mediaId" = "\d+"}, name="KunstmaanMediaBundle_media_delete", methods={"GET", "POST"})
*
* @return RedirectResponse
*/
public function deleteAction(Request $request, $mediaId)
{
// NEXT_MAJOR: remove check and change methods property in route annotation
if ($request->isMethod(Request::METHOD_GET)) {
@trigger_error(sprintf('Calling the action "%s" with a GET request is deprecated since KunstmaanMediaBundle 5.10 and will only allow a POST request in KunstmaanMediaBundle 6.0.', __METHOD__), E_USER_DEPRECATED);
}

$csrfId = 'media-delete';
$hasToken = $request->request->has('token');
// NEXT_MAJOR remove hasToken check and make csrf token required
if (!$hasToken) {
@trigger_error(sprintf('Not passing as csrf token with id "%s" in field "token" is deprecated in KunstmaanMediaBundle 5.10 and will be required in KunstmaanMediaBundle 6.0. If you override the adminlist delete action template make sure to post a csrf token.', $csrfId), E_USER_DEPRECATED);
}

if ($hasToken && !$this->isCsrfTokenValid($csrfId, $request->request->get('token'))) {
return new RedirectResponse($this->generateUrl(''));
}

$em = $this->getDoctrine()->getManager();

/* @var Media $media */
Expand Down
Expand Up @@ -21,12 +21,16 @@

<!-- Footer -->
<div class="modal-footer">
<a href="{{ path('KunstmaanMediaBundle_folder_delete', { 'folderId' : folder.id , 'type' : type }) }}" class="js-add-app-loading btn btn-danger btn--raise-on-hover">
{{ 'form.delete' |trans }}
</a>
<button type="button" class="btn btn-default btn--raise-on-hover" data-dismiss="modal">
{{ 'form.cancel' |trans }}
</button>
<form action="{{ path('KunstmaanMediaBundle_folder_delete', { 'folderId' : folder.id , 'type' : type }) }}" method="post" novalidate="novalidate">
<input type="hidden" name="token" value="{{ csrf_token('media-folder-delete') }}"/>

<button type="submit" name="submit" class="btn btn-danger btn--raise-on-hover">
{{ 'form.delete' | trans }}
</button>
<button type="button" class="btn btn-default btn--raise-on-hover" data-dismiss="modal">
{{ 'form.cancel' |trans }}
</button>
</form>
</div>
</div>
</div>
Expand Down
Expand Up @@ -19,12 +19,16 @@

<!-- Footer -->
<div class="modal-footer">
<a href="{{ path('KunstmaanMediaBundle_media_delete', { 'mediaId': media.id }) }}" class="btn btn-danger btn--raise-on-hover">
{{ 'form.delete' | trans }}
</a>
<button class="btn btn-default btn--raise-on-hover" data-dismiss="modal">
{{ 'form.cancel' | trans }}
</button>
<form action="{{ path('KunstmaanMediaBundle_media_delete', { 'mediaId': media.id }) }}" method="post" novalidate="novalidate">
<input type="hidden" name="token" value="{{ csrf_token('media-delete') }}"/>

<button type="submit" name="submit" class="btn btn-danger btn--raise-on-hover">
{{ 'form.delete' | trans }}
</button>
<button class="btn btn-default btn--raise-on-hover" data-dismiss="modal">
{{ 'form.cancel' | trans }}
</button>
</form>
</div>
</div>
</div>
Expand Down

0 comments on commit 4f5612a

Please sign in to comment.