Issue #2228393 by almaudoh, andypost, pfrenssen, znerol, cpj, Dom.: Decouple session from cookie based user authentication

core/includes/install.core.inc

@@ -141,6 +141,10 @@ function install_drupal($class_loader, $settings = array()) {
   // installations can send output to the browser or redirect the user to the
   // next page.
   if ($state['interactive']) {
+    // If a session has been initiated in this request, make sure to save it.
+    if ($session = \Drupal::request()->getSession()) {
+      $session->save();
+    }
     if ($state['parameters_changed']) {
       // Redirect to the correct page if the URL parameters have changed.
       install_goto(install_redirect_url($state));
@@ -596,8 +600,9 @@ function install_run_task($task, &$install_state) {
       $url = Url::fromUri('base:install.php', ['query' => $install_state['parameters'], 'script' => '']);
       $response = batch_process($url, clone $url);
       if ($response instanceof Response) {
-        // Save $_SESSION data from batch.
+        if ($session = \Drupal::request()->getSession()) {
-        \Drupal::service('session')->save();
+          $session->save();
+        }
         // Send the response.
         $response->send();
         exit;
@@ -1549,12 +1554,13 @@ function install_load_profile(&$install_state) {
 
 /**
  * Performs a full bootstrap of Drupal during installation.
- *
- * @param $install_state
- *   An array of information about the current installation state.
  */
 function install_bootstrap_full() {
-  \Drupal::service('session')->start();
+  // Store the session on the request object and start it.
+  /** @var \Symfony\Component\HttpFoundation\Session\SessionInterface $session */
+  $session = \Drupal::service('session');
+  \Drupal::request()->setSession($session);
+  $session->start();
 }
 
 /**

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

@@ -1,56 +0,0 @@
-<?php
-
-/**
- * @file
- * Contains \Drupal\Core\Authentication\Provider\Cookie.
- */
-
-namespace Drupal\Core\Authentication\Provider;
-
-use Drupal\Core\Authentication\AuthenticationProviderInterface;
-use Drupal\Core\Session\SessionConfigurationInterface;
-use Symfony\Component\HttpFoundation\Request;
-
-/**
- * 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() && $this->sessionConfiguration->hasSession($request);
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function authenticate(Request $request) {
-    if ($request->getSession()->start()) {
-      // @todo Remove global in https://www.drupal.org/node/2228393
-      global $_session_user;
-      return $_session_user;
-    }
-
-    return NULL;
-  }
-
-}

core/lib/Drupal/Core/Session/SessionHandler.php

@@ -10,7 +10,6 @@ namespace Drupal\Core\Session;
 use Drupal\Component\Utility\Crypt;
 use Drupal\Core\Database\Connection;
 use Drupal\Core\DependencyInjection\DependencySerializationTrait;
-use Drupal\Core\Site\Settings;
 use Drupal\Core\Utility\Error;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Session\Storage\Proxy\AbstractProxy;
@@ -36,13 +35,6 @@ class SessionHandler extends AbstractProxy implements \SessionHandlerInterface {
    */
   protected $connection;
 
-  /**
-   * An associative array of obsolete sessions with session id as key, and db-key as value.
-   *
-   * @var array
-   */
-  protected $obsoleteSessionIds = array();
-
   /**
    * Constructs a new SessionHandler instance.
    *
@@ -67,59 +59,27 @@ class SessionHandler extends AbstractProxy implements \SessionHandlerInterface {
    * {@inheritdoc}
    */
   public function read($sid) {
-    // @todo Remove global in https://www.drupal.org/node/2228393
+    $data = '';
-    global $_session_user;
+    if (!empty($sid)) {
-
+      // Read the session data from the database.
-    // Handle the case of first time visitors and clients that don't store
+      $query = $this->connection
-    // cookies (eg. web crawlers).
+        ->queryRange('SELECT session FROM {sessions} WHERE sid = :sid', 0, 1, ['sid' => Crypt::hashBase64($sid)]);
-    $cookies = $this->requestStack->getCurrentRequest()->cookies;
+      $data = (string) $query->fetchField();
-    if (empty($sid) || !$cookies->has($this->getName())) {
-      $_session_user = new UserSession();
-      return '';
     }
-
+    return $data;
-    $values = $this->connection->query("SELECT u.*, s.* FROM {users_field_data} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE u.default_langcode = 1 AND s.sid = :sid", array(
-      ':sid' => Crypt::hashBase64($sid),
-    ))->fetchAssoc();
-
-    // We found the client's session record and they are an authenticated,
-    // active user.
-    if ($values && $values['uid'] > 0 && $values['status'] == 1) {
-      // Add roles element to $user.
-      $rids = $this->connection->query("SELECT ur.roles_target_id as rid FROM {user__roles} ur WHERE ur.entity_id = :uid", array(
-        ':uid' => $values['uid'],
-      ))->fetchCol();
-      $values['roles'] = array_merge(array(AccountInterface::AUTHENTICATED_ROLE), $rids);
-      $_session_user = new UserSession($values);
-    }
-    elseif ($values) {
-      // The user is anonymous or blocked. Only preserve two fields from the
-      // {sessions} table.
-      $_session_user = new UserSession(array(
-        'session' => $values['session'],
-        'access' => $values['access'],
-      ));
-    }
-    else {
-      // The session has expired.
-      $_session_user = new UserSession();
-    }
-
-    return $_session_user->session;
   }
 
   /**
    * {@inheritdoc}
    */
   public function write($sid, $value) {
-    $user = \Drupal::currentUser();
-
     // The exception handler is not active at this point, so we need to do it
     // manually.
     try {
+      $request = $this->requestStack->getCurrentRequest();
       $fields = array(
-        'uid' => $user->id(),
+        'uid' => $request->getSession()->get('uid', 0),
-        'hostname' => $this->requestStack->getCurrentRequest()->getClientIP(),
+        'hostname' => $request->getClientIP(),
         'session' => $value,
         'timestamp' => REQUEST_TIME,
       );
@@ -127,13 +87,6 @@ class SessionHandler extends AbstractProxy implements \SessionHandlerInterface {
         ->keys(array('sid' => Crypt::hashBase64($sid)))
         ->fields($fields)
         ->execute();
-
-      // Likewise, do not update access time more than once per 180 seconds.
-      if ($user->isAuthenticated() && REQUEST_TIME - $user->getLastAccessedTime() > Settings::get('session_write_interval', 180)) {
-        /** @var \Drupal\user\UserStorageInterface $storage */
-        $storage = \Drupal::entityManager()->getStorage('user');
-        $storage->updateLastAccessTimestamp($user, REQUEST_TIME);
-      }
       return TRUE;
     }
     catch (\Exception $exception) {
@@ -159,21 +112,11 @@ class SessionHandler extends AbstractProxy implements \SessionHandlerInterface {
    * {@inheritdoc}
    */
   public function destroy($sid) {
-
-
     // Delete session data.
     $this->connection->delete('sessions')
       ->condition('sid', Crypt::hashBase64($sid))
       ->execute();
 
-    // Reset $_SESSION and current user to prevent a new session from being
-    // started in \Drupal\Core\Session\SessionManager::save().
-    $_SESSION = array();
-    \Drupal::currentUser()->setAccount(new AnonymousUserSession());
-
-    // Unset the session cookies.
-    $this->deleteCookie($this->getName());
-
     return TRUE;
   }
 
@@ -192,19 +135,4 @@ class SessionHandler extends AbstractProxy implements \SessionHandlerInterface {
     return TRUE;
   }
 
-  /**
-   * Deletes a session cookie.
-   *
-   * @param string $name
-   *   Name of session cookie to delete.
-   */
-  protected function deleteCookie($name) {
-    $cookies = $this->requestStack->getCurrentRequest()->cookies;
-    if ($cookies->has($name)) {
-      $params = session_get_cookie_params();
-      setcookie($name, '', REQUEST_TIME - 3600, $params['path'], $params['domain'], $params['secure'], $params['httponly']);
-      $cookies->remove($name);
-    }
-  }
-
 }

core/lib/Drupal/Core/Session/SessionManager.php

@@ -124,10 +124,6 @@ class SessionManager extends NativeSessionStorage implements SessionManagerInter
     }
 
     if (empty($result)) {
-      // @todo Remove global in https://www.drupal.org/node/2228393
-      global $_session_user;
-      $_session_user = new AnonymousUserSession();
-
       // Randomly generate a session identifier for this request. This is
       // necessary because \Drupal\user\SharedTempStoreFactory::get() wants to
       // know the future session ID of a lazily started session in advance.
@@ -184,18 +180,16 @@ class SessionManager extends NativeSessionStorage implements SessionManagerInter
    * {@inheritdoc}
    */
   public function save() {
-    $user = \Drupal::currentUser();
-
     if ($this->isCli()) {
       // We don't have anything to do if we are not allowed to save the session.
       return;
     }
 
-    if ($user->isAnonymous() && $this->isSessionObsolete()) {
+    if ($this->isSessionObsolete()) {
       // There is no session data to store, destroy the session if it was
       // previously started.
       if ($this->getSaveHandler()->isActive()) {
-        session_destroy();
+        $this->destroy();
       }
     }
     else {
@@ -215,8 +209,6 @@ class SessionManager extends NativeSessionStorage implements SessionManagerInter
    * {@inheritdoc}
    */
   public function regenerate($destroy = FALSE, $lifetime = NULL) {
-    $user = \Drupal::currentUser();
-
     // Nothing to do if we are not allowed to change the session.
     if ($this->isCli()) {
       return;
@@ -261,6 +253,22 @@ class SessionManager extends NativeSessionStorage implements SessionManagerInter
       ->execute();
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function destroy() {
+    session_destroy();
+
+    // Unset the session cookies.
+    $session_name = $this->getName();
+    $cookies = $this->requestStack->getCurrentRequest()->cookies;
+    if ($cookies->has($session_name)) {
+      $params = session_get_cookie_params();
+      setcookie($session_name, '', REQUEST_TIME - 3600, $params['path'], $params['domain'], $params['secure'], $params['httponly']);
+      $cookies->remove($session_name);
+    }
+  }
+
   /**
    * {@inheritdoc}
    */

core/lib/Drupal/Core/Session/SessionManagerInterface.php

@@ -22,6 +22,11 @@ interface SessionManagerInterface extends SessionStorageInterface {
    */
   public function delete($uid);
 
+  /**
+   * Destroys the current session and removes session cookies.
+   */
+  public function destroy();
+
   /**
    * Sets the write safe session handler.
    *

core/modules/user/src/Authentication/Provider/Cookie.php

@@ -0,0 +1,103 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\user\Authentication\Provider\Cookie.
+ */
+
+namespace Drupal\user\Authentication\Provider;
+
+use Drupal\Core\Authentication\AuthenticationProviderInterface;
+use Drupal\Core\Database\Connection;
+use Drupal\Core\Session\AccountInterface;
+use Drupal\Core\Session\UserSession;
+use Drupal\Core\Session\SessionConfigurationInterface;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Session\SessionInterface;
+
+/**
+ * Cookie based authentication provider.
+ */
+class Cookie implements AuthenticationProviderInterface {
+
+  /**
+   * The session configuration.
+   *
+   * @var \Drupal\Core\Session\SessionConfigurationInterface
+   */
+  protected $sessionConfiguration;
+
+  /**
+   * The database connection.
+   *
+   * @var \Drupal\Core\Database\Connection
+   */
+  protected $connection;
+
+  /**
+   * Constructs a new cookie authentication provider.
+   *
+   * @param \Drupal\Core\Session\SessionConfigurationInterface $session_configuration
+   *   The session configuration.
+   * @param \Drupal\Core\Database\Connection $connection
+   *   The database connection.
+   */
+  public function __construct(SessionConfigurationInterface $session_configuration, Connection $connection) {
+    $this->sessionConfiguration = $session_configuration;
+    $this->connection = $connection;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function applies(Request $request) {
+    return $request->hasSession() && $this->sessionConfiguration->hasSession($request);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function authenticate(Request $request) {
+    return $this->getUserFromSession($request->getSession());
+  }
+
+  /**
+   * Returns the UserSession object for the given session.
+   *
+   * @param \Symfony\Component\HttpFoundation\Session\SessionInterface $session
+   *   The session.
+   *
+   * @return \Drupal\Core\Session\AccountInterface|NULL
+   *   The UserSession object for the current user, or NULL if this is an
+   *   anonymous session.
+   */
+  protected function getUserFromSession(SessionInterface $session) {
+    if ($uid = $session->get('uid')) {
+      // @todo Load the User entity in SessionHandler so we don't need queries.
+      // @see https://www.drupal.org/node/2345611
+      $values = $this->connection
+        ->query('SELECT * FROM {users_field_data} u WHERE u.uid = :uid AND u.default_langcode = 1', [':uid' => $uid])
+        ->fetchAssoc();
+
+      // Check if the user data was found and the user is active.
+      if (!empty($values) && $values['status'] == 1) {
+        // UserSession::getLastAccessedTime() returns session save timestamp,
+        // while User::getLastAccessedTime() returns the user 'access'
+        // timestamp. This ensures they are synchronized.
+        $values['timestamp'] = $values['access'];
+
+        // Add the user's roles.
+        $rids = $this->connection
+          ->query('SELECT roles_target_id FROM {user__roles} WHERE entity_id = :uid', [':uid' => $values['uid']])
+          ->fetchCol();
+        $values['roles'] = array_merge([AccountInterface::AUTHENTICATED_ROLE], $rids);
+
+        return new UserSession($values);
+      }
+    }
+
+    // This is an anonymous session.
+    return NULL;
+  }
+
+}

core/modules/user/src/EventSubscriber/UserRequestSubscriber.php

@@ -0,0 +1,75 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\user\EventSubscriber\UserRequestSubscriber.
+ */
+
+namespace Drupal\user\EventSubscriber;
+
+use Drupal\Core\Entity\EntityManagerInterface;
+use Drupal\Core\Session\AccountInterface;
+use Drupal\Core\Site\Settings;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\HttpKernel\Event\PostResponseEvent;
+use Symfony\Component\HttpKernel\KernelEvents;
+
+/**
+ * Updates the current user's last access time.
+ */
+class UserRequestSubscriber implements EventSubscriberInterface {
+
+  /**
+   * The current account.
+   *
+   * @var \Drupal\Core\Session\AccountInterface
+   */
+  protected $account;
+
+  /**
+   * The entity manager.
+   *
+   * @var \Drupal\Core\Entity\EntityManagerInterface
+   */
+  protected $entityManager;
+
+  /**
+   * Constructs a new UserRequestSubscriber.
+   *
+   * @param \Drupal\Core\Session\AccountInterface $account
+   *   The current user.
+   * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
+   *   The entity manager.
+   */
+  public function __construct(AccountInterface $account, EntityManagerInterface $entity_manager) {
+    $this->account = $account;
+    $this->entityManager = $entity_manager;
+  }
+
+  /**
+   * Updates the current user's last access time.
+   *
+   * @param \Symfony\Component\HttpKernel\Event\PostResponseEvent $event
+   *   The event to process.
+   */
+  public function onKernelTerminate(PostResponseEvent $event) {
+    if ($this->account->isAuthenticated() && REQUEST_TIME - $this->account->getLastAccessedTime() > Settings::get('session_write_interval', 180)) {
+      // Do that no more than once per 180 seconds.
+      /** @var \Drupal\user\UserStorageInterface $storage */
+      $storage = $this->entityManager->getStorage('user');
+      $storage->updateLastAccessTimestamp($this->account, REQUEST_TIME);
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getSubscribedEvents() {
+    // Should go before other subscribers start to write their caches. Notably
+    // before \Drupal\Core\EventSubscriber\KernelDestructionSubscriber to
+    // prevent instantiation of destructed services.
+    $events[KernelEvents::TERMINATE][] = ['onKernelTerminate', 300];
+    return $events;
+  }
+
+}

core/modules/user/user.module

@@ -528,7 +528,7 @@ function user_login_finalize(UserInterface $account) {
   // fails or incorrectly does a redirect which would leave the old session
   // in place.
   \Drupal::service('session')->migrate();
-
+  \Drupal::service('session')->set('uid', $account->id());
   \Drupal::moduleHandler()->invokeAll('user_login', array($account));
 }
 
@@ -1386,7 +1386,8 @@ function user_logout() {
   // Session::invalidate(). Regrettably this method is currently broken and may
   // lead to the creation of spurious session records in the database.
   // @see https://github.com/symfony/symfony/issues/12375
-  session_destroy();
+  \Drupal::service('session_manager')->destroy();
+  $user->setAccount(new AnonymousUserSession());
 }
 
 /**

core/modules/user/user.services.yml

@@ -16,8 +16,8 @@ services:
     tags:
       - { name: access_check, applies_to: _user_is_logged_in }
   authentication.cookie:
-    class: Drupal\Core\Authentication\Provider\Cookie
+    class: Drupal\user\Authentication\Provider\Cookie
-    arguments: ['@session_configuration']
+    arguments: ['@session_configuration', '@database']
     tags:
       - { name: authentication_provider, priority: 0 }
   user.data:
@@ -35,6 +35,11 @@ services:
     arguments: ['@current_user', '@url_generator']
     tags:
       - { name: event_subscriber }
+  user_last_access_subscriber:
+    class: Drupal\user\EventSubscriber\UserRequestSubscriber
+    arguments: ['@current_user', '@entity.manager']
+    tags:
+      - { name: event_subscriber }
   theme.negotiator.admin_theme:
     class: Drupal\user\Theme\AdminNegotiator
     arguments: ['@current_user', '@config.factory', '@entity.manager', '@router.admin_context']