From 531f95eb455507dc5a0a7043dae8f46c3b5426ad Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Thu, 19 Mar 2015 12:34:11 +0000 Subject: [PATCH] Issue #2286971 by znerol, Berdir, almaudoh, cilefen: Remove dependency of current_user on request and authentication manager --- core/authorize.php | 11 +- core/core.services.yml | 8 +- .../Authentication/AuthenticationManager.php | 288 +++++++++++------- .../AuthenticationManagerInterface.php | 23 -- ...thenticationProviderChallengeInterface.php | 34 +++ .../AuthenticationProviderFilterInterface.php | 39 +++ .../AuthenticationProviderInterface.php | 36 +-- .../Core/Authentication/Provider/Cookie.php | 36 ++- core/lib/Drupal/Core/DrupalKernel.php | 10 + .../AuthenticationSubscriber.php | 120 ++++++-- .../SpecialAttributesRouteSubscriber.php | 2 - .../Drupal/Core/Routing/AccessAwareRouter.php | 4 - .../Enhancer/AuthenticationEnhancer.php | 81 ----- core/lib/Drupal/Core/Session/AccountProxy.php | 77 +++-- .../Core/Session/AccountProxyInterface.php | 12 +- .../Drupal/Core/Session/SessionHandler.php | 2 +- .../Drupal/Core/Session/SessionManager.php | 2 +- .../Drupal/Core/StackMiddleware/Session.php | 4 +- .../src/Authentication/Provider/BasicAuth.php | 32 +- .../block/src/Tests/BlockLanguageTest.php | 4 + .../early_translation_test/src/Auth.php | 13 - core/modules/rest/rest.services.yml | 1 + .../rest/src/Access/CSRFAccessCheck.php | 22 +- core/modules/rest/src/Tests/AuthTest.php | 14 +- core/modules/rest/src/Tests/CsrfTest.php | 3 - core/modules/user/user.services.yml | 1 + .../AuthenticationManagerTest.php | 88 ++++++ .../SpecialAttributesRouteSubscriberTest.php | 1 - 28 files changed, 583 insertions(+), 385 deletions(-) delete mode 100644 core/lib/Drupal/Core/Authentication/AuthenticationManagerInterface.php create mode 100644 core/lib/Drupal/Core/Authentication/AuthenticationProviderChallengeInterface.php create mode 100644 core/lib/Drupal/Core/Authentication/AuthenticationProviderFilterInterface.php delete mode 100644 core/lib/Drupal/Core/Routing/Enhancer/AuthenticationEnhancer.php create mode 100644 core/tests/Drupal/Tests/Core/Authentication/AuthenticationManagerTest.php diff --git a/core/authorize.php b/core/authorize.php index 0d61004466c..35277f6b379 100644 --- a/core/authorize.php +++ b/core/authorize.php @@ -47,10 +47,17 @@ const MAINTENANCE_MODE = 'update'; * The killswitch in settings.php overrides all else, otherwise, the user must * have access to the 'administer software updates' permission. * + * @param \Symfony\Component\HttpFoundation\Request $request + * The incoming request. + * * @return bool * TRUE if the current user can run authorize.php, and FALSE if not. */ -function authorize_access_allowed() { +function authorize_access_allowed(Request $request) { + $account = \Drupal::service('authentication')->authenticate($request); + if ($account) { + \Drupal::currentUser()->setAccount($account); + } return Settings::get('allow_authorize_operations', TRUE) && \Drupal::currentUser()->hasPermission('administer software updates'); } @@ -79,7 +86,7 @@ $content = []; $show_messages = TRUE; $response = new Response(); -if (authorize_access_allowed()) { +if (authorize_access_allowed($request)) { // Load both the Form API and Batch API. require_once __DIR__ . '/includes/form.inc'; require_once __DIR__ . '/includes/batch.inc'; diff --git a/core/core.services.yml b/core/core.services.yml index 6681e7daa71..4e42b3ee878 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -733,11 +733,6 @@ services: tags: - { name: route_enhancer } - { name: event_subscriber } - route_enhancer.authentication: - class: Drupal\Core\Routing\Enhancer\AuthenticationEnhancer - tags: - - { name: route_enhancer, priority: 1000 } - arguments: ['@authentication', '@current_user'] route_enhancer.entity: class: Drupal\Core\Entity\Enhancer\EntityRouteEnhancer tags: @@ -1110,15 +1105,14 @@ services: - { name: service_collector, tag: authentication_provider, call: addProvider } authentication_subscriber: class: Drupal\Core\EventSubscriber\AuthenticationSubscriber + arguments: ['@authentication', '@current_user'] tags: - { name: event_subscriber } - arguments: ['@authentication'] account_switcher: class: Drupal\Core\Session\AccountSwitcher arguments: ['@current_user', '@session_handler.write_safe'] current_user: class: Drupal\Core\Session\AccountProxy - arguments: ['@authentication', '@request_stack'] session_configuration: class: Drupal\Core\Session\SessionConfiguration arguments: ['%session.storage.options%'] diff --git a/core/lib/Drupal/Core/Authentication/AuthenticationManager.php b/core/lib/Drupal/Core/Authentication/AuthenticationManager.php index 3bc40d8f7ef..28860307974 100644 --- a/core/lib/Drupal/Core/Authentication/AuthenticationManager.php +++ b/core/lib/Drupal/Core/Authentication/AuthenticationManager.php @@ -8,28 +8,24 @@ namespace Drupal\Core\Authentication; use Drupal\Core\Routing\RouteMatch; -use Drupal\Core\Session\AnonymousUserSession; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; /** * Manager for authentication. * * On each request, let all authentication providers try to authenticate the * user. The providers are iterated according to their priority and the first - * provider detecting credentials for its method will become the triggered - * provider. No further provider will get triggered. + * provider detecting credentials for its method wins. No further provider will + * get triggered. * - * If no provider was triggered the lowest-priority provider is assumed to - * be responsible. If no provider set an active user then the user is set to - * anonymous. + * If no provider set an active user then the user is set to anonymous. */ -class AuthenticationManager implements AuthenticationProviderInterface, AuthenticationManagerInterface { +class AuthenticationManager implements AuthenticationProviderInterface, AuthenticationProviderFilterInterface, AuthenticationProviderChallengeInterface { /** * Array of all registered authentication providers, keyed by ID. * - * @var array + * @var \Drupal\Core\Authentication\AuthenticationProviderInterface[] */ protected $providers; @@ -43,16 +39,45 @@ class AuthenticationManager implements AuthenticationProviderInterface, Authenti /** * Sorted list of registered providers. * - * @var array + * @var \Drupal\Core\Authentication\AuthenticationProviderInterface[] */ protected $sortedProviders; /** - * Id of the provider that authenticated the user. + * List of providers which implement the filter interface. * - * @var string + * @var \Drupal\Core\Authentication\AuthenticationProviderFilterInterface[] */ - protected $triggeredProviderId = ''; + protected $filters; + + /** + * List of providers which implement the challenge interface. + * + * @var \Drupal\Core\Authentication\AuthenticationProviderChallengeInterface[] + */ + protected $challengers; + + /** + * List of providers which are allowed on routes with no _auth option. + * + * @var string[] + */ + protected $globalProviders; + + /** + * Constructs an authentication manager. + * + * @todo Revisit service construction. Especially write a custom compiler pass + * which is capable of collecting, sorting and injecting all providers + * (including global/vs non global), filters and challengers on compile + * time in https://www.drupal.org/node/2432585. + * + * @param array $global_providers + * List of global providers, keyed by the provier ID. + */ + public function __construct($global_providers = ['cookie' => TRUE]) { + $this->globalProviders = $global_providers; + } /** * Adds a provider to the array of registered providers. @@ -72,70 +97,166 @@ class AuthenticationManager implements AuthenticationProviderInterface, Authenti $this->providerOrders[$priority][$id] = $provider; // Force the builders to be re-sorted. $this->sortedProviders = NULL; + + if ($provider instanceof AuthenticationProviderFilterInterface) { + $this->filters[$id] = $provider; + } + if ($provider instanceof AuthenticationProviderChallengeInterface) { + $this->challengers[$id] = $provider; + } } /** * {@inheritdoc} */ public function applies(Request $request) { - return TRUE; + return (bool) $this->getProvider($request); } /** * {@inheritdoc} */ public function authenticate(Request $request) { - $account = NULL; - - // Iterate the allowed providers. - foreach ($this->filterProviders($this->getSortedProviders(), $request) as $provider_id => $provider) { - if ($provider->applies($request)) { - // Try to authenticate with this provider, skipping all others. - $account = $provider->authenticate($request); - $this->triggeredProviderId = $provider_id; - break; - } - } - - // No provider returned a valid account, so set the user to anonymous. - if (!$account) { - $account = new AnonymousUserSession(); - } - - // No provider was fired, so assume the one with the least priority - // should have. - if (!$this->triggeredProviderId) { - $this->triggeredProviderId = $this->defaultProviderId(); - } - - // Save the authenticated account and the provider that supplied it - // for later access. - $request->attributes->set('_authentication_provider', $this->triggeredProviderId); - - return $account; + $provider_id = $this->getProvider($request); + return $this->providers[$provider_id]->authenticate($request); } /** - * Returns the default provider ID. - * - * The default provider is the one with the lowest registered priority. - * - * @return string - * The ID of the default provider. + * {@inheritdoc} */ - public function defaultProviderId() { - $providers = $this->getSortedProviders(); - $provider_ids = array_keys($providers); - return end($provider_ids); + public function appliesToRoutedRequest(Request $request, $authenticated) { + $result = FALSE; + + if ($authenticated) { + $result = $this->applyFilter($request, $authenticated, $this->getProvider($request)); + } + else { + foreach ($this->getSortedProviders() as $provider_id => $provider) { + if ($this->applyFilter($request, $authenticated, $provider_id)) { + $result = TRUE; + break; + } + } + } + + return $result; + } + + /** + * {@inheritdoc} + */ + public function challengeException(Request $request, \Exception $previous) { + $provider_id = $this->getChallenger($request); + if ($provider_id) { + return $this->challengers[$provider_id]->challengeException($request, $previous); + } + } + + /** + * Returns the id of the authentication provider for a request. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The incoming request. + * + * @return string|NULL + * The id of the first authentication provider which applies to the request. + * If no application detects appropriate credentials, then NULL is returned. + */ + protected function getProvider(Request $request) { + foreach ($this->getSortedProviders() as $provider_id => $provider) { + if ($provider->applies($request)) { + return $provider_id; + } + } + } + + /** + * Returns the id of the challenge provider for a request. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The incoming request. + * + * @return string|NULL + * The id of the first authentication provider which applies to the request. + * If no application detects appropriate credentials, then NULL is returned. + */ + protected function getChallenger(Request $request) { + if (!empty($this->challengers)) { + foreach ($this->getSortedProviders($request, FALSE) as $provider_id => $provider) { + if (isset($this->challengers[$provider_id]) && !$provider->applies($request) && $this->applyFilter($request, FALSE, $provider_id)) { + return $provider_id; + } + } + } + } + + /** + * Checks whether a provider is allowed on the given request. + * + * If no filter is registered for the given provider id, the default filter + * is applied. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The incoming request. + * @param bool $authenticated + * Whether or not the request is authenticated. + * @param string $provider_id + * The id of the authentication provider to check access for. + * + * @return bool + * TRUE if provider is allowed, FALSE otherwise. + */ + protected function applyFilter(Request $request, $authenticated, $provider_id) { + if (isset($this->filters[$provider_id])) { + $result = $this->filters[$provider_id]->appliesToRoutedRequest($request, $authenticated); + } + else { + $result = $this->defaultFilter($request, $provider_id); + } + + return $result; + } + + /** + * Default implementation of the provider filter. + * + * Checks whether a provider is allowed as per the _auth option on a route. If + * the option is not set or if the request did not match any route, only + * providers from the global provider set are allowed. + * + * If no filter is registered for the given provider id, the default filter + * is applied. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The incoming request. + * @param string $provider_id + * The id of the authentication provider to check access for. + * + * @return bool + * TRUE if provider is allowed, FALSE otherwise. + */ + protected function defaultFilter(Request $request, $provider_id) { + $route = RouteMatch::createFromRequest($request)->getRouteObject(); + $has_auth_option = isset($route) && $route->hasOption('_auth'); + + if ($has_auth_option) { + return in_array($provider_id, $route->getOption('_auth')); + } + else { + return isset($this->globalProviders[$provider_id]); + } } /** * Returns the sorted array of authentication providers. * - * @return array + * @todo Replace with a list of providers sorted during compile time in + * https://www.drupal.org/node/2432585. + * + * @return \Drupal\Core\Authentication\AuthenticationProviderInterface[] * An array of authentication provider objects. */ - public function getSortedProviders() { + protected function getSortedProviders() { if (!isset($this->sortedProviders)) { // Sort the builders according to priority. krsort($this->providerOrders); @@ -148,63 +269,4 @@ class AuthenticationManager implements AuthenticationProviderInterface, Authenti return $this->sortedProviders; } - /** - * Filters a list of providers and only return those allowed on the request. - * - * @param \Drupal\Core\Authentication\AuthenticationProviderInterface[] $providers - * An array of authentication provider objects. - * @param Request $request - * The request object. - * - * @return \Drupal\Core\Authentication\AuthenticationProviderInterface[] - * The filtered array authentication provider objects. - */ - protected function filterProviders(array $providers, Request $request) { - $route = RouteMatch::createFromRequest($request)->getRouteObject(); - $allowed_providers = array(); - if ($route && $route->hasOption('_auth')) { - $allowed_providers = $route->getOption('_auth'); - } - elseif ($default_provider = $this->defaultProviderId()) { - // @todo Mirrors the defective behavior of AuthenticationEnhancer and - // restricts the list of allowed providers to the default provider if no - // _auth was specified on the current route. - // - // This restriction will be removed by https://www.drupal.org/node/2286971 - // See also https://www.drupal.org/node/2283637 - $allowed_providers = array($default_provider); - } - - return array_intersect_key($providers, array_flip($allowed_providers)); - } - - /** - * Cleans up the authentication. - * - * Allow the triggered provider to clean up before the response is sent, e.g. - * trigger a session commit. - * - * @param \Symfony\Component\HttpFoundation\Request $request - * The request object. - * - * @see \Drupal\Core\Authentication\Provider\Cookie::cleanup() - */ - public function cleanup(Request $request) { - if (empty($this->providers[$this->triggeredProviderId])) { - return; - } - $this->providers[$this->triggeredProviderId]->cleanup($request); - } - - /** - * {@inheritdoc} - */ - public function handleException(GetResponseForExceptionEvent $event) { - foreach ($this->filterProviders($this->getSortedProviders(), $event->getRequest()) as $provider) { - if ($provider->handleException($event) === TRUE) { - break; - } - } - } - } diff --git a/core/lib/Drupal/Core/Authentication/AuthenticationManagerInterface.php b/core/lib/Drupal/Core/Authentication/AuthenticationManagerInterface.php deleted file mode 100644 index 0b4c0ff331f..00000000000 --- a/core/lib/Drupal/Core/Authentication/AuthenticationManagerInterface.php +++ /dev/null @@ -1,23 +0,0 @@ -sessionConfiguration = $session_configuration; + } + /** * {@inheritdoc} */ public function applies(Request $request) { - return $request->hasSession(); + return $request->hasSession() && $this->sessionConfiguration->hasSession($request); } /** @@ -29,7 +45,7 @@ class Cookie implements AuthenticationProviderInterface { */ public function authenticate(Request $request) { if ($request->getSession()->start()) { - // @todo Remove global in https://www.drupal.org/node/2286971 + // @todo Remove global in https://www.drupal.org/node/2228393 global $_session_user; return $_session_user; } @@ -37,16 +53,4 @@ class Cookie implements AuthenticationProviderInterface { return NULL; } - /** - * {@inheritdoc} - */ - public function cleanup(Request $request) { - } - - /** - * {@inheritdoc} - */ - public function handleException(GetResponseForExceptionEvent $event) { - return FALSE; - } } diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php index eb841b29dda..be9821d9cac 100644 --- a/core/lib/Drupal/Core/DrupalKernel.php +++ b/core/lib/Drupal/Core/DrupalKernel.php @@ -685,6 +685,11 @@ class DrupalKernel implements DrupalKernelInterface, TerminableInterface { $this->containerNeedsDumping = FALSE; $session_manager_started = FALSE; if (isset($this->container)) { + // Save the id of the currently logged in user. + if ($this->container->initialized('current_user')) { + $current_user_id = $this->container->get('current_user')->id(); + } + // If there is a session manager, close and save the session. if ($this->container->initialized('session_manager')) { $session_manager = $this->container->get('session_manager'); @@ -731,6 +736,11 @@ class DrupalKernel implements DrupalKernelInterface, TerminableInterface { } } } + + if (!empty($current_user_id)) { + $this->container->get('current_user')->setInitialAccountId($current_user_id); + } + \Drupal::setContainer($this->container); // If needs dumping flag was set, dump the container. diff --git a/core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php index 80f77962f67..6fd0b9997c4 100644 --- a/core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php @@ -7,71 +7,139 @@ namespace Drupal\Core\EventSubscriber; +use Drupal\Core\Authentication\AuthenticationProviderFilterInterface; +use Drupal\Core\Authentication\AuthenticationProviderChallengeInterface; use Drupal\Core\Authentication\AuthenticationProviderInterface; +use Drupal\Core\Session\AccountProxyInterface; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\HttpKernel\Event\GetResponseEvent; +use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\KernelEvents; -use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; -use Symfony\Component\HttpKernel\Event\FilterResponseEvent; -use Symfony\Component\EventDispatcher\EventSubscriberInterface; /** * Authentication subscriber. * - * Trigger authentication and cleanup during the request. + * Trigger authentication during the request. */ class AuthenticationSubscriber implements EventSubscriberInterface { /** * Authentication provider. * - * @var AuthenticationProviderInterface + * @var \Drupal\Core\Authentication\AuthenticationProviderInterface */ protected $authenticationProvider; /** - * Keep authentication manager as private variable. + * Authentication provider filter. * - * @param AuthenticationProviderInterface $authentication_manager - * The authentication manager. + * @var \Drupal\Core\Authentication\AuthenticationProviderFilterInterface|NULL */ - public function __construct(AuthenticationProviderInterface $authentication_provider) { + protected $filter; + + /** + * Authentication challenge provider. + * + * @var \Drupal\Core\Authentication\AuthenticationProviderChallengeInterface|NULL + */ + protected $challengeProvider; + + /** + * Account proxy. + * + * @var \Drupal\Core\Session\AccountProxyInterface + */ + protected $accountProxy; + + /** + * Constructs an authentication subscriber. + * + * @param \Drupal\Core\Authentication\AuthenticationProviderInterface $authentication_provider + * An authentication provider. + * @param \Drupal\Core\Session\AccountProxyInterface $account_proxy + * Account proxy. + */ + public function __construct(AuthenticationProviderInterface $authentication_provider, AccountProxyInterface $account_proxy) { $this->authenticationProvider = $authentication_provider; + $this->filter = ($authentication_provider instanceof AuthenticationProviderFilterInterface) ? $authentication_provider : NULL; + $this->challengeProvider = ($authentication_provider instanceof AuthenticationProviderChallengeInterface) ? $authentication_provider : NULL; + $this->accountProxy = $account_proxy; } /** - * Triggers authentication clean up on response. + * Authenticates user on request. * - * @see \Drupal\Core\Authentication\AuthenticationProviderInterface::cleanup() + * @param \Symfony\Component\HttpKernel\Event\GetResponseEvent $event + * The request event. + * + * @see \Drupal\Core\Authentication\AuthenticationProviderInterface::authenticate() */ - public function onRespond(FilterResponseEvent $event) { - if ($event->getRequestType() == HttpKernelInterface::MASTER_REQUEST) { + public function onKernelRequestAuthenticate(GetResponseEvent $event) { + if ($event->getRequestType() === HttpKernelInterface::MASTER_REQUEST) { $request = $event->getRequest(); - $this->authenticationProvider->cleanup($request); + if ($this->authenticationProvider->applies($request)) { + $account = $this->authenticationProvider->authenticate($request); + if ($account) { + $this->accountProxy->setAccount($account); + } + } } } /** - * Pass exception handling to authentication manager. + * Denies access if authentication provider is not allowed on this route. * - * @param GetResponseForExceptionEvent $event + * @param \Symfony\Component\HttpKernel\Event\GetResponseEvent $event + * The request event. */ - public function onException(GetResponseForExceptionEvent $event) { - if ($event->getRequestType() == HttpKernelInterface::MASTER_REQUEST) { - $this->authenticationProvider->handleException($event); + public function onKernelRequestFilterProvider(GetResponseEvent $event) { + if (isset($this->filter) && $event->getRequestType() === HttpKernelInterface::MASTER_REQUEST) { + $request = $event->getRequest(); + if ($this->authenticationProvider->applies($request) && !$this->filter->appliesToRoutedRequest($request, TRUE)) { + throw new AccessDeniedHttpException(); + } + } + } + + /** + * Respond with a challenge on access denied exceptions if appropriate. + * + * On a 403 (access denied), if there are no credentials on the request, some + * authentication methods (e.g. basic auth) require that a challenge is sent + * to the client. + * + * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event + * The exception event. + */ + public function onExceptionSendChallenge(GetResponseForExceptionEvent $event) { + if (isset($this->challengeProvider) && $event->getRequestType() === HttpKernelInterface::MASTER_REQUEST) { + $request = $event->getRequest(); + $exception = $event->getException(); + if ($exception instanceof AccessDeniedHttpException && !$this->authenticationProvider->applies($request) && (!isset($this->filter) || $this->filter->appliesToRoutedRequest($request, FALSE))) { + $challenge_exception = $this->challengeProvider->challengeException($request, $exception); + if ($challenge_exception) { + $event->setException($challenge_exception); + } + } } } /** * {@inheritdoc} - * - * The priority for request must be higher than the highest event subscriber - * accessing the current user. - * The priority for the response must be as low as possible allowing e.g the - * Cookie provider to send all relevant session data to the user. */ public static function getSubscribedEvents() { - $events[KernelEvents::RESPONSE][] = ['onRespond', 0]; - $events[KernelEvents::EXCEPTION][] = ['onException', 75]; + // The priority for authentication must be higher than the highest event + // subscriber accessing the current user. Especially it must be higher than + // LanguageRequestSubscriber as LanguageManager accesses the current user if + // the language module is enabled. + $events[KernelEvents::REQUEST][] = ['onKernelRequestAuthenticate', 300]; + + // Access check must be performed after routing. + $events[KernelEvents::REQUEST][] = ['onKernelRequestFilterProvider', 31]; + $events[KernelEvents::EXCEPTION][] = ['onExceptionSendChallenge', 75]; return $events; } + } diff --git a/core/lib/Drupal/Core/EventSubscriber/SpecialAttributesRouteSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/SpecialAttributesRouteSubscriber.php index e2e7137df5d..d096531b75e 100644 --- a/core/lib/Drupal/Core/EventSubscriber/SpecialAttributesRouteSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/SpecialAttributesRouteSubscriber.php @@ -7,7 +7,6 @@ namespace Drupal\Core\EventSubscriber; -use Drupal\Component\Utility\String; use Drupal\Core\Routing\RouteBuildEvent; use Drupal\Core\Routing\RouteSubscriberBase; use Symfony\Cmf\Component\Routing\RouteObjectInterface; @@ -25,7 +24,6 @@ class SpecialAttributesRouteSubscriber extends RouteSubscriberBase { $special_variables = array( 'system_path', '_legacy', - '_authentication_provider', '_raw_variables', RouteObjectInterface::ROUTE_OBJECT, RouteObjectInterface::ROUTE_NAME, diff --git a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php index 93a31c31b77..1cf937c4f3c 100644 --- a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php +++ b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php @@ -88,10 +88,6 @@ class AccessAwareRouter implements AccessAwareRouterInterface { public function matchRequest(Request $request) { $parameters = $this->chainRouter->matchRequest($request); $request->attributes->add($parameters); - // Trigger a session start and authentication by accessing any property of - // the current user. - // @todo This will be removed in https://www.drupal.org/node/2229145. - $this->account->id(); $this->checkAccess($request); // We can not return $parameters because the access check can change the // request attributes. diff --git a/core/lib/Drupal/Core/Routing/Enhancer/AuthenticationEnhancer.php b/core/lib/Drupal/Core/Routing/Enhancer/AuthenticationEnhancer.php deleted file mode 100644 index b1639ec0741..00000000000 --- a/core/lib/Drupal/Core/Routing/Enhancer/AuthenticationEnhancer.php +++ /dev/null @@ -1,81 +0,0 @@ -manager = $manager; - $this->currentUser = $current_user; - } - - /** - * {@inheritdoc} - */ - public function enhance(array $defaults, Request $request) { - $auth_provider_triggered = $request->attributes->get('_authentication_provider'); - if (!empty($auth_provider_triggered)) { - $route = isset($defaults[RouteObjectInterface::ROUTE_OBJECT]) ? $defaults[RouteObjectInterface::ROUTE_OBJECT] : NULL; - - $auth_providers = ($route && $route->getOption('_auth')) ? $route->getOption('_auth') : array($this->manager->defaultProviderId()); - // If the request was authenticated with a non-permitted provider, - // force the user back to anonymous. - if (!in_array($auth_provider_triggered, $auth_providers)) { - $anonymous_user = new AnonymousUserSession(); - - $this->currentUser->setAccount($anonymous_user); - } - } - return $defaults; - } - - /** - * {@inheritdoc} - */ - public function applies(Route $route) { - return TRUE; - } - -} diff --git a/core/lib/Drupal/Core/Session/AccountProxy.php b/core/lib/Drupal/Core/Session/AccountProxy.php index 2aa8fa5e466..eae71e7731d 100644 --- a/core/lib/Drupal/Core/Session/AccountProxy.php +++ b/core/lib/Drupal/Core/Session/AccountProxy.php @@ -7,9 +7,6 @@ namespace Drupal\Core\Session; -use Drupal\Core\Authentication\AuthenticationManagerInterface; -use Symfony\Component\HttpFoundation\RequestStack; - /** * A proxied implementation of AccountInterface. * @@ -23,20 +20,6 @@ use Symfony\Component\HttpFoundation\RequestStack; */ class AccountProxy implements AccountProxyInterface { - /** - * The current request. - * - * @var \Symfony\Component\HttpFoundation\RequestStack - */ - protected $requestStack; - - /** - * The authentication manager. - * - * @var \Drupal\Core\Authentication\AuthenticationManagerInterface - */ - protected $authenticationManager; - /** * The instantiated account. * @@ -45,17 +28,11 @@ class AccountProxy implements AccountProxyInterface { protected $account; /** - * Constructs a new AccountProxy. + * Initial account id. * - * @param \Drupal\Core\Authentication\AuthenticationManagerInterface $authentication_manager - * The authentication manager. - * @param \Symfony\Component\HttpFoundation\Request $request - * The request object used for authenticating. + * @var int */ - public function __construct(AuthenticationManagerInterface $authentication_manager, RequestStack $requestStack) { - $this->authenticationManager = $authentication_manager; - $this->requestStack = $requestStack; - } + protected $initialAccountId; /** * {@inheritdoc} @@ -75,10 +52,17 @@ class AccountProxy implements AccountProxyInterface { */ public function getAccount() { if (!isset($this->account)) { - // Use the master request to prevent subrequests authenticating to a - // different user. - $this->setAccount($this->authenticationManager->authenticate($this->requestStack->getMasterRequest())); + if ($this->initialAccountId) { + // After the container is rebuilt, DrupalKernel sets the initial + // account to the id of the logged in user. This is necessary in order + // to refresh the user account reference here. + $this->account = $this->loadUserEntity($this->initialAccountId); + } + else { + $this->account = new AnonymousUserSession(); + } } + return $this->account; } @@ -187,5 +171,38 @@ class AccountProxy implements AccountProxyInterface { return $this->getAccount()->getLastAccessedTime(); } -} + /** + * {@inheritdoc} + */ + public function setInitialAccountId($account_id) { + if (isset($this->account)) { + throw new \LogicException('AccountProxyInterface::setInitialAccountId() cannot be called after an account was set on the AccountProxy'); + } + $this->initialAccountId = $account_id; + } + + /** + * Load a user entity. + * + * The entity manager requires additional initialization code and cache + * clearing after the list of modules is changed. Therefore it is necessary to + * retrieve it as late as possible. + * + * Because of serialization issues it is currently not possible to inject the + * container into the AccountProxy. Thus it is necessary to retrieve the + * entity manager statically. + * + * @see https://www.drupal.org/node/2430447 + * + * @param int $account_id + * The id of an account to load. + * + * @return \Drupal\Core\Session\AccountInterface|NULL + * An account or NULL if none is found. + */ + protected function loadUserEntity($account_id) { + return \Drupal::entityManager()->getStorage('user')->load($account_id); + } + +} diff --git a/core/lib/Drupal/Core/Session/AccountProxyInterface.php b/core/lib/Drupal/Core/Session/AccountProxyInterface.php index 017bd8b080a..7ac9758202e 100644 --- a/core/lib/Drupal/Core/Session/AccountProxyInterface.php +++ b/core/lib/Drupal/Core/Session/AccountProxyInterface.php @@ -37,5 +37,15 @@ interface AccountProxyInterface extends AccountInterface { */ public function getAccount(); -} + /** + * Sets the id of the initial account. + * + * Never use this method, its sole purpose is to work around weird effects + * during mid-request container rebuilds. + * + * @param int $account_id + * The id of the initial account. + */ + public function setInitialAccountId($account_id); +} diff --git a/core/lib/Drupal/Core/Session/SessionHandler.php b/core/lib/Drupal/Core/Session/SessionHandler.php index 81877f9b3a4..8d9aa960eef 100644 --- a/core/lib/Drupal/Core/Session/SessionHandler.php +++ b/core/lib/Drupal/Core/Session/SessionHandler.php @@ -64,7 +64,7 @@ class SessionHandler extends AbstractProxy implements \SessionHandlerInterface { * {@inheritdoc} */ public function read($sid) { - // @todo Remove global in https://www.drupal.org/node/2286971 + // @todo Remove global in https://www.drupal.org/node/2228393 global $_session_user; // Handle the case of first time visitors and clients that don't store diff --git a/core/lib/Drupal/Core/Session/SessionManager.php b/core/lib/Drupal/Core/Session/SessionManager.php index 706e7ab20ad..7c3ac21b5b8 100644 --- a/core/lib/Drupal/Core/Session/SessionManager.php +++ b/core/lib/Drupal/Core/Session/SessionManager.php @@ -121,7 +121,7 @@ class SessionManager extends NativeSessionStorage implements SessionManagerInter } if (empty($result)) { - // @todo Remove global in https://www.drupal.org/node/2286971 + // @todo Remove global in https://www.drupal.org/node/2228393 global $_session_user; $_session_user = new AnonymousUserSession(); diff --git a/core/lib/Drupal/Core/StackMiddleware/Session.php b/core/lib/Drupal/Core/StackMiddleware/Session.php index f92e778b828..e0ed0e4e236 100644 --- a/core/lib/Drupal/Core/StackMiddleware/Session.php +++ b/core/lib/Drupal/Core/StackMiddleware/Session.php @@ -54,7 +54,9 @@ class Session implements HttpKernelInterface { */ public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = TRUE) { if ($type === self::MASTER_REQUEST && PHP_SAPI !== 'cli') { - $request->setSession($this->container->get($this->sessionServiceName)); + $session = $this->container->get($this->sessionServiceName); + $session->start(); + $request->setSession($session); } $result = $this->httpKernel->handle($request, $type, $catch); diff --git a/core/modules/basic_auth/src/Authentication/Provider/BasicAuth.php b/core/modules/basic_auth/src/Authentication/Provider/BasicAuth.php index 2fed1ed71b2..c2b8483ffb4 100644 --- a/core/modules/basic_auth/src/Authentication/Provider/BasicAuth.php +++ b/core/modules/basic_auth/src/Authentication/Provider/BasicAuth.php @@ -7,21 +7,20 @@ namespace Drupal\basic_auth\Authentication\Provider; -use \Drupal\Component\Utility\String; +use Drupal\Component\Utility\String; use Drupal\Core\Authentication\AuthenticationProviderInterface; +use Drupal\Core\Authentication\AuthenticationProviderChallengeInterface; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Flood\FloodInterface; use Drupal\user\UserAuthInterface; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; use Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException; -use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; /** * HTTP Basic authentication provider. */ -class BasicAuth implements AuthenticationProviderInterface { +class BasicAuth implements AuthenticationProviderInterface, AuthenticationProviderChallengeInterface { /** * The config factory. @@ -131,25 +130,12 @@ class BasicAuth implements AuthenticationProviderInterface { /** * {@inheritdoc} */ - public function cleanup(Request $request) {} - - /** - * {@inheritdoc} - */ - public function handleException(GetResponseForExceptionEvent $event) { - $exception = $event->getException(); - if (\Drupal::currentUser()->isAnonymous() && $exception instanceof AccessDeniedHttpException) { - if (!$this->applies($event->getRequest())) { - $site_name = $this->configFactory->get('system.site')->get('name'); - global $base_url; - $challenge = String::format('Basic realm="@realm"', array( - '@realm' => !empty($site_name) ? $site_name : $base_url, - )); - $event->setException(new UnauthorizedHttpException($challenge, 'No authentication credentials provided.', $exception)); - } - return TRUE; - } - return FALSE; + public function challengeException(Request $request, \Exception $previous) { + $site_name = $this->configFactory->get('system.site')->get('name'); + $challenge = String::format('Basic realm="@realm"', array( + '@realm' => !empty($site_name) ? $site_name : 'Access restricted', + )); + return new UnauthorizedHttpException($challenge, 'No authentication credentials provided.', $previous); } } diff --git a/core/modules/block/src/Tests/BlockLanguageTest.php b/core/modules/block/src/Tests/BlockLanguageTest.php index d8e9f1889ee..c3f87a115f1 100644 --- a/core/modules/block/src/Tests/BlockLanguageTest.php +++ b/core/modules/block/src/Tests/BlockLanguageTest.php @@ -149,6 +149,10 @@ class BlockLanguageTest extends WebTestBase { $this->drupalGet('node', ['query' => ['language' => 'fr']]); $this->assertText('Powered by Drupal', 'The body of the block appears on the page.'); + // Re-login in order to clear the interface language stored in the session. + $this->drupalLogout(); + $this->drupalLogin($this->adminUser); + // Content language does not depend on session/request arguments. // It will fall back on English (site default) and not display the block. $this->drupalGet('en'); diff --git a/core/modules/locale/tests/modules/early_translation_test/src/Auth.php b/core/modules/locale/tests/modules/early_translation_test/src/Auth.php index afb5ea1685f..12da491c612 100644 --- a/core/modules/locale/tests/modules/early_translation_test/src/Auth.php +++ b/core/modules/locale/tests/modules/early_translation_test/src/Auth.php @@ -10,7 +10,6 @@ namespace Drupal\early_translation_test; use Drupal\Core\Authentication\AuthenticationProviderInterface; use Drupal\Core\Entity\EntityManagerInterface; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; /** * Test authentication provider. @@ -53,16 +52,4 @@ class Auth implements AuthenticationProviderInterface { return NULL; } - /** - * {@inheritdoc} - */ - public function cleanup(Request $request) {} - - /** - * {@inheritdoc} - */ - public function handleException(GetResponseForExceptionEvent $event) { - return FALSE; - } - } diff --git a/core/modules/rest/rest.services.yml b/core/modules/rest/rest.services.yml index 14e4ab219b6..bfba7bb5688 100644 --- a/core/modules/rest/rest.services.yml +++ b/core/modules/rest/rest.services.yml @@ -11,6 +11,7 @@ services: arguments: [rest] access_check.rest.csrf: class: Drupal\rest\Access\CSRFAccessCheck + arguments: ['@session_configuration'] tags: - { name: access_check } rest.link_manager: diff --git a/core/modules/rest/src/Access/CSRFAccessCheck.php b/core/modules/rest/src/Access/CSRFAccessCheck.php index 563a0a4cdec..2e39c61d1b4 100644 --- a/core/modules/rest/src/Access/CSRFAccessCheck.php +++ b/core/modules/rest/src/Access/CSRFAccessCheck.php @@ -10,6 +10,7 @@ namespace Drupal\rest\Access; use Drupal\Core\Access\AccessCheckInterface; use Drupal\Core\Access\AccessResult; use Drupal\Core\Session\AccountInterface; +use Drupal\Core\Session\SessionConfigurationInterface; use Symfony\Component\Routing\Route; use Symfony\Component\HttpFoundation\Request; @@ -18,6 +19,23 @@ use Symfony\Component\HttpFoundation\Request; */ class CSRFAccessCheck implements AccessCheckInterface { + /** + * The session configuration. + * + * @var \Drupal\Core\Session\SessionConfigurationInterface + */ + protected $sessionConfiguration; + + /** + * Constructs a new rest CSRF access check. + * + * @param \Drupal\Core\Session\SessionConfigurationInterface $session_configuration + * The session configuration. + */ + public function __construct(SessionConfigurationInterface $session_configuration) { + $this->sessionConfiguration = $session_configuration; + } + /** * Implements AccessCheckInterface::applies(). */ @@ -54,7 +72,6 @@ class CSRFAccessCheck implements AccessCheckInterface { */ public function access(Request $request, AccountInterface $account) { $method = $request->getMethod(); - $cookie = $request->attributes->get('_authentication_provider') == 'cookie'; // This check only applies if // 1. this is a write operation @@ -62,7 +79,7 @@ class CSRFAccessCheck implements AccessCheckInterface { // 3. the request comes with a session cookie. if (!in_array($method, array('GET', 'HEAD', 'OPTIONS', 'TRACE')) && $account->isAuthenticated() - && $cookie + && $this->sessionConfiguration->hasSession($request) ) { $csrf_token = $request->headers->get('X-CSRF-Token'); if (!\Drupal::csrfToken()->validate($csrf_token, 'rest')) { @@ -72,4 +89,5 @@ class CSRFAccessCheck implements AccessCheckInterface { // Let other access checkers decide if the request is legit. return AccessResult::allowed()->setCacheable(FALSE); } + } diff --git a/core/modules/rest/src/Tests/AuthTest.php b/core/modules/rest/src/Tests/AuthTest.php index c8a8e3b81c1..09537c8ff1b 100644 --- a/core/modules/rest/src/Tests/AuthTest.php +++ b/core/modules/rest/src/Tests/AuthTest.php @@ -56,15 +56,15 @@ class AuthTest extends RESTTestBase { // Try to read the resource with session cookie authentication, which is // not enabled and should not work. $this->httpRequest($entity->urlInfo(), 'GET', NULL, $this->defaultMimeType); - $this->assertResponse('401', 'HTTP response code is 401 when the request is authenticated but not authorized.'); + $this->assertResponse('403', 'HTTP response code is 403 when the request was authenticated by the wrong authentication provider.'); // Ensure that cURL settings/headers aren't carried over to next request. unset($this->curlHandle); // Now read it with the Basic authentication which is enabled and should // work. - $this->basicAuthGet($entity->urlInfo(), $account->getUsername(), $account->pass_raw); - $this->assertResponse('200', 'HTTP response code is 200 for successfully authorized requests.'); + $this->basicAuthGet($entity->urlInfo(), $account->getUsername(), $account->pass_raw, $this->defaultMimeType); + $this->assertResponse('200', 'HTTP response code is 200 for successfully authenticated requests.'); $this->curlClose(); } @@ -80,11 +80,16 @@ class AuthTest extends RESTTestBase { * The user name to authenticate with. * @param string $password * The password. + * @param string $mime_type + * The MIME type for the Accept header. * * @return string * Curl output. */ - protected function basicAuthGet(Url $url, $username, $password) { + protected function basicAuthGet(Url $url, $username, $password, $mime_type = NULL) { + if (!isset($mime_type)) { + $mime_type = $this->defaultMimeType; + } $out = $this->curlExec( array( CURLOPT_HTTPGET => TRUE, @@ -92,6 +97,7 @@ class AuthTest extends RESTTestBase { CURLOPT_NOBODY => FALSE, CURLOPT_HTTPAUTH => CURLAUTH_BASIC, CURLOPT_USERPWD => $username . ':' . $password, + CURLOPT_HTTPHEADER => array('Accept: ' . $mime_type), ) ); diff --git a/core/modules/rest/src/Tests/CsrfTest.php b/core/modules/rest/src/Tests/CsrfTest.php index d44d7876f3c..a686f448049 100644 --- a/core/modules/rest/src/Tests/CsrfTest.php +++ b/core/modules/rest/src/Tests/CsrfTest.php @@ -61,9 +61,6 @@ class CsrfTest extends RESTTestBase { * Tests that CSRF check is not triggered for Basic Auth requests. */ public function testBasicAuth() { - // Login so the session cookie is sent in addition to the basic auth header. - $this->drupalLogin($this->account); - $curl_options = $this->getCurlOptions(); $curl_options[CURLOPT_HTTPAUTH] = CURLAUTH_BASIC; $curl_options[CURLOPT_USERPWD] = $this->account->getUsername() . ':' . $this->account->pass_raw; diff --git a/core/modules/user/user.services.yml b/core/modules/user/user.services.yml index 50405b56dec..4bee8698182 100644 --- a/core/modules/user/user.services.yml +++ b/core/modules/user/user.services.yml @@ -17,6 +17,7 @@ services: - { name: access_check, applies_to: _user_is_logged_in } authentication.cookie: class: Drupal\Core\Authentication\Provider\Cookie + arguments: ['@session_configuration'] tags: - { name: authentication_provider, priority: 0 } cache_context.user: diff --git a/core/tests/Drupal/Tests/Core/Authentication/AuthenticationManagerTest.php b/core/tests/Drupal/Tests/Core/Authentication/AuthenticationManagerTest.php new file mode 100644 index 00000000000..370cc8cdc59 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Authentication/AuthenticationManagerTest.php @@ -0,0 +1,88 @@ + TRUE]) { + $authentication_manager = new AuthenticationManager($global_providers); + $auth_provider = $this->getMock('Drupal\Core\Authentication\AuthenticationProviderInterface'); + $authentication_manager->addProvider($auth_provider, 'authentication.' . $provider_id); + + $request = new Request(); + if ($has_route) { + $route = new Route('/example'); + if ($auth_option) { + $route->setOption('_auth', $auth_option); + } + $request->attributes->set(RouteObjectInterface::ROUTE_OBJECT, $route); + } + + $this->assertSame($applies, $authentication_manager->appliesToRoutedRequest($request, FALSE)); + } + + /** + * @covers ::applyFilter + */ + public function testApplyFilterWithFilterprovider() { + $authentication_manager = new AuthenticationManager(); + $auth_provider = $this->getMock('Drupal\Tests\Core\Authentication\TestAuthenticationProviderInterface'); + $authentication_manager->addProvider($auth_provider, 'authentication.filtered'); + + $auth_provider->expects($this->once()) + ->method('appliesToRoutedRequest') + ->willReturn(TRUE); + + $request = new Request(); + $this->assertTrue($authentication_manager->appliesToRoutedRequest($request, FALSE)); + } + + /** + * Provides data to self::testDefaultFilter(). + */ + public function providerTestDefaultFilter() { + $data = []; + // No route, cookie is global, should apply. + $data[] = [TRUE, FALSE, [], 'cookie']; + // No route, cookie is not global, should not apply. + $data[] = [FALSE, FALSE, [], 'cookie', ['other' => TRUE]]; + // Route, no _auth, cookie is global, should apply. + $data[] = [TRUE, TRUE, [], 'cookie']; + // Route, no _auth, cookie is not global, should not apply. + $data[] = [FALSE, TRUE, [], 'cookie', ['other' => TRUE]]; + // Route, with _auth and non-matching provider, should not apply. + $data[] = [FALSE, TRUE, ['basic_auth'], 'cookie']; + // Route, with _auth and matching provider should not apply. + $data[] = [TRUE, TRUE, ['basic_auth'], 'basic_auth']; + return $data; + } + +} + +/** + * Helper interface to mock two interfaces at once. + */ +interface TestAuthenticationProviderInterface extends AuthenticationProviderFilterInterface, AuthenticationProviderInterface {} diff --git a/core/tests/Drupal/Tests/Core/EventSubscriber/SpecialAttributesRouteSubscriberTest.php b/core/tests/Drupal/Tests/Core/EventSubscriber/SpecialAttributesRouteSubscriberTest.php index 1c90a626ba3..4b9405514ae 100644 --- a/core/tests/Drupal/Tests/Core/EventSubscriber/SpecialAttributesRouteSubscriberTest.php +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/SpecialAttributesRouteSubscriberTest.php @@ -46,7 +46,6 @@ class SpecialAttributesRouteSubscriberTest extends UnitTestCase { $routes = array(); $routes[] = array(new Route('/test/{system_path}')); $routes[] = array(new Route('/test/{_legacy}')); - $routes[] = array(new Route('/test/{_authentication_provider}')); $routes[] = array(new Route('/test/{' . RouteObjectInterface::ROUTE_OBJECT . '}')); $routes[] = array(new Route('/test/{' . RouteObjectInterface::ROUTE_NAME . '}')); $routes[] = array(new Route('/test/{_content}'));