diff --git a/core/assets/scaffold/files/default.services.yml b/core/assets/scaffold/files/default.services.yml index 45d986cd8e..9aa48df92d 100644 --- a/core/assets/scaffold/files/default.services.yml +++ b/core/assets/scaffold/files/default.services.yml @@ -36,6 +36,18 @@ parameters: # @default none # cookie_domain: '.example.com' # + # Specify the length of session ID string. Session ID length can be between + # 22 to 256. The PHP recommended value is 48. See + # https://www.php.net/manual/session.security.ini.php for more information. + # @default 48 + sid_length: 48 + # + # Specify the number of bits in encoded session ID character. The possible + # values are '4' (0-9, a-f), '5' (0-9, a-v), and '6' (0-9, a-z, A-Z, "-", + # ","). The PHP recommended value is 5. See + # https://www.php.net/manual/session.security.ini.php for more information. + # @default 6 + sid_bits_per_character: 6 twig.config: # Twig debugging: # diff --git a/core/core.services.yml b/core/core.services.yml index 5d01d87667..f672ffab52 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -9,6 +9,8 @@ parameters: gc_divisor: 100 gc_maxlifetime: 200000 cookie_lifetime: 2000000 + sid_length: 48 + sid_bits_per_character: 6 twig.config: debug: false auto_reload: null @@ -1724,7 +1726,7 @@ services: - { name: backend_overridable } tempstore.shared: class: Drupal\Core\TempStore\SharedTempStoreFactory - arguments: ['@keyvalue.expirable', '@lock', '@request_stack', '%tempstore.expire%'] + arguments: ['@keyvalue.expirable', '@lock', '@request_stack', '@current_user', '%tempstore.expire%'] tags: - { name: backend_overridable } pager.manager: diff --git a/core/includes/install.core.inc b/core/includes/install.core.inc index 77d3df7bad..61ff66b3fa 100644 --- a/core/includes/install.core.inc +++ b/core/includes/install.core.inc @@ -664,8 +664,12 @@ function install_run_task($task, &$install_state) { // @todo Replace this when we refactor the installer to use a request- // response workflow. if ($output instanceof Response) { + if (\Drupal::request()->hasSession()) { + \Drupal::request()->getSession()->save(); + } + // Send the response. $output->send(); - $output = NULL; + exit; } // The task is complete when we try to access the batch page and receive // FALSE in return, since this means we are at a URL where we are no diff --git a/core/lib/Drupal/Core/Session/MetadataBag.php b/core/lib/Drupal/Core/Session/MetadataBag.php index ee0fbd4855..46bacc01ae 100644 --- a/core/lib/Drupal/Core/Session/MetadataBag.php +++ b/core/lib/Drupal/Core/Session/MetadataBag.php @@ -2,6 +2,7 @@ namespace Drupal\Core\Session; +use Drupal\Component\Utility\Crypt; use Drupal\Core\Site\Settings; use Symfony\Component\HttpFoundation\Session\Storage\MetadataBag as SymfonyMetadataBag; @@ -48,10 +49,22 @@ public function getCsrfTokenSeed() { } } + /** + * {@inheritdoc} + */ + public function stampNew($lifetime = NULL) { + parent::stampNew($lifetime); + + // We set token seed immediately to avoid race condition between two + // simultaneous requests without a seed. + $this->setCsrfTokenSeed(Crypt::randomBytesBase64()); + } + /** * Clear the CSRF token seed. */ public function clearCsrfTokenSeed() { + @trigger_error('Calling ' . __METHOD__ . '() is deprecated in drupal:9.2.0 and will be removed in drupal:10.0.0. Use \Drupal\Core\Session\MetadataBag::stampNew() instead. See https://www.drupal.org/node/3187914', E_USER_DEPRECATED); unset($this->meta[static::CSRF_TOKEN_SEED]); } diff --git a/core/lib/Drupal/Core/Session/SessionConfiguration.php b/core/lib/Drupal/Core/Session/SessionConfiguration.php index bbdde7cb5d..cecc0a350f 100644 --- a/core/lib/Drupal/Core/Session/SessionConfiguration.php +++ b/core/lib/Drupal/Core/Session/SessionConfiguration.php @@ -22,9 +22,12 @@ class SessionConfiguration implements SessionConfigurationInterface { * * @see \Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage::__construct() * @see http://php.net/manual/session.configuration.php + * @see https://www.php.net/manual/session.security.ini.php */ public function __construct($options = []) { - $this->options = $options; + // Provide sensible defaults for sid_length and sid_bits_per_character. + // See core/assets/scaffold/files/default.services.yml for more information. + $this->options = $options + ['sid_length' => 48, 'sid_bits_per_character' => 6]; } /** diff --git a/core/lib/Drupal/Core/Session/SessionManager.php b/core/lib/Drupal/Core/Session/SessionManager.php index 42a9d8f273..5e42186f71 100644 --- a/core/lib/Drupal/Core/Session/SessionManager.php +++ b/core/lib/Drupal/Core/Session/SessionManager.php @@ -119,17 +119,6 @@ public function start() { } if (empty($result)) { - // Randomly generate a session identifier for this request. This is - // necessary because \Drupal\Core\TempStore\SharedTempStoreFactory::get() - // wants to know the future session ID of a lazily started session in - // advance. - // - // @todo: With current versions of PHP there is little reason to generate - // the session id from within application code. Consider using the - // default php session id instead of generating a custom one: - // https://www.drupal.org/node/2238561 - $this->setId(Crypt::randomBytesBase64()); - // Initialize the session global and attach the Symfony session bags. $_SESSION = []; $this->loadSession(); @@ -145,6 +134,23 @@ public function start() { return $result; } + /** + * {@inheritdoc} + */ + public function getId() { + $id = parent::getId(); + + if (empty($id)) { + // Legacy code might rely on the existence of a session ID before a real + // session exists. In this case, generate a random session ID to provide + // backwards compatibility. + @trigger_error('Calling ' . __METHOD__ . '() outside of an actual existing session is deprecated in drupal:9.2.0 and will be removed in drupal:10.0.0. This is often used for anonymous users. See https://www.drupal.org/node/3006306', E_USER_DEPRECATED); + $id = Crypt::randomBytesBase64(); + $this->setId($id); + } + return $id; + } + /** * Forcibly start a PHP session. * @@ -210,34 +216,23 @@ public function regenerate($destroy = FALSE, $lifetime = NULL) { return; } - // We do not support the optional $destroy and $lifetime parameters as long - // as #2238561 remains open. - if ($destroy || isset($lifetime)) { - throw new \InvalidArgumentException('The optional parameters $destroy and $lifetime of SessionManager::regenerate() are not supported currently'); - } - - if ($this->isStarted()) { - $old_session_id = $this->getId(); - // Save and close the old session. Call the parent method to avoid issue - // with session destruction due to the session being considered obsolete. - parent::save(); - // Ensure the session is reloaded correctly. - $this->startedLazy = TRUE; + // The default behaviour in Drupal when regenerating a session is to destroy + // the existing session. This is inline with the @link https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html#renew-the-session-id-after-any-privilege-level-change + // OWASP session management cheat sheet. @endlink + if (count(func_get_args())) { + $destroy = TRUE; } - session_id(Crypt::randomBytesBase64()); - // We set token seed immediately to avoid race condition between two - // simultaneous requests without a seed. - $this->getMetadataBag()->setCsrfTokenSeed(Crypt::randomBytesBase64()); + $regenerated = parent::regenerate($destroy, $lifetime); - if (isset($old_session_id)) { - $params = session_get_cookie_params(); - $expire = $params['lifetime'] ? REQUEST_TIME + $params['lifetime'] : 0; - setcookie($this->getName(), $this->getId(), $expire, $params['path'], $params['domain'], $params['secure'], $params['httponly']); - $this->migrateStoredSession($old_session_id); + if (!$regenerated && $destroy) { + // Ensure the metadata bag has been stamped. If the parent::regenerate() + // is called prior to the session being started it will not refresh the + // metadata as expected. + $this->getMetadataBag()->stampNew($lifetime); } - $this->startNow(); + return $regenerated; } /** @@ -261,6 +256,12 @@ public function destroy() { return; } + // Symfony suggests using Session::invalidate() instead of session_destroy() + // however the former calls session_regenerate_id(TRUE), which while + // destroying the current session creates a new ID; Drupal has historically + // decided to only set sessions when absolutely necessary (e.g., to increase + // anonymous user cache hit rates) and as such we cannot use the Symfony + // convenience method here. session_destroy(); // Unset the session cookies. @@ -332,18 +333,4 @@ protected function getSessionDataMask() { return array_intersect_key($mask, $_SESSION); } - /** - * Migrates the current session to a new session id. - * - * @param string $old_session_id - * The old session ID. The new session ID is $this->getId(). - */ - protected function migrateStoredSession($old_session_id) { - $fields = ['sid' => Crypt::hashBase64($this->getId())]; - $this->connection->update('sessions') - ->fields($fields) - ->condition('sid', Crypt::hashBase64($old_session_id)) - ->execute(); - } - } diff --git a/core/lib/Drupal/Core/TempStore/PrivateTempStore.php b/core/lib/Drupal/Core/TempStore/PrivateTempStore.php index 4c89ff4170..6696671534 100644 --- a/core/lib/Drupal/Core/TempStore/PrivateTempStore.php +++ b/core/lib/Drupal/Core/TempStore/PrivateTempStore.php @@ -2,6 +2,7 @@ namespace Drupal\Core\TempStore; +use Drupal\Component\Utility\Crypt; use Drupal\Core\DependencyInjection\DependencySerializationTrait; use Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface; use Drupal\Core\Lock\LockBackendInterface; @@ -119,17 +120,16 @@ public function get($key) { * Thrown when a lock for the backend storage could not be acquired. */ public function set($key, $value) { - // Ensure that an anonymous user has a session created for them, as - // otherwise subsequent page loads will not be able to retrieve their - // tempstore data. if ($this->currentUser->isAnonymous()) { - // @todo when https://www.drupal.org/node/2865991 is resolved, use force - // start session API rather than setting an arbitrary value directly. + // Ensure that an anonymous user has a session created for them, as + // otherwise subsequent page loads will not be able to retrieve their + // tempstore data. Note this has to be done before the key is created as + // the owner is used in key creation. $this->startSession(); - $this->requestStack - ->getCurrentRequest() - ->getSession() - ->set('core.tempstore.private', TRUE); + $session = $this->requestStack->getCurrentRequest()->getSession(); + if (!$session->has('core.tempstore.private.owner')) { + $session->set('core.tempstore.private.owner', Crypt::randomBytesBase64()); + } } $key = $this->createkey($key); @@ -224,8 +224,10 @@ protected function createkey($key) { protected function getOwner() { $owner = $this->currentUser->id(); if ($this->currentUser->isAnonymous()) { + // Check to see if an owner key exists in the session. $this->startSession(); - $owner = $this->requestStack->getCurrentRequest()->getSession()->getId(); + $session = $this->requestStack->getCurrentRequest()->getSession(); + $owner = $session->get('core.tempstore.private.owner'); } return $owner; } diff --git a/core/lib/Drupal/Core/TempStore/SharedTempStore.php b/core/lib/Drupal/Core/TempStore/SharedTempStore.php index 6bc0340752..06973f5c3c 100644 --- a/core/lib/Drupal/Core/TempStore/SharedTempStore.php +++ b/core/lib/Drupal/Core/TempStore/SharedTempStore.php @@ -5,6 +5,7 @@ use Drupal\Core\DependencyInjection\DependencySerializationTrait; use Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface; use Drupal\Core\Lock\LockBackendInterface; +use Drupal\Core\Session\AccountProxyInterface; use Symfony\Component\HttpFoundation\RequestStack; /** @@ -68,6 +69,13 @@ class SharedTempStore { */ protected $owner; + /** + * The current user. + * + * @var \Drupal\Core\Session\AccountProxyInterface + */ + protected $currentUser; + /** * The time to live for items in seconds. * @@ -90,14 +98,28 @@ class SharedTempStore { * The owner key to store along with the data (e.g. a user or session ID). * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack * The request stack. + * @param \Drupal\Core\Session\AccountProxyInterface $current_user + * The current user. * @param int $expire * The time to live for items, in seconds. */ - public function __construct(KeyValueStoreExpirableInterface $storage, LockBackendInterface $lock_backend, $owner, RequestStack $request_stack, $expire = 604800) { + public function __construct(KeyValueStoreExpirableInterface $storage, LockBackendInterface $lock_backend, $owner, RequestStack $request_stack, $current_user = NULL, $expire = 604800) { $this->storage = $storage; $this->lockBackend = $lock_backend; $this->owner = $owner; $this->requestStack = $request_stack; + if ($current_user instanceof AccountProxyInterface) { + $this->currentUser = $current_user; + } + else { + @trigger_error('Calling ' . __METHOD__ . '() without the $current_user argument is deprecated in drupal:9.2.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3006268', E_USER_DEPRECATED); + $this->currentUser = \Drupal::currentUser(); + if (is_int($current_user)) { + // If the $current_user argument is numeric then this object has been + // instantiated with the old constructor signature. + $expire = $current_user; + } + } $this->expire = $expire; } @@ -150,7 +172,9 @@ public function setIfNotExists($key, $value) { 'data' => $value, 'updated' => (int) $this->requestStack->getMasterRequest()->server->get('REQUEST_TIME'), ]; - return $this->storage->setWithExpireIfNotExists($key, $value, $this->expire); + $this->ensureAnonymousSession(); + $set = $this->storage->setWithExpireIfNotExists($key, $value, $this->expire); + return $set; } /** @@ -208,6 +232,7 @@ public function set($key, $value) { 'data' => $value, 'updated' => (int) $this->requestStack->getMasterRequest()->server->get('REQUEST_TIME'), ]; + $this->ensureAnonymousSession(); $this->storage->setWithExpire($key, $value, $this->expire); $this->lockBackend->release($key); } @@ -279,4 +304,17 @@ public function deleteIfOwner($key) { return FALSE; } + /** + * Stores the owner in session if the user is anonymous. + * + * This method should be called when a value is set. + */ + protected function ensureAnonymousSession() { + // If this is being run from the CLI then the request will not have + // session. + if ($this->currentUser->isAnonymous() && $this->requestStack->getCurrentRequest()->hasSession()) { + $this->requestStack->getCurrentRequest()->getSession()->set('core.tempstore.shared.owner', $this->owner); + } + } + } diff --git a/core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php b/core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php index 90e816a7dd..151b254f87 100644 --- a/core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php +++ b/core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php @@ -2,8 +2,10 @@ namespace Drupal\Core\TempStore; +use Drupal\Component\Utility\Crypt; use Drupal\Core\KeyValueStore\KeyValueExpirableFactoryInterface; use Drupal\Core\Lock\LockBackendInterface; +use Drupal\Core\Session\AccountProxyInterface; use Symfony\Component\HttpFoundation\RequestStack; /** @@ -32,6 +34,13 @@ class SharedTempStoreFactory { */ protected $requestStack; + /** + * The current user. + * + * @var \Drupal\Core\Session\AccountProxyInterface + */ + protected $currentUser; + /** * The time to live for items in seconds. * @@ -48,13 +57,29 @@ class SharedTempStoreFactory { * The lock object used for this data. * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack * The request stack. + * @param \Drupal\Core\Session\AccountProxyInterface $current_user + * The current user. * @param int $expire * The time to live for items, in seconds. */ - public function __construct(KeyValueExpirableFactoryInterface $storage_factory, LockBackendInterface $lock_backend, RequestStack $request_stack, $expire = 604800) { + public function __construct(KeyValueExpirableFactoryInterface $storage_factory, LockBackendInterface $lock_backend, RequestStack $request_stack, $current_user = NULL, $expire = 604800) { $this->storageFactory = $storage_factory; $this->lockBackend = $lock_backend; $this->requestStack = $request_stack; + if ($current_user instanceof AccountProxyInterface) { + $this->currentUser = $current_user; + } + else { + $this->currentUser = \Drupal::currentUser(); + } + if (!($current_user instanceof AccountProxyInterface)) { + @trigger_error('Calling ' . __METHOD__ . '() without the $current_user argument is deprecated in drupal:9.2.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3006268', E_USER_DEPRECATED); + if (is_int($current_user)) { + // If the $current_user argument is numeric then this object has been + // instantiated with the old constructor signature. + $expire = $current_user; + } + } $this->expire = $expire; } @@ -76,12 +101,20 @@ public function get($collection, $owner = NULL) { // Use the currently authenticated user ID or the active user ID unless // the owner is overridden. if (!isset($owner)) { - $owner = \Drupal::currentUser()->id() ?: session_id(); + $owner = $this->currentUser->id(); + if ($this->currentUser->isAnonymous()) { + $owner = Crypt::randomBytesBase64(); + if ($this->requestStack->getCurrentRequest()->hasSession()) { + // Anonymous users store a random identifier in session if session is + // available. + $owner = $this->requestStack->getCurrentRequest()->getSession()->get('core.tempstore.shared.owner', $owner); + } + } } // Store the data for this collection in the database. $storage = $this->storageFactory->get("tempstore.shared.$collection"); - return new SharedTempStore($storage, $this->lockBackend, $owner, $this->requestStack, $this->expire); + return new SharedTempStore($storage, $this->lockBackend, $owner, $this->requestStack, $this->currentUser, $this->expire); } } diff --git a/core/misc/cspell/dictionary.txt b/core/misc/cspell/dictionary.txt index 684dfa4ff9..c8d9694e86 100644 --- a/core/misc/cspell/dictionary.txt +++ b/core/misc/cspell/dictionary.txt @@ -1130,6 +1130,7 @@ overrider's overriders overridetest overwritable +owasp pageable pagecache pagetop diff --git a/core/modules/user/tests/src/Functional/UserCancelTest.php b/core/modules/user/tests/src/Functional/UserCancelTest.php index 7b454f4302..9e4a18f6f5 100644 --- a/core/modules/user/tests/src/Functional/UserCancelTest.php +++ b/core/modules/user/tests/src/Functional/UserCancelTest.php @@ -476,6 +476,11 @@ public function testUserDelete() { $user_storage->resetCache([$account->id()]); $this->assertNull($user_storage->load($account->id()), 'User is not found in the database.'); + // Confirm there's only one session in the database. The user will be logged + // out and their session migrated. + // @see _user_cancel_session_regenerate() + $this->assertSame(1, (int) \Drupal::database()->select('sessions', 's')->countQuery()->execute()->fetchField()); + // Confirm that user's content has been deleted. $node_storage->resetCache([$node->id()]); $this->assertNull($node_storage->load($node->id()), 'Node of the user has been deleted.'); diff --git a/core/modules/user/tests/src/Functional/UserEditTest.php b/core/modules/user/tests/src/Functional/UserEditTest.php index 4cf8179e43..0054bc4374 100644 --- a/core/modules/user/tests/src/Functional/UserEditTest.php +++ b/core/modules/user/tests/src/Functional/UserEditTest.php @@ -79,6 +79,11 @@ public function testUserEdit() { $this->drupalPostForm("user/" . $user1->id() . "/edit", $edit, 'Save'); $this->assertRaw(t("The changes have been saved.")); + // Confirm there's only one session in the database as the existing session + // has been migrated when the password is changed. + // @see \Drupal\user\Entity\User::postSave() + $this->assertSame(1, (int) \Drupal::database()->select('sessions', 's')->countQuery()->execute()->fetchField()); + // Make sure the changed timestamp is updated. $this->assertEqual($user1->getChangedTime(), REQUEST_TIME, 'Changing a user sets "changed" timestamp.'); diff --git a/core/modules/user/user.module b/core/modules/user/user.module index 383c093878..9f713e2c95 100644 --- a/core/modules/user/user.module +++ b/core/modules/user/user.module @@ -460,8 +460,10 @@ function user_login_finalize(UserInterface $account) { // This is called before hook_user_login() in case one of those functions // 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()); + /** @var \Symfony\Component\HttpFoundation\Session\Session $session */ + $session = \Drupal::service('session'); + $session->migrate(TRUE); + $session->set('uid', $account->id()); \Drupal::moduleHandler()->invokeAll('user_login', [$account]); } diff --git a/core/tests/Drupal/KernelTests/Core/TempStore/TempStoreDatabaseTest.php b/core/tests/Drupal/KernelTests/Core/TempStore/TempStoreDatabaseTest.php index 791d79dfab..d2e633ee5b 100644 --- a/core/tests/Drupal/KernelTests/Core/TempStore/TempStoreDatabaseTest.php +++ b/core/tests/Drupal/KernelTests/Core/TempStore/TempStoreDatabaseTest.php @@ -3,6 +3,7 @@ namespace Drupal\KernelTests\Core\TempStore; use Drupal\Core\KeyValueStore\KeyValueExpirableFactory; +use Drupal\Core\Session\AccountProxyInterface; use Drupal\KernelTests\KernelTestBase; use Drupal\Core\TempStore\SharedTempStoreFactory; use Drupal\Core\Lock\DatabaseLockBackend; @@ -28,7 +29,14 @@ public function testSharedTempStore() { // Create a key/value collection. $database = Database::getConnection(); - $factory = new SharedTempStoreFactory(new KeyValueExpirableFactory(\Drupal::getContainer()), new DatabaseLockBackend($database), $this->container->get('request_stack')); + // Mock the current user service so that isAnonymous returns FALSE. + $current_user = $this->prophesize(AccountProxyInterface::class); + $factory = new SharedTempStoreFactory( + new KeyValueExpirableFactory(\Drupal::getContainer()), + new DatabaseLockBackend($database), + $this->container->get('request_stack'), + $current_user->reveal() + ); $collection = $this->randomMachineName(); // Create two mock users. diff --git a/core/tests/Drupal/Tests/Core/Session/MetadataBagTest.php b/core/tests/Drupal/Tests/Core/Session/MetadataBagTest.php new file mode 100644 index 0000000000..fbd7187aad --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Session/MetadataBagTest.php @@ -0,0 +1,36 @@ +setCsrfTokenSeed('a_cryptographically_secure_long_random_string_should_used_here'); + $metadata->stampNew(); + $this->assertNotEquals('a_cryptographically_secure_long_random_string_should_used_here', $metadata->getCsrfTokenSeed()); + } + + /** + * @covers ::clearCsrfTokenSeed + * @group legacy + */ + public function testDeprecatedClearCsrfTokenSeed() { + $this->expectDeprecation('Calling Drupal\Core\Session\MetadataBag::clearCsrfTokenSeed() is deprecated in drupal:9.2.0 and will be removed in drupal:10.0.0. Use \Drupal\Core\Session\MetadataBag::stampNew() instead. See https://www.drupal.org/node/3187914'); + + $metadata = new MetadataBag(new Settings([])); + $metadata->clearCsrfTokenSeed(); + } + +} diff --git a/core/tests/Drupal/Tests/Core/Session/SessionConfigurationTest.php b/core/tests/Drupal/Tests/Core/Session/SessionConfigurationTest.php index 70847cb78b..e39913caff 100644 --- a/core/tests/Drupal/Tests/Core/Session/SessionConfigurationTest.php +++ b/core/tests/Drupal/Tests/Core/Session/SessionConfigurationTest.php @@ -246,4 +246,33 @@ public function providerTestEnforcedSessionName() { }, $data); } + /** + * Tests constructor's default settings. + * + * @covers ::__construct + * + * @dataProvider providerTestConstructorDefaultSettings + */ + public function testConstructorDefaultSettings(array $options, int $expected_sid_length, int $expected_sid_bits_per_character) { + $config = $this->createSessionConfiguration($options); + $options = $config->getOptions(Request::createFromGlobals()); + $this->assertSame($expected_sid_length, $options['sid_length']); + $this->assertSame($expected_sid_bits_per_character, $options['sid_bits_per_character']); + } + + /** + * Data provider for the constructor test. + * + * @returns array + * Test data + */ + public function providerTestConstructorDefaultSettings() { + return [ + [[], 48, 6], + [['sid_length' => 100], 100, 6], + [['sid_bits_per_character' => 5], 48, 5], + [['sid_length' => 100, 'sid_bits_per_character' => 5], 100, 5], + ]; + } + } diff --git a/core/tests/Drupal/Tests/Core/Session/SessionManagerTest.php b/core/tests/Drupal/Tests/Core/Session/SessionManagerTest.php new file mode 100644 index 0000000000..12a69854da --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Session/SessionManagerTest.php @@ -0,0 +1,57 @@ +createMock(Connection::class); + $session_configuration = $this->createMock(SessionConfigurationInterface::class); + $session_manager = new SessionManager( + new RequestStack(), + $connection, + new MetadataBag(new Settings([])), + $session_configuration, + new FakeAbstractProxy() + ); + + $this->expectDeprecation('Calling Drupal\Core\Session\SessionManager::getId() outside of an actual existing session is deprecated in drupal:9.2.0 and will be removed in drupal:10.0.0. This is often used for anonymous users. See https://www.drupal.org/node/3006306'); + $this->assertNotEmpty($session_manager->getId()); + } + +} + +class FakeAbstractProxy extends AbstractProxy { + + /** + * Stores the fake session ID. + * + * @var string + */ + protected $id; + + public function setId($id) { + $this->id = $id; + } + + public function getId() { + return $this->id; + } + +} diff --git a/core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreTest.php b/core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreTest.php index 1abe54d184..585fc47ac2 100644 --- a/core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreTest.php +++ b/core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreTest.php @@ -2,13 +2,18 @@ namespace Drupal\Tests\Core\TempStore; +use Drupal\Core\DependencyInjection\ContainerBuilder; +use Drupal\Core\KeyValueStore\KeyValueExpirableFactoryInterface; +use Drupal\Core\Session\AccountProxyInterface; use Drupal\Core\TempStore\Lock; +use Drupal\Core\TempStore\SharedTempStoreFactory; use Drupal\Tests\UnitTestCase; use Drupal\Core\TempStore\SharedTempStore; use Drupal\Core\TempStore\TempStoreException; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpFoundation\Session\SessionInterface; /** * @coversDefaultClass \Drupal\Core\TempStore\SharedTempStore @@ -75,9 +80,12 @@ protected function setUp(): void { $this->lock = $this->createMock('Drupal\Core\Lock\LockBackendInterface'); $this->requestStack = new RequestStack(); $request = Request::createFromGlobals(); + $session = $this->createMock(SessionInterface::class); + $request->setSession($session); $this->requestStack->push($request); + $current_user = $this->createMock(AccountProxyInterface::class); - $this->tempStore = new SharedTempStore($this->keyValue, $this->lock, $this->owner, $this->requestStack, 604800); + $this->tempStore = new SharedTempStore($this->keyValue, $this->lock, $this->owner, $this->requestStack, $current_user, 604800); $this->ownObject = (object) [ 'data' => 'test_data', @@ -383,4 +391,50 @@ public function testSerialization() { $this->assertSame($unserializable_request, $request_stack->pop()); } + /** + * @group legacy + */ + public function testLegacyConstructor() { + $this->expectDeprecation('Calling Drupal\Core\TempStore\SharedTempStore::__construct() without the $current_user argument is deprecated in drupal:9.2.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3006268'); + + $container = new ContainerBuilder(); + $current_user = $this->createMock(AccountProxyInterface::class); + $container->set('current_user', $current_user); + \Drupal::setContainer($container); + $store = new SharedTempStore($this->keyValue, $this->lock, 2, $this->requestStack, 1000); + $reflection_class = new \ReflectionClass(SharedTempStore::class); + + $current_user_property = $reflection_class->getProperty('currentUser'); + $current_user_property->setAccessible(TRUE); + $this->assertSame($current_user, $current_user_property->getValue($store)); + + $expire_property = $reflection_class->getProperty('expire'); + $expire_property->setAccessible(TRUE); + $this->assertSame(1000, $expire_property->getValue($store)); + } + + /** + * @group legacy + * @covers \Drupal\Core\TempStore\SharedTempStoreFactory::__construct + */ + public function testLegacyFactoryConstructor() { + $this->expectDeprecation('Calling Drupal\Core\TempStore\SharedTempStoreFactory::__construct() without the $current_user argument is deprecated in drupal:9.2.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3006268'); + + $container = new ContainerBuilder(); + $current_user = $this->createMock(AccountProxyInterface::class); + $container->set('current_user', $current_user); + \Drupal::setContainer($container); + $key_value_factory = $this->prophesize(KeyValueExpirableFactoryInterface::class); + $store = new SharedTempStoreFactory($key_value_factory->reveal(), $this->lock, $this->requestStack, 1000); + $reflection_class = new \ReflectionClass(SharedTempStoreFactory::class); + + $current_user_property = $reflection_class->getProperty('currentUser'); + $current_user_property->setAccessible(TRUE); + $this->assertSame($current_user, $current_user_property->getValue($store)); + + $expire_property = $reflection_class->getProperty('expire'); + $expire_property->setAccessible(TRUE); + $this->assertSame(1000, $expire_property->getValue($store)); + } + } diff --git a/sites/default/default.services.yml b/sites/default/default.services.yml index 45d986cd8e..9aa48df92d 100644 --- a/sites/default/default.services.yml +++ b/sites/default/default.services.yml @@ -36,6 +36,18 @@ parameters: # @default none # cookie_domain: '.example.com' # + # Specify the length of session ID string. Session ID length can be between + # 22 to 256. The PHP recommended value is 48. See + # https://www.php.net/manual/session.security.ini.php for more information. + # @default 48 + sid_length: 48 + # + # Specify the number of bits in encoded session ID character. The possible + # values are '4' (0-9, a-f), '5' (0-9, a-v), and '6' (0-9, a-z, A-Z, "-", + # ","). The PHP recommended value is 5. See + # https://www.php.net/manual/session.security.ini.php for more information. + # @default 6 + sid_bits_per_character: 6 twig.config: # Twig debugging: #