Commit acc31be4 authored by catch's avatar catch

Issue #1798296 by damiankloip, Xano, David_Rothstein, Crell, dawehner:...

Issue #1798296 by damiankloip, Xano, David_Rothstein, Crell, dawehner: Integrate CSRF link token directly into routing system.
parent f35ebac3
......@@ -264,7 +264,7 @@ services:
- [setFinalMatcher, ['@router.matcher.final_matcher']]
url_generator:
class: Drupal\Core\Routing\UrlGenerator
arguments: ['@router.route_provider', '@path_processor_manager', '@config.factory', '@settings']
arguments: ['@router.route_provider', '@path_processor_manager', '@route_processor_manager', '@config.factory', '@settings']
calls:
- [setRequest, ['@?request']]
- [setContext, ['@?router.request_context']]
......@@ -449,6 +449,11 @@ services:
arguments: ['@controller_resolver']
tags:
- { name: access_check }
access_check.csrf:
class: Drupal\Core\Access\CsrfAccessCheck
tags:
- { name: access_check }
arguments: ['@csrf_token']
maintenance_mode_subscriber:
class: Drupal\Core\EventSubscriber\MaintenanceModeSubscriber
tags:
......@@ -507,6 +512,8 @@ services:
tags:
- { name: event_subscriber }
arguments: [['@exception_controller', execute]]
route_processor_manager:
class: Drupal\Core\RouteProcessor\RouteProcessorManager
path_processor_manager:
class: Drupal\Core\PathProcessor\PathProcessorManager
path_processor_decode:
......@@ -525,6 +532,11 @@ services:
- { name: path_processor_inbound, priority: 100 }
- { name: path_processor_outbound, priority: 300 }
arguments: ['@path.alias_manager']
route_processer_csrf:
class: Drupal\Core\Access\RouteProcessorCsrf
tags:
- { name: route_processor_outbound }
arguments: ['@csrf_token']
transliteration:
class: Drupal\Core\Transliteration\PHPTransliteration
flood:
......
<?php
/**
* @file
* Contains \Drupal\Core\Access\CsrfAccessCheck.
*/
namespace Drupal\Core\Access;
use Drupal\Core\Access\CsrfTokenGenerator;
use Drupal\Core\Session\AccountInterface;
use Symfony\Component\Routing\Route;
use Symfony\Component\HttpFoundation\Request;
/**
* Allows access to routes to be controlled by a '_csrf_token' parameter.
*
* To use this check, add a "token" GET parameter to URLs of which the value is
* a token generated by \Drupal::csrfToken()->get() using the same value as the
* "_csrf_token" parameter in the route.
*/
class CsrfAccessCheck implements StaticAccessCheckInterface {
/**
* The CSRF token generator.
*
* @var \Drupal\Core\Access\CsrfTokenGenerator
*/
protected $csrfToken;
/**
* Constructs a CsrfAccessCheck object.
*
* @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token
* The CSRF token generator.
*/
function __construct(CsrfTokenGenerator $csrf_token) {
$this->csrfToken = $csrf_token;
}
/**
* {@inheritdoc}
*/
public function appliesTo() {
return array('_csrf_token');
}
/**
* {@inheritdoc}
*/
public function access(Route $route, Request $request, AccountInterface $account) {
// If this is the controller request, check CSRF access as normal.
if ($request->attributes->get('_controller_request')) {
return $this->csrfToken->validate($request->query->get('token'), $route->getRequirement('_csrf_token')) ? static::ALLOW : static::KILL;
}
// Otherwise, this could be another requested access check that we don't
// want to check CSRF tokens on.
$conjunction = $route->getOption('_access_mode') ?: 'ANY';
// Return ALLOW if all access checks are needed.
if ($conjunction == 'ALL') {
return static::ALLOW;
}
// Return DENY otherwise, as another access checker should grant access
// for the route.
else {
return static::DENY;
}
}
}
<?php
/**
* @file
* Contains \Drupal\Core\Access\RouteProcessorCsrf.
*/
namespace Drupal\Core\Access;
use Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface;
use Drupal\Core\Access\CsrfTokenGenerator;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\Route;
/**
* Processes the inbound path by resolving it to the front page if empty.
*/
class RouteProcessorCsrf implements OutboundRouteProcessorInterface {
/**
* The CSRF token generator.
*
* @var \Drupal\Core\Access\CsrfTokenGenerator
*/
protected $csrfToken;
/**
* Constructs a RouteProcessorCsrf object.
*
* @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token
* The CSRF token generator.
*/
function __construct(CsrfTokenGenerator $csrf_token) {
$this->csrfToken = $csrf_token;
}
/**
* {@inheritdoc}
*/
public function processOutbound(Route $route, array &$parameters) {
if ($route->hasRequirement('_csrf_token')) {
// Adding this to the parameters means it will get merged into the query
// string when the route is compiled.
$parameters['token'] = $this->csrfToken->get($route->getRequirement('_csrf_token'));
}
}
}
......@@ -14,6 +14,7 @@
use Drupal\Core\DependencyInjection\Compiler\RegisterKernelListenersPass;
use Drupal\Core\DependencyInjection\Compiler\RegisterAccessChecksPass;
use Drupal\Core\DependencyInjection\Compiler\RegisterPathProcessorsPass;
use Drupal\Core\DependencyInjection\Compiler\RegisterRouteProcessorsPass;
use Drupal\Core\DependencyInjection\Compiler\RegisterRouteFiltersPass;
use Drupal\Core\DependencyInjection\Compiler\RegisterRouteEnhancersPass;
use Drupal\Core\DependencyInjection\Compiler\RegisterParamConvertersPass;
......@@ -63,6 +64,7 @@ public function register(ContainerBuilder $container) {
$container->addCompilerPass(new RegisterServicesForDestructionPass());
// Add the compiler pass that will process the tagged services.
$container->addCompilerPass(new RegisterPathProcessorsPass());
$container->addCompilerPass(new RegisterRouteProcessorsPass());
$container->addCompilerPass(new ListCacheBinsPass());
// Add the compiler pass for appending string translators.
$container->addCompilerPass(new RegisterStringTranslatorsPass());
......
<?php
/**
* @file
* Contains \Drupal\Core\DependencyInjection\Compiler\RegisterRouteProcessorsPass.
*/
namespace Drupal\Core\DependencyInjection\Compiler;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
/**
* Adds services to the route_processor_manager service.
*/
class RegisterRouteProcessorsPass implements CompilerPassInterface {
/**
* {@inheritdoc}
*/
public function process(ContainerBuilder $container) {
if (!$container->hasDefinition('route_processor_manager')) {
return;
}
$manager = $container->getDefinition('route_processor_manager');
// Add outbound route processors.
foreach ($container->findTaggedServiceIds('route_processor_outbound') as $id => $attributes) {
$priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0;
$manager->addMethodCall('addOutbound', array(new Reference($id), $priority));
}
}
}
......@@ -11,6 +11,7 @@
use Drupal\Core\Session\AccountInterface;
use Symfony\Cmf\Component\Routing\RouteObjectInterface;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
......@@ -59,13 +60,29 @@ public function __construct(AccessManager $access_manager, AccountInterface $cur
*/
public function onKernelRequestAccessCheck(GetResponseEvent $event) {
$request = $event->getRequest();
// The controller is being handled by the HTTP kernel, so add an attribute
// to tell us this is the controller request.
$request->attributes->set('_controller_request', TRUE);
if (!$request->attributes->has(RouteObjectInterface::ROUTE_OBJECT)) {
// If no Route is available it is likely a static resource and access is
// handled elsewhere.
return;
}
$access = $this->accessManager->check($request->attributes->get(RouteObjectInterface::ROUTE_OBJECT), $request, $this->currentUser);
// Wrap this in a try/catch to ensure the '_controller_request' attribute
// can always be removed.
try {
$access = $this->accessManager->check($request->attributes->get(RouteObjectInterface::ROUTE_OBJECT), $request, $this->currentUser);
}
catch (\Exception $e) {
$request->attributes->remove('_controller_request');
throw $e;
}
$request->attributes->remove('_controller_request');
if (!$access) {
throw new AccessDeniedHttpException();
}
......
......@@ -8,6 +8,7 @@
namespace Drupal\Core\PathProcessor;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\Route;
/**
* Defines an interface for classes that process the outbound path.
......
<?php
/**
* @file
* Contains \Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface.
*/
namespace Drupal\Core\RouteProcessor;
use Symfony\Component\Routing\Route;
/**
* Defines an interface for classes that process the outbound route.
*/
interface OutboundRouteProcessorInterface {
/**
* Processes the outbound route.
*
* @param \Symfony\Component\Routing\Route $route
* The outbound route to process.
*
* @param array $parameters
* An array of parameters to be passed to the route compiler. Passed by
* reference.
*
* @return
* The processed path.
*/
public function processOutbound(Route $route, array &$parameters);
}
<?php
/**
* @file
* Contains \Drupal\Tests\Core\RouteProcessor\RouteProcessorManager.
*/
namespace Drupal\Core\RouteProcessor;
use Symfony\Component\Routing\Route;
/**
* Route processor manager.
*
* Holds an array of route processor objects and uses them to sequentially
* process an outbound route, in order of processor priority.
*/
class RouteProcessorManager implements OutboundRouteProcessorInterface {
/**
* Holds the array of outbound processors to cycle through.
*
* @var array
* An array whose keys are priorities and whose values are arrays of path
* processor objects.
*/
protected $outboundProcessors = array();
/**
* Holds the array of outbound processors, sorted by priority.
*
* @var array
* An array of path processor objects.
*/
protected $sortedOutbound = array();
/**
* Adds an outbound processor object to the $outboundProcessors property.
*
* @param \Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface $processor
* The processor object to add.
*
* @param int $priority
* The priority of the processor being added.
*/
public function addOutbound(OutboundRouteProcessorInterface $processor, $priority = 0) {
$this->outboundProcessors[$priority][] = $processor;
$this->sortedOutbound = array();
}
/**
* {@inheritdoc}
*/
public function processOutbound(Route $route, array &$parameters) {
$processors = $this->getOutbound();
foreach ($processors as $processor) {
$processor->processOutbound($route, $parameters);
}
}
/**
* Returns the sorted array of outbound processors.
*
* @return array
* An array of processor objects.
*/
protected function getOutbound() {
if (empty($this->sortedOutbound)) {
$this->sortedOutbound = $this->sortProcessors();
}
return $this->sortedOutbound;
}
/**
* Sorts the processors according to priority.
*/
protected function sortProcessors() {
$sorted = array();
krsort($this->outboundProcessors);
foreach ($this->outboundProcessors as $processors) {
$sorted = array_merge($sorted, $processors);
}
return $sorted;
}
}
......@@ -9,6 +9,7 @@
use Symfony\Component\Routing\RequestContext;
use Symfony\Component\Routing\Exception\RouteNotFoundException;
use Symfony\Component\Routing\Route;
/**
* No-op implementation of a Url Generator, needed for backward compatibility.
......
......@@ -19,6 +19,7 @@
use Drupal\Component\Utility\Url;
use Drupal\Core\Config\ConfigFactory;
use Drupal\Core\PathProcessor\OutboundPathProcessorInterface;
use Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface;
/**
* Generates URLs from route names and parameters.
......@@ -39,6 +40,13 @@ class UrlGenerator extends ProviderBasedGenerator implements UrlGeneratorInterfa
*/
protected $pathProcessor;
/**
* The route processor.
*
* @var \Drupal\Tests\Core\RouteProcessor\OutboundRouteProcessorInterface
*/
protected $routeProcessor;
/**
* The base path to use for urls.
*
......@@ -77,10 +85,11 @@ class UrlGenerator extends ProviderBasedGenerator implements UrlGeneratorInterfa
* @param \Symfony\Component\HttpKernel\Log\LoggerInterface $logger
* An optional logger for recording errors.
*/
public function __construct(RouteProviderInterface $provider, OutboundPathProcessorInterface $path_processor, ConfigFactory $config, Settings $settings, LoggerInterface $logger = NULL) {
public function __construct(RouteProviderInterface $provider, OutboundPathProcessorInterface $path_processor, OutboundRouteProcessorInterface $route_processor, ConfigFactory $config, Settings $settings, LoggerInterface $logger = NULL) {
parent::__construct($provider, $logger);
$this->pathProcessor = $path_processor;
$this->routeProcessor = $route_processor;
$this->mixedModeSessions = $settings->get('mixed_mode_sessions', FALSE);
$allowed_protocols = $config->get('system.filter')->get('protocols') ?: array('http', 'https');
Url::setAllowedProtocols($allowed_protocols);
......@@ -167,10 +176,13 @@ public function generate($name, $parameters = array(), $absolute = FALSE) {
public function generateFromRoute($name, $parameters = array(), $options = array()) {
$absolute = !empty($options['absolute']);
$route = $this->getRoute($name);
$this->processRoute($route, $parameters);
// Symfony adds any parameters that are not path slugs as query strings.
if (isset($options['query']) && is_array($options['query'])) {
$parameters = (array) $parameters + $options['query'];
}
$path = $this->getInternalPathFromRoute($route, $parameters);
$path = $this->processPath($path, $options);
$fragment = '';
......@@ -179,6 +191,7 @@ public function generateFromRoute($name, $parameters = array(), $options = array
$fragment = '#' . $fragment;
}
}
$base_url = $this->context->getBaseUrl();
if (!$absolute || !$host = $this->context->getHost()) {
return $base_url . $path . $fragment;
......@@ -335,6 +348,19 @@ protected function processPath($path, &$options = array()) {
return $path;
}
/**
* Passes the route to the processor manager for altering before complation.
*
* @param \Symfony\Component\Routing\Route $route
* The route object to process.
*
* @param array $parameters
* An array of parameters to be passed to the route compiler.
*/
protected function processRoute(SymfonyRoute $route, array &$parameters) {
$this->routeProcessor->processOutbound($route, $parameters);
}
/**
* Returns whether or not the url generator has been initialized.
*
......
......@@ -33,9 +33,8 @@ class ShortcutSetController extends ControllerBase {
* @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
*/
public function addShortcutLinkInline(ShortcutSetInterface $shortcut_set, Request $request) {
$token = $request->query->get('token');
$link = $request->query->get('link');
if (isset($token) && drupal_valid_token($token, 'shortcut-add-link') && shortcut_valid_link($link)) {
if (shortcut_valid_link($link)) {
$item = menu_get_item($link);
$title = ($item && $item['title']) ? $item['title'] : $link;
$link = array(
......
......@@ -451,14 +451,15 @@ function shortcut_preprocess_page(&$variables) {
$link_mode = isset($mlid) ? "remove" : "add";
if ($link_mode == "add") {
$query['token'] = drupal_get_token('shortcut-add-link');
$link_text = shortcut_set_switch_access() ? t('Add to %shortcut_set shortcuts', array('%shortcut_set' => $shortcut_set->label())) : t('Add to shortcuts');
$link_path = 'admin/config/user-interface/shortcut/manage/' . $shortcut_set->id() . '/add-link-inline';
$route_name = 'shortcut.link_add_inline';
$route_parameters = array('shortcut_set' => $shortcut_set->id());
}
else {
$query['mlid'] = $mlid;
$link_text = shortcut_set_switch_access() ? t('Remove from %shortcut_set shortcuts', array('%shortcut_set' => $shortcut_set->label())) : t('Remove from shortcuts');
$link_path = 'admin/config/user-interface/shortcut/link/' . $mlid . '/delete';
$route_name = 'shortcut.link_delete';
$route_parameters = array('menu_link' => $mlid);
}
if (theme_get_setting('shortcut_module_link')) {
......@@ -471,7 +472,8 @@ function shortcut_preprocess_page(&$variables) {
'#prefix' => '<div class="add-or-remove-shortcuts ' . $link_mode . '-shortcut">',
'#type' => 'link',
'#title' => '<span class="icon">'. t('Add or remove shortcut') .'</span><span class="text">' . $link_text . '</span>',
'#href' => $link_path,
'#route_name' => $route_name,
'#route_parameters' => $route_parameters,
'#options' => array('query' => $query, 'html' => TRUE),
'#suffix' => '</div>',
);
......
......@@ -40,6 +40,7 @@ shortcut.link_add_inline:
_controller: 'Drupal\shortcut\Controller\ShortcutSetController::addShortcutLinkInline'
requirements:
_entity_access: 'shortcut_set.update'
_csrf_token: 'shortcut-add-link'
shortcut.set_customize:
path: '/admin/config/user-interface/shortcut/manage/{shortcut_set}'
......
<?php
/**
* @file
* Contains \Drupal\Tests\Core\Access\CsrfAccessCheckTest.
*/
namespace Drupal\Tests\Core\Access;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\Route;
use Drupal\Core\Access\CsrfAccessCheck;
use Drupal\Core\Access\AccessInterface;
use Drupal\Tests\UnitTestCase;
/**
* Tests the CSRF access checker..
*
* @group Drupal
* @group Access
*
* @see \Drupal\Core\Access\CsrfAccessCheck
*/
class CsrfAccessCheckTest extends UnitTestCase {
/**
* The mock CSRF token generator.
*
* @var \Drupal\Core\Access\CsrfTokenGenerator|\PHPUnit_Framework_MockObject_MockObject
*/
protected $csrfToken;
/**
* The access checker.
*
* @var \Drupal\Core\Access\CsrfAccessCheck
*/
protected $accessCheck;
/**
* The mock user account.
*
* @var \Drupal\Core\Session\AccountInterface|\PHPUnit_Framework_MockObject_MockObject
*/
protected $account;
public static function getInfo() {
return array(
'name' => 'CSRF access checker',
'description' => 'Tests CSRF access control for routes.',
'group' => 'Routing',
);
}
public function setUp() {
$this->csrfToken = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator')
->disableOriginalConstructor()
->getMock();
$this->account = $this->getMock('Drupal\Core\Session\AccountInterface');
$this->accessCheck = new CsrfAccessCheck($this->csrfToken);
}
/**
* Tests CsrfAccessCheck::appliesTo().
*/
public function testAppliesTo() {
$this->assertEquals($this->accessCheck->appliesTo(), array('_csrf_token'), 'Access checker returned the expected appliesTo() array.');
}
/**
* Tests the access() method with a valid token.
*/
public function testAccessTokenPass() {
$this->csrfToken->expects($this->once())
->method('validate')
->with('test_query', 'test')
->will($this->returnValue(TRUE));
$route = new Route('', array(), array('_csrf_token' => 'test'));
$request = new Request(array(
'token' => 'test_query',
));
// Set the _controller_request flag so tokens are validated.
$request->attributes->set('_controller_request', TRUE);
$this->assertSame(AccessInterface::ALLOW, $this->accessCheck->access($route, $request, $this->account));
}
/**
* Tests the access() method with an invalid token.
*/
public function testAccessTokenFail() {
$this->csrfToken->expects($this->once())
->method('validate')
->with('test_query', 'test')
->will($this->returnValue(FALSE));
$route = new Route('', array(), array('_csrf_token' => 'test'));
$request = new Request(array(
'token' => 'test_query',
));
// Set the _controller_request flag so tokens are validated.
$request->attributes->set('_controller_request', TRUE);
$this->assertSame(AccessInterface::KILL, $this->accessCheck->access($route, $request, $this->account));
}
/**
* Tests the access() method with no _controller_request attribute set.
*
* This will default to the 'ANY' access conjuction.
*/
public function testAccessTokenMissAny() {
$this->csrfToken->expects($this->never())
->method('validate');
$route = new Route('', array(), array('_csrf_token' => 'test'));
$request = new Request(array(
'token' => 'test_query',
));
$this->assertSame(AccessInterface::DENY, $this->accessCheck->access($route, $request, $this->account));
}
/**
* Tests the access() method with no _controller_request attribute set.
*
* This will use the 'ALL' access conjuction.
*/
public function testAccessTokenMissAll() {
$this->csrfToken->expects($this->never())
->method('validate');
$route = new Route('', array(), array('_csrf_token' => 'test'), array('_access_mode' => 'ALL'));
$request = new Request(array(
'token' => 'test_query',
));
$this->assertSame(AccessInterface::ALLOW, $this->accessCheck->access($route, $request, $this->account));
}
}
<?php
/**
* @file
* Contains \Drupal\Tests\Core\Access\RouteProcessorCsrfTest.
*/
namespace Drupal\Tests\Core\Access;
use Drupal\Tests\UnitTestCase;
use Drupal\Core\Access\RouteProcessorCsrf;
use Symfony\Component\Routing\Route;
/**
* Tests the CSRF route processor.
*
* @see Drupal
* @see Routing
*
* @see \Drupal\Core\Access\RouteProcessorCsrf
*/
class RouteProcessorCsrfTest extends UnitTestCase {
/**
* The mock CSRF token generator.
*
* @var \Drupal\Core\Access\CsrfTokenGenerator|\PHPUnit_Framework_MockObject_MockObject
*/
protected $csrfToken;
/**
* The route processor.
*
* @var \Drupal\Core\Access\RouteProcessorCsrf
*/
protected $processor;
public static function getInfo() {
return array(
'name' => 'CSRF access checker',
'description' => 'Tests CSRF access control for routes.',
'group' => 'Routing',
);
}
public function setUp() {
$this->csrfToken = $this->getMockBuilder('Drupal\Core\Access\CsrfTokenGenerator')
->disableOriginalConstructor()