Issue #2286971 by znerol, Berdir, almaudoh, cilefen: Remove dependency of...

Issue #2286971 by znerol, Berdir, almaudoh, cilefen: Remove dependency of current_user on request and authentication manager

core/authorize.php

@@ -47,10 +47,17 @@
  * 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 @@ function authorize_access_allowed() {
 $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';

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%']

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 \Drupal\Core\Authentication\AuthenticationProviderFilterInterface[]
+   */
+  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
+   * @var string[]
    */
-  protected $triggeredProviderId = '';
+  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,139 +97,176 @@ public function addProvider(AuthenticationProviderInterface $provider, $id, $pri
     $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;
+    $provider_id = $this->getProvider($request);
+    return $this->providers[$provider_id]->authenticate($request);
+  }
 
-    // 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;
-      }
-    }
+  /**
+   * {@inheritdoc}
+   */
+  public function appliesToRoutedRequest(Request $request, $authenticated) {
+    $result = FALSE;
 
-    // No provider returned a valid account, so set the user to anonymous.
-    if (!$account) {
-      $account = new AnonymousUserSession();
+    if ($authenticated) {
+      $result = $this->applyFilter($request, $authenticated, $this->getProvider($request));
     }
-
-    // No provider was fired, so assume the one with the least priority
-    // should have.
-    if (!$this->triggeredProviderId) {
-      $this->triggeredProviderId = $this->defaultProviderId();
+    else {
+      foreach ($this->getSortedProviders() as $provider_id => $provider) {
+        if ($this->applyFilter($request, $authenticated, $provider_id)) {
+          $result = TRUE;
+          break;
+        }
+      }
     }
 
-    // Save the authenticated account and the provider that supplied it
-    //  for later access.
-    $request->attributes->set('_authentication_provider', $this->triggeredProviderId);
+    return $result;
+  }
 
-    return $account;
+  /**
+   * {@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 default provider ID.
+   * Returns the id of the authentication provider for a request.
    *
-   * The default provider is the one with the lowest registered priority.
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The incoming request.
    *
-   * @return string
-   *   The ID of the default provider.
+   * @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.
    */
-  public function defaultProviderId() {
-    $providers = $this->getSortedProviders();
-    $provider_ids = array_keys($providers);
-    return end($provider_ids);
+  protected function getProvider(Request $request) {
+    foreach ($this->getSortedProviders() as $provider_id => $provider) {
+      if ($provider->applies($request)) {
+        return $provider_id;
+      }
+    }
   }
 
   /**
-   * Returns the sorted array of authentication providers.
+   * Returns the id of the challenge provider for a request.
    *
-   * @return array
-   *   An array of authentication provider objects.
+   * @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.
    */
-  public function getSortedProviders() {
-    if (!isset($this->sortedProviders)) {
-      // Sort the builders according to priority.
-      krsort($this->providerOrders);
-      // Merge nested providers from $this->providers into $this->sortedProviders.
-      $this->sortedProviders = array();
-      foreach ($this->providerOrders as $providers) {
-        $this->sortedProviders = array_merge($this->sortedProviders, $providers);
+  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;
+        }
       }
     }
-    return $this->sortedProviders;
   }
 
   /**
-   * Filters a list of providers and only return those allowed on the request.
+   * Checks whether a provider is allowed on the given request.
    *
-   * @param \Drupal\Core\Authentication\AuthenticationProviderInterface[] $providers
-   *   An array of authentication provider objects.
-   * @param Request $request
-   *   The request object.
+   * If no filter is registered for the given provider id, the default filter
+   * is applied.
    *
-   * @return \Drupal\Core\Authentication\AuthenticationProviderInterface[]
-   *   The filtered array authentication provider objects.
+   * @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 filterProviders(array $providers, Request $request) {
-    $route = RouteMatch::createFromRequest($request)->getRouteObject();
-    $allowed_providers = array();
-    if ($route && $route->hasOption('_auth')) {
-      $allowed_providers = $route->getOption('_auth');
+  protected function applyFilter(Request $request, $authenticated, $provider_id) {
+    if (isset($this->filters[$provider_id])) {
+      $result = $this->filters[$provider_id]->appliesToRoutedRequest($request, $authenticated);
     }
-    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);
+    else {
+      $result = $this->defaultFilter($request, $provider_id);
     }
 
-    return array_intersect_key($providers, array_flip($allowed_providers));
+    return $result;
   }
 
   /**
-   * Cleans up the authentication.
+   * Default implementation of the provider filter.
    *
-   * Allow the triggered provider to clean up before the response is sent, e.g.
-   * trigger a session commit.
+   * 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 request object.
+   *   The incoming request.
+   * @param string $provider_id
+   *   The id of the authentication provider to check access for.
    *
-   * @see \Drupal\Core\Authentication\Provider\Cookie::cleanup()
+   * @return bool
+   *   TRUE if provider is allowed, FALSE otherwise.
    */
-  public function cleanup(Request $request) {
-    if (empty($this->providers[$this->triggeredProviderId])) {
-      return;
+  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]);
     }
-    $this->providers[$this->triggeredProviderId]->cleanup($request);
   }
 
   /**
-   * {@inheritdoc}
+   * Returns the sorted array of authentication providers.
+   *
+   * @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 handleException(GetResponseForExceptionEvent $event) {
-    foreach ($this->filterProviders($this->getSortedProviders(), $event->getRequest()) as $provider) {
-      if ($provider->handleException($event) === TRUE) {
-        break;
+  protected function getSortedProviders() {
+    if (!isset($this->sortedProviders)) {
+      // Sort the builders according to priority.
+      krsort($this->providerOrders);
+      // Merge nested providers from $this->providers into $this->sortedProviders.
+      $this->sortedProviders = array();
+      foreach ($this->providerOrders as $providers) {
+        $this->sortedProviders = array_merge($this->sortedProviders, $providers);
       }
     }
+    return $this->sortedProviders;
   }
 
 }

core/lib/Drupal/Core/Authentication/AuthenticationManagerInterface.php

-<?php
-
-/**
- * @file
- * Contains Drupal\Core\Authentication\AuthenticationManagerInterface.
- */
-
-namespace Drupal\Core\Authentication;
-
-/**
- * Defines an interface for authentication managers.
- */
-interface AuthenticationManagerInterface extends AuthenticationProviderInterface {
-
-  /**
-   * Returns the service id of the default authentication provider.
-   *
-   * @return string
-   *   The service id of the default authentication provider.
-   */
-  public function defaultProviderId();
-
-}

