diff --git a/core/core.services.yml b/core/core.services.yml index b7c686a..d72d30c 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -673,11 +673,11 @@ services: csrf_token: class: Drupal\Core\Access\CsrfTokenGenerator arguments: ['@private_key', '@session_manager.metadata_bag'] - access_arguments_resolver: - class: Drupal\Core\Access\AccessArgumentsResolver + access_arguments_resolver_factory: + class: Drupal\Core\Access\AccessArgumentsResolverFactory access_manager: class: Drupal\Core\Access\AccessManager - arguments: ['@router.route_provider', '@url_generator', '@paramconverter_manager', '@access_arguments_resolver', '@request_stack', '@current_user'] + arguments: ['@router.route_provider', '@paramconverter_manager', '@access_arguments_resolver_factory', '@current_user'] calls: - [setContainer, ['@service_container']] access_route_subscriber: @@ -704,13 +704,13 @@ services: - { name: access_check, applies_to: _access_theme } access_check.custom: class: Drupal\Core\Access\CustomAccessCheck - arguments: ['@controller_resolver', '@access_arguments_resolver'] + arguments: ['@controller_resolver', '@access_arguments_resolver_factory'] tags: - { name: access_check, applies_to: _custom_access } access_check.csrf: class: Drupal\Core\Access\CsrfAccessCheck tags: - - { name: access_check, applies_to: _csrf_token } + - { name: access_check, applies_to: _csrf_token, needs_incoming_request: TRUE } arguments: ['@csrf_token'] maintenance_mode: class: Drupal\Core\Site\MaintenanceMode diff --git a/core/lib/Drupal/Component/Utility/ArgumentsResolver.php b/core/lib/Drupal/Component/Utility/ArgumentsResolver.php new file mode 100644 index 0000000..ecb4ce1 --- /dev/null +++ b/core/lib/Drupal/Component/Utility/ArgumentsResolver.php @@ -0,0 +1,150 @@ +scalars = $scalars; + $this->objects = $objects; + $this->wildcards = $wildcards; + } + + /** + * {@inheritdoc} + */ + public function getArguments(callable $callable) { + $arguments = array(); + foreach ($this->getReflector($callable)->getParameters() as $parameter) { + $arguments[] = $this->getArgument($parameter); + } + return $arguments; + } + + /** + * Returns the argument value for a parameter. + * + * @param \ReflectionParameter $parameter + * The parameter of a callable to get the value for. + * + * @return mixed + * The value of the requested parameter value. + * + * @throws \RuntimeException + * Thrown when there is a missing parameter. + */ + protected function getArgument(\ReflectionParameter $parameter) { + $parameter_type_hint = $parameter->getClass(); + $parameter_name = $parameter->getName(); + + // If the argument exists and is NULL, return it, regardless of + // parameter type hint. + if (!isset($this->objects[$parameter_name]) && array_key_exists($parameter_name, $this->objects)) { + return NULL; + } + + if ($parameter_type_hint) { + // If the argument exists and complies with the type hint, return it. + if (isset($this->objects[$parameter_name]) && is_object($this->objects[$parameter_name]) && $parameter_type_hint->isInstance($this->objects[$parameter_name])) { + return $this->objects[$parameter_name]; + } + // Otherwise, resolve wildcard arguments by type matching. + foreach ($this->wildcards as $wildcard) { + if ($parameter_type_hint->isInstance($wildcard)) { + return $wildcard; + } + } + } + elseif (isset($this->scalars[$parameter_name])) { + return $this->scalars[$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('Callable "%s" requires a value for the "$%s" argument.', $function_name, $parameter->getName())); + } + +} diff --git a/core/lib/Drupal/Component/Utility/ArgumentsResolverInterface.php b/core/lib/Drupal/Component/Utility/ArgumentsResolverInterface.php new file mode 100644 index 0000000..3b27b7c --- /dev/null +++ b/core/lib/Drupal/Component/Utility/ArgumentsResolverInterface.php @@ -0,0 +1,26 @@ +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 AccessManagerInterface::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/AccessArgumentsResolverFactory.php b/core/lib/Drupal/Core/Access/AccessArgumentsResolverFactory.php new file mode 100644 index 0000000..cdef380 --- /dev/null +++ b/core/lib/Drupal/Core/Access/AccessArgumentsResolverFactory.php @@ -0,0 +1,45 @@ +getRouteObject(); + + // Defaults for the parameters defined on the route object need to be added + // to the raw arguments. + $raw_route_arguments = $route_match->getRawParameters()->all() + $route->getDefaults(); + + $upcasted_route_arguments = $route_match->getParameters()->all(); + + // Parameters which are not defined on the route object, but still are + // essential for access checking are passed as wildcards to the argument + // resolver. An access-check method with a parameter of type Route, + // RouteMatchInterface, AccountInterface or Request will receive those + // arguments regardless of the parameter name. + $wildcard_arguments = [$route, $route_match, $account]; + if (isset($request)) { + $wildcard_arguments[] = $request; + } + + return new ArgumentsResolver($raw_route_arguments, $upcasted_route_arguments, $wildcard_arguments); + } + +} diff --git a/core/lib/Drupal/Core/Access/AccessArgumentsResolverFactoryInterface.php b/core/lib/Drupal/Core/Access/AccessArgumentsResolverFactoryInterface.php new file mode 100644 index 0000000..b4c3219 --- /dev/null +++ b/core/lib/Drupal/Core/Access/AccessArgumentsResolverFactoryInterface.php @@ -0,0 +1,34 @@ +routeProvider = $route_provider; - $this->urlGenerator = $url_generator; $this->paramConverterManager = $paramconverter_manager; - $this->argumentsResolver = $arguments_resolver; - $this->requestStack = $requestStack; + $this->argumentsResolverFactory = $arguments_resolver_factory; $this->currentUser = $current_user; } /** * {@inheritdoc} */ - public function addCheckService($service_id, $service_method, array $applies_checks = array()) { + public function addCheckService($service_id, $service_method, array $applies_checks = array(), $needs_incoming_request = FALSE) { $this->checkIds[] = $service_id; $this->checkMethods[$service_id] = $service_method; + if ($needs_incoming_request) { + $this->checkNeedsRequest[$service_id] = $service_id; + } foreach ($applies_checks as $applies_check) { $this->staticRequirementMap[$applies_check][] = $service_id; } @@ -192,22 +179,17 @@ protected function applies(Route $route) { /** * {@inheritdoc} */ - public function checkNamedRoute($route_name, array $parameters = array(), AccountInterface $account = NULL, Request $route_request = NULL) { + public function checkNamedRoute($route_name, array $parameters = array(), AccountInterface $account = NULL) { try { $route = $this->routeProvider->getRouteByName($route_name, $parameters); - if (empty($route_request)) { - // Create a cloned request with fresh attributes. - $route_request = RequestHelper::duplicate($this->requestStack->getCurrentRequest(), $this->urlGenerator->generate($route_name, $parameters)); - $route_request->attributes->replace(array()); - - // Populate $route_request->attributes with both raw and converted - // parameters. - $parameters += $route->getDefaults(); - $route_request->attributes->set('_raw_variables', new ParameterBag($parameters)); - $parameters[RouteObjectInterface::ROUTE_OBJECT] = $route; - $route_request->attributes->add($this->paramConverterManager->convert($parameters)); - } - return $this->check($route, $route_request, $account); + + // ParamConverterManager relies on the route object being available + // from the parameters array. + $parameters[RouteObjectInterface::ROUTE_OBJECT] = $route; + $upcasted_parameters = $this->paramConverterManager->convert($parameters + $route->getDefaults(), new Request()); + + $route_match = new RouteMatch($route_name, $route, $upcasted_parameters, $parameters); + return $this->check($route_match, $account); } catch (RouteNotFoundException $e) { return FALSE; @@ -220,19 +202,40 @@ public function checkNamedRoute($route_name, array $parameters = array(), Accoun /** * {@inheritdoc} */ - public function check(Route $route, Request $request, AccountInterface $account = NULL) { + public function checkRequest(Request $request, AccountInterface $account = NULL) { + $route_match = RouteMatch::createFromRequest($request); + return $this->check($route_match, $account, $request); + } + + /** + * {@inheritdoc} + */ + public function check(RouteMatchInterface $route_match, AccountInterface $account = NULL, Request $request = NULL) { + $access = FALSE; + if (!isset($account)) { $account = $this->currentUser; } + $route = $route_match->getRouteObject(); $checks = $route->getOption('_access_checks') ?: array(); $conjunction = $route->getOption('_access_mode') ?: static::ACCESS_MODE_ALL; - if ($conjunction == static::ACCESS_MODE_ALL) { - return $this->checkAll($checks, $route, $request, $account); + // Filter out checks which require the incoming request. + if (!isset($request)) { + $checks = array_diff($checks, $this->checkNeedsRequest); } - else { - return $this->checkAny($checks, $route, $request, $account); + + if (!empty($checks)) { + $arguments_resolver = $this->argumentsResolverFactory->getArgumentsResolver($route_match, $account, $request); + if ($conjunction == static::ACCESS_MODE_ALL) { + return $this->checkAll($checks, $arguments_resolver); + } + else { + return $this->checkAny($checks, $arguments_resolver); + } } + + return $access; } /** @@ -240,17 +243,13 @@ public function check(Route $route, Request $request, AccountInterface $account * * @param array $checks * Contains the list of checks on the route definition. - * @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. + * @param \Drupal\Component\Utility\ArgumentsResolverInterface $arguments_resolver + * The parametrized arguments resolver instance. * * @return bool - * Returns TRUE if the user has access to the route, else FALSE. + * Returns TRUE if the user has access to the route, else FALSE. */ - protected function checkAll(array $checks, Route $route, Request $request, AccountInterface $account) { + protected function checkAll(array $checks, ArgumentsResolverInterface $arguments_resolver) { $access = FALSE; foreach ($checks as $service_id) { @@ -258,7 +257,7 @@ protected function checkAll(array $checks, Route $route, Request $request, Accou $this->loadCheck($service_id); } - $service_access = $this->performCheck($service_id, $route, $request, $account); + $service_access = $this->performCheck($service_id, $arguments_resolver); if ($service_access === AccessInterface::ALLOW) { $access = TRUE; @@ -278,17 +277,13 @@ protected function checkAll(array $checks, Route $route, Request $request, Accou * * @param array $checks * Contains the list of checks on the route definition. - * @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. + * @param \Drupal\Component\Utility\ArgumentsResolverInterface $arguments_resolver + * The parametrized arguments resolver instance. * * @return bool - * Returns TRUE if the user has access to the route, else FALSE. + * Returns TRUE if the user has access to the route, else FALSE. */ - protected function checkAny(array $checks, $route, $request, AccountInterface $account) { + protected function checkAny(array $checks, ArgumentsResolverInterface $arguments_resolver) { // No checks == deny by default. $access = FALSE; @@ -297,7 +292,7 @@ protected function checkAny(array $checks, $route, $request, AccountInterface $a $this->loadCheck($service_id); } - $service_access = $this->performCheck($service_id, $route, $request, $account); + $service_access = $this->performCheck($service_id, $arguments_resolver); if ($service_access === AccessInterface::ALLOW) { $access = TRUE; @@ -315,12 +310,8 @@ protected function checkAny(array $checks, $route, $request, AccountInterface $a * * @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. + * @param \Drupal\Component\Utility\ArgumentsResolverInterface $arguments_resolver + * The parametrized arguments resolver instance. * * @throws \Drupal\Core\Access\AccessException * Thrown when the access check returns an invalid value. @@ -328,9 +319,9 @@ protected function checkAny(array $checks, $route, $request, AccountInterface $a * @return string * A \Drupal\Core\Access\AccessInterface constant value. */ - protected function performCheck($service_id, $route, $request, $account) { + protected function performCheck($service_id, ArgumentsResolverInterface $arguments_resolver) { $callable = array($this->checks[$service_id], $this->checkMethods[$service_id]); - $arguments = $this->argumentsResolver->getArguments($callable, $route, $request, $account); + $arguments = $arguments_resolver->getArguments($callable); $service_access = call_user_func_array($callable, $arguments); if (!in_array($service_access, array(AccessInterface::ALLOW, AccessInterface::DENY, AccessInterface::KILL), TRUE)) { diff --git a/core/lib/Drupal/Core/Access/AccessManagerInterface.php b/core/lib/Drupal/Core/Access/AccessManagerInterface.php index db6cfa3..00ac6fb 100644 --- a/core/lib/Drupal/Core/Access/AccessManagerInterface.php +++ b/core/lib/Drupal/Core/Access/AccessManagerInterface.php @@ -7,10 +7,10 @@ namespace Drupal\Core\Access; -use Symfony\Component\Routing\Route; -use Symfony\Component\Routing\RouteCollection; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Routing\RouteCollection; use Drupal\Core\Session\AccountInterface; +use Drupal\Core\Routing\RouteMatchInterface; /** * Provides an interface for attaching and running access check services. @@ -47,14 +47,22 @@ * @param \Drupal\Core\Session\AccountInterface $account * (optional) Run access checks for this account. Defaults to the current * user. - * @param \Symfony\Component\HttpFoundation\Request $route_request - * Optional incoming request object. If not provided, one will be built - * using the route information and the current request from the container. * * @return bool * Returns TRUE if the user has access to the route, otherwise FALSE. */ - public function checkNamedRoute($route_name, array $parameters = array(), AccountInterface $account = NULL, Request $route_request = NULL); + public function checkNamedRoute($route_name, array $parameters = array(), AccountInterface $account = NULL); + + /** + * Execute access checks against the incoming request. + * + * @param Request $request + * The incoming request. + * @param \Drupal\Core\Session\AccountInterface $account + * (optional) Run access checks for this account. Defaults to the current + * user. + */ + public function checkRequest(Request $request, AccountInterface $account = NULL); /** * For each route, saves a list of applicable access checks to the route. @@ -74,25 +82,28 @@ public function setChecks(RouteCollection $routes); * @param array $applies_checks * (optional) An array of route requirement keys the checker service applies * to. + * @param bool $needs_incoming_request + * (optional) True if access-check method only acts on an incoming request. */ - public function addCheckService($service_id, $service_method, array $applies_checks = array()); + public function addCheckService($service_id, $service_method, array $applies_checks = array(), $needs_incoming_request = FALSE); /** * Checks a route against applicable access check services. * * Determines whether the route is accessible or not. * - * @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\Routing\RouteMatchInterface $route_match + * The route match. * @param \Drupal\Core\Session\AccountInterface $account * (optional) Run access checks for this account. Defaults to the current * user. + * @param \Symfony\Component\HttpFoundation\Request $request + * Optional, a request. Only supply this parameter when checking the + * incoming request, do not specify when checking routes on output. * * @return bool * Returns TRUE if the user has access to the route, otherwise FALSE. */ - public function check(Route $route, Request $request, AccountInterface $account = NULL); + public function check(RouteMatchInterface $route_match, AccountInterface $account = NULL, Request $request = NULL); } diff --git a/core/lib/Drupal/Core/Access/CsrfAccessCheck.php b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php index f6b60d3..c8988dc 100644 --- a/core/lib/Drupal/Core/Access/CsrfAccessCheck.php +++ b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php @@ -33,7 +33,7 @@ class CsrfAccessCheck implements RoutingAccessInterface { * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token * The CSRF token generator. */ - function __construct(CsrfTokenGenerator $csrf_token) { + public function __construct(CsrfTokenGenerator $csrf_token) { $this->csrfToken = $csrf_token; } @@ -49,25 +49,9 @@ function __construct(CsrfTokenGenerator $csrf_token) { * A \Drupal\Core\Access\AccessInterface constant value. */ public function access(Route $route, Request $request) { - // If this is the controller request, check CSRF access as normal. - if ($request->attributes->get('_controller_request')) { - // @todo Remove dependency on the internal _system_path attribute: - // https://www.drupal.org/node/2293501. - return $this->csrfToken->validate($request->query->get('token'), $request->attributes->get('_system_path')) ? static::ALLOW : static::KILL; - } - - // Otherwise, this could be another requested access check that we don't - // want to check CSRF tokens on. - $conjunction = $route->getOption('_access_mode') ?: AccessManagerInterface::ACCESS_MODE_ANY; - // Return ALLOW if all access checks are needed. - if ($conjunction == AccessManagerInterface::ACCESS_MODE_ALL) { - return static::ALLOW; - } - // Return DENY otherwise, as another access checker should grant access - // for the route. - else { - return static::DENY; - } + // @todo Remove dependency on the internal _system_path attribute: + // https://www.drupal.org/node/2293501. + 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 c128a5b..9f8664e 100644 --- a/core/lib/Drupal/Core/Access/CustomAccessCheck.php +++ b/core/lib/Drupal/Core/Access/CustomAccessCheck.php @@ -9,8 +9,8 @@ use Drupal\Core\Controller\ControllerResolverInterface; use Drupal\Core\Routing\Access\AccessInterface as RoutingAccessInterface; +use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; /** @@ -35,39 +35,39 @@ class CustomAccessCheck implements RoutingAccessInterface { /** * The arguments resolver. * - * @var \Drupal\Core\Access\AccessArgumentsResolverInterface + * @var \Drupal\Core\Access\AccessArgumentsResolverFactoryInterface */ - protected $argumentsResolver; + protected $argumentsResolverFactory; /** * Constructs a CustomAccessCheck instance. * * @param \Drupal\Core\Controller\ControllerResolverInterface $controller_resolver * The controller resolver. - * @param \Drupal\Core\Access\AccessArgumentsResolverInterface $arguments_resolver - * The arguments resolver. + * @param \Drupal\Core\Access\AccessArgumentsResolverFactoryInterface $arguments_resolver_factory + * The arguments resolver factory. */ - public function __construct(ControllerResolverInterface $controller_resolver, AccessArgumentsResolverInterface $arguments_resolver) { + public function __construct(ControllerResolverInterface $controller_resolver, AccessArgumentsResolverFactoryInterface $arguments_resolver_factory) { $this->controllerResolver = $controller_resolver; - $this->argumentsResolver = $arguments_resolver; + $this->argumentsResolverFactory = $arguments_resolver_factory; } /** * 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\Routing\RouteMatchInterface $route_match + * The route match object to be checked. * @param \Drupal\Core\Session\AccountInterface $account - * The currently logged in account. + * The account being checked. * * @return string * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Route $route, RouteMatchInterface $route_match, AccountInterface $account) { $callable = $this->controllerResolver->getControllerFromDefinition($route->getRequirement('_custom_access')); - $arguments = $this->argumentsResolver->getArguments($callable, $route, $request, $account); + $arguments_resolver = $this->argumentsResolverFactory->getArgumentsResolver($route_match, $account); + $arguments = $arguments_resolver->getArguments($callable); + return call_user_func_array($callable, $arguments); } diff --git a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php index 69494af..4864eaf 100644 --- a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php @@ -28,6 +28,7 @@ public function process(ContainerBuilder $container) { foreach ($container->findTaggedServiceIds('access_check') as $id => $attributes) { $applies = array(); $method = 'access'; + $needs_incoming_request = FALSE; foreach ($attributes as $attribute) { if (isset($attribute['applies_to'])) { $applies[] = $attribute['applies_to']; @@ -35,8 +36,11 @@ public function process(ContainerBuilder $container) { if (isset($attribute['method'])) { $method = $attribute['method']; } + if (!empty($attribute['needs_incoming_request'])) { + $needs_incoming_request = TRUE; + } } - $access_manager->addMethodCall('addCheckService', array($id, $method, $applies)); + $access_manager->addMethodCall('addCheckService', array($id, $method, $applies, $needs_incoming_request)); } } } diff --git a/core/lib/Drupal/Core/Entity/EntityAccessCheck.php b/core/lib/Drupal/Core/Entity/EntityAccessCheck.php index 951abb0..7e8f9a2 100644 --- a/core/lib/Drupal/Core/Entity/EntityAccessCheck.php +++ b/core/lib/Drupal/Core/Entity/EntityAccessCheck.php @@ -8,9 +8,9 @@ namespace Drupal\Core\Entity; use Drupal\Core\Routing\Access\AccessInterface; +use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; /** * Provides a generic access checker for entities. @@ -32,21 +32,22 @@ class EntityAccessCheck implements AccessInterface { * * @param \Symfony\Component\Routing\Route $route * The route to check against. - * @param \Symfony\Component\HttpFoundation\Request $request - * The request object. + * @param \Drupal\Core\Routing\RouteMatchInterface $route_match + * The parametrized route * @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, RouteMatchInterface $route_match, AccountInterface $account) { // Split the entity type and the operation. $requirement = $route->getRequirement('_entity_access'); list($entity_type, $operation) = explode('.', $requirement); // If there is valid entity of the given entity type, check its access. - if ($request->attributes->has($entity_type)) { - $entity = $request->attributes->get($entity_type); + $parameters = $route_match->getParameters(); + if ($parameters->has($entity_type)) { + $entity = $parameters->get($entity_type); if ($entity instanceof EntityInterface) { return $entity->access($operation, $account) ? static::ALLOW : static::DENY; } diff --git a/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php b/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php index e3fab39..fc68a97 100644 --- a/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php +++ b/core/lib/Drupal/Core/Entity/EntityCreateAccessCheck.php @@ -8,8 +8,8 @@ namespace Drupal\Core\Entity; use Drupal\Core\Routing\Access\AccessInterface; +use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; /** @@ -46,22 +46,22 @@ public function __construct(EntityManagerInterface $entity_manager) { * * @param \Symfony\Component\Routing\Route $route * The route to check against. - * @param \Symfony\Component\HttpFoundation\Request $request - * The request object. + * @param \Drupal\Core\Routing\RouteMatchInterface $route_match + * The parametrized route. * @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, RouteMatchInterface $route_match, AccountInterface $account) { list($entity_type, $bundle) = explode(':', $route->getRequirement($this->requirementsKey) . ':'); // The bundle argument can contain request argument placeholders like // {name}, loop over the raw variables and attempt to replace them in the // bundle name. If a placeholder does not exist, it won't get replaced. if ($bundle && strpos($bundle, '{') !== FALSE) { - foreach ($request->get('_raw_variables')->all() as $name => $value) { + foreach ($route_match->getRawParameters()->all() as $name => $value) { $bundle = str_replace('{' . $name . '}', $value, $bundle); } // If we were unable to replace all placeholders, deny access. diff --git a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php index b8409df..3ca0c1e 100644 --- a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php +++ b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php @@ -10,7 +10,6 @@ use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Cmf\Component\Routing\ChainRouter; -use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\Routing\RequestContext; @@ -102,21 +101,8 @@ public function matchRequest(Request $request) { * The request to access check. */ protected function checkAccess(Request $request) { - // The controller is being handled by the HTTP kernel, so add an attribute - // to tell us this is the controller request. - $request->attributes->set('_controller_request', TRUE); - $e = FALSE; - try { - if (!$this->accessManager->check($request->attributes->get(RouteObjectInterface::ROUTE_OBJECT), $request, $this->account)) { - $e = new AccessDeniedHttpException(); - } - } - catch (\Exception $e) { - } - // @todo Once PHP 5.5 is a requirement refactor this using finally. - $request->attributes->remove('_controller_request'); - if ($e) { - throw $e; + if (!$this->accessManager->checkRequest($request, $this->account)) { + throw new AccessDeniedHttpException(); } } diff --git a/core/lib/Drupal/Core/Routing/RequestHelper.php b/core/lib/Drupal/Core/Routing/RequestHelper.php index a56ed83..55d85aa 100644 --- a/core/lib/Drupal/Core/Routing/RequestHelper.php +++ b/core/lib/Drupal/Core/Routing/RequestHelper.php @@ -15,120 +15,6 @@ class RequestHelper { /** - * Duplicates a request for another path. - * - * This method does basically the same as Request::create() but keeping all - * the previous variables to speed it up. - * - * @param \Symfony\Component\HttpFoundation\Request $original_request - * The original request object to clone. - * @param string $uri - * The URI. - * @param string $method - * The HTTP method. - * @param array $parameters - * The query (GET) or request (POST) parameters. - * @param array $query - * The GET parameters. - * @param array $post - * The POST parameters. - * @param array $attributes - * The request attributes (parameters parsed from the PATH_INFO, ...). - * @param array $cookies - * The COOKIE parameters. - * @param array $files - * The FILES parameters. - * @param array $server - * The SERVER parameters. - * - * @return \Symfony\Component\HttpFoundation\Request - * The cloned request instance. - * - * @see \Symfony\Component\HttpFoundation\Request::create() - * @see \Symfony\Component\HttpFoundation\Request::duplicate() - */ - public static function duplicate(Request $original_request, $uri, $method = 'GET', $parameters = array(), array $query = NULL, array $post = NULL, array $attributes = NULL, array $cookies = NULL, array $files = NULL, array $server = NULL) { - $request = $original_request->duplicate($query, $post, $attributes, $cookies, $files, $server); - - $server = array(); - - $server['PATH_INFO'] = ''; - $server['REQUEST_METHOD'] = strtoupper($method); - - $components = parse_url($uri); - if (isset($components['host'])) { - $server['SERVER_NAME'] = $components['host']; - $server['HTTP_HOST'] = $components['host']; - } - - if (isset($components['scheme'])) { - if ('https' === $components['scheme']) { - $server['HTTPS'] = 'on'; - $server['SERVER_PORT'] = 443; - } - else { - unset($server['HTTPS']); - $server['SERVER_PORT'] = 80; - } - } - - if (isset($components['port'])) { - $server['SERVER_PORT'] = $components['port']; - $server['HTTP_HOST'] = $server['HTTP_HOST'] . ':' . $components['port']; - } - - if (isset($components['user'])) { - $server['PHP_AUTH_USER'] = $components['user']; - } - - if (isset($components['pass'])) { - $server['PHP_AUTH_PW'] = $components['pass']; - } - - if (!isset($components['path'])) { - $components['path'] = '/'; - } - - switch (strtoupper($method)) { - case 'POST': - case 'PUT': - case 'DELETE': - if (!isset($server['CONTENT_TYPE'])) { - $server['CONTENT_TYPE'] = 'application/x-www-form-urlencoded'; - } - case 'PATCH': - $post = $parameters; - $query = array(); - break; - default: - $post = array(); - $query = $parameters; - break; - } - - if (isset($components['query'])) { - parse_str(html_entity_decode($components['query']), $query_string); - $query = array_replace($query_string, $query); - } - $query_string = http_build_query($query, '', '&'); - - // Prepend a ? if there is a query string. - if ($query_string !== '') { - $query_string = '?' . $query_string; - } - - $server['REQUEST_URI'] = $components['path'] . $query_string; - $server['QUERY_STRING'] = $query_string; - $request->server->add($server); - // The 'request' attribute name corresponds to $_REQUEST, but Symfony - // documents it as holding the POST parameters. - $request->request->add($post); - $request->query->add($query); - - return $request; - } - - /** * Returns whether the request is using a clean URL. * * A clean URL is one that does not include the script name. For example, diff --git a/core/modules/config_translation/config_translation.services.yml b/core/modules/config_translation/config_translation.services.yml index f4ed510..e6c00b6 100644 --- a/core/modules/config_translation/config_translation.services.yml +++ b/core/modules/config_translation/config_translation.services.yml @@ -7,13 +7,13 @@ services: config_translation.access.overview: class: Drupal\config_translation\Access\ConfigTranslationOverviewAccess - arguments: ['@plugin.manager.config_translation.mapper'] + arguments: ['@plugin.manager.config_translation.mapper', '@language_manager'] tags: - { name: access_check, applies_to: _config_translation_overview_access } config_translation.access.form: class: Drupal\config_translation\Access\ConfigTranslationFormAccess - arguments: ['@plugin.manager.config_translation.mapper'] + arguments: ['@plugin.manager.config_translation.mapper', '@language_manager'] tags: - { name: access_check, applies_to: _config_translation_form_access } diff --git a/core/modules/config_translation/src/Access/ConfigTranslationFormAccess.php b/core/modules/config_translation/src/Access/ConfigTranslationFormAccess.php index 791ef85..f9a5116 100644 --- a/core/modules/config_translation/src/Access/ConfigTranslationFormAccess.php +++ b/core/modules/config_translation/src/Access/ConfigTranslationFormAccess.php @@ -8,7 +8,6 @@ namespace Drupal\config_translation\Access; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; /** @@ -19,12 +18,12 @@ class ConfigTranslationFormAccess extends ConfigTranslationOverviewAccess { /** * {@inheritdoc} */ - public function access(Route $route, Request $request, AccountInterface $account) { + public function access(Route $route, AccountInterface $account, $langcode = NULL) { // For the translation forms we have a target language, so we need some // checks in addition to the checks performed for the translation overview. - $base_access = parent::access($route, $request, $account); + $base_access = parent::access($route, $account); if ($base_access === static::ALLOW) { - $target_language = language_load($request->attributes->get('langcode')); + $target_language = $this->languageManager->getLanguage($langcode); // Make sure that the target language is not locked, and that the target // language is not the original submission language. Although technically @@ -33,7 +32,7 @@ public function access(Route $route, Request $request, AccountInterface $account $access = !empty($target_language) && !$target_language->locked && - $target_language->id != $this->sourceLanguage->id; + (empty($this->sourceLanguage) || ($target_language->id != $this->sourceLanguage->id)); return $access ? static::ALLOW : static::DENY; } diff --git a/core/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php b/core/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php index 529bc4a..b8203a2 100644 --- a/core/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php +++ b/core/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php @@ -7,10 +7,10 @@ namespace Drupal\config_translation\Access; +use Drupal\Core\Language\LanguageManagerInterface; use Drupal\config_translation\ConfigMapperManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; /** @@ -26,6 +26,13 @@ class ConfigTranslationOverviewAccess implements AccessInterface { protected $configMapperManager; /** + * The language manager. + * + * @var \Drupal\Core\Language\LanguageManagerInterface + */ + protected $languageManager; + + /** * The source language. * * @var \Drupal\Core\Language\LanguageInterface @@ -38,8 +45,9 @@ class ConfigTranslationOverviewAccess implements AccessInterface { * @param \Drupal\config_translation\ConfigMapperManagerInterface $config_mapper_manager * The mapper plugin discovery service. */ - public function __construct(ConfigMapperManagerInterface $config_mapper_manager) { + public function __construct(ConfigMapperManagerInterface $config_mapper_manager, LanguageManagerInterface $language_manager) { $this->configMapperManager = $config_mapper_manager; + $this->languageManager = $language_manager; } /** @@ -47,29 +55,25 @@ public function __construct(ConfigMapperManagerInterface $config_mapper_manager) * * @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) { + public function access(Route $route, AccountInterface $account) { /** @var \Drupal\config_translation\ConfigMapperInterface $mapper */ $mapper = $this->configMapperManager->createInstance($route->getDefault('plugin_id')); - $mapper->populateFromRequest($request); - - $this->sourceLanguage = $mapper->getLanguageWithFallback(); + $this->sourceLanguage = $this->languageManager->getLanguage($mapper->getLangcode()); // Allow access to the translation overview if the proper permission is // granted, the configuration has translatable pieces, and the source - // language is not locked. + // language is not locked if it is present. $access = $account->hasPermission('translate configuration') && $mapper->hasSchema() && $mapper->hasTranslatable() && - !$this->sourceLanguage->locked; + empty($this->sourceLanguage->locked); return $access ? static::ALLOW : static::DENY; } diff --git a/core/modules/content_translation/src/Access/ContentTranslationManageAccessCheck.php b/core/modules/content_translation/src/Access/ContentTranslationManageAccessCheck.php index 5f12452..b8a22a6 100644 --- a/core/modules/content_translation/src/Access/ContentTranslationManageAccessCheck.php +++ b/core/modules/content_translation/src/Access/ContentTranslationManageAccessCheck.php @@ -11,8 +11,8 @@ use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Language\LanguageManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; +use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; /** @@ -52,8 +52,8 @@ public function __construct(EntityManagerInterface $manager, LanguageManagerInte * * @param \Symfony\Component\Routing\Route $route * The route to check against. - * @param \Symfony\Component\HttpFoundation\Request $request - * The request object. + * @param \Drupal\Core\Routing\RouteMatchInterface $route_match + * The parametrized route. * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * @param string $source @@ -69,9 +69,9 @@ public function __construct(EntityManagerInterface $manager, LanguageManagerInte * @return string * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account, $source = NULL, $target = NULL, $language = NULL, $entity_type_id = NULL) { + public function access(Route $route, RouteMatchInterface $route_match, AccountInterface $account, $source = NULL, $target = NULL, $language = NULL, $entity_type_id = NULL) { /* @var \Drupal\Core\Entity\ContentEntityInterface $entity */ - if ($entity = $request->attributes->get($entity_type_id)) { + if ($entity = $route_match->getParameter($entity_type_id)) { $operation = $route->getRequirement('_access_content_translation_manage'); diff --git a/core/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php b/core/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php index 893b8b1..d3c1b0a 100644 --- a/core/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php +++ b/core/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php @@ -9,8 +9,8 @@ use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; +use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Component\HttpFoundation\Request; /** * Access check for entity translation overview. @@ -37,23 +37,25 @@ public function __construct(EntityManagerInterface $manager) { /** * Checks access to the translation overview for the entity and bundle. * - * @param \Symfony\Component\HttpFoundation\Request $request - * The request object. + * @param \Drupal\Core\Routing\RouteMatchInterface $route_match + * The parametrized route. * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. + * @param string $entity_type_id + * The entity type ID. * * @return string * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Request $request, AccountInterface $account) { - $entity_type = $request->attributes->get('entity_type_id'); - $entity = $request->attributes->get($entity_type); + public function access(RouteMatchInterface $route_match, AccountInterface $account, $entity_type_id) { + /* @var \Drupal\Core\Entity\ContentEntityInterface $entity */ + $entity = $route_match->getParameter($entity_type_id); if ($entity && $entity->isTranslatable()) { // Get entity base info. $bundle = $entity->bundle(); // Get entity access callback. - $definition = $this->entityManager->getDefinition($entity_type); + $definition = $this->entityManager->getDefinition($entity_type_id); $translation = $definition->get('translation'); $access_callback = $translation['content_translation']['access_callback']; if (call_user_func($access_callback, $entity)) { @@ -66,9 +68,9 @@ public function access(Request $request, AccountInterface $account) { } // Check per entity permission. - $permission = "translate {$entity_type}"; + $permission = "translate {$entity_type_id}"; if ($definition->getPermissionGranularity() == 'bundle') { - $permission = "translate {$bundle} {$entity_type}"; + $permission = "translate {$bundle} {$entity_type_id}"; } if ($account->hasPermission($permission)) { return static::ALLOW; diff --git a/core/modules/content_translation/tests/src/Unit/Access/ContentTranslationManageAccessCheckTest.php b/core/modules/content_translation/tests/src/Unit/Access/ContentTranslationManageAccessCheckTest.php index 7bdfc98..1048811 100644 --- a/core/modules/content_translation/tests/src/Unit/Access/ContentTranslationManageAccessCheckTest.php +++ b/core/modules/content_translation/tests/src/Unit/Access/ContentTranslationManageAccessCheckTest.php @@ -11,7 +11,6 @@ use Drupal\Core\Access\AccessInterface; use Drupal\Core\Language\Language; use Drupal\Tests\UnitTestCase; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; /** @@ -74,9 +73,12 @@ public function testCreateAccess() { $route = new Route('test_route'); $route->setRequirement('_access_content_translation_manage', 'create'); - // Set the request attributes. - $request = Request::create('node/1'); - $request->attributes->set('node', $entity); + // Set up the route match. + $route_match = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); + $route_match->expects($this->once()) + ->method('getParameter') + ->with('node') + ->will($this->returnValue($entity)); // Set the mock account. $account = $this->getMock('Drupal\Core\Session\AccountInterface'); @@ -88,7 +90,7 @@ public function testCreateAccess() { $language = 'en'; $entity_type_id = 'node'; - $this->assertEquals($check->access($route, $request, $account, $source, $target, $language, $entity_type_id), AccessInterface::ALLOW, "The access check matches"); + $this->assertEquals($check->access($route, $route_match, $account, $source, $target, $language, $entity_type_id), AccessInterface::ALLOW, "The access check matches"); } } diff --git a/core/modules/field_ui/src/Access/FormModeAccessCheck.php b/core/modules/field_ui/src/Access/FormModeAccessCheck.php index d35dd15..3a7e57a 100644 --- a/core/modules/field_ui/src/Access/FormModeAccessCheck.php +++ b/core/modules/field_ui/src/Access/FormModeAccessCheck.php @@ -9,9 +9,9 @@ use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; +use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; /** * Defines an access check for entity form mode routes. @@ -42,8 +42,8 @@ public function __construct(EntityManagerInterface $entity_manager) { * * @param \Symfony\Component\Routing\Route $route * The route to check against. - * @param \Symfony\Component\HttpFoundation\Request $request - * The request object. + * @param \Drupal\Core\Routing\RouteMatchInterface $route_match + * The parametrized route. * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * @param string $form_mode_name @@ -60,11 +60,11 @@ public function __construct(EntityManagerInterface $entity_manager) { * @return string * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account, $form_mode_name = 'default', $bundle = NULL) { + public function access(Route $route, RouteMatchInterface $route_match, AccountInterface $account, $form_mode_name = 'default', $bundle = NULL) { if ($entity_type_id = $route->getDefault('entity_type_id')) { if (!isset($bundle)) { $entity_type = $this->entityManager->getDefinition($entity_type_id); - $bundle = $request->attributes->get('_raw_variables')->get($entity_type->getBundleEntityType()); + $bundle = $route_match->getRawParameter($entity_type->getBundleEntityType()); } $visibility = FALSE; diff --git a/core/modules/field_ui/src/Access/ViewModeAccessCheck.php b/core/modules/field_ui/src/Access/ViewModeAccessCheck.php index 8e93040..4fef23e 100644 --- a/core/modules/field_ui/src/Access/ViewModeAccessCheck.php +++ b/core/modules/field_ui/src/Access/ViewModeAccessCheck.php @@ -9,9 +9,9 @@ use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; +use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\Routing\Route; -use Symfony\Component\HttpFoundation\Request; /** * Defines an access check for entity view mode routes. @@ -42,8 +42,8 @@ public function __construct(EntityManagerInterface $entity_manager) { * * @param \Symfony\Component\Routing\Route $route * The route to check against. - * @param \Symfony\Component\HttpFoundation\Request $request - * The request object. + * @param \Drupal\Core\Routing\RouteMatchInterface $route_match + * The parametrized route. * @param \Drupal\Core\Session\AccountInterface $account * The currently logged in account. * @param string $view_mode_name @@ -60,11 +60,11 @@ public function __construct(EntityManagerInterface $entity_manager) { * @return string * A \Drupal\Core\Access\AccessInterface constant value. */ - public function access(Route $route, Request $request, AccountInterface $account, $view_mode_name = 'default', $bundle = NULL) { + public function access(Route $route, RouteMatchInterface $route_match, AccountInterface $account, $view_mode_name = 'default', $bundle = NULL) { if ($entity_type_id = $route->getDefault('entity_type_id')) { if (!isset($bundle)) { $entity_type = $this->entityManager->getDefinition($entity_type_id); - $bundle = $request->attributes->get('_raw_variables')->get($entity_type->getBundleEntityType()); + $bundle = $route_match->getRawParameter($entity_type->getBundleEntityType()); } $visibility = FALSE; diff --git a/core/modules/system/src/PathBasedBreadcrumbBuilder.php b/core/modules/system/src/PathBasedBreadcrumbBuilder.php index 7f1bec6..1075ee7 100644 --- a/core/modules/system/src/PathBasedBreadcrumbBuilder.php +++ b/core/modules/system/src/PathBasedBreadcrumbBuilder.php @@ -139,11 +139,7 @@ public function build(RouteMatchInterface $route_match) { // Copy the path elements for up-casting. $route_request = $this->getRequestForPath(implode('/', $path_elements), $exclude); if ($route_request) { - $route_name = $route_request->attributes->get(RouteObjectInterface::ROUTE_NAME); - // Note that the parameters don't really matter here since we're - // passing in the request which already has the upcast attributes. - $parameters = array(); - $access = $this->accessManager->checkNamedRoute($route_name, $parameters, $this->currentUser, $route_request); + $access = $this->accessManager->checkRequest($route_request, $this->currentUser); if ($access) { $title = $this->titleResolver->getTitle($route_request, $route_request->attributes->get(RouteObjectInterface::ROUTE_OBJECT)); } diff --git a/core/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php b/core/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php index 5d24f20..3115b18 100644 --- a/core/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php +++ b/core/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php @@ -325,7 +325,7 @@ public function testBuildWithUserPath() { */ public function setupAccessManagerWithTrue() { $this->accessManager->expects($this->any()) - ->method('checkNamedRoute') + ->method('checkRequest') ->will($this->returnValue(TRUE)); } diff --git a/core/modules/toolbar/src/Controller/ToolbarController.php b/core/modules/toolbar/src/Controller/ToolbarController.php index 11d17b4..7290160 100644 --- a/core/modules/toolbar/src/Controller/ToolbarController.php +++ b/core/modules/toolbar/src/Controller/ToolbarController.php @@ -10,7 +10,6 @@ use Drupal\Core\Access\AccessInterface; use Drupal\Core\Controller\ControllerBase; use Symfony\Component\HttpFoundation\JsonResponse; -use Symfony\Component\HttpFoundation\Request; /** * Defines a controller for the toolbar module. @@ -33,8 +32,8 @@ public function subtreesJsonp() { /** * Checks access for the subtree controller. * - * @param \Symfony\Component\HttpFoundation\Request $request - * The current request. + * @param string $hash + * The hash of the toolbar subtrees. * @param string $langcode * The langcode of the requested site, NULL if none given. * @@ -42,8 +41,7 @@ public function subtreesJsonp() { * Returns AccessInterface::ALLOW when access was granted, otherwise * AccessInterface::DENY. */ - public function checkSubTreeAccess(Request $request, $langcode) { - $hash = $request->get('hash'); + public function checkSubTreeAccess($hash, $langcode) { return ($this->currentUser()->hasPermission('access toolbar') && ($hash == _toolbar_get_subtrees_hash($langcode))) ? AccessInterface::ALLOW : AccessInterface::DENY; } diff --git a/core/tests/Drupal/Tests/Component/Utility/ArgumentsResolverTest.php b/core/tests/Drupal/Tests/Component/Utility/ArgumentsResolverTest.php new file mode 100644 index 0000000..660ff2a --- /dev/null +++ b/core/tests/Drupal/Tests/Component/Utility/ArgumentsResolverTest.php @@ -0,0 +1,221 @@ +getArguments($callable); + $this->assertSame($expected, $arguments); + } + + /** + * Provides test data to testGetArgument(). + */ + public function providerTestGetArgument() { + $data = []; + + // Test an optional parameter with no provided value. + $data[] = [ + function($foo = 'foo') {}, [], [], [] , ['foo'], + ]; + + // Test an optional parameter with a provided value. + $data[] = [ + function($foo = 'foo') {}, ['foo' => 'bar'], [], [], ['bar'], + ]; + + // Test with a provided value. + $data[] = [ + function($foo) {}, ['foo' => 'bar'], [], [], ['bar'], + ]; + + // Test with an explicitly NULL value. + $data[] = [ + function($foo) {}, [], ['foo' => NULL], [], [NULL], + ]; + + // Test with a raw value that overrides the provided upcast value, since + // it is not typehinted. + $scalars = ['foo' => 'baz']; + $objects = ['foo' => new \stdClass()]; + $data[] = [ + function($foo) {}, $scalars, $objects, [], ['baz'], + ]; + + return $data; + } + + /** + * Tests getArgument() with an object. + */ + public function testGetArgumentObject() { + $callable = function(\stdClass $object) {}; + + $object = new \stdClass(); + $arguments = (new ArgumentsResolver([], ['object' => $object], []))->getArguments($callable); + $this->assertSame([$object], $arguments); + } + + /** + * Tests getArgument() with a wildcard object for a parameter with a custom name. + */ + public function testGetWildcardArgument() { + $callable = function(\stdClass $custom_name) {}; + + $object = new \stdClass(); + $arguments = (new ArgumentsResolver([], [], [$object]))->getArguments($callable); + $this->assertSame([$object], $arguments); + } + + /** + * Tests getArgument() with a Route, Request, and Account object. + */ + public function testGetArgumentOrder() { + $a1 = $this->getMock('\Drupal\Tests\Component\Utility\TestInterface1'); + $a2 = $this->getMock('\Drupal\Tests\Component\Utility\TestClass'); + $a3 = $this->getMock('\Drupal\Tests\Component\Utility\TestInterface2'); + + $objects = [ + 't1' => $a1, + 'tc' => $a2, + ]; + $wildcards = [$a3]; + $resolver = new ArgumentsResolver([], $objects, $wildcards); + + $callable = function(TestInterface1 $t1, TestClass $tc, TestInterface2 $t2) {}; + $arguments = $resolver->getArguments($callable); + $this->assertSame([$a1, $a2, $a3], $arguments); + + // Test again, but with the arguments in a different order. + $callable = function(TestInterface2 $t2, TestClass $tc, TestInterface1 $t1) {}; + $arguments = $resolver->getArguments($callable); + $this->assertSame([$a3, $a2, $a1], $arguments); + } + + /** + * Tests getArgument() with a wildcard parameter with no typehint. + * + * Without the typehint, the wildcard object will not be passed to the callable. + * + * @expectedException \RuntimeException + * @expectedExceptionMessage requires a value for the "$route" argument. + */ + public function testGetWildcardArgumentNoTypehint() { + $a = $this->getMock('\Drupal\Tests\Component\Utility\TestInterface1'); + $wildcards = [$a]; + $resolver = new ArgumentsResolver([], [], $wildcards); + + $callable = function($route) {}; + $arguments = $resolver->getArguments($callable); + $this->assertNull($arguments); + } + + /** + * Tests getArgument() with a named parameter with no typehint and a value. + * + * Without the typehint, passing a value to a named parameter will still + * receive the provided value. + */ + public function testGetArgumentRouteNoTypehintAndValue() { + $scalars = ['route' => 'foo']; + $resolver = new ArgumentsResolver($scalars, [], []); + + $callable = function($route) {}; + $arguments = $resolver->getArguments($callable); + $this->assertSame(['foo'], $arguments); + } + + /** + * Tests handleUnresolvedArgument() for a scalar argument. + * + * @expectedException \RuntimeException + * @expectedExceptionMessage requires a value for the "$foo" argument. + */ + public function testHandleNotUpcastedArgument() { + $objects = ['foo' => 'bar']; + $scalars = ['foo' => 'baz']; + $resolver = new ArgumentsResolver($scalars, $objects, []); + + $callable = function(\stdClass $foo) {}; + $arguments = $resolver->getArguments($callable); + $this->assertNull($arguments); + } + + /** + * Tests handleUnresolvedArgument() for missing arguments. + * + * @expectedException \RuntimeException + * @expectedExceptionMessage requires a value for the "$foo" argument. + * + * @dataProvider providerTestHandleUnresolvedArgument + */ + public function testHandleUnresolvedArgument($callable) { + $resolver = new ArgumentsResolver([], [], []); + $arguments = $resolver->getArguments($callable); + $this->assertNull($arguments); + } + + /** + * Provides test data to testHandleUnresolvedArgument(). + */ + public function providerTestHandleUnresolvedArgument() { + $data = []; + $data[] = [function($foo) {}]; + $data[] = [[new TestClass(), 'access']]; + $data[] = ['test_access_arguments_resolver_access']; + return $data; + } + +} + +/** + * Provides a test class. + */ +class TestClass { + public function access($foo) { + } +} + +/** + * Provides a test interface. + */ +interface TestInterface1 { +} + +/** + * Provides a different test interface. + */ +interface TestInterface2 { +} + +} + +namespace { + function test_access_arguments_resolver_access($foo) { + } +} diff --git a/core/tests/Drupal/Tests/Core/Access/AccessArgumentsResolverTest.php b/core/tests/Drupal/Tests/Core/Access/AccessArgumentsResolverTest.php deleted file mode 100644 index 2a46556..0000000 --- a/core/tests/Drupal/Tests/Core/Access/AccessArgumentsResolverTest.php +++ /dev/null @@ -1,242 +0,0 @@ -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 0f45b83..f2f39b1 100644 --- a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php @@ -10,14 +10,13 @@ use Drupal\Core\Access\AccessCheckInterface; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Routing\Access\AccessInterface; +use Drupal\Core\Routing\RouteMatch; use Drupal\Core\Access\AccessManager; use Drupal\Core\Access\DefaultAccessCheck; use Drupal\Tests\UnitTestCase; use Drupal\router_test\Access\DefinedTestAccessCheck; use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\Routing\Exception\RouteNotFoundException; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; @@ -57,13 +56,6 @@ class AccessManagerTest extends UnitTestCase { protected $routeProvider; /** - * The url generator - * - * @var \PHPUnit_Framework_MockObject_MockObject - */ - protected $urlGenerator; - - /** * The parameter converter. * * @var \Drupal\Core\ParamConverter\ParamConverterManagerInterface|\PHPUnit_Framework_MockObject_MockObject @@ -80,16 +72,9 @@ class AccessManagerTest extends UnitTestCase { /** * The access arguments resolver. * - * @var \Drupal\Core\Access\AccessArgumentsResolverInterface|\PHPUnit_Framework_MockObject_MockObject - */ - protected $argumentsResolver; - - /** - * The request stack. - * - * @var \Symfony\Component\HttpFoundation\RequestStack + * @var \Drupal\Core\Access\AccessArgumentsResolverFactoryInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $requestStack; + protected $argumentsResolverFactory; /** * @var \Drupal\Core\Session\AccountInterface|\PHPUnit_Framework_MockObject_MockObject @@ -126,20 +111,13 @@ protected function setUp() { $map[] = array('test_route_3', array(), '/test-route-3'); $map[] = array('test_route_4', array('value' => 'example'), '/test-route-4/example'); - $this->urlGenerator = $this->getMock('Symfony\Component\Routing\Generator\UrlGeneratorInterface'); - $this->urlGenerator->expects($this->any()) - ->method('generate') - ->will($this->returnValueMap($map)); - $this->paramConverter = $this->getMock('Drupal\Core\ParamConverter\ParamConverterManagerInterface'); $this->account = $this->getMock('Drupal\Core\Session\AccountInterface'); $this->currentUser = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->argumentsResolver = $this->getMock('Drupal\Core\Access\AccessArgumentsResolverInterface'); - - $this->requestStack = new RequestStack(); + $this->argumentsResolverFactory = $this->getMock('Drupal\Core\Access\AccessArgumentsResolverFactoryInterface'); - $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->argumentsResolver, $this->requestStack, $this->currentUser); + $this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser); $this->accessManager->setContainer($this->container); } @@ -168,7 +146,7 @@ public function testSetChecks() { */ public function testSetChecksWithDynamicAccessChecker() { // Setup the access manager. - $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->argumentsResolver, $this->requestStack, $this->currentUser); + $this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser); $this->accessManager->setContainer($this->container); // Setup the dynamic access checker. @@ -198,31 +176,32 @@ public function testSetChecksWithDynamicAccessChecker() { * Tests \Drupal\Core\Access\AccessManager::check(). */ public function testCheck() { - $request = new Request(); + $route_matches = []; - // Check check without any access checker defined yet. - foreach ($this->routeCollection->all() as $route) { - $this->assertFalse($this->accessManager->check($route, $request, $this->account)); + // Construct route match objects. + foreach ($this->routeCollection->all() as $route_name => $route) { + $route_matches[$route_name] = new RouteMatch($route_name, $route, [], []); + } + + // Test check without any access checker defined yet. + foreach ($route_matches as $route_match) { + $this->assertFalse($this->accessManager->check($route_match, $this->account)); } $this->setupAccessChecker(); // An access checker got setup, but the routes haven't been setup using // setChecks. - foreach ($this->routeCollection->all() as $route) { - $this->assertFalse($this->accessManager->check($route, $request, $this->account)); + foreach ($route_matches as $route_match) { + $this->assertFalse($this->accessManager->check($route_match, $this->account)); } $this->accessManager->setChecks($this->routeCollection); - $this->argumentsResolver->expects($this->any()) - ->method('getArguments') - ->will($this->returnCallback(function ($callable, $route, $request, $account) { - return array($route); - })); + $this->setupAccessArgumentsResolverFactory(); - $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)); - $this->assertFalse($this->accessManager->check($this->routeCollection->get('test_route_3'), $request, $this->account)); + $this->assertFalse($this->accessManager->check($route_matches['test_route_1'], $this->account)); + $this->assertTrue($this->accessManager->check($route_matches['test_route_2'], $this->account)); + $this->assertFalse($this->accessManager->check($route_matches['test_route_3'], $this->account)); } /** @@ -233,15 +212,16 @@ public function testCheck() { public function testCheckWithNullAccount() { $this->setupAccessChecker(); $this->accessManager->setChecks($this->routeCollection); - $request = new Request; + $route = $this->routeCollection->get('test_route_2'); - $this->argumentsResolver->expects($this->once()) - ->method('getArguments') - ->with(array($this->container->get('test_access_default'), 'access'), $route, $request, $this->currentUser) - ->will($this->returnCallback(function ($callable, $route, $request, $account) { - return array($route); - })); - $this->accessManager->check($route, $request); + $route_match = new RouteMatch('test_route_2', $route, [], []); + + // Asserts that the current user is passed to the access arguments resolver + // factory. + $this->setupAccessArgumentsResolverFactory() + ->with($route_match, $this->currentUser, NULL); + + $this->assertTrue($this->accessManager->check($route_match)); } /** @@ -395,8 +375,6 @@ public function testCheckConjunctions($conjunction, $name, $condition_one, $cond $this->container->register('test_access_defined', $access_check); $this->accessManager->addCheckService('test_access_defined', 'access', array('_test_access')); - $request = new Request(); - $route_collection = new RouteCollection(); // Setup a test route for each access configuration. $requirements = array( @@ -406,14 +384,12 @@ public function testCheckConjunctions($conjunction, $name, $condition_one, $cond $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); + $this->setupAccessArgumentsResolverFactory(); + + $route_match = new RouteMatch($name, $route, array(), array()); + $this->assertSame($this->accessManager->check($route_match, $this->account), $expected_access); } /** @@ -424,25 +400,7 @@ public function testCheckConjunctions($conjunction, $name, $condition_one, $cond 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(); - $this->assertTrue($this->accessManager->checkNamedRoute('test_route_2', array(), $this->account, $request)); - $this->assertFalse($this->accessManager->checkNamedRoute('test_route_3', array(), $this->account, $request)); - - // Tests the access with routes with parameters with given request. - $request = new Request(); - $request->attributes->set('value', 'example'); - $request->attributes->set('value2', 'example2'); - $this->assertTrue($this->accessManager->checkNamedRoute('test_route_4', array(), $this->account, $request)); - - // Tests the access with routes without given request. - $this->requestStack->push(new Request()); + $this->setupAccessArgumentsResolverFactory(); $this->paramConverter->expects($this->at(0)) ->method('convert') @@ -478,28 +436,19 @@ public function testCheckNamedRouteWithUpcastedValues() { $map = array(); $map[] = array('test_route_1', array('value' => 'example'), '/test-route-1/example'); - $this->urlGenerator = $this->getMock('Symfony\Component\Routing\Generator\UrlGeneratorInterface'); - $this->urlGenerator->expects($this->any()) - ->method('generate') - ->will($this->returnValueMap($map)); - $this->paramConverter = $this->getMock('Drupal\Core\ParamConverter\ParamConverterManagerInterface'); $this->paramConverter->expects($this->at(0)) ->method('convert') ->with(array('value' => 'example', RouteObjectInterface::ROUTE_OBJECT => $route)) ->will($this->returnValue(array('value' => 'upcasted_value'))); - $this->argumentsResolver->expects($this->atLeastOnce()) - ->method('getArguments') - ->will($this->returnCallback(function ($callable, $route, $request, $account) { - return array($route); + $this->setupAccessArgumentsResolverFactory($this->once()) + ->with($this->callback(function ($route_match) { + return $route_match->getParameters()->get('value') == 'upcasted_value'; })); - $subrequest = Request::create('/test-route-1/example'); - - $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->argumentsResolver, $this->requestStack, $this->currentUser); + $this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser); $this->accessManager->setContainer($this->container); - $this->requestStack->push(new Request()); $access_check = $this->getMock('Drupal\Tests\Core\Access\TestAccessCheckInterface'); $access_check->expects($this->atLeastOnce()) @@ -509,7 +458,6 @@ public function testCheckNamedRouteWithUpcastedValues() { ->method('access') ->will($this->returnValue(AccessInterface::KILL)); - $subrequest->attributes->set('value', 'upcasted_value'); $this->container->set('test_access', $access_check); $this->accessManager->addCheckService('test_access', 'access'); @@ -518,7 +466,7 @@ public function testCheckNamedRouteWithUpcastedValues() { $this->assertFalse($this->accessManager->checkNamedRoute('test_route_1', array('value' => 'example'), $this->account)); } - /** + /** * Tests the checkNamedRoute with default values. * * @covers \Drupal\Core\Access\AccessManager::checkNamedRoute() @@ -537,29 +485,19 @@ public function testCheckNamedRouteWithDefaultValue() { $map = array(); $map[] = array('test_route_1', array('value' => 'example'), '/test-route-1/example'); - $this->urlGenerator = $this->getMock('Symfony\Component\Routing\Generator\UrlGeneratorInterface'); - $this->urlGenerator->expects($this->any()) - ->method('generate') - ->with('test_route_1', array()) - ->will($this->returnValueMap($map)); - $this->paramConverter = $this->getMock('Drupal\Core\ParamConverter\ParamConverterManagerInterface'); $this->paramConverter->expects($this->at(0)) ->method('convert') ->with(array('value' => 'example', RouteObjectInterface::ROUTE_OBJECT => $route)) ->will($this->returnValue(array('value' => 'upcasted_value'))); - $this->argumentsResolver->expects($this->atLeastOnce()) - ->method('getArguments') - ->will($this->returnCallback(function ($callable, $route, $request, $account) { - return array($route); + $this->setupAccessArgumentsResolverFactory($this->once()) + ->with($this->callback(function ($route_match) { + return $route_match->getParameters()->get('value') == 'upcasted_value'; })); - $subrequest = Request::create('/test-route-1/example'); - - $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->argumentsResolver, $this->requestStack, $this->currentUser); + $this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser); $this->accessManager->setContainer($this->container); - $this->requestStack->push(new Request()); $access_check = $this->getMock('Drupal\Tests\Core\Access\TestAccessCheckInterface'); $access_check->expects($this->atLeastOnce()) @@ -569,7 +507,6 @@ public function testCheckNamedRouteWithDefaultValue() { ->method('access') ->will($this->returnValue(AccessInterface::KILL)); - $subrequest->attributes->set('value', 'upcasted_value'); $this->container->set('test_access', $access_check); $this->accessManager->addCheckService('test_access', 'access'); @@ -618,13 +555,13 @@ public function testCheckException($return_value, $access_mode) { $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(); + $this->paramConverter = $this->getMock('Drupal\Core\ParamConverter\ParamConverterManagerInterface'); + $this->paramConverter->expects($this->any()) + ->method('convert') + ->will($this->returnValue(array())); + + $this->setupAccessArgumentsResolverFactory(); $container = new ContainerBuilder(); @@ -635,11 +572,11 @@ public function testCheckException($return_value, $access_mode) { ->will($this->returnValue($return_value)); $container->set('test_incorrect_value', $access_check); - $access_manager = new AccessManager($route_provider, $this->urlGenerator, $this->paramConverter, $this->argumentsResolver, $this->requestStack, $this->currentUser); + $access_manager = new AccessManager($route_provider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser); $access_manager->setContainer($container); $access_manager->addCheckService('test_incorrect_value', 'access'); - $access_manager->checkNamedRoute('test_incorrect_value', array(), $this->account, $request); + $access_manager->checkNamedRoute('test_incorrect_value', array(), $this->account); } /** @@ -690,13 +627,33 @@ protected static function convertAccessCheckInterfaceToString($constant) { * 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->argumentsResolver, $this->requestStack, $this->currentUser); + $this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser); $this->accessManager->setContainer($this->container); $access_check = new DefaultAccessCheck(); $this->container->register('test_access_default', $access_check); $this->accessManager->addCheckService('test_access_default', 'access', array('_access')); } + /** + * Add default expectations to the access arguments resolver factory. + */ + protected function setupAccessArgumentsResolverFactory($constraint = NULL) { + if (!isset($constraint)) { + $constraint = $this->any(); + } + return $this->argumentsResolverFactory->expects($constraint) + ->method('getArgumentsResolver') + ->will($this->returnCallback(function ($route_match, $account) { + $resolver = $this->getMock('Drupal\Component\Utility\ArgumentsResolverInterface'); + $resolver->expects($this->any()) + ->method('getArguments') + ->will($this->returnCallback(function ($callable) use ($route_match) { + return array($route_match->getRouteObject()); + })); + return $resolver; + })); + } + } /** diff --git a/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php index 55c47ec..3ec5d21 100644 --- a/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php @@ -7,7 +7,6 @@ namespace Drupal\Tests\Core\Access; -use Drupal\Core\Access\AccessManagerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; use Drupal\Core\Access\CsrfAccessCheck; @@ -63,8 +62,6 @@ public function testAccessTokenPass() { $route = new Route('/test-path', array(), array('_csrf_token' => 'TRUE')); $request = Request::create('/test-path?token=test_query'); $request->attributes->set('_system_path', '/test-path'); - // Set the _controller_request flag so tokens are validated. - $request->attributes->set('_controller_request', TRUE); $this->assertSame(AccessInterface::ALLOW, $this->accessCheck->access($route, $request, $this->account)); } @@ -81,48 +78,8 @@ public function testAccessTokenFail() { $route = new Route('/test-path', array(), array('_csrf_token' => 'TRUE')); $request = Request::create('/test-path?token=test_query'); $request->attributes->set('_system_path', '/test-path'); - // Set the _controller_request flag so tokens are validated. - $request->attributes->set('_controller_request', TRUE); $this->assertSame(AccessInterface::KILL, $this->accessCheck->access($route, $request, $this->account)); } - /** - * Tests the access() method with no _controller_request attribute set. - * - * This will default to the AccessManagerInterface::ACCESS_MODE_ANY access conjunction. - * - * @see Drupal\Core\Access\AccessManagerInterface::ACCESS_MODE_ANY - */ - public function testAccessTokenMissAny() { - $this->csrfToken->expects($this->never()) - ->method('validate'); - - $route = new Route('/test-path', array(), array('_csrf_token' => 'TRUE')); - $request = new Request(array( - 'token' => 'test_query', - )); - - $this->assertSame(AccessInterface::DENY, $this->accessCheck->access($route, $request, $this->account)); - } - - /** - * Tests the access() method with no _controller_request attribute set. - * - * This will use the AccessManagerInterface::ACCESS_MODE_ALL access conjunction. - * - * @see Drupal\Core\Access\AccessManagerInterface::ACCESS_MODE_ALL - */ - public function testAccessTokenMissAll() { - $this->csrfToken->expects($this->never()) - ->method('validate'); - - $route = new Route('/test-path', array(), array('_csrf_token' => 'TRUE'), array('_access_mode' => AccessManagerInterface::ACCESS_MODE_ALL)); - $request = new Request(array( - 'token' => 'test_query', - )); - - $this->assertSame(AccessInterface::ALLOW, $this->accessCheck->access($route, $request, $this->account)); - } - } diff --git a/core/tests/Drupal/Tests/Core/Access/CustomAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Access/CustomAccessCheckTest.php index 23f500d..1322391 100644 --- a/core/tests/Drupal/Tests/Core/Access/CustomAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Access/CustomAccessCheckTest.php @@ -10,7 +10,6 @@ use Drupal\Core\Access\AccessInterface; use Drupal\Core\Access\CustomAccessCheck; use Drupal\Tests\UnitTestCase; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; /** @@ -36,9 +35,9 @@ class CustomAccessCheckTest extends UnitTestCase { /** * The mocked arguments resolver. * - * @var \Drupal\Core\Access\AccessArgumentsResolverInterface|\PHPUnit_Framework_MockObject_MockObject + * @var \Drupal\Core\Access\AccessArgumentsResolverFactoryInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $argumentsResolver; + protected $argumentsResolverFactory; /** * {@inheritdoc} @@ -47,52 +46,64 @@ protected function setUp() { parent::setUp(); $this->controllerResolver = $this->getMock('Drupal\Core\Controller\ControllerResolverInterface'); - $this->argumentsResolver = $this->getMock('Drupal\Core\Access\AccessArgumentsResolverInterface'); - $this->accessChecker = new CustomAccessCheck($this->controllerResolver, $this->argumentsResolver); + $this->argumentsResolverFactory = $this->getMock('Drupal\Core\Access\AccessArgumentsResolverFactoryInterface'); + $this->accessChecker = new CustomAccessCheck($this->controllerResolver, $this->argumentsResolverFactory); } /** * Test the access method. */ public function testAccess() { - $request = new Request(array()); + $route_match = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); $this->controllerResolver->expects($this->at(0)) ->method('getControllerFromDefinition') ->with('\Drupal\Tests\Core\Access\TestController::accessDeny') ->will($this->returnValue(array(new TestController(), 'accessDeny'))); - $this->argumentsResolver->expects($this->at(0)) + $resolver0 = $this->getMock('Drupal\Component\Utility\ArgumentsResolverInterface'); + $resolver0->expects($this->once()) ->method('getArguments') ->will($this->returnValue(array())); + $this->argumentsResolverFactory->expects($this->at(0)) + ->method('getArgumentsResolver') + ->will($this->returnValue($resolver0)); $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)) + $resolver1 = $this->getMock('Drupal\Component\Utility\ArgumentsResolverInterface'); + $resolver1->expects($this->once()) ->method('getArguments') ->will($this->returnValue(array())); + $this->argumentsResolverFactory->expects($this->at(1)) + ->method('getArgumentsResolver') + ->will($this->returnValue($resolver1)); $this->controllerResolver->expects($this->at(2)) ->method('getControllerFromDefinition') ->with('\Drupal\Tests\Core\Access\TestController::accessParameter') ->will($this->returnValue(array(new TestController(), 'accessParameter'))); - $this->argumentsResolver->expects($this->at(2)) + $resolver2 = $this->getMock('Drupal\Component\Utility\ArgumentsResolverInterface'); + $resolver2->expects($this->once()) ->method('getArguments') ->will($this->returnValue(array('parameter' => 'TRUE'))); + $this->argumentsResolverFactory->expects($this->at(2)) + ->method('getArgumentsResolver') + ->will($this->returnValue($resolver2)); $route = new Route('/test-route', array(), array('_custom_access' => '\Drupal\Tests\Core\Access\TestController::accessDeny')); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertSame(AccessInterface::DENY, $this->accessChecker->access($route, $request, $account)); + $this->assertSame(AccessInterface::DENY, $this->accessChecker->access($route, $route_match, $account)); $route = new Route('/test-route', array(), array('_custom_access' => '\Drupal\Tests\Core\Access\TestController::accessAllow')); - $this->assertSame(AccessInterface::ALLOW, $this->accessChecker->access($route, $request, $account)); + $this->assertSame(AccessInterface::ALLOW, $this->accessChecker->access($route, $route_match, $account)); $route = new Route('/test-route', array('parameter' => 'TRUE'), array('_custom_access' => '\Drupal\Tests\Core\Access\TestController::accessParameter')); - $this->assertSame(AccessInterface::ALLOW, $this->accessChecker->access($route, $request, $account)); + $this->assertSame(AccessInterface::ALLOW, $this->accessChecker->access($route, $route_match, $account)); } } diff --git a/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php index ba5ffcf..7c73d63 100644 --- a/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php @@ -7,7 +7,7 @@ namespace Drupal\Tests\Core\Entity; -use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\ParameterBag; use Symfony\Component\Routing\Route; use Drupal\Core\Access\AccessCheckInterface; use Drupal\Core\Entity\EntityAccessCheck; @@ -25,7 +25,11 @@ class EntityAccessCheckTest extends UnitTestCase { */ public function testAccess() { $route = new Route('/foo', array(), array('_entity_access' => 'node.update')); - $request = new Request(); + $upcasted_arguments = new ParameterBag(); + $route_match = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); + $route_match->expects($this->once()) + ->method('getParameters') + ->will($this->returnValue($upcasted_arguments)); $node = $this->getMockBuilder('Drupal\node\Entity\Node') ->disableOriginalConstructor() ->getMock(); @@ -33,9 +37,9 @@ public function testAccess() { ->method('access') ->will($this->returnValue(TRUE)); $access_check = new EntityAccessCheck(); - $request->attributes->set('node', $node); + $upcasted_arguments->set('node', $node); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $access = $access_check->access($route, $request, $account); + $access = $access_check->access($route, $route_match, $account); $this->assertSame(AccessCheckInterface::ALLOW, $access); } diff --git a/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php index bf2ebf3..6ccba22 100644 --- a/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php @@ -11,7 +11,6 @@ use Drupal\Core\Entity\EntityCreateAccessCheck; use Drupal\Tests\UnitTestCase; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; -use Symfony\Component\HttpFoundation\Request; /** * @coversDefaultClass \Drupal\Core\Entity\EntityCreateAccessCheck @@ -86,17 +85,18 @@ public function testAccess($entity_bundle, $requirement, $access, $expected) { ->with('_entity_create_access') ->will($this->returnValue($requirement)); - $request = new Request(); $raw_variables = new ParameterBag(); if ($entity_bundle) { - // Add the bundle as a raw variable and an upcasted attribute. - $request->attributes->set('bundle_argument', new \stdClass()); $raw_variables->set('bundle_argument', $entity_bundle); } - $request->attributes->set('_raw_variables', $raw_variables); + + $route_match = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); + $route_match->expects($this->any()) + ->method('getRawParameters') + ->will($this->returnValue($raw_variables)); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertEquals($expected, $applies_check->access($route, $request, $account)); + $this->assertEquals($expected, $applies_check->access($route, $route_match, $account)); } } diff --git a/core/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php b/core/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php index edd14ec..b8fee18 100644 --- a/core/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php @@ -342,8 +342,8 @@ public function testGetContextualLinksArrayByGroupAccessCheck() { $this->accessManager->expects($this->any()) ->method('checkNamedRoute') ->will($this->returnValueMap(array( - array('test_route', array('key' => 'value'), $this->account, NULL, TRUE), - array('test_route2', array('key' => 'value'), $this->account, NULL, FALSE), + array('test_route', array('key' => 'value'), $this->account, TRUE), + array('test_route2', array('key' => 'value'), $this->account, FALSE), ))); // Set up mocking of the plugin factory. diff --git a/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php b/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php index cbd81f5..f75b188 100644 --- a/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php +++ b/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php @@ -154,10 +154,10 @@ public function testCheckAccess() { $this->accessManager->expects($this->exactly(4)) ->method('checkNamedRoute') ->will($this->returnValueMap(array( - array('example1', array(), $this->currentUser, NULL, FALSE), - array('example2', array('foo' => 'bar'), $this->currentUser, NULL, TRUE), - array('example3', array('baz' => 'qux'), $this->currentUser, NULL, FALSE), - array('example5', array(), $this->currentUser, NULL, TRUE), + array('example1', array(), $this->currentUser, FALSE), + array('example2', array('foo' => 'bar'), $this->currentUser, TRUE), + array('example3', array('baz' => 'qux'), $this->currentUser, FALSE), + array('example5', array(), $this->currentUser, TRUE), ))); $this->mockTree(); @@ -343,11 +343,11 @@ public function testCheckNodeAccess() { // Ensure that the access manager is just called for the non-node routes. $this->accessManager->expects($this->at(0)) ->method('checkNamedRoute') - ->with('test_route', [], $this->currentUser, NULL) + ->with('test_route', [], $this->currentUser) ->willReturn(TRUE); $this->accessManager->expects($this->at(1)) ->method('checkNamedRoute') - ->with('test_route', [], $this->currentUser, NULL) + ->with('test_route', [], $this->currentUser) ->willReturn(FALSE); $tree = $this->defaultMenuTreeManipulators->checkAccess($tree); diff --git a/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php b/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php index 846e1ed..84b1661 100644 --- a/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php @@ -72,10 +72,11 @@ protected function setupRouter() { */ public function testMatchRequestAllowed() { $this->setupRouter(); + $request = new Request(); $this->accessManager->expects($this->once()) - ->method('check') + ->method('checkRequest') + ->with($request) ->will($this->returnValue(TRUE)); - $request = new Request(); $parameters = $this->router->matchRequest($request); $this->assertSame($request->attributes->all(), array(RouteObjectInterface::ROUTE_OBJECT => $this->route)); $this->assertSame($parameters, array(RouteObjectInterface::ROUTE_OBJECT => $this->route)); @@ -88,10 +89,12 @@ public function testMatchRequestAllowed() { */ public function testMatchRequestDenied() { $this->setupRouter(); + $request = new Request(); $this->accessManager->expects($this->once()) - ->method('check') + ->method('checkRequest') + ->with($request) ->will($this->returnValue(FALSE)); - $this->router->matchRequest(new Request()); + $this->router->matchRequest($request); } /**