From 0fa529e4f0cb8be7e582b0d1330d2971b50fc38c Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Sat, 18 Apr 2015 12:06:53 +0200
Subject: [PATCH] Issue #2228393 by almaudoh, andypost, pfrenssen, znerol, cpj,
Dom.: Decouple session from cookie based user authentication
---
core/includes/install.core.inc | 18 ++-
.../Core/Authentication/Provider/Cookie.php | 56 ----------
.../Drupal/Core/Session/SessionHandler.php | 92 ++--------------
.../Drupal/Core/Session/SessionManager.php | 28 +++--
.../Core/Session/SessionManagerInterface.php | 5 +
.../src/Authentication/Provider/Cookie.php | 103 ++++++++++++++++++
.../EventSubscriber/UserRequestSubscriber.php | 75 +++++++++++++
core/modules/user/user.module | 5 +-
core/modules/user/user.services.yml | 9 +-
9 files changed, 233 insertions(+), 158 deletions(-)
delete mode 100644 core/lib/Drupal/Core/Authentication/Provider/Cookie.php
create mode 100644 core/modules/user/src/Authentication/Provider/Cookie.php
create mode 100644 core/modules/user/src/EventSubscriber/UserRequestSubscriber.php
diff --git a/core/includes/install.core.inc b/core/includes/install.core.inc
index 5579dbf1b525..9b947caffa5d 100644
--- a/core/includes/install.core.inc
+++ b/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.
- \Drupal::service('session')->save();
+ if ($session = \Drupal::request()->getSession()) {
+ $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();
}
/**
diff --git a/core/lib/Drupal/Core/Authentication/Provider/Cookie.php b/core/lib/Drupal/Core/Authentication/Provider/Cookie.php
deleted file mode 100644
index 7db5d9b3f2b3..000000000000
--- a/core/lib/Drupal/Core/Authentication/Provider/Cookie.php
+++ /dev/null
@@ -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;
- }
-
-}
diff --git a/core/lib/Drupal/Core/Session/SessionHandler.php b/core/lib/Drupal/Core/Session/SessionHandler.php
index 92f67418982a..ec1e43024f7a 100644
--- a/core/lib/Drupal/Core/Session/SessionHandler.php
+++ b/core/lib/Drupal/Core/Session/SessionHandler.php
@@ -10,7 +10,6 @@
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 @@ public function open($save_path, $name) {
* {@inheritdoc}
*/
public function read($sid) {
- // @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
- // cookies (eg. web crawlers).
- $cookies = $this->requestStack->getCurrentRequest()->cookies;
- if (empty($sid) || !$cookies->has($this->getName())) {
- $_session_user = new UserSession();
- return '';
- }
-
- $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);
+ $data = '';
+ if (!empty($sid)) {
+ // Read the session data from the database.
+ $query = $this->connection
+ ->queryRange('SELECT session FROM {sessions} WHERE sid = :sid', 0, 1, ['sid' => Crypt::hashBase64($sid)]);
+ $data = (string) $query->fetchField();
}
- 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;
+ return $data;
}
/**
* {@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(),
- 'hostname' => $this->requestStack->getCurrentRequest()->getClientIP(),
+ 'uid' => $request->getSession()->get('uid', 0),
+ 'hostname' => $request->getClientIP(),
'session' => $value,
'timestamp' => REQUEST_TIME,
);
@@ -127,13 +87,6 @@ public function write($sid, $value) {
->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 @@ public function close() {
* {@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 @@ public function gc($lifetime) {
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);
- }
- }
-
}
diff --git a/core/lib/Drupal/Core/Session/SessionManager.php b/core/lib/Drupal/Core/Session/SessionManager.php
index 3c71930cb3ed..1d470e2f6bf2 100644
--- a/core/lib/Drupal/Core/Session/SessionManager.php
+++ b/core/lib/Drupal/Core/Session/SessionManager.php
@@ -124,10 +124,6 @@ public function start() {
}
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 @@ protected function startNow() {
* {@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 @@ public function save() {
* {@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 @@ public function delete($uid) {
->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}
*/
diff --git a/core/lib/Drupal/Core/Session/SessionManagerInterface.php b/core/lib/Drupal/Core/Session/SessionManagerInterface.php
index d194002fa466..c755687d6aa8 100644
--- a/core/lib/Drupal/Core/Session/SessionManagerInterface.php
+++ b/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.
*
diff --git a/core/modules/user/src/Authentication/Provider/Cookie.php b/core/modules/user/src/Authentication/Provider/Cookie.php
new file mode 100644
index 000000000000..bac9e15396ca
--- /dev/null
+++ b/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;
+ }
+
+}
diff --git a/core/modules/user/src/EventSubscriber/UserRequestSubscriber.php b/core/modules/user/src/EventSubscriber/UserRequestSubscriber.php
new file mode 100644
index 000000000000..6f35a2b4bf65
--- /dev/null
+++ b/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;
+ }
+
+}
diff --git a/core/modules/user/user.module b/core/modules/user/user.module
index 5bcea7cfc0f1..3ca469844b39 100644
--- a/core/modules/user/user.module
+++ b/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());
}
/**
diff --git a/core/modules/user/user.services.yml b/core/modules/user/user.services.yml
index 0cf12ef0843f..0a55b6ee5f8f 100644
--- a/core/modules/user/user.services.yml
+++ b/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
- arguments: ['@session_configuration']
+ class: Drupal\user\Authentication\Provider\Cookie
+ 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']
--
GitLab