From ada9c9da11bcb945df5972ac2c4f5d146a7067ab Mon Sep 17 00:00:00 2001
From: webchick <webchick@24967.no-reply.drupal.org>
Date: Tue, 3 Dec 2013 20:27:53 -0800
Subject: [PATCH] Issue #2109303 by damiankloip, ParisLiakos: Convert CSRF
 checks in controllers to the routing system.

---
 .../Drupal/Core/Access/RouteProcessorCsrf.php |  2 +-
 .../modules/aggregator/aggregator.routing.yml |  1 +
 .../Controller/AggregatorController.php       | 13 +--------
 core/modules/comment/comment.routing.yml      |  1 +
 .../lib/Drupal/comment/CommentViewBuilder.php |  4 +--
 .../comment/Controller/CommentController.php  | 28 ++-----------------
 core/modules/shortcut/shortcut.routing.yml    |  2 +-
 .../system/Controller/ThemeController.php     |  6 ++--
 core/modules/system/system.admin.inc          | 12 ++++----
 core/modules/system/system.routing.yml        |  3 ++
 .../views_ui/Controller/ViewsUIController.php |  7 -----
 .../Drupal/views_ui/ViewListController.php    |  5 +++-
 core/modules/views_ui/views_ui.routing.yml    |  1 +
 .../Core/Access/RouteProcessorCsrfTest.php    |  7 +++--
 14 files changed, 28 insertions(+), 64 deletions(-)

