diff --git a/core/lib/Drupal/Core/Access/AccessManager.php b/core/lib/Drupal/Core/Access/AccessManager.php index 33d3e9d..533b131 100644 --- a/core/lib/Drupal/Core/Access/AccessManager.php +++ b/core/lib/Drupal/Core/Access/AccessManager.php @@ -120,11 +120,16 @@ public function check(Route $route) { $this->loadCheck($service_id); } - $access = $this->checks[$service_id]->access($route, $this->request); - if ($access === FALSE) { + $service_access = $this->checks[$service_id]->access($route, $this->request); + if ($service_access === FALSE) { // A check has denied access, no need to continue checking. + $access = FALSE; break; } + elseif ($service_access === TRUE) { + // A check has explicitly granted access, so we need to remember that. + $access = TRUE; + } } // Access has been denied or not explicily approved. diff --git a/core/lib/Drupal/Core/Access/DefaultAccessCheck.php b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php index 80afdef..31c6da3 100644 --- a/core/lib/Drupal/Core/Access/DefaultAccessCheck.php +++ b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php @@ -26,6 +26,6 @@ public function applies(Route $route) { * Implements AccessCheckInterface::access(). */ public function access(Route $route, Request $request) { - return $route->getRequirement('_access'); + return (bool) $route->getRequirement('_access'); } } diff --git a/core/modules/rest/lib/Drupal/rest/Access/CSRFAccessCheck.php b/core/modules/rest/lib/Drupal/rest/Access/CSRFAccessCheck.php new file mode 100644 index 0000000..584be0c --- /dev/null +++ b/core/modules/rest/lib/Drupal/rest/Access/CSRFAccessCheck.php @@ -0,0 +1,66 @@ +getRequirements(); + if (array_key_exists('_access_rest_csrf', $requirements)) { + if (isset($requirements['_method'])) { + // There could be more than one method requirement separated with '|'. + $methods = explode('|', $requirements['_method']); + // CSRF protection only applies to write operations, so we can filter + // out any routes that require reading methods only. + $write_methods = array_diff($methods, array('GET', 'HEAD', 'OPTIONS', 'TRACE')); + if (empty($write_methods)) { + return FALSE; + } + } + // No method requirement given, so we run this access check to be on the + // safe side. + return TRUE; + } + return FALSE; + } + + /** + * Implements AccessCheckInterface::access(). + */ + public function access(Route $route, Request $request) { + $method = $request->getMethod(); + $cookie = $request->cookies->get(session_name(), FALSE); + // This check only applies if + // 1. this is a write operation + // 2. the user was successfully authenticated and + // 3. the request comes with a session cookie. + if (!in_array($method, array('GET', 'HEAD', 'OPTIONS', 'TRACE')) + && user_is_logged_in() + && $cookie + ) { + $csrf_token = $request->headers->get('X-CSRF-Token'); + if (!drupal_valid_token($csrf_token, 'rest')) { + return FALSE; + } + } + // As we do not perform any authorization here we always return NULL to + // indicate that other access checkers should decide if the request is + // legit. + return NULL; + } +} diff --git a/core/modules/rest/lib/Drupal/rest/EventSubscriber/RouteSubscriber.php b/core/modules/rest/lib/Drupal/rest/EventSubscriber/RouteSubscriber.php index 47e9420..d4711e0 100644 --- a/core/modules/rest/lib/Drupal/rest/EventSubscriber/RouteSubscriber.php +++ b/core/modules/rest/lib/Drupal/rest/EventSubscriber/RouteSubscriber.php @@ -60,9 +60,8 @@ public function dynamicRoutes(RouteBuildEvent $event) { foreach ($enabled as $key => $resource) { $plugin = $this->manager->getInstance(array('id' => $key)); - // @todo Switch to ->addCollection() once http://drupal.org/node/1819018 is resolved. foreach ($plugin->routes() as $name => $route) { - $route->setRequirement('_access', 'TRUE'); + $route->setRequirement('_access_rest_csrf', 'TRUE'); $collection->add("rest.$name", $route); } } diff --git a/core/modules/rest/lib/Drupal/rest/RequestHandler.php b/core/modules/rest/lib/Drupal/rest/RequestHandler.php index ba8ae75..fe74eec 100644 --- a/core/modules/rest/lib/Drupal/rest/RequestHandler.php +++ b/core/modules/rest/lib/Drupal/rest/RequestHandler.php @@ -30,10 +30,6 @@ class RequestHandler extends ContainerAware { * The response object. */ public function handle(Request $request, $id = NULL) { - if (!$this->csrfValidation($request)) { - return new Response('CSRF validation failed.', 403, array('Content-Type' => 'text/plain')); - } - $plugin = $request->attributes->get(RouteObjectInterface::ROUTE_OBJECT)->getDefault('_plugin'); $method = strtolower($request->getMethod()); @@ -74,35 +70,6 @@ public function handle(Request $request, $id = NULL) { } /** - * Validates a request to prevent CSRF vulnerabilities. - * - * This method checks the X-CSRF-Token header on write operations (POST, PUT, - * DELETE etc.) if it has been authenticated with session cookies. - * - * @param \Symfony\Component\HttpFoundation\Request $request - * The request object. - * - * @return bool - * TRUE if the request was successfully verified, FALSE otherwise. - */ - protected function csrfValidation(Request $request) { - $method = $request->getMethod(); - $cookie = $request->cookies->get(session_name(), FALSE); - // This check only applies if - // 1. this is a write operation - // 2. the user was successfully authenticated and - // 3. the request comes with a session cookie. - if (!in_array($method, array('GET', 'HEAD', 'OPTIONS', 'TRACE')) - && user_is_logged_in() - && $cookie - ) { - $csrf_token = $request->headers->get('X-CSRF-Token'); - return isset($csrf_token) && drupal_valid_token($csrf_token, 'rest'); - } - return TRUE; - } - - /** * Generates a CSRF protecting session token. * * @return \Symfony\Component\HttpFoundation\Response diff --git a/core/modules/rest/lib/Drupal/rest/RestBundle.php b/core/modules/rest/lib/Drupal/rest/RestBundle.php index 67d3c59..9d9360f 100644 --- a/core/modules/rest/lib/Drupal/rest/RestBundle.php +++ b/core/modules/rest/lib/Drupal/rest/RestBundle.php @@ -28,5 +28,8 @@ public function build(ContainerBuilder $container) { ->addArgument(new Reference('plugin.manager.rest')) ->addArgument(new Reference('config.factory')) ->addTag('event_subscriber'); + + $container->register('access_check.rest.csrf', 'Drupal\rest\Access\CSRFAccessCheck') + ->addTag('access_check'); } }