From 809b361f20b7666d9ec41614f3169976ebec6895 Mon Sep 17 00:00:00 2001 From: Dries Date: Fri, 16 May 2014 12:45:50 -0400 Subject: [PATCH] Issue #2222719 by effulgentsia, tim.plunkett, xjm, dawehner: Use parameter matching via reflection for access checks instead of pulling variables from request attributes. --- core/core.services.yml | 6 +- .../Core/Access/AccessArgumentsResolver.php | 131 +++++++++ .../AccessArgumentsResolverInterface.php | 39 +++ .../Drupal/Core/Access/AccessInterface.php | 3 - core/lib/Drupal/Core/Access/AccessManager.php | 77 +++++- .../Drupal/Core/Access/CsrfAccessCheck.php | 13 +- .../Drupal/Core/Access/CustomAccessCheck.php | 33 ++- .../Drupal/Core/Access/DefaultAccessCheck.php | 12 +- .../Compiler/RegisterAccessChecksPass.php | 6 +- .../Drupal/Core/Entity/EntityAccessCheck.php | 12 +- .../Core/Entity/EntityCreateAccessCheck.php | 12 +- .../Core/Routing/Access/AccessInterface.php | 19 +- .../Drupal/Core/Theme/ThemeAccessCheck.php | 19 +- .../Access/BookNodeIsRemovableAccessCheck.php | 20 +- .../ConfigTranslationOverviewAccess.php | 12 +- .../contact/Access/ContactPageAccess.php | 17 +- .../ContentTranslationManageAccessCheck.php | 33 ++- .../ContentTranslationOverviewAccess.php | 13 +- .../ContentTranslationRouteSubscriber.php | 1 - .../field_ui/Access/FormModeAccessCheck.php | 32 ++- .../field_ui/Access/ViewModeAccessCheck.php | 32 ++- .../Drupal/node/Access/NodeAddAccessCheck.php | 28 +- .../node/Access/NodeRevisionAccessCheck.php | 36 ++- .../Access/EditEntityAccessCheck.php | 24 +- .../Access/EditEntityFieldAccessCheck.php | 27 +- .../Access/EditEntityAccessCheckTest.php | 10 +- .../Access/EditEntityFieldAccessCheckTest.php | 36 ++- .../Drupal/rest/Access/CSRFAccessCheck.php | 12 +- core/modules/shortcut/shortcut.services.yml | 10 - .../shortcut/src/Access/LinkAccessCheck.php | 32 --- .../src/Access/ShortcutSetEditAccessCheck.php | 37 --- .../Access/ShortcutSetSwitchAccessCheck.php | 20 +- .../Drupal/system/Access/CronAccessCheck.php | 14 +- .../Access/DefinedTestAccessCheck.php | 12 +- .../router_test/Access/TestAccessCheck.php | 10 +- .../Access/ViewOwnTrackerAccessCheck.php | 18 +- .../Access/UpdateManagerAccessCheck.php | 10 +- .../Drupal/user/Access/LoginStatusCheck.php | 13 +- .../user/Access/PermissionAccessCheck.php | 14 +- .../user/Access/RegisterAccessCheck.php | 13 +- .../Drupal/user/Access/RoleAccessCheck.php | 13 +- .../lib/Drupal/views/ViewsAccessCheck.php | 15 +- .../Access/AccessArgumentsResolverTest.php | 254 ++++++++++++++++++ .../Tests/Core/Access/AccessManagerTest.php | 69 +++-- .../Core/Access/CustomAccessCheckTest.php | 30 ++- .../Tests/Core/Route/RoleAccessCheckTest.php | 10 +- 46 files changed, 966 insertions(+), 343 deletions(-) create mode 100644 core/lib/Drupal/Core/Access/AccessArgumentsResolver.php create mode 100644 core/lib/Drupal/Core/Access/AccessArgumentsResolverInterface.php delete mode 100644 core/modules/shortcut/src/Access/LinkAccessCheck.php delete mode 100644 core/modules/shortcut/src/Access/ShortcutSetEditAccessCheck.php create mode 100644 core/tests/Drupal/Tests/Core/Access/AccessArgumentsResolverTest.php diff --git a/core/core.services.yml b/core/core.services.yml index dc55af1cffe..93231810fe2 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -515,9 +515,11 @@ services: csrf_token: class: Drupal\Core\Access\CsrfTokenGenerator arguments: ['@private_key'] + access_arguments_resolver: + class: Drupal\Core\Access\AccessArgumentsResolver access_manager: class: Drupal\Core\Access\AccessManager - arguments: ['@router.route_provider', '@url_generator', '@paramconverter_manager'] + arguments: ['@router.route_provider', '@url_generator', '@paramconverter_manager', '@access_arguments_resolver'] calls: - [setContainer, ['@service_container']] - [setRequest, ['@?request']] @@ -552,7 +554,7 @@ services: - { name: access_check, applies_to: _access_theme } access_check.custom: class: Drupal\Core\Access\CustomAccessCheck - arguments: ['@controller_resolver'] + arguments: ['@controller_resolver', '@access_arguments_resolver'] tags: - { name: access_check, applies_to: _custom_access } access_check.csrf: diff --git a/core/lib/Drupal/Core/Access/AccessArgumentsResolver.php b/core/lib/Drupal/Core/Access/AccessArgumentsResolver.php new file mode 100644 index 00000000000..4c61fac3005 --- /dev/null +++ b/core/lib/Drupal/Core/Access/AccessArgumentsResolver.php @@ -0,0 +1,131 @@ +getReflector($callable)->getParameters() as $parameter) { + $arguments[] = $this->getArgument($parameter, $route, $request, $account); + } + return $arguments; + } + + /** + * Returns the argument value for a parameter. + * + * @param \ReflectionParameter $parameter + * The parameter of a callable to get the value for. + * @param \Symfony\Component\Routing\Route $route + * The access checked route. + * @param \Symfony\Component\HttpFoundation\Request $request + * The current request. + * @param \Drupal\Core\Session\AccountInterface $account + * The current user. + * + * @return mixed + * The value of the requested parameter value. + * + * @throws \RuntimeException + * Thrown when there is a missing parameter. + */ + protected function getArgument(\ReflectionParameter $parameter, Route $route, Request $request, AccountInterface $account) { + $upcasted_route_arguments = $request->attributes->all(); + $raw_route_arguments = isset($upcasted_route_arguments['_raw_variables']) ? $upcasted_route_arguments['_raw_variables']->all() : $upcasted_route_arguments; + $parameter_type_hint = $parameter->getClass(); + $parameter_name = $parameter->getName(); + + // @todo Remove this once AccessManager::checkNamedRoute() is fixed to not + // leak _raw_variables from the request being duplicated. + // @see https://drupal.org/node/2265939 + $raw_route_arguments += $upcasted_route_arguments; + + // If the route argument exists and is NULL, return it, regardless of + // parameter type hint. + if (!isset($upcasted_route_arguments[$parameter_name]) && array_key_exists($parameter_name, $upcasted_route_arguments)) { + return NULL; + } + + if ($parameter_type_hint) { + // If the argument exists and complies with the type hint, return it. + if (isset($upcasted_route_arguments[$parameter_name]) && is_object($upcasted_route_arguments[$parameter_name]) && $parameter_type_hint->isInstance($upcasted_route_arguments[$parameter_name])) { + return $upcasted_route_arguments[$parameter_name]; + } + // Otherwise, resolve $request, $route, and $account by type matching + // only. This way, the callable may rename them in case the route + // defines other parameters with these names. + foreach (array($request, $route, $account) as $special_argument) { + if ($parameter_type_hint->isInstance($special_argument)) { + return $special_argument; + } + } + } + elseif (isset($raw_route_arguments[$parameter_name])) { + return $raw_route_arguments[$parameter_name]; + } + + // If the callable provides a default value, use it. + if ($parameter->isDefaultValueAvailable()) { + return $parameter->getDefaultValue(); + } + + // Can't resolve it: call a method that throws an exception or can be + // overridden to do something else. + return $this->handleUnresolvedArgument($parameter); + } + + /** + * Returns a reflector for the access check callable. + * + * The access checker may be either a procedural function (in which case the + * callable is the function name) or a method (in which case the callable is + * an array of the object and method name). + * + * @param callable $callable + * The callable (either a function or a method). + * + * @return \ReflectionFunctionAbstract + * The ReflectionMethod or ReflectionFunction to introspect the callable. + */ + protected function getReflector(callable $callable) { + return is_array($callable) ? new \ReflectionMethod($callable[0], $callable[1]) : new \ReflectionFunction($callable); + } + + /** + * Handles unresolved arguments for getArgument(). + * + * Subclasses that override this method may return a default value + * instead of throwing an exception. + * + * @throws \RuntimeException + * Thrown when there is a missing parameter. + */ + protected function handleUnresolvedArgument(\ReflectionParameter $parameter) { + $class = $parameter->getDeclaringClass(); + $function = $parameter->getDeclaringFunction(); + if ($class && !$function->isClosure()) { + $function_name = $class->getName() . '::' . $function->getName(); + } + else { + $function_name = $function->getName(); + } + throw new \RuntimeException(sprintf('Access callable "%s" requires a value for the "$%s" argument.', $function_name, $parameter->getName())); + } + +} diff --git a/core/lib/Drupal/Core/Access/AccessArgumentsResolverInterface.php b/core/lib/Drupal/Core/Access/AccessArgumentsResolverInterface.php new file mode 100644 index 00000000000..ce0ecf8a750 --- /dev/null +++ b/core/lib/Drupal/Core/Access/AccessArgumentsResolverInterface.php @@ -0,0 +1,39 @@ +routeProvider = $route_provider; $this->urlGenerator = $url_generator; $this->paramConverterManager = $paramconverter_manager; + $this->argumentsResolver = $arguments_resolver; } /** @@ -121,12 +137,15 @@ class AccessManager implements ContainerAwareInterface { * * @param string $service_id * The ID of the service in the Container that provides a check. + * @param string $service_method + * The method to invoke on the service object for performing the check. * @param array $applies_checks * (optional) An array of route requirement keys the checker service applies * to. */ - public function addCheckService($service_id, array $applies_checks = array()) { + public function addCheckService($service_id, $service_method, array $applies_checks = array()) { $this->checkIds[] = $service_id; + $this->checkMethods[$service_id] = $service_method; foreach ($applies_checks as $applies_check) { $this->staticRequirementMap[$applies_check][] = $service_id; } @@ -267,11 +286,7 @@ class AccessManager implements ContainerAwareInterface { $this->loadCheck($service_id); } - $service_access = $this->checks[$service_id]->access($route, $request, $account); - - if (!in_array($service_access, array(AccessInterface::ALLOW, AccessInterface::DENY, AccessInterface::KILL), TRUE)) { - throw new AccessException("Access error in $service_id. Access services can only return AccessInterface::ALLOW, AccessInterface::DENY, or AccessInterface::KILL constants."); - } + $service_access = $this->performCheck($service_id, $route, $request, $account); if ($service_access === AccessInterface::ALLOW) { $access = TRUE; @@ -310,11 +325,7 @@ class AccessManager implements ContainerAwareInterface { $this->loadCheck($service_id); } - $service_access = $this->checks[$service_id]->access($route, $request, $account); - - if (!in_array($service_access, array(AccessInterface::ALLOW, AccessInterface::DENY, AccessInterface::KILL), TRUE)) { - throw new AccessException("Access error in $service_id. Access services can only return AccessInterface::ALLOW, AccessInterface::DENY, or AccessInterface::KILL constants."); - } + $service_access = $this->performCheck($service_id, $route, $request, $account); if ($service_access === AccessInterface::ALLOW) { $access = TRUE; @@ -327,11 +338,46 @@ class AccessManager implements ContainerAwareInterface { return $access; } + /** + * Performs the specified access check. + * + * @param string $service_id + * The access check service ID to use. + * @param \Symfony\Component\Routing\Route $route + * The route to check access to. + * @param \Symfony\Component\HttpFoundation\Request $request + * The incoming request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The current user. + * + * @throws \Drupal\Core\Access\AccessException + * Thrown when the access check returns an invalid value. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. + */ + protected function performCheck($service_id, $route, $request, $account) { + $callable = array($this->checks[$service_id], $this->checkMethods[$service_id]); + $arguments = $this->argumentsResolver->getArguments($callable, $route, $request, $account); + $service_access = call_user_func_array($callable, $arguments); + + if (!in_array($service_access, array(AccessInterface::ALLOW, AccessInterface::DENY, AccessInterface::KILL), TRUE)) { + throw new AccessException("Access error in $service_id. Access services can only return AccessInterface::ALLOW, AccessInterface::DENY, or AccessInterface::KILL constants."); + } + + return $service_access; + } + /** * Lazy-loads access check services. * * @param string $service_id * The service id of the access check service to load. + * + * @throws \InvalidArgumentException + * Thrown when the service hasn't been registered in addCheckService(). + * @throws \Drupal\Core\Access\AccessException + * Thrown when the service doesn't implement the required interface. */ protected function loadCheck($service_id) { if (!in_array($service_id, $this->checkIds)) { @@ -340,9 +386,12 @@ class AccessManager implements ContainerAwareInterface { $check = $this->container->get($service_id); - if (!($check instanceof RoutingAccessInterface)) { + if (!($check instanceof AccessInterface)) { throw new AccessException('All access checks must implement AccessInterface.'); } + if (!is_callable(array($check, $this->checkMethods[$service_id]))) { + throw new AccessException(sprintf('Access check method %s in service %s must be callable.', $this->checkMethods[$service_id], $service_id)); + } $this->checks[$service_id] = $check; } diff --git a/core/lib/Drupal/Core/Access/CsrfAccessCheck.php b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php index 9744d6c196d..1e8847d57a1 100644 --- a/core/lib/Drupal/Core/Access/CsrfAccessCheck.php +++ b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php @@ -7,7 +7,6 @@ namespace Drupal\Core\Access; -use Drupal\Core\Session\AccountInterface; use Drupal\Core\Routing\Access\AccessInterface as RoutingAccessInterface; use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; @@ -39,9 +38,17 @@ class CsrfAccessCheck implements RoutingAccessInterface { } /** - * {@inheritdoc} + * Checks access based on a CSRF token for the request. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Route $route, Request $request) { // 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'), $request->attributes->get('_system_path')) ? static::ALLOW : static::KILL; diff --git a/core/lib/Drupal/Core/Access/CustomAccessCheck.php b/core/lib/Drupal/Core/Access/CustomAccessCheck.php index c116e669ace..c128a5b435e 100644 --- a/core/lib/Drupal/Core/Access/CustomAccessCheck.php +++ b/core/lib/Drupal/Core/Access/CustomAccessCheck.php @@ -32,26 +32,43 @@ class CustomAccessCheck implements RoutingAccessInterface { */ protected $controllerResolver; + /** + * The arguments resolver. + * + * @var \Drupal\Core\Access\AccessArgumentsResolverInterface + */ + protected $argumentsResolver; + /** * Constructs a CustomAccessCheck instance. * * @param \Drupal\Core\Controller\ControllerResolverInterface $controller_resolver * The controller resolver. + * @param \Drupal\Core\Access\AccessArgumentsResolverInterface $arguments_resolver + * The arguments resolver. */ - public function __construct(ControllerResolverInterface $controller_resolver) { + public function __construct(ControllerResolverInterface $controller_resolver, AccessArgumentsResolverInterface $arguments_resolver) { $this->controllerResolver = $controller_resolver; + $this->argumentsResolver = $arguments_resolver; } /** - * {@inheritdoc} + * Checks access for the account and route using the custom access checker. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ public function access(Route $route, Request $request, AccountInterface $account) { - $access_controller = $route->getRequirement('_custom_access'); - - $controller = $this->controllerResolver->getControllerFromDefinition($access_controller); - $arguments = $this->controllerResolver->getArguments($request, $controller); - - return call_user_func_array($controller, $arguments); + $callable = $this->controllerResolver->getControllerFromDefinition($route->getRequirement('_custom_access')); + $arguments = $this->argumentsResolver->getArguments($callable, $route, $request, $account); + return call_user_func_array($callable, $arguments); } } diff --git a/core/lib/Drupal/Core/Access/DefaultAccessCheck.php b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php index dc260672576..485dad862cb 100644 --- a/core/lib/Drupal/Core/Access/DefaultAccessCheck.php +++ b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php @@ -7,10 +7,8 @@ namespace Drupal\Core\Access; -use Drupal\Core\Session\AccountInterface; use Drupal\Core\Routing\Access\AccessInterface as RoutingAccessInterface; use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; /** * Allows access to routes to be controlled by an '_access' boolean parameter. @@ -18,9 +16,15 @@ use Symfony\Component\HttpFoundation\Request; class DefaultAccessCheck implements RoutingAccessInterface { /** - * {@inheritdoc} + * Checks access to the route based on the _access parameter. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Route $route) { if ($route->getRequirement('_access') === 'TRUE') { return static::ALLOW; } diff --git a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php index ae6685118e3..69494affcc0 100644 --- a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php @@ -27,12 +27,16 @@ class RegisterAccessChecksPass implements CompilerPassInterface { $access_manager = $container->getDefinition('access_manager'); foreach ($container->findTaggedServiceIds('access_check') as $id => $attributes) { $applies = array(); + $method = 'access'; foreach ($attributes as $attribute) { if (isset($attribute['applies_to'])) { $applies[] = $attribute['applies_to']; } + if (isset($attribute['method'])) { + $method = $attribute['method']; + } } - $access_manager->addMethodCall('addCheckService', array($id, $applies)); + $access_manager->addMethodCall('addCheckService', array($id, $method, $applies)); } } } diff --git a/core/lib/Drupal/Core/Entity/EntityAccessCheck.php b/core/lib/Drupal/Core/Entity/EntityAccessCheck.php index 1e3b0aea82c..951abb0aee0 100644 --- a/core/lib/Drupal/Core/Entity/EntityAccessCheck.php +++ b/core/lib/Drupal/Core/Entity/EntityAccessCheck.php @@ -18,7 +18,7 @@ use Symfony\Component\HttpFoundation\Request; class EntityAccessCheck implements AccessInterface { /** - * Implements \Drupal\Core\Access\AccessCheckInterface::access(). + * Checks access to the entity operation on the given route. * * The value of the '_entity_access' key must be in the pattern * 'entity_type.operation.' The entity type must match the {entity_type} @@ -29,6 +29,16 @@ class EntityAccessCheck implements AccessInterface { * _entity_access: 'node.update' * @endcode * Available operations are 'view', 'update', 'create', and 'delete'. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ public function access(Route $route, Request $request, AccountInterface $account) { // Split the entity type and the operation. diff --git a/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php b/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php index 1cf7d8715b5..bf7073d87e4 100644 --- a/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php +++ b/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php @@ -42,7 +42,17 @@ class EntityCreateAccessCheck implements AccessInterface { } /** - * {@inheritdoc} + * Checks access to create the entity type and bundle for the given route. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ public function access(Route $route, Request $request, AccountInterface $account) { list($entity_type, $bundle) = explode(':', $route->getRequirement($this->requirementsKey) . ':'); diff --git a/core/lib/Drupal/Core/Routing/Access/AccessInterface.php b/core/lib/Drupal/Core/Routing/Access/AccessInterface.php index 2d80c6df7cf..24c943d6b16 100644 --- a/core/lib/Drupal/Core/Routing/Access/AccessInterface.php +++ b/core/lib/Drupal/Core/Routing/Access/AccessInterface.php @@ -8,28 +8,13 @@ namespace Drupal\Core\Routing\Access; use Drupal\Core\Access\AccessInterface as GenericAccessInterface; -use Drupal\Core\Session\AccountInterface; -use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\Routing\Route; /** * An access check service determines access rules for particular routes. */ interface AccessInterface extends GenericAccessInterface { - /** - * Checks for access to a route. - * - * @param \Symfony\Component\Routing\Route $route - * The route to check against. - * @param \Symfony\Component\HttpFoundation\Request $request - * The request object. - * @param \Drupal\Core\Session\AccountInterface $account - * The currently logged in account. - * - * @return bool|null - * self::ALLOW, self::DENY, or self::KILL. - */ - public function access(Route $route, Request $request, AccountInterface $account); + // @todo Remove this interface since it no longer defines any methods? + // @see https://drupal.org/node/2266817. } diff --git a/core/lib/Drupal/Core/Theme/ThemeAccessCheck.php b/core/lib/Drupal/Core/Theme/ThemeAccessCheck.php index 75afddcb9df..289803484ec 100644 --- a/core/lib/Drupal/Core/Theme/ThemeAccessCheck.php +++ b/core/lib/Drupal/Core/Theme/ThemeAccessCheck.php @@ -8,24 +8,27 @@ namespace Drupal\Core\Theme; use Drupal\Core\Routing\Access\AccessInterface; -use Drupal\Core\Session\AccountInterface; -use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\Routing\Route; /** - * Access check for a theme. + * Provides access checking for themes for routing and theme negotiation. */ class ThemeAccessCheck implements AccessInterface { /** - * {@inheritdoc} + * Checks access to the theme for routing. + * + * @param string $theme + * The name of a theme. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { - return $this->checkAccess($request->attributes->get('theme')) ? static::ALLOW : static::DENY; + public function access($theme) { + return $this->checkAccess($theme) ? static::ALLOW : static::DENY; } /** - * Checks access to a theme. + * Indicates whether the theme is accessible based on whether it is enabled. * * @param string $theme * The name of a theme. diff --git a/core/modules/book/lib/Drupal/book/Access/BookNodeIsRemovableAccessCheck.php b/core/modules/book/lib/Drupal/book/Access/BookNodeIsRemovableAccessCheck.php index 6a5385c1dc0..861c25268b5 100644 --- a/core/modules/book/lib/Drupal/book/Access/BookNodeIsRemovableAccessCheck.php +++ b/core/modules/book/lib/Drupal/book/Access/BookNodeIsRemovableAccessCheck.php @@ -9,9 +9,7 @@ namespace Drupal\book\Access; use Drupal\book\BookManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; -use Drupal\Core\Session\AccountInterface; -use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; +use Drupal\node\NodeInterface; /** * Determines whether the requested node can be removed from its book. @@ -36,14 +34,16 @@ class BookNodeIsRemovableAccessCheck implements AccessInterface { } /** - * {@inheritdoc} + * Checks access for removing the node from its book. + * + * @param \Drupal\node\NodeInterface $node + * The node requested to be removed from its book. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { - $node = $request->attributes->get('node'); - if (!empty($node)) { - return $this->bookManager->checkNodeIsRemovable($node) ? static::ALLOW : static::DENY; - } - return static::DENY; + public function access(NodeInterface $node) { + return $this->bookManager->checkNodeIsRemovable($node) ? static::ALLOW : static::DENY; } } diff --git a/core/modules/config_translation/lib/Drupal/config_translation/Access/ConfigTranslationOverviewAccess.php b/core/modules/config_translation/lib/Drupal/config_translation/Access/ConfigTranslationOverviewAccess.php index 183441640dc..1eef1ffb012 100644 --- a/core/modules/config_translation/lib/Drupal/config_translation/Access/ConfigTranslationOverviewAccess.php +++ b/core/modules/config_translation/lib/Drupal/config_translation/Access/ConfigTranslationOverviewAccess.php @@ -43,7 +43,17 @@ class ConfigTranslationOverviewAccess implements AccessInterface { } /** - * {@inheritdoc} + * Checks access to the overview based on permissions and translatability. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ public function access(Route $route, Request $request, AccountInterface $account) { /** @var \Drupal\config_translation\ConfigMapperInterface $mapper */ diff --git a/core/modules/contact/lib/Drupal/contact/Access/ContactPageAccess.php b/core/modules/contact/lib/Drupal/contact/Access/ContactPageAccess.php index 548d1faf8bc..97a7b54fee2 100644 --- a/core/modules/contact/lib/Drupal/contact/Access/ContactPageAccess.php +++ b/core/modules/contact/lib/Drupal/contact/Access/ContactPageAccess.php @@ -11,8 +11,7 @@ use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Drupal\user\UserDataInterface; -use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; +use Drupal\user\UserInterface; /** * Access check for contact_personal_page route. @@ -47,10 +46,18 @@ class ContactPageAccess implements AccessInterface { } /** - * {@inheritdoc} + * Checks access to the given user's contact page. + * + * @param \Drupal\user\UserInterface $user + * The user being contacted. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { - $contact_account = $request->attributes->get('user'); + public function access(UserInterface $user, AccountInterface $account) { + $contact_account = $user; // Anonymous users cannot have contact forms. if ($contact_account->isAnonymous()) { diff --git a/core/modules/content_translation/lib/Drupal/content_translation/Access/ContentTranslationManageAccessCheck.php b/core/modules/content_translation/lib/Drupal/content_translation/Access/ContentTranslationManageAccessCheck.php index b9b49042dba..b0e0fb6bbfd 100644 --- a/core/modules/content_translation/lib/Drupal/content_translation/Access/ContentTranslationManageAccessCheck.php +++ b/core/modules/content_translation/lib/Drupal/content_translation/Access/ContentTranslationManageAccessCheck.php @@ -37,14 +37,30 @@ class ContentTranslationManageAccessCheck implements AccessInterface { } /** - * {@inheritdoc} + * Checks translation access for the entity and operation on the given route. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * @param string $source + * (optional) For a create operation, the language code of the source. + * @param string $target + * (optional) For a create operation, the language code of the translation. + * @param string $language + * (optional) For an update or delete operation, the language code of the + * translation being updated or deleted. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Route $route, Request $request, AccountInterface $account, $source = NULL, $target = NULL, $language = NULL) { $entity_type = $request->attributes->get('_entity_type_id'); /** @var $entity \Drupal\Core\Entity\EntityInterface */ if ($entity = $request->attributes->get($entity_type)) { - $route_requirements = $route->getRequirements(); - $operation = $route_requirements['_access_content_translation_manage']; + $operation = $route->getRequirement('_access_content_translation_manage'); $controller = content_translation_controller($entity_type, $account); // Load translation. @@ -53,10 +69,8 @@ class ContentTranslationManageAccessCheck implements AccessInterface { switch ($operation) { case 'create': - $source = language_load($request->attributes->get('source')); - $target = language_load($request->attributes->get('target')); - $source = !empty($source) ? $source : $entity->language(); - $target = !empty($target) ? $target : \Drupal::languageManager()->getCurrentLanguage(Language::TYPE_CONTENT); + $source = language_load($source) ?: $entity->language(); + $target = language_load($target) ?: \Drupal::languageManager()->getCurrentLanguage(Language::TYPE_CONTENT); return ($source->id != $target->id && isset($languages[$source->id]) && isset($languages[$target->id]) @@ -66,8 +80,7 @@ class ContentTranslationManageAccessCheck implements AccessInterface { case 'update': case 'delete': - $language = language_load($request->attributes->get('language')); - $language = !empty($language) ? $language : \Drupal::languageManager()->getCurrentLanguage(Language::TYPE_CONTENT); + $language = language_load($language) ?: \Drupal::languageManager()->getCurrentLanguage(Language::TYPE_CONTENT); return isset($languages[$language->id]) && $language->id != $entity->getUntranslated()->language()->id && isset($translations[$language->id]) diff --git a/core/modules/content_translation/lib/Drupal/content_translation/Access/ContentTranslationOverviewAccess.php b/core/modules/content_translation/lib/Drupal/content_translation/Access/ContentTranslationOverviewAccess.php index bc30db84dc0..fabd61b954a 100644 --- a/core/modules/content_translation/lib/Drupal/content_translation/Access/ContentTranslationOverviewAccess.php +++ b/core/modules/content_translation/lib/Drupal/content_translation/Access/ContentTranslationOverviewAccess.php @@ -10,7 +10,6 @@ namespace Drupal\content_translation\Access; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; /** @@ -36,9 +35,17 @@ class ContentTranslationOverviewAccess implements AccessInterface { } /** - * {@inheritdoc} + * Checks access to the translation overview for the entity and bundle. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Request $request, AccountInterface $account) { $entity_type = $request->attributes->get('_entity_type_id'); if ($entity = $request->attributes->get($entity_type)) { // Get entity base info. diff --git a/core/modules/content_translation/lib/Drupal/content_translation/Routing/ContentTranslationRouteSubscriber.php b/core/modules/content_translation/lib/Drupal/content_translation/Routing/ContentTranslationRouteSubscriber.php index 8b89d2719f2..51731b66891 100644 --- a/core/modules/content_translation/lib/Drupal/content_translation/Routing/ContentTranslationRouteSubscriber.php +++ b/core/modules/content_translation/lib/Drupal/content_translation/Routing/ContentTranslationRouteSubscriber.php @@ -50,7 +50,6 @@ class ContentTranslationRouteSubscriber extends RouteSubscriberBase { $path, array( '_content' => '\Drupal\content_translation\Controller\ContentTranslationController::overview', - 'account' => 'NULL', '_entity_type_id' => $entity_type_id, ), array( diff --git a/core/modules/field_ui/lib/Drupal/field_ui/Access/FormModeAccessCheck.php b/core/modules/field_ui/lib/Drupal/field_ui/Access/FormModeAccessCheck.php index e8d3acdfdab..5c038066389 100644 --- a/core/modules/field_ui/lib/Drupal/field_ui/Access/FormModeAccessCheck.php +++ b/core/modules/field_ui/lib/Drupal/field_ui/Access/FormModeAccessCheck.php @@ -36,22 +36,40 @@ class FormModeAccessCheck implements AccessInterface { } /** - * {@inheritdoc} + * Checks access to the form mode. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * @param string $form_mode_name + * (optional) The form mode. Defaults to 'default'. + * @param string $bundle + * (optional) The bundle. Different entity types can have different names + * for their bundle key, so if not specified on the route via a {bundle} + * parameter, the access checker determines the appropriate key name, and + * gets the value from the corresponding request attribute. For example, + * for nodes, the bundle key is "node_type", so the value would be + * available via the {node_type} parameter rather than a {bundle} + * parameter. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Route $route, Request $request, AccountInterface $account, $form_mode_name = 'default', $bundle = NULL) { if ($entity_type_id = $route->getDefault('entity_type_id')) { - $form_mode = $request->attributes->get('form_mode_name'); - - if (!($bundle = $request->attributes->get('bundle'))) { + if (!isset($bundle)) { $entity_type = $this->entityManager->getDefinition($entity_type_id); $bundle = $request->attributes->get('_raw_variables')->get($entity_type->getBundleEntityType()); } $visibility = FALSE; - if (!$form_mode || $form_mode == 'default') { + if ($form_mode_name == 'default') { $visibility = TRUE; } - elseif ($entity_display = $this->entityManager->getStorage('entity_form_display')->load($entity_type_id . '.' . $bundle . '.' . $form_mode)) { + elseif ($entity_display = $this->entityManager->getStorage('entity_form_display')->load($entity_type_id . '.' . $bundle . '.' . $form_mode_name)) { $visibility = $entity_display->status(); } diff --git a/core/modules/field_ui/lib/Drupal/field_ui/Access/ViewModeAccessCheck.php b/core/modules/field_ui/lib/Drupal/field_ui/Access/ViewModeAccessCheck.php index baa15f88424..ee17c8020f7 100644 --- a/core/modules/field_ui/lib/Drupal/field_ui/Access/ViewModeAccessCheck.php +++ b/core/modules/field_ui/lib/Drupal/field_ui/Access/ViewModeAccessCheck.php @@ -36,22 +36,40 @@ class ViewModeAccessCheck implements AccessInterface { } /** - * {@inheritdoc} + * Checks access to the view mode. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * @param string $view_mode_name + * (optional) The view mode. Defaults to 'default'. + * @param string $bundle + * (optional) The bundle. Different entity types can have different names + * for their bundle key, so if not specified on the route via a {bundle} + * parameter, the access checker determines the appropriate key name, and + * gets the value from the corresponding request attribute. For example, + * for nodes, the bundle key is "node_type", so the value would be + * available via the {node_type} parameter rather than a {bundle} + * parameter. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Route $route, Request $request, AccountInterface $account, $view_mode_name = 'default', $bundle = NULL) { if ($entity_type_id = $route->getDefault('entity_type_id')) { - $view_mode = $request->attributes->get('view_mode_name'); - - if (!($bundle = $request->attributes->get('bundle'))) { + if (!isset($bundle)) { $entity_type = $this->entityManager->getDefinition($entity_type_id); $bundle = $request->attributes->get('_raw_variables')->get($entity_type->getBundleEntityType()); } $visibility = FALSE; - if (!$view_mode || $view_mode == 'default') { + if ($view_mode_name == 'default') { $visibility = TRUE; } - elseif ($entity_display = $this->entityManager->getStorage('entity_view_display')->load($entity_type_id . '.' . $bundle . '.' . $view_mode)) { + elseif ($entity_display = $this->entityManager->getStorage('entity_view_display')->load($entity_type_id . '.' . $bundle . '.' . $view_mode_name)) { $visibility = $entity_display->status(); } diff --git a/core/modules/node/lib/Drupal/node/Access/NodeAddAccessCheck.php b/core/modules/node/lib/Drupal/node/Access/NodeAddAccessCheck.php index 8d72223a01a..c7bb0e778eb 100644 --- a/core/modules/node/lib/Drupal/node/Access/NodeAddAccessCheck.php +++ b/core/modules/node/lib/Drupal/node/Access/NodeAddAccessCheck.php @@ -10,8 +10,7 @@ namespace Drupal\node\Access; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; +use Drupal\node\NodeTypeInterface; /** * Determines access to for node add pages. @@ -36,17 +35,26 @@ class NodeAddAccessCheck implements AccessInterface { } /** - * {@inheritdoc} + * Checks access to the node add page for the node type. + * + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * @param \Drupal\node\NodeTypeInterface $node_type + * (optional) The node type. If not specified, access is allowed if there + * exists at least one node type for which the user may create a node. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(AccountInterface $account, NodeTypeInterface $node_type = NULL) { $access_controller = $this->entityManager->getAccessController('node'); - // If a node type is set on the request, just check that. - if ($request->attributes->has('node_type')) { - return $access_controller->createAccess($request->attributes->get('node_type')->type, $account) ? static::ALLOW : static::DENY; + // If checking whether a node of a particular type may be created. + if ($node_type) { + return $access_controller->createAccess($node_type->id(), $account) ? static::ALLOW : static::DENY; } - foreach (node_permissions_get_configured_types() as $type) { - if ($access_controller->createAccess($type->type, $account)) { - // Allow access if at least one type is permitted. + // If checking whether a node of any type may be created. + foreach (node_permissions_get_configured_types() as $node_type) { + if ($access_controller->createAccess($node_type->id(), $account)) { return static::ALLOW; } } diff --git a/core/modules/node/lib/Drupal/node/Access/NodeRevisionAccessCheck.php b/core/modules/node/lib/Drupal/node/Access/NodeRevisionAccessCheck.php index 9e60caeebb1..66dc7e20881 100644 --- a/core/modules/node/lib/Drupal/node/Access/NodeRevisionAccessCheck.php +++ b/core/modules/node/lib/Drupal/node/Access/NodeRevisionAccessCheck.php @@ -12,7 +12,6 @@ use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Drupal\node\NodeInterface; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; /** @@ -63,21 +62,30 @@ class NodeRevisionAccessCheck implements AccessInterface { } /** - * {@inheritdoc} + * Checks routing access for the node revision. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * @param int $node_revision + * (optional) The node revision ID. If not specified, but $node is, access + * is checked for that object's revision. + * @param \Drupal\node\NodeInterface $node + * (optional) A node object. Used for checking access to a node's default + * revision when $node_revision is unspecified. Ignored when $node_revision + * is specified. If neither $node_revision nor $node are specified, then + * access is denied. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { - // If the route has a {node_revision} placeholder, load the node for that - // revision. Otherwise, try to use a {node} placeholder. - if ($request->attributes->has('node_revision')) { - $node = $this->nodeStorage->loadRevision($request->attributes->get('node_revision')); + public function access(Route $route, AccountInterface $account, $node_revision = NULL, NodeInterface $node = NULL) { + if ($node_revision) { + $node = $this->nodeStorage->loadRevision($node_revision); } - elseif ($request->attributes->has('node')) { - $node = $request->attributes->get('node'); - } - else { - return static::DENY; - } - return $this->checkAccess($node, $account, $route->getRequirement('_access_node_revision')) ? static::ALLOW : static::DENY; + $operation = $route->getRequirement('_access_node_revision'); + return ($node && $this->checkAccess($node, $account, $operation)) ? static::ALLOW : static::DENY; } /** diff --git a/core/modules/quickedit/lib/Drupal/quickedit/Access/EditEntityAccessCheck.php b/core/modules/quickedit/lib/Drupal/quickedit/Access/EditEntityAccessCheck.php index 535c74afbca..a80aa2e55c4 100644 --- a/core/modules/quickedit/lib/Drupal/quickedit/Access/EditEntityAccessCheck.php +++ b/core/modules/quickedit/lib/Drupal/quickedit/Access/EditEntityAccessCheck.php @@ -10,13 +10,11 @@ namespace Drupal\quickedit\Access; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Drupal\Core\Entity\EntityInterface; /** - * Access check for editing entities. + * Access check for editing entities with QuickEdit. */ class EditEntityAccessCheck implements AccessInterface { @@ -38,12 +36,20 @@ class EditEntityAccessCheck implements AccessInterface { } /** - * {@inheritdoc} + * Checks Quick Edit access to the entity. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. + * + * @todo Replace $request parameter with $entity once + * https://drupal.org/node/1837388 is fixed. */ - public function access(Route $route, Request $request, AccountInterface $account) { - // @todo Request argument validation and object loading should happen - // elsewhere in the request processing pipeline: - // http://drupal.org/node/1798214. + public function access(Request $request, AccountInterface $account) { if (!$this->validateAndUpcastRequestAttributes($request)) { return static::KILL; } @@ -60,6 +66,8 @@ class EditEntityAccessCheck implements AccessInterface { /** * Validates and upcasts request attributes. + * + * @todo Remove once https://drupal.org/node/1837388 is fixed. */ protected function validateAndUpcastRequestAttributes(Request $request) { // Load the entity. diff --git a/core/modules/quickedit/lib/Drupal/quickedit/Access/EditEntityFieldAccessCheck.php b/core/modules/quickedit/lib/Drupal/quickedit/Access/EditEntityFieldAccessCheck.php index f8819ba589e..6eaba8a42d9 100644 --- a/core/modules/quickedit/lib/Drupal/quickedit/Access/EditEntityFieldAccessCheck.php +++ b/core/modules/quickedit/lib/Drupal/quickedit/Access/EditEntityFieldAccessCheck.php @@ -10,7 +10,6 @@ namespace Drupal\quickedit\Access; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; use Drupal\Core\Entity\EntityInterface; @@ -37,17 +36,29 @@ class EditEntityFieldAccessCheck implements AccessInterface, EditEntityFieldAcce } /** - * {@inheritdoc} + * Checks Quick Edit access to the field. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param string $field_name. + * The field name. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. + * + * @todo Replace $request parameter with $entity once + * https://drupal.org/node/1837388 is fixed. + * + * @todo Use the $account argument: https://drupal.org/node/2266809. */ - public function access(Route $route, Request $request, AccountInterface $account) { - // @todo Request argument validation and object loading should happen - // elsewhere in the request processing pipeline: - // http://drupal.org/node/1798214. + public function access(Request $request, $field_name, AccountInterface $account) { if (!$this->validateAndUpcastRequestAttributes($request)) { return static::KILL; } - return $this->accessEditEntityField($request->attributes->get('entity'), $request->attributes->get('field_name')) ? static::ALLOW : static::DENY; + return $this->accessEditEntityField($request->attributes->get('entity'), $field_name) ? static::ALLOW : static::DENY; } /** @@ -59,6 +70,8 @@ class EditEntityFieldAccessCheck implements AccessInterface, EditEntityFieldAcce /** * Validates and upcasts request attributes. + * + * @todo Remove once https://drupal.org/node/1837388 is fixed. */ protected function validateAndUpcastRequestAttributes(Request $request) { // Load the entity. diff --git a/core/modules/quickedit/tests/Drupal/quickedit/Tests/Access/EditEntityAccessCheckTest.php b/core/modules/quickedit/tests/Drupal/quickedit/Tests/Access/EditEntityAccessCheckTest.php index c3f87dab421..6596ccb77dd 100644 --- a/core/modules/quickedit/tests/Drupal/quickedit/Tests/Access/EditEntityAccessCheckTest.php +++ b/core/modules/quickedit/tests/Drupal/quickedit/Tests/Access/EditEntityAccessCheckTest.php @@ -8,7 +8,6 @@ namespace Drupal\quickedit\Tests\Access; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\Routing\Route; use Drupal\Core\Access\AccessCheckInterface; use Drupal\quickedit\Access\EditEntityAccessCheck; use Drupal\Tests\UnitTestCase; @@ -103,7 +102,6 @@ class EditEntityAccessCheckTest extends UnitTestCase { * @dataProvider providerTestAccess */ public function testAccess(EntityInterface $entity, $expected_result) { - $route = new Route('/quickedit/form/test_entity/1/body/und/full', array(), array('_access_quickedit_entity' => 'TRUE')); $request = new Request(); // Prepare the request to be valid. @@ -111,7 +109,7 @@ class EditEntityAccessCheckTest extends UnitTestCase { $request->attributes->set('entity_type', 'test_entity'); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $access = $this->editAccessCheck->access($route, $request, $account); + $access = $this->editAccessCheck->access($request, $account); $this->assertSame($expected_result, $access); } @@ -119,7 +117,6 @@ class EditEntityAccessCheckTest extends UnitTestCase { * Tests the access method with an undefined entity type. */ public function testAccessWithUndefinedEntityType() { - $route = new Route('/quickedit/form/test_entity/1/body/und/full', array(), array('_access_quickedit_entity' => 'TRUE')); $request = new Request(); $request->attributes->set('entity_type', 'non_valid'); @@ -129,14 +126,13 @@ class EditEntityAccessCheckTest extends UnitTestCase { ->will($this->returnValue(NULL)); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($route, $request, $account)); + $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, $account)); } /** * Tests the access method with a non existing entity. */ public function testAccessWithNotExistingEntity() { - $route = new Route('/quickedit/form/test_entity/1/body/und/full', array(), array('_access_quickedit_entity_field' => 'TRUE')); $request = new Request(); $request->attributes->set('entity_type', 'entity_test'); $request->attributes->set('entity', 1); @@ -152,7 +148,7 @@ class EditEntityAccessCheckTest extends UnitTestCase { ->will($this->returnValue(NULL)); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($route, $request, $account)); + $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, $account)); } } diff --git a/core/modules/quickedit/tests/Drupal/quickedit/Tests/Access/EditEntityFieldAccessCheckTest.php b/core/modules/quickedit/tests/Drupal/quickedit/Tests/Access/EditEntityFieldAccessCheckTest.php index 68954224a34..d1b022850d2 100644 --- a/core/modules/quickedit/tests/Drupal/quickedit/Tests/Access/EditEntityFieldAccessCheckTest.php +++ b/core/modules/quickedit/tests/Drupal/quickedit/Tests/Access/EditEntityFieldAccessCheckTest.php @@ -8,7 +8,6 @@ namespace Drupal\quickedit\Tests\Access; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\Routing\Route; use Drupal\Core\Access\AccessCheckInterface; use Drupal\quickedit\Access\EditEntityFieldAccessCheck; use Drupal\Tests\UnitTestCase; @@ -118,13 +117,13 @@ class EditEntityFieldAccessCheckTest extends UnitTestCase { * @dataProvider providerTestAccess */ public function testAccess(EntityInterface $entity, FieldConfigInterface $field = NULL, $expected_result) { - $route = new Route('/quickedit/form/test_entity/1/body/und/full', array(), array('_access_quickedit_entity_field' => 'TRUE')); $request = new Request(); + $field_name = 'valid'; $entity_with_field = clone $entity; $entity_with_field->expects($this->any()) ->method('get') - ->with('valid') + ->with($field_name) ->will($this->returnValue($field)); $entity_with_field->expects($this->once()) ->method('hasTranslation') @@ -134,11 +133,11 @@ class EditEntityFieldAccessCheckTest extends UnitTestCase { // Prepare the request to be valid. $request->attributes->set('entity_type', 'test_entity'); $request->attributes->set('entity', $entity_with_field); - $request->attributes->set('field_name', 'valid'); + $request->attributes->set('field_name', $field_name); $request->attributes->set('langcode', Language::LANGCODE_NOT_SPECIFIED); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $access = $this->editAccessCheck->access($route, $request, $account); + $access = $this->editAccessCheck->access($request, $field_name, $account); $this->assertSame($expected_result, $access); } @@ -146,7 +145,6 @@ class EditEntityFieldAccessCheckTest extends UnitTestCase { * Tests the access method with an undefined entity type. */ public function testAccessWithUndefinedEntityType() { - $route = new Route('/quickedit/form/test_entity/1/body/und/full', array(), array('_access_quickedit_entity_field' => 'TRUE')); $request = new Request(); $request->attributes->set('entity_type', 'non_valid'); @@ -156,14 +154,13 @@ class EditEntityFieldAccessCheckTest extends UnitTestCase { ->will($this->returnValue(NULL)); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($route, $request, $account)); + $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, NULL, $account)); } /** * Tests the access method with a non existing entity. */ public function testAccessWithNotExistingEntity() { - $route = new Route('/quickedit/form/test_entity/1/body/und/full', array(), array('_access_quickedit_entity_field' => 'TRUE')); $request = new Request(); $request->attributes->set('entity_type', 'entity_test'); $request->attributes->set('entity', 1); @@ -179,48 +176,47 @@ class EditEntityFieldAccessCheckTest extends UnitTestCase { ->will($this->returnValue(NULL)); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($route, $request, $account)); + $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, NULL, $account)); } /** * Tests the access method with a forgotten passed field_name. */ public function testAccessWithNotPassedFieldName() { - $route = new Route('/quickedit/form/test_entity/1/body/und/full', array(), array('_access_quickedit_entity_field' => 'TRUE')); $request = new Request(); $request->attributes->set('entity_type', 'entity_test'); $request->attributes->set('entity', $this->createMockEntity()); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($route, $request, $account)); + $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, NULL, $account)); } /** * Tests the access method with a non existing field. */ public function testAccessWithNonExistingField() { - $route = new Route('/quickedit/form/test_entity/1/body/und/full', array(), array('_access_quickedit_entity_field' => 'TRUE')); $request = new Request(); + $field_name = 'not_valid'; $request->attributes->set('entity_type', 'entity_test'); $request->attributes->set('entity', $this->createMockEntity()); - $request->attributes->set('field_name', 'not_valid'); + $request->attributes->set('field_name', $field_name); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($route, $request, $account)); + $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, $field_name, $account)); } /** * Tests the access method with a forgotten passed language. */ public function testAccessWithNotPassedLanguage() { - $route = new Route('/quickedit/form/test_entity/1/body/und/full', array(), array('_access_quickedit_entity_field' => 'TRUE')); $request = new Request(); + $field_name = 'valid'; $request->attributes->set('entity_type', 'entity_test'); $request->attributes->set('entity', $this->createMockEntity()); - $request->attributes->set('field_name', 'valid'); + $request->attributes->set('field_name', $field_name); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($route, $request, $account)); + $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, $field_name, $account)); } /** @@ -233,15 +229,15 @@ class EditEntityFieldAccessCheckTest extends UnitTestCase { ->with('xx-lolspeak') ->will($this->returnValue(FALSE)); - $route = new Route('/quickedit/form/test_entity/1/body/und/full', array(), array('_access_quickedit_entity_field' => 'TRUE')); $request = new Request(); + $field_name = 'valid'; $request->attributes->set('entity_type', 'entity_test'); $request->attributes->set('entity', $entity); - $request->attributes->set('field_name', 'valid'); + $request->attributes->set('field_name', $field_name); $request->attributes->set('langcode', 'xx-lolspeak'); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($route, $request, $account)); + $this->assertSame(AccessCheckInterface::KILL, $this->editAccessCheck->access($request, $field_name, $account)); } /** diff --git a/core/modules/rest/lib/Drupal/rest/Access/CSRFAccessCheck.php b/core/modules/rest/lib/Drupal/rest/Access/CSRFAccessCheck.php index 396b3a17992..6d50ff07308 100644 --- a/core/modules/rest/lib/Drupal/rest/Access/CSRFAccessCheck.php +++ b/core/modules/rest/lib/Drupal/rest/Access/CSRFAccessCheck.php @@ -41,9 +41,17 @@ class CSRFAccessCheck implements AccessCheckInterface { } /** - * Implements AccessCheckInterface::access(). + * Checks access. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Request $request, AccountInterface $account) { $method = $request->getMethod(); $cookie = $request->attributes->get('_authentication_provider') == 'cookie'; diff --git a/core/modules/shortcut/shortcut.services.yml b/core/modules/shortcut/shortcut.services.yml index a6605940e76..dc8d30cd0ac 100644 --- a/core/modules/shortcut/shortcut.services.yml +++ b/core/modules/shortcut/shortcut.services.yml @@ -1,14 +1,4 @@ services: - access_check.shortcut.link: - class: Drupal\shortcut\Access\LinkAccessCheck - tags: - - { name: access_check, applies_to: _access_shortcut_link } - - access_check.shortcut.shortcut_set_edit: - class: Drupal\shortcut\Access\ShortcutSetEditAccessCheck - tags: - - { name: access_check, applies_to: _access_shortcut_set_edit } - access_check.shortcut.shortcut_set_switch: class: Drupal\shortcut\Access\ShortcutSetSwitchAccessCheck tags: diff --git a/core/modules/shortcut/src/Access/LinkAccessCheck.php b/core/modules/shortcut/src/Access/LinkAccessCheck.php deleted file mode 100644 index b85fb9f7a64..00000000000 --- a/core/modules/shortcut/src/Access/LinkAccessCheck.php +++ /dev/null @@ -1,32 +0,0 @@ -attributes->get('menu_link'); - $set_name = str_replace('shortcut-', '', $menu_link['menu_name']); - if ($shortcut_set = shortcut_set_load($set_name)) { - return shortcut_set_edit_access($shortcut_set) ? static::ALLOW : static::DENY; - } - return static::DENY; - } - -} diff --git a/core/modules/shortcut/src/Access/ShortcutSetEditAccessCheck.php b/core/modules/shortcut/src/Access/ShortcutSetEditAccessCheck.php deleted file mode 100644 index 21bbda8fba9..00000000000 --- a/core/modules/shortcut/src/Access/ShortcutSetEditAccessCheck.php +++ /dev/null @@ -1,37 +0,0 @@ -attributes->get('shortcut_set'); - // Sufficiently-privileged users can edit their currently displayed shortcut - // set, but not other sets. Shortcut administrators can edit any set. - if ($account->hasPermission('administer shortcuts')) { - return static::ALLOW; - } - if ($account->hasPermission('customize shortcut links')) { - return !isset($shortcut_set) || $shortcut_set == shortcut_current_displayed_set() ? static::ALLOW : static::DENY; - } - return static::DENY; - } - -} diff --git a/core/modules/shortcut/src/Access/ShortcutSetSwitchAccessCheck.php b/core/modules/shortcut/src/Access/ShortcutSetSwitchAccessCheck.php index 6d844edfd0d..51463f86087 100644 --- a/core/modules/shortcut/src/Access/ShortcutSetSwitchAccessCheck.php +++ b/core/modules/shortcut/src/Access/ShortcutSetSwitchAccessCheck.php @@ -9,18 +9,25 @@ namespace Drupal\shortcut\Access; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; +use Drupal\user\UserInterface; /** - * Provides an access check for shortcut link delete routes. + * Checks access to switch a user's shortcut set. */ class ShortcutSetSwitchAccessCheck implements AccessInterface { /** - * {@inheritdoc} + * Checks access. + * + * @param \Drupal\user\UserInterface $user + * The owner of the shortcut set. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(UserInterface $user, AccountInterface $account) { if ($account->hasPermission('administer shortcuts')) { // Administrators can switch anyone's shortcut set. return static::ALLOW; @@ -31,8 +38,7 @@ class ShortcutSetSwitchAccessCheck implements AccessInterface { return static::DENY; } - $user = $request->attributes->get('account'); - if (!isset($user) || $user->id() == $account->id()) { + if ($user->id() == $account->id()) { // Users with the 'switch shortcut sets' permission can switch their own // shortcuts sets. return static::ALLOW; diff --git a/core/modules/system/lib/Drupal/system/Access/CronAccessCheck.php b/core/modules/system/lib/Drupal/system/Access/CronAccessCheck.php index ef094fd7fb3..7aa9a9d2f51 100644 --- a/core/modules/system/lib/Drupal/system/Access/CronAccessCheck.php +++ b/core/modules/system/lib/Drupal/system/Access/CronAccessCheck.php @@ -8,9 +8,6 @@ namespace Drupal\system\Access; use Drupal\Core\Routing\Access\AccessInterface; -use Drupal\Core\Session\AccountInterface; -use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; /** * Access check for cron routes. @@ -18,10 +15,15 @@ use Symfony\Component\HttpFoundation\Request; class CronAccessCheck implements AccessInterface { /** - * Implements AccessCheckInterface::access(). + * Checks access. + * + * @param string $key + * The cron key. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { - $key = $request->attributes->get('key'); + public function access($key) { if ($key != \Drupal::state()->get('system.cron_key')) { watchdog('cron', 'Cron could not run because an invalid key was used.', array(), WATCHDOG_NOTICE); return static::KILL; diff --git a/core/modules/system/tests/modules/router_test_directory/lib/Drupal/router_test/Access/DefinedTestAccessCheck.php b/core/modules/system/tests/modules/router_test_directory/lib/Drupal/router_test/Access/DefinedTestAccessCheck.php index 7aa50dc022a..1469d20ead6 100644 --- a/core/modules/system/tests/modules/router_test_directory/lib/Drupal/router_test/Access/DefinedTestAccessCheck.php +++ b/core/modules/system/tests/modules/router_test_directory/lib/Drupal/router_test/Access/DefinedTestAccessCheck.php @@ -8,8 +8,6 @@ namespace Drupal\router_test\Access; use Drupal\Core\Routing\Access\AccessInterface; -use Drupal\Core\Session\AccountInterface; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; /** @@ -18,9 +16,15 @@ use Symfony\Component\Routing\Route; class DefinedTestAccessCheck implements AccessInterface { /** - * {@inheritdoc} + * Checks access. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Route $route) { if ($route->getRequirement('_test_access') === 'TRUE') { return static::ALLOW; } diff --git a/core/modules/system/tests/modules/router_test_directory/lib/Drupal/router_test/Access/TestAccessCheck.php b/core/modules/system/tests/modules/router_test_directory/lib/Drupal/router_test/Access/TestAccessCheck.php index eda75608d59..d8af2c66cdf 100644 --- a/core/modules/system/tests/modules/router_test_directory/lib/Drupal/router_test/Access/TestAccessCheck.php +++ b/core/modules/system/tests/modules/router_test_directory/lib/Drupal/router_test/Access/TestAccessCheck.php @@ -8,9 +8,6 @@ namespace Drupal\router_test\Access; use Drupal\Core\Routing\Access\AccessInterface; -use Drupal\Core\Session\AccountInterface; -use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; /** * Access check for test routes. @@ -18,9 +15,12 @@ use Symfony\Component\HttpFoundation\Request; class TestAccessCheck implements AccessInterface { /** - * Implements AccessCheckInterface::access(). + * Checks access. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access() { // No opinion, so other access checks should decide if access should be // allowed or not. return static::DENY; diff --git a/core/modules/tracker/lib/Drupal/tracker/Access/ViewOwnTrackerAccessCheck.php b/core/modules/tracker/lib/Drupal/tracker/Access/ViewOwnTrackerAccessCheck.php index a4c594e7d34..4421b77e89c 100644 --- a/core/modules/tracker/lib/Drupal/tracker/Access/ViewOwnTrackerAccessCheck.php +++ b/core/modules/tracker/lib/Drupal/tracker/Access/ViewOwnTrackerAccessCheck.php @@ -9,8 +9,7 @@ namespace Drupal\tracker\Access; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; +use Drupal\user\UserInterface; /** * Access check for user tracker routes. @@ -18,12 +17,17 @@ use Symfony\Component\HttpFoundation\Request; class ViewOwnTrackerAccessCheck implements AccessInterface { /** - * {@inheritdoc} + * Checks access. + * + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * @param \Drupal\user\UserInterface $user + * The user whose tracker page is being accessed. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { - // The user object from the User ID in the path. - $user = $request->attributes->get('user'); + public function access(AccountInterface $account, UserInterface $user) { return ($user && $account->isAuthenticated() && ($user->id() == $account->id())) ? static::ALLOW : static::DENY; } } - diff --git a/core/modules/update/lib/Drupal/update/Access/UpdateManagerAccessCheck.php b/core/modules/update/lib/Drupal/update/Access/UpdateManagerAccessCheck.php index 5fba57fc763..5fc20b03da2 100644 --- a/core/modules/update/lib/Drupal/update/Access/UpdateManagerAccessCheck.php +++ b/core/modules/update/lib/Drupal/update/Access/UpdateManagerAccessCheck.php @@ -8,10 +8,7 @@ namespace Drupal\update\Access; use Drupal\Core\Routing\Access\AccessInterface; -use Drupal\Core\Session\AccountInterface; use Drupal\Core\Site\Settings; -use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; /** * Determines whether allow authorized operations is set. @@ -36,9 +33,12 @@ class UpdateManagerAccessCheck implements AccessInterface { } /** - * {@inheritdoc} + * Checks access. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access() { return $this->settings->get('allow_authorize_operations', TRUE) ? static::ALLOW : static::DENY; } diff --git a/core/modules/user/lib/Drupal/user/Access/LoginStatusCheck.php b/core/modules/user/lib/Drupal/user/Access/LoginStatusCheck.php index 01d7a6cbeac..fbceacd006d 100644 --- a/core/modules/user/lib/Drupal/user/Access/LoginStatusCheck.php +++ b/core/modules/user/lib/Drupal/user/Access/LoginStatusCheck.php @@ -9,7 +9,6 @@ namespace Drupal\user\Access; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; /** @@ -18,9 +17,17 @@ use Symfony\Component\HttpFoundation\Request; class LoginStatusCheck implements AccessInterface { /** - * {@inheritdoc} + * Checks access. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Request $request, AccountInterface $account) { return ($request->attributes->get('_menu_admin') || $account->isAuthenticated()) ? static::ALLOW : static::DENY; } diff --git a/core/modules/user/lib/Drupal/user/Access/PermissionAccessCheck.php b/core/modules/user/lib/Drupal/user/Access/PermissionAccessCheck.php index a159b1c14a1..2672a4e4591 100644 --- a/core/modules/user/lib/Drupal/user/Access/PermissionAccessCheck.php +++ b/core/modules/user/lib/Drupal/user/Access/PermissionAccessCheck.php @@ -10,7 +10,6 @@ namespace Drupal\user\Access; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; /** * Determines access to routes based on permissions defined via hook_permission(). @@ -18,11 +17,18 @@ use Symfony\Component\HttpFoundation\Request; class PermissionAccessCheck implements AccessInterface { /** - * Implements AccessCheckInterface::access(). + * Checks access. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Route $route, AccountInterface $account) { $permission = $route->getRequirement('_permission'); - // If the access check fails, return NULL to give other checks a chance. return $account->hasPermission($permission) ? static::ALLOW : static::DENY; } } diff --git a/core/modules/user/lib/Drupal/user/Access/RegisterAccessCheck.php b/core/modules/user/lib/Drupal/user/Access/RegisterAccessCheck.php index eff984ba071..aec179b041c 100644 --- a/core/modules/user/lib/Drupal/user/Access/RegisterAccessCheck.php +++ b/core/modules/user/lib/Drupal/user/Access/RegisterAccessCheck.php @@ -9,7 +9,6 @@ namespace Drupal\user\Access; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; /** @@ -18,9 +17,17 @@ use Symfony\Component\HttpFoundation\Request; class RegisterAccessCheck implements AccessInterface { /** - * Implements AccessCheckInterface::access(). + * Checks access. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request object. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Request $request, AccountInterface $account) { return ($request->attributes->get('_menu_admin') || $account->isAnonymous()) && (\Drupal::config('user.settings')->get('register') != USER_REGISTER_ADMINISTRATORS_ONLY) ? static::ALLOW : static::DENY; } } diff --git a/core/modules/user/lib/Drupal/user/Access/RoleAccessCheck.php b/core/modules/user/lib/Drupal/user/Access/RoleAccessCheck.php index e3ace16fa33..e8cfda4ba75 100644 --- a/core/modules/user/lib/Drupal/user/Access/RoleAccessCheck.php +++ b/core/modules/user/lib/Drupal/user/Access/RoleAccessCheck.php @@ -9,7 +9,6 @@ namespace Drupal\user\Access; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; /** @@ -22,9 +21,17 @@ use Symfony\Component\Routing\Route; class RoleAccessCheck implements AccessInterface { /** - * {@inheritdoc} + * Checks access. + * + * @param \Symfony\Component\Routing\Route $route + * The route to check against. + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Route $route, AccountInterface $account) { // Requirements just allow strings, so this might be a comma separated list. $rid_string = $route->getRequirement('_role'); diff --git a/core/modules/views/lib/Drupal/views/ViewsAccessCheck.php b/core/modules/views/lib/Drupal/views/ViewsAccessCheck.php index 8a81fb70ef0..8a3b508f6a9 100644 --- a/core/modules/views/lib/Drupal/views/ViewsAccessCheck.php +++ b/core/modules/views/lib/Drupal/views/ViewsAccessCheck.php @@ -9,7 +9,6 @@ namespace Drupal\views; use Drupal\Core\Access\AccessCheckInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; /** @@ -27,12 +26,16 @@ class ViewsAccessCheck implements AccessCheckInterface { } /** - * Implements AccessCheckInterface::applies(). + * Checks access. + * + * @param \Drupal\Core\Session\AccountInterface $account + * The currently logged in account. + * + * @return string + * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { - $access = $account->hasPermission('access all views'); - - return $access ? static::ALLOW : static::DENY; + public function access(AccountInterface $account) { + return $account->hasPermission('access all views') ? static::ALLOW : static::DENY; } } diff --git a/core/tests/Drupal/Tests/Core/Access/AccessArgumentsResolverTest.php b/core/tests/Drupal/Tests/Core/Access/AccessArgumentsResolverTest.php new file mode 100644 index 00000000000..1aa8171152f --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Access/AccessArgumentsResolverTest.php @@ -0,0 +1,254 @@ + 'Access arguments resolver tests', + 'description' => 'Test for the AccessArgumentsResolver object.', + 'group' => 'Routing', + ); + } + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + $this->account = $this->getMock('Drupal\Core\Session\AccountInterface'); + $this->route = new Route('/test'); + } + + /** + * Tests the getArgument() method. + * + * @dataProvider providerTestGetArgument + */ + public function testGetArgument($callable, $request, $expected) { + $arguments = (new AccessArgumentsResolver())->getArguments($callable, $this->route, $request, $this->account); + $this->assertSame($expected, $arguments); + } + + /** + * Provides test data to testGetArgument(). + */ + public function providerTestGetArgument() { + $data = array(); + + // Test an optional parameter with no provided value. + $data[] = array( + function($foo = 'foo') {}, new Request(), array('foo'), + ); + + // Test an optional parameter with a provided value. + $request = new Request(); + $request->attributes->set('foo', 'bar'); + $data[] = array( + function($foo = 'foo') {}, $request, array('bar'), + ); + + // Test with a provided value. + $request = new Request(); + $request->attributes->set('foo', 'bar'); + $data[] = array( + function($foo) {}, $request, array('bar'), + ); + + // Test with an explicitly NULL value. + $request = new Request(); + $request->attributes->set('foo', NULL); + $data[] = array( + function($foo) {}, $request, array(NULL), + ); + + // Test with a raw value that overrides the provided upcast value, since + // it is not typehinted. + $request = new Request(); + $request->attributes->set('foo', 'bar'); + $request->attributes->set('_raw_variables', new ParameterBag(array('foo' => 'baz'))); + $data[] = array( + function($foo) {}, $request, array('baz'), + ); + + return $data; + } + + /** + * Tests getArgument() with a Route object. + */ + public function testGetArgumentRoute() { + $callable = function(Route $route) {}; + $request = new Request(); + + $arguments = (new AccessArgumentsResolver())->getArguments($callable, $this->route, $request, $this->account); + $this->assertSame(array($this->route), $arguments); + } + + /** + * Tests getArgument() with a Route object for a parameter with a custom name. + */ + public function testGetArgumentRouteCustomName() { + $callable = function(Route $custom_name) {}; + $request = new Request(); + + $arguments = (new AccessArgumentsResolver())->getArguments($callable, $this->route, $request, $this->account); + $this->assertSame(array($this->route), $arguments); + } + + /** + * Tests getArgument() with a Route, Request, and Account object. + */ + public function testGetArgumentRouteRequestAccount() { + $callable = function(Route $route, Request $request, AccountInterface $account) {}; + $request = new Request(); + + $arguments = (new AccessArgumentsResolver())->getArguments($callable, $this->route, $request, $this->account); + $this->assertSame(array($this->route, $request, $this->account), $arguments); + + // Test again, but with the arguments in a different order. + $callable = function(AccountInterface $account, Request $request, Route $route) {}; + $request = new Request(); + + $arguments = (new AccessArgumentsResolver())->getArguments($callable, $this->route, $request, $this->account); + $this->assertSame(array($this->account, $request, $this->route), $arguments); + } + + /** + * Tests getArgument() with a '$route' parameter with no typehint. + * + * Without the typehint, the Route object will not be passed to the callable. + * + * @expectedException \RuntimeException + * @expectedExceptionMessage requires a value for the "$route" argument. + */ + public function testGetArgumentRouteNoTypehint() { + $callable = function($route) {}; + $request = new Request(); + + $arguments = (new AccessArgumentsResolver())->getArguments($callable, $this->route, $request, $this->account); + $this->assertNull($arguments); + } + + /** + * Tests getArgument() with a '$route' parameter with no typehint and a value. + * + * Without the typehint, passing a value to a parameter named '$route' will + * still receive the provided value. + */ + public function testGetArgumentRouteNoTypehintAndValue() { + $callable = function($route) {}; + $request = new Request(); + $request->attributes->set('route', 'foo'); + + $arguments = (new AccessArgumentsResolver())->getArguments($callable, $this->route, $request, $this->account); + $this->assertSame(array('foo'), $arguments); + } + + /** + * Tests getArgument() when upcasting is bypassed. + */ + public function testGetArgumentBypassUpcasting() { + $callable = function(Route $route = NULL) {}; + + $request = new Request(); + $request->attributes->set('route', NULL); + + $arguments = (new AccessArgumentsResolver())->getArguments($callable, $this->route, $request, $this->account); + $this->assertSame(array(NULL), $arguments); + } + + /** + * Tests handleUnresolvedArgument() for a non-upcast argument. + * + * @expectedException \RuntimeException + * @expectedExceptionMessage requires a value for the "$foo" argument. + */ + public function testHandleNotUpcastedArgument() { + $callable = function(\stdClass $foo) {}; + + $request = new Request(); + $request->attributes->set('foo', 'bar'); + $request->attributes->set('_raw_variables', new ParameterBag(array('foo' => 'baz'))); + + $arguments = (new AccessArgumentsResolver())->getArguments($callable, $this->route, $request, $this->account); + $this->assertNull($arguments); + } + + /** + * Tests handleUnresolvedArgument() for missing arguments. + * + * @expectedException \RuntimeException + * @expectedExceptionMessage requires a value for the "$foo" argument. + * + * @dataProvider providerTestHandleUnresolvedArgument + */ + public function testHandleUnresolvedArgument($callable) { + $request = new Request(); + + $arguments = (new AccessArgumentsResolver())->getArguments($callable, $this->route, $request, $this->account); + $this->assertNull($arguments); + } + + /** + * Provides test data to testHandleUnresolvedArgument(). + */ + public function providerTestHandleUnresolvedArgument() { + $data = array(); + $data[] = array(function($foo) {}); + $data[] = array(array(new TestClass(), 'access')); + $data[] = array('test_access_arguments_resolver_access'); + return $data; + } + +} + +/** + * Provides a test class. + */ +class TestClass { + public function access($foo) { + } +} + +} + +namespace { + function test_access_arguments_resolver_access($foo) { + } +} diff --git a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php index 0e830e2b1bb..8221ea1f9d9 100644 --- a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php @@ -76,6 +76,13 @@ class AccessManagerTest extends UnitTestCase { */ protected $account; + /** + * The access arguments resolver. + * + * @var \Drupal\Core\Access\AccessArgumentsResolverInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $argumentsResolver; + public static function getInfo() { return array( 'name' => 'Access manager tests', @@ -122,8 +129,9 @@ class AccessManagerTest extends UnitTestCase { $this->paramConverter = $this->getMock('Drupal\Core\ParamConverter\ParamConverterManagerInterface'); $this->account = $this->getMock('Drupal\Core\Session\AccountInterface'); + $this->argumentsResolver = $this->getMock('Drupal\Core\Access\AccessArgumentsResolverInterface'); - $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->account); + $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->argumentsResolver); $this->accessManager->setContainer($this->container); } @@ -152,13 +160,13 @@ class AccessManagerTest extends UnitTestCase { */ public function testSetChecksWithDynamicAccessChecker() { // Setup the access manager. - $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->account); + $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->argumentsResolver); $this->accessManager->setContainer($this->container); // Setup the dynamic access checker. - $access_check = $this->getMock('Drupal\Core\Access\AccessCheckInterface'); + $access_check = $this->getMock('Drupal\Tests\Core\Access\TestAccessCheckInterface'); $this->container->set('test_access', $access_check); - $this->accessManager->addCheckService('test_access'); + $this->accessManager->addCheckService('test_access', 'access'); $route = new Route('/test-path', array(), array('_foo' => '1', '_bar' => '1')); $route2 = new Route('/test-path', array(), array('_foo' => '1', '_bar' => '2')); @@ -198,6 +206,11 @@ class AccessManagerTest extends UnitTestCase { } $this->accessManager->setChecks($this->routeCollection); + $this->argumentsResolver->expects($this->any()) + ->method('getArguments') + ->will($this->returnCallback(function ($callable, $route, $request, $account) { + return array($route); + })); $this->assertFalse($this->accessManager->check($this->routeCollection->get('test_route_1'), $request, $this->account)); $this->assertTrue($this->accessManager->check($this->routeCollection->get('test_route_2'), $request, $this->account)); @@ -353,7 +366,7 @@ class AccessManagerTest extends UnitTestCase { $this->setupAccessChecker(); $access_check = new DefinedTestAccessCheck(); $this->container->register('test_access_defined', $access_check); - $this->accessManager->addCheckService('test_access_defined', array('_test_access')); + $this->accessManager->addCheckService('test_access_defined', 'access', array('_test_access')); $request = new Request(); @@ -366,6 +379,11 @@ class AccessManagerTest extends UnitTestCase { $options = $conjunction ? array('_access_mode' => $conjunction) : array(); $route = new Route($name, array(), $requirements, $options); $route_collection->add($name, $route); + $this->argumentsResolver->expects($this->any()) + ->method('getArguments') + ->will($this->returnCallback(function ($callable, $route, $request, $account) { + return array($route, $request, $account); + })); $this->accessManager->setChecks($route_collection); $this->assertSame($this->accessManager->check($route, $request, $this->account), $expected_access); @@ -379,6 +397,11 @@ class AccessManagerTest extends UnitTestCase { public function testCheckNamedRoute() { $this->setupAccessChecker(); $this->accessManager->setChecks($this->routeCollection); + $this->argumentsResolver->expects($this->any()) + ->method('getArguments') + ->will($this->returnCallback(function ($callable, $route, $request, $account) { + return array($route, $request, $account); + })); // Tests the access with routes without parameters. $request = new Request(); @@ -444,23 +467,22 @@ class AccessManagerTest extends UnitTestCase { ->with('/test-route-1/example') ->will($this->returnValue($subrequest)); - $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->account); + $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->argumentsResolver); $this->accessManager->setContainer($this->container); $this->accessManager->setRequest(new Request()); - $access_check = $this->getMock('Drupal\Core\Access\AccessCheckInterface'); + $access_check = $this->getMock('Drupal\Tests\Core\Access\TestAccessCheckInterface'); $access_check->expects($this->any()) ->method('applies') ->will($this->returnValue(TRUE)); $access_check->expects($this->any()) ->method('access') - ->with($route, $subrequest) ->will($this->returnValue(AccessInterface::KILL)); $subrequest->attributes->set('value', 'upcasted_value'); $this->container->register('test_access', $access_check); - $this->accessManager->addCheckService('test_access'); + $this->accessManager->addCheckService('test_access', 'access'); $this->accessManager->setChecks($this->routeCollection); $this->assertFalse($this->accessManager->checkNamedRoute('test_route_1', array('value' => 'example'), $this->account)); @@ -504,23 +526,22 @@ class AccessManagerTest extends UnitTestCase { ->with('/test-route-1/example') ->will($this->returnValue($subrequest)); - $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->account); + $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->argumentsResolver); $this->accessManager->setContainer($this->container); $this->accessManager->setRequest(new Request()); - $access_check = $this->getMock('Drupal\Core\Access\AccessCheckInterface'); + $access_check = $this->getMock('Drupal\Tests\Core\Access\TestAccessCheckInterface'); $access_check->expects($this->any()) ->method('applies') ->will($this->returnValue(TRUE)); $access_check->expects($this->any()) ->method('access') - ->with($route, $subrequest) ->will($this->returnValue(AccessInterface::KILL)); $subrequest->attributes->set('value', 'upcasted_value'); $this->container->register('test_access', $access_check); - $this->accessManager->addCheckService('test_access'); + $this->accessManager->addCheckService('test_access', 'access'); $this->accessManager->setChecks($this->routeCollection); $this->assertFalse($this->accessManager->checkNamedRoute('test_route_1', array(), $this->account)); @@ -566,21 +587,26 @@ class AccessManagerTest extends UnitTestCase { $route_provider->expects($this->any()) ->method('getRouteByName') ->will($this->returnValue($route)); + $this->argumentsResolver->expects($this->any()) + ->method('getArguments') + ->will($this->returnCallback(function ($callable, $route, $request, $account) { + return array($route); + })); $request = new Request(); $container = new ContainerBuilder(); // Register a service that will return an incorrect value. - $access_check = $this->getMock('Drupal\Core\Routing\Access\AccessInterface'); + $access_check = $this->getMock('Drupal\Tests\Core\Access\TestAccessCheckInterface'); $access_check->expects($this->any()) ->method('access') ->will($this->returnValue($return_value)); $container->set('test_incorrect_value', $access_check); - $access_manager = new AccessManager($route_provider, $this->urlGenerator, $this->paramConverter); + $access_manager = new AccessManager($route_provider, $this->urlGenerator, $this->paramConverter, $this->argumentsResolver); $access_manager->setContainer($container); - $access_manager->addCheckService('test_incorrect_value'); + $access_manager->addCheckService('test_incorrect_value', 'access'); $access_manager->checkNamedRoute('test_incorrect_value', array(), $this->account, $request); } @@ -633,11 +659,18 @@ class AccessManagerTest extends UnitTestCase { * Adds a default access check service to the container and the access manager. */ protected function setupAccessChecker() { - $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->account); + $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->argumentsResolver); $this->accessManager->setContainer($this->container); $access_check = new DefaultAccessCheck(); $this->container->register('test_access_default', $access_check); - $this->accessManager->addCheckService('test_access_default', array('_access')); + $this->accessManager->addCheckService('test_access_default', 'access', array('_access')); } } + +/** + * Defines an interface with a defined access() method for mocking. + */ +interface TestAccessCheckInterface extends AccessCheckInterface { + public function access(); +} diff --git a/core/tests/Drupal/Tests/Core/Access/CustomAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Access/CustomAccessCheckTest.php index 19a8bff1307..dca56200a39 100644 --- a/core/tests/Drupal/Tests/Core/Access/CustomAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Access/CustomAccessCheckTest.php @@ -34,6 +34,13 @@ class CustomAccessCheckTest extends UnitTestCase { */ protected $controllerResolver; + /** + * The mocked arguments resolver. + * + * @var \Drupal\Core\Access\AccessArgumentsResolverInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $argumentsResolver; + public static function getInfo() { return array( 'name' => 'Custom access check', @@ -49,7 +56,8 @@ class CustomAccessCheckTest extends UnitTestCase { parent::setUp(); $this->controllerResolver = $this->getMock('Drupal\Core\Controller\ControllerResolverInterface'); - $this->accessChecker = new CustomAccessCheck($this->controllerResolver); + $this->argumentsResolver = $this->getMock('Drupal\Core\Access\AccessArgumentsResolverInterface'); + $this->accessChecker = new CustomAccessCheck($this->controllerResolver, $this->argumentsResolver); } /** @@ -63,25 +71,25 @@ class CustomAccessCheckTest extends UnitTestCase { ->with('\Drupal\Tests\Core\Access\TestController::accessDeny') ->will($this->returnValue(array(new TestController(), 'accessDeny'))); + $this->argumentsResolver->expects($this->at(0)) + ->method('getArguments') + ->will($this->returnValue(array())); + $this->controllerResolver->expects($this->at(1)) + ->method('getControllerFromDefinition') + ->with('\Drupal\Tests\Core\Access\TestController::accessAllow') + ->will($this->returnValue(array(new TestController(), 'accessAllow'))); + + $this->argumentsResolver->expects($this->at(1)) ->method('getArguments') ->will($this->returnValue(array())); $this->controllerResolver->expects($this->at(2)) - ->method('getControllerFromDefinition') - ->with('\Drupal\Tests\Core\Access\TestController::accessAllow') - ->will($this->returnValue(array(new TestController(), 'accessAllow'))); - - $this->controllerResolver->expects($this->at(3)) - ->method('getArguments') - ->will($this->returnValue(array())); - - $this->controllerResolver->expects($this->at(4)) ->method('getControllerFromDefinition') ->with('\Drupal\Tests\Core\Access\TestController::accessParameter') ->will($this->returnValue(array(new TestController(), 'accessParameter'))); - $this->controllerResolver->expects($this->at(5)) + $this->argumentsResolver->expects($this->at(2)) ->method('getArguments') ->will($this->returnValue(array('parameter' => 'TRUE'))); diff --git a/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php index 873ff0e0174..aec50c2171a 100644 --- a/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php @@ -8,13 +8,9 @@ namespace Drupal\Tests\Core\Route; use Drupal\Core\Access\AccessCheckInterface; -use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Session\UserSession; use Drupal\Tests\UnitTestCase; use Drupal\user\Access\RoleAccessCheck; -use Symfony\Component\DependencyInjection\Container; -use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; @@ -159,16 +155,14 @@ class RoleAccessCheckTest extends UnitTestCase { $collection = $this->getTestRouteCollection(); foreach ($grant_accounts as $account) { - $subrequest = Request::create($path, 'GET'); $message = sprintf('Access granted for user with the roles %s on path: %s', implode(', ', $account->getRoles()), $path); - $this->assertSame(AccessCheckInterface::ALLOW, $role_access_check->access($collection->get($path), $subrequest, $account), $message); + $this->assertSame(AccessCheckInterface::ALLOW, $role_access_check->access($collection->get($path), $account), $message); } // Check all users which don't have access. foreach ($deny_accounts as $account) { - $subrequest = Request::create($path, 'GET'); $message = sprintf('Access denied for user %s with the roles %s on path: %s', $account->id(), implode(', ', $account->getRoles()), $path); - $has_access = $role_access_check->access($collection->get($path), $subrequest, $account); + $has_access = $role_access_check->access($collection->get($path), $account); $this->assertSame(AccessCheckInterface::DENY, $has_access , $message); } }