core/lib/Drupal/Core/Authentication/AuthenticationProviderChallengeInterface.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Authentication\AuthenticationProviderChallengeInterface
+ */
+
+namespace Drupal\Core\Authentication;
+
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * Generate a challenge when access is denied for unauthenticated users.
+ *
+ * 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.
+ */
+interface AuthenticationProviderChallengeInterface {
+
+  /**
+   * Constructs an exception which is used to generate the challenge.
+   *
+   * @var \Symfony\Component\HttpFoundation\Request
+   *   The request.
+   * @var \Exception $exception
+   *   The previous exception.
+   *
+   * @return \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface|NULL
+   *   An exception to be used in order to generate an authentication challenge.
+   */
+  public function challengeException(Request $request, \Exception $previous);
+
+}

core/lib/Drupal/Core/Authentication/AuthenticationProviderFilterInterface.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Authentication\AuthenticationProviderFilterInterface
+ */
+
+namespace Drupal\Core\Authentication;
+
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * Restrict authentication methods to a subset of the site.
+ *
+ * Some authentication methods should not be available throughout a whole site.
+ * E.g., there are good reasons to restrict insecure methods like HTTP basic
+ * auth or an URL token authentication method to API-only routes.
+ */
+interface AuthenticationProviderFilterInterface {
+
+  /**
+   * Checks whether the authentication method is allowed on a given route.
+   *
+   * While authentication itself is run before routing, this method is called
+   * after routing, hence RouteMatch is available and can be used to inspect
+   * route options.
+   *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The request.
+   * @param bool $authenticated
+   *   Whether or not the request is authenticated.
+   *
+   * @return bool
+   *   TRUE if an authentication method is allowed on the request, otherwise
+   *   FALSE.
+   */
+  public function appliesToRoutedRequest(Request $request, $authenticated);
+
+}