diff --git a/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php
index f9c12d89efee..183c2a24b45b 100644
--- a/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php
+++ b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php
@@ -39,7 +39,7 @@ function __construct(CsrfTokenGenerator $csrf_token) {
    */
   public function processOutbound(Route $route, array &$parameters) {
     if ($route->hasRequirement('_csrf_token')) {
-      $path = $route->getPath();
+      $path = ltrim($route->getPath(), '/');
       // Replace the path parameters with values from the parameters array.
       foreach ($parameters as $param => $value) {
         $path = str_replace("{{$param}}", $value, $path);
diff --git a/core/modules/aggregator/aggregator.routing.yml b/core/modules/aggregator/aggregator.routing.yml
index bfcf7b3a9d04..2fc29c21ce82 100644
--- a/core/modules/aggregator/aggregator.routing.yml
+++ b/core/modules/aggregator/aggregator.routing.yml
@@ -61,6 +61,7 @@ aggregator.feed_refresh:
     _title: 'Update items'
   requirements:
     _permission: 'administer news feeds'
+    _csrf_token: 'TRUE'
 
 aggregator.opml_add:
   path: '/admin/config/services/aggregator/add/opml'
diff --git a/core/modules/aggregator/lib/Drupal/aggregator/Controller/AggregatorController.php b/core/modules/aggregator/lib/Drupal/aggregator/Controller/AggregatorController.php
index 0a98fd037399..8640c560a4dd 100644
--- a/core/modules/aggregator/lib/Drupal/aggregator/Controller/AggregatorController.php
+++ b/core/modules/aggregator/lib/Drupal/aggregator/Controller/AggregatorController.php
@@ -142,8 +142,6 @@ protected function buildPageList(array $items, $feed_source = '') {
    *
    * @param \Drupal\aggregator\FeedInterface $aggregator_feed
    *   An object describing the feed to be refreshed.
-   * @param \Symfony\Component\HttpFoundation\Request $request
-   *   The current request object containing the search string.
    *
    * @return \Symfony\Component\HttpFoundation\RedirectResponse
    *   A redirection to the admin overview page.
@@ -151,15 +149,7 @@ protected function buildPageList(array $items, $feed_source = '') {
    * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
    *   If the query token is missing or invalid.
    */
-  public function feedRefresh(FeedInterface $aggregator_feed, Request $request) {
-    // @todo CSRF tokens are validated in page callbacks rather than access
-    //   callbacks, because access callbacks are also invoked during menu link
-    //   generation. Add token support to routing: http://drupal.org/node/755584.
-    $token = $request->query->get('token');
-    if (!isset($token) || !drupal_valid_token($token, 'aggregator/update/' . $aggregator_feed->id())) {
-      throw new AccessDeniedHttpException();
-    }
-
+  public function feedRefresh(FeedInterface $aggregator_feed) {
     // @todo after https://drupal.org/node/1972246 find a new place for it.
     aggregator_refresh($aggregator_feed);
     return $this->redirect('aggregator.admin_overview');
@@ -202,7 +192,6 @@ public function adminOverview() {
         'title' => $this->t('Update items'),
         'route_name' => 'aggregator.feed_refresh',
         'route_parameters' => array('aggregator_feed' => $feed->fid),
-        'query' => array('token' => drupal_get_token("aggregator/update/$feed->fid")),
       );
       $row[] = array(
         'data' => array(
diff --git a/core/modules/comment/comment.routing.yml b/core/modules/comment/comment.routing.yml
index fcb8bab7feeb..dcb3baa3a180 100644
--- a/core/modules/comment/comment.routing.yml
+++ b/core/modules/comment/comment.routing.yml
@@ -32,6 +32,7 @@ comment.approve:
     entity_type: 'comment'
   requirements:
     _entity_access: 'comment.approve'
+    _csrf_token: 'TRUE'
 
 comment.permalink:
   path: '/comment/{comment}'
diff --git a/core/modules/comment/lib/Drupal/comment/CommentViewBuilder.php b/core/modules/comment/lib/Drupal/comment/CommentViewBuilder.php
index b38460b5630b..230bd49c0151 100644
--- a/core/modules/comment/lib/Drupal/comment/CommentViewBuilder.php
+++ b/core/modules/comment/lib/Drupal/comment/CommentViewBuilder.php
@@ -234,9 +234,9 @@ protected static function buildLinks(CommentInterface $entity, EntityInterface $
       if ($entity->status->value == COMMENT_NOT_PUBLISHED && $entity->access('approve')) {
         $links['comment-approve'] = array(
           'title' => t('Approve'),
-          'href' => "comment/{$entity->id()}/approve",
+          'route_name' => 'comment.approve',
+          'route_parameters' => array('comment' => $entity->id()),
           'html' => TRUE,
-          'query' => array('token' => $container->get('csrf_token')->get("comment/{$entity->id()}/approve")),
         );
       }
       if (empty($links)) {
diff --git a/core/modules/comment/lib/Drupal/comment/Controller/CommentController.php b/core/modules/comment/lib/Drupal/comment/Controller/CommentController.php
index 2f13edb67654..ccb3a4aee3f5 100644
--- a/core/modules/comment/lib/Drupal/comment/Controller/CommentController.php
+++ b/core/modules/comment/lib/Drupal/comment/Controller/CommentController.php
@@ -10,7 +10,6 @@
 use Drupal\comment\CommentInterface;
 use Drupal\comment\CommentManagerInterface;
 use Drupal\field\FieldInfo;
-use Drupal\Core\Access\CsrfTokenGenerator;
 use Drupal\Core\Controller\ControllerBase;
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
@@ -37,13 +36,6 @@ class CommentController extends ControllerBase implements ContainerInjectionInte
    */
   protected $httpKernel;
 
-  /**
-   * The CSRF token manager service.
-   *
-   * @var \Drupal\Core\Access\CsrfTokenGenerator
-   */
-  protected $csrfToken;
-
   /**
    * The current user service.
    *
@@ -70,8 +62,6 @@ class CommentController extends ControllerBase implements ContainerInjectionInte
    *
    * @param \Symfony\Component\HttpKernel\HttpKernelInterface $httpKernel
    *   HTTP kernel to handle requests.
-   * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token
-   *   The CSRF token manager service.
    * @param \Drupal\Core\Session\AccountInterface $current_user
    *   The current user service.
    * @param \Drupal\field\FieldInfo $field_info
@@ -79,9 +69,8 @@ class CommentController extends ControllerBase implements ContainerInjectionInte
    * @param \Drupal\comment\CommentManagerInterface $comment_manager
    *   The comment manager service.
    */
-  public function __construct(HttpKernelInterface $httpKernel, CsrfTokenGenerator $csrf_token, AccountInterface $current_user, FieldInfo $field_info, CommentManagerInterface $comment_manager) {
+  public function __construct(HttpKernelInterface $httpKernel, AccountInterface $current_user, FieldInfo $field_info, CommentManagerInterface $comment_manager) {
     $this->httpKernel = $httpKernel;
-    $this->csrfToken = $csrf_token;
     $this->currentUser = $current_user;
     $this->fieldInfo = $field_info;
     $this->commentManager = $comment_manager;
@@ -93,7 +82,6 @@ public function __construct(HttpKernelInterface $httpKernel, CsrfTokenGenerator
   public static function create(ContainerInterface $container) {
     return new static(
       $container->get('http_kernel'),
-      $container->get('csrf_token'),
       $container->get('current_user'),
       $container->get('field.info'),
       $container->get('comment.manager')
@@ -103,24 +91,12 @@ public static function create(ContainerInterface $container) {
   /**
    * Publishes the specified comment.
    *
-   * @param \Symfony\Component\HttpFoundation\Request $request
-   *   The request object.
    * @param \Drupal\comment\CommentInterface $comment
    *   A comment entity.
    *
-   * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
    * @return \Symfony\Component\HttpFoundation\RedirectResponse.
    */
-  public function commentApprove(Request $request, CommentInterface $comment) {
-    // @todo CRSF tokens are validated in the content controller until it gets
-    //   moved to the access layer:
-    //   Integrate CSRF link token directly into routing system:
-    //   https://drupal.org/node/1798296.
-    $token = $request->query->get('token');
-    if (!isset($token) || !$this->csrfToken->validate($token, 'comment/' . $comment->id() . '/approve')) {
-      throw new AccessDeniedHttpException();
-    }
-
+  public function commentApprove(CommentInterface $comment) {
     $comment->status->value = COMMENT_PUBLISHED;
     $comment->save();
 
diff --git a/core/modules/shortcut/shortcut.routing.yml b/core/modules/shortcut/shortcut.routing.yml
index abbe1e6eda1b..be220e7c5ce3 100644
--- a/core/modules/shortcut/shortcut.routing.yml
+++ b/core/modules/shortcut/shortcut.routing.yml
@@ -44,7 +44,7 @@ shortcut.link_add_inline:
     _controller: 'Drupal\shortcut\Controller\ShortcutSetController::addShortcutLinkInline'
   requirements:
     _entity_access: 'shortcut_set.update'
-    _csrf_token: 'shortcut-add-link'
+    _csrf_token: 'TRUE'
 
 shortcut.set_customize:
   path: '/admin/config/user-interface/shortcut/manage/{shortcut_set}'
diff --git a/core/modules/system/lib/Drupal/system/Controller/ThemeController.php b/core/modules/system/lib/Drupal/system/Controller/ThemeController.php
index d46e4cfbacf6..852d2b665b61 100644
--- a/core/modules/system/lib/Drupal/system/Controller/ThemeController.php
+++ b/core/modules/system/lib/Drupal/system/Controller/ThemeController.php
@@ -60,9 +60,8 @@ public static function create(ContainerInterface $container) {
    */
   public function disable(Request $request) {
     $theme = $request->get('theme');
-    $token = $request->get('token');
 
-    if (isset($theme) && isset($token) && drupal_valid_token($token, 'system-theme-operation-link')) {
+    if (isset($theme)) {
       // Get current list of themes.
       $themes = list_themes();
 
@@ -102,9 +101,8 @@ public function disable(Request $request) {
    */
   public function enable(Request $request) {
     $theme = $request->get('theme');
-    $token = $request->get('token');
 
-    if (isset($theme) && isset($token) && drupal_valid_token($token, 'system-theme-operation-link')) {
+    if (isset($theme)) {
       // Get current list of themes.
       $themes = list_themes(TRUE);
 
diff --git a/core/modules/system/system.admin.inc b/core/modules/system/system.admin.inc
index 29237fdba97c..3af30c83ad9a 100644
--- a/core/modules/system/system.admin.inc
+++ b/core/modules/system/system.admin.inc
@@ -64,7 +64,6 @@ function system_themes_page() {
       // Confirm that the theme engine is available.
       $theme->incompatible_engine = (isset($theme->info['engine']) && !isset($theme->owner));
     }
-    $query['token'] = drupal_get_token('system-theme-operation-link');
     $theme->operations = array();
     if (!empty($theme->status) || !$theme->incompatible_core && !$theme->incompatible_php && !$theme->incompatible_base && !$theme->incompatible_engine) {
       // Create the operations links.
@@ -81,14 +80,14 @@ function system_themes_page() {
           if ($theme->name != $admin_theme) {
             $theme->operations[] = array(
               'title' => t('Disable'),
-              'href' => 'admin/appearance/disable',
+              'route_name' => 'system.theme_disable',
               'query' => $query,
               'attributes' => array('title' => t('Disable !theme theme', array('!theme' => $theme->info['name']))),
             );
           }
           $theme->operations[] = array(
             'title' => t('Set default'),
-            'href' => 'admin/appearance/default',
+            'route_name' => 'system.theme_set_default',
             'query' => $query,
             'attributes' => array('title' => t('Set !theme as default theme', array('!theme' => $theme->info['name']))),
           );
@@ -98,13 +97,13 @@ function system_themes_page() {
       else {
         $theme->operations[] = array(
           'title' => t('Enable'),
-          'href' => 'admin/appearance/enable',
+          'route_name' => 'system.theme_enable',
           'query' => $query,
           'attributes' => array('title' => t('Enable !theme theme', array('!theme' => $theme->info['name']))),
         );
         $theme->operations[] = array(
           'title' => t('Enable and set default'),
-          'href' => 'admin/appearance/default',
+          'route_name' => 'system.theme_set_default',
           'query' => $query,
           'attributes' => array('title' => t('Enable !theme as default theme', array('!theme' => $theme->info['name']))),
         );
@@ -191,8 +190,7 @@ function system_themes_admin_form_submit($form, &$form_state) {
 function system_theme_default() {
   $request = \Drupal::request();
   $theme = $request->get('theme');
-  $token = $request->get('token');
-  if (!empty($theme) && !empty($token) && drupal_valid_token($token, 'system-theme-operation-link')) {
+  if (!empty($theme)) {
     // Get current list of themes.
     $themes = list_themes();
 
diff --git a/core/modules/system/system.routing.yml b/core/modules/system/system.routing.yml
index 5bf659df148b..c1a7a377f652 100644
--- a/core/modules/system/system.routing.yml
+++ b/core/modules/system/system.routing.yml
@@ -257,6 +257,7 @@ system.theme_disable:
     _controller: 'Drupal\system\Controller\ThemeController::disable'
   requirements:
     _permission: 'administer themes'
+    _csrf_token: 'TRUE'
 
 system.theme_enable:
   path: '/admin/appearance/enable'
@@ -264,6 +265,7 @@ system.theme_enable:
     _controller: 'Drupal\system\Controller\ThemeController::enable'
   requirements:
     _permission: 'administer themes'
+    _csrf_token: 'TRUE'
 
 system.status:
   path: '/admin/reports/status'
@@ -318,6 +320,7 @@ system.theme_set_default:
     _content: '\Drupal\system\Controller\SystemController::themeSetDefault'
   requirements:
     _permission: 'administer themes'
+    _csrf_token: 'TRUE'
 
 system.theme_settings:
   path: '/admin/appearance/settings'
diff --git a/core/modules/views_ui/lib/Drupal/views_ui/Controller/ViewsUIController.php b/core/modules/views_ui/lib/Drupal/views_ui/Controller/ViewsUIController.php
index df736b57589e..90c1d58f117a 100644
--- a/core/modules/views_ui/lib/Drupal/views_ui/Controller/ViewsUIController.php
+++ b/core/modules/views_ui/lib/Drupal/views_ui/Controller/ViewsUIController.php
@@ -19,7 +19,6 @@
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Drupal\Core\Ajax\AjaxResponse;
 use Drupal\Core\Ajax\ReplaceCommand;
-use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
 use Drupal\Core\Routing\UrlGeneratorInterface;
 use Drupal\Core\Utility\LinkGeneratorInterface;
 
@@ -181,14 +180,8 @@ public function reportPlugins() {
    *   Either returns a rebuilt listing page as an AJAX response, or redirects
    *   back to the listing page.
    *
-   * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
    */
   public function ajaxOperation(ViewStorageInterface $view, $op, Request $request) {
-    if (!drupal_valid_token($request->query->get('token'), $op)) {
-      // Throw an access denied exception if the token is invalid or missing.
-      throw new AccessDeniedHttpException();
-    }
-
     // Perform the operation.
     $view->$op()->save();
 
diff --git a/core/modules/views_ui/lib/Drupal/views_ui/ViewListController.php b/core/modules/views_ui/lib/Drupal/views_ui/ViewListController.php
index ef88707e7091..0c389088b615 100644
--- a/core/modules/views_ui/lib/Drupal/views_ui/ViewListController.php
+++ b/core/modules/views_ui/lib/Drupal/views_ui/ViewListController.php
@@ -155,7 +155,10 @@ public function getOperations(EntityInterface $entity) {
     foreach (array('enable', 'disable') as $op) {
       if (isset($operations[$op])) {
         $operations[$op]['ajax'] = TRUE;
-        $operations[$op]['query']['token'] = drupal_get_token($op);
+        $operations[$op]['route_name'] = 'views_ui.operation';
+        $operations[$op]['route_parameters'] = array('view' => $entity->id(), 'op' => $op);
+        // @todo Remove this when entity links use route_names.
+        unset($operations[$op]['href']);
       }
     }
 
diff --git a/core/modules/views_ui/views_ui.routing.yml b/core/modules/views_ui/views_ui.routing.yml
index a064b24d8cc6..3d3be396dbfe 100644
--- a/core/modules/views_ui/views_ui.routing.yml
+++ b/core/modules/views_ui/views_ui.routing.yml
@@ -52,6 +52,7 @@ views_ui.operation:
     _controller: '\Drupal\views_ui\Controller\ViewsUIController::ajaxOperation'
   requirements:
     _permission: 'administer views'
+    _csrf_token: 'TRUE'
     op: 'enable|disable'
 
 views_ui.clone:
diff --git a/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php b/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php
index b57d74b69962..3cf0803efb4b 100644
--- a/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php
+++ b/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php
@@ -72,7 +72,8 @@ public function testProcessOutboundNoRequirement() {
   public function testProcessOutbound() {
     $this->csrfToken->expects($this->once())
       ->method('get')
-      ->with('/test-path')
+      // The leading '/' will be stripped from the path.
+      ->with('test-path')
       ->will($this->returnValue('test_token'));
 
     $route = new Route('/test-path', array(), array('_csrf_token' => 'TRUE'));
@@ -90,7 +91,7 @@ public function testProcessOutbound() {
   public function testProcessOutboundDynamicOne() {
     $this->csrfToken->expects($this->once())
       ->method('get')
-      ->with('/test-path/100')
+      ->with('test-path/100')
       ->will($this->returnValue('test_token'));
 
     $route = new Route('/test-path/{slug}', array(), array('_csrf_token' => 'TRUE'));
@@ -105,7 +106,7 @@ public function testProcessOutboundDynamicOne() {
   public function testProcessOutboundDynamicTwo() {
     $this->csrfToken->expects($this->once())
       ->method('get')
-      ->with('/100/test-path/test')
+      ->with('100/test-path/test')
       ->will($this->returnValue('test_token'));
 
     $route = new Route('{slug_1}/test-path/{slug_2}', array(), array('_csrf_token' => 'TRUE'));
-- 
GitLab