Issue #2109303 by damiankloip, ParisLiakos: Convert CSRF checks in controllers...

Issue #2109303 by damiankloip, ParisLiakos: Convert CSRF checks in controllers to the routing system.

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);

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'

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(

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}'

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)) {

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();
 

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}'

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);
 

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();
 

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'

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();
 

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']);
       }
     }
 

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:

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'));