core/lib/Drupal/Core/Authentication/AuthenticationProviderInterface.php

@@ -8,7 +8,6 @@
 namespace Drupal\Core\Authentication;
 
 use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
 
 /**
  * Interface for authentication providers.
@@ -16,52 +15,27 @@
 interface AuthenticationProviderInterface {
 
   /**
-   * Declares whether the provider applies to a specific request or not.
+   * Checks whether suitable authentication credentials are on the request.
    *
    * @param \Symfony\Component\HttpFoundation\Request $request
    *   The request object.
    *
    * @return bool
-   *   TRUE if the provider applies to the passed request, FALSE otherwise.
+   *   TRUE if authentication credentials suitable for this provider are on the
+   *   request, FALSE otherwise.
    */
   public function applies(Request $request);
 
   /**
    * Authenticates the user.
    *
-   * @param \Symfony\Component\HttpFoundation\Request|null $request
+   * @param \Symfony\Component\HttpFoundation\Request|NULL $request
    *   The request object.
    *
-   * @return \Drupal\Core\Session\AccountInterface|null
+   * @return \Drupal\Core\Session\AccountInterface|NULL
    *   AccountInterface - in case of a successful authentication.
    *   NULL - in case where authentication failed.
    */
   public function authenticate(Request $request);
 
-  /**
-   * Performs cleanup tasks at the end of a request.
-   *
-   * Allow the authentication provider to clean up before the response is sent.
-   * This is uses for instance in \Drupal\Core\Authentication\Provider\Cookie to
-   * ensure the session gets committed.
-   *
-   * @param Request $request
-   *   The request object.
-   */
-  public function cleanup(Request $request);
-
-  /**
-   * Handles an exception.
-   *
-   * In case exception has happened we allow authentication providers react.
-   * Used in \Drupal\Core\Authentication\Provider\BasicAuth to set up headers to
-   * prompt login.
-   *
-   * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event
-   *
-   * @return bool
-   *   TRUE - exception handled. No need to run through other providers.
-   *   FALSE - no actions have been done. Run through other providers.
-   */
-  public function handleException(GetResponseForExceptionEvent $event);
 }

core/lib/Drupal/Core/Authentication/Provider/Cookie.php

@@ -8,20 +8,36 @@
 namespace Drupal\Core\Authentication\Provider;
 
 use Drupal\Core\Authentication\AuthenticationProviderInterface;
-use Drupal\Core\Session\SessionManagerInterface;
+use Drupal\Core\Session\SessionConfigurationInterface;
 use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
 
 /**
  * Cookie based authentication provider.
  */
 class Cookie implements AuthenticationProviderInterface {
 
+  /**
+   * The session configuration.
+   *
+   * @var \Drupal\Core\Session\SessionConfigurationInterface
+   */
+  protected $sessionConfiguration;
+
+  /**
+   * Constructs a new cookie authentication provider.
+   *
+   * @param \Drupal\Core\Session\SessionConfigurationInterface $session_configuration
+   *   The session configuration.
+   */
+  public function __construct(SessionConfigurationInterface $session_configuration) {
+    $this->sessionConfiguration = $session_configuration;
+  }
+
   /**
    * {@inheritdoc}
    */
   public function applies(Request $request) {
-    return $request->hasSession();
+    return $request->hasSession() && $this->sessionConfiguration->hasSession($request);
   }
 
   /**
@@ -29,7 +45,7 @@ public function applies(Request $request) {