diff --git a/core/core.services.yml b/core/core.services.yml index 1d9b2d2..49fd29d 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -975,7 +975,7 @@ services: arguments: ['@authentication'] current_user: class: Drupal\Core\Session\AccountProxy - arguments: ['@authentication', '@request_stack'] + arguments: ['@authentication', '@request_stack', '@session_manager'] session_manager: class: Drupal\Core\Session\SessionManager arguments: ['@request_stack', '@database', '@session_manager.metadata_bag', '@settings'] diff --git a/core/lib/Drupal/Core/Cron.php b/core/lib/Drupal/Core/Cron.php index bc256f8..41af5d5 100644 --- a/core/lib/Drupal/Core/Cron.php +++ b/core/lib/Drupal/Core/Cron.php @@ -117,14 +117,9 @@ public function run() { // Allow execution to continue even if the request gets cancelled. @ignore_user_abort(TRUE); - // Prevent session information from being saved while cron is running. - $original_session_saving = $this->sessionManager->isEnabled(); - $this->sessionManager->disable(); - // Force the current user to anonymous to ensure consistent permissions on // cron runs. - $original_user = $this->currentUser->getAccount(); - $this->currentUser->setAccount(new AnonymousUserSession()); + $this->currentUser->impersonateAccount(new AnonymousUserSession()); // Try to allocate enough time to run all the hook_cron implementations. drupal_set_time_limit(240); @@ -151,10 +146,7 @@ public function run() { $this->processQueues(); // Restore the user. - $this->currentUser->setAccount($original_user); - if ($original_session_saving) { - $this->sessionManager->enable(); - } + $this->currentUser->revertAccount(); return $return; } diff --git a/core/lib/Drupal/Core/Installer/InstallerServiceProvider.php b/core/lib/Drupal/Core/Installer/InstallerServiceProvider.php index 6e93561..9d8902d 100644 --- a/core/lib/Drupal/Core/Installer/InstallerServiceProvider.php +++ b/core/lib/Drupal/Core/Installer/InstallerServiceProvider.php @@ -48,6 +48,8 @@ public function register(ContainerBuilder $container) { ->addArgument(new Reference('request_stack')); $container ->register('router.dumper', 'Drupal\Core\Routing\NullMatcherDumper'); + $container + ->register('current_user', 'Drupal\Core\Session\NullAccountProxy'); // Replace the route builder with an empty implementation. // @todo Convert installer steps into routes; add an installer.routing.yml. diff --git a/core/lib/Drupal/Core/Session/AccountProxy.php b/core/lib/Drupal/Core/Session/AccountProxy.php index f9d0d35..cb6fbd2 100644 --- a/core/lib/Drupal/Core/Session/AccountProxy.php +++ b/core/lib/Drupal/Core/Session/AccountProxy.php @@ -45,16 +45,40 @@ class AccountProxy implements AccountProxyInterface { protected $account; /** + * A stack of previous overridden accounts. + * + * @var \Drupal\Core\Session\AccountInterface[] + */ + protected $accountStack = array(); + + /** + * The session manager. + * + * @var \Drupal\Core\Session\SessionManager + */ + protected $sessionManager; + + /** + * The original state of session saving prior to user impersonations. + * + * @var bool + */ + protected $originalSessionSaving; + + /** * Constructs a new AccountProxy. * * @param \Drupal\Core\Authentication\AuthenticationManagerInterface $authentication_manager * The authentication manager. * @param \Symfony\Component\HttpFoundation\Request $request * The request object used for authenticating. + * @param \Drupal\Core\Session\SessionManager $session_manager + * The session manager. */ - public function __construct(AuthenticationManagerInterface $authentication_manager, RequestStack $requestStack) { + public function __construct(AuthenticationManagerInterface $authentication_manager, RequestStack $requestStack, SessionManager $session_manager) { $this->authenticationManager = $authentication_manager; $this->requestStack = $requestStack; + $this->sessionManager = $session_manager; } /** @@ -84,6 +108,57 @@ public function getAccount() { /** * {@inheritdoc} */ + public function impersonateAccount(AccountInterface $account) { + // Prevent session information from being saved and push the previous account. + array_push($this->accountStack, $this->account); + $this->originalSessionSaving = $this->sessionManager->isEnabled(); + $this->sessionManager->disable(); + $this->setAccount($account); + return $this; + } + + /** + * {@inheritdoc} + */ + public function revertAccount() { + // Restore the previous account from the stack. + if (!empty($this->accountStack)) { + $this->account = array_pop($this->accountStack); + } + else { + throw new \RuntimeException('No more account impersonations to revert.'); + } + // Restore original session saving status if all impersonations are reverted. + if (empty($this->accountStack)) { + if ($this->originalSessionSaving) { + $this->sessionManager->enable(); + } + } + return $this; + } + + /** + * {@inheritdoc} + */ + public function revertAll() { + // Restore the original account from the stack. + if (!empty($this->accountStack)) { + $this->account = array_shift($this->accountStack); + $this->accountStack = array(); + } + else { + throw new \RuntimeException('No more account impersonations to revert.'); + } + // Restore original session saving status if all impersonations are reverted. + if ($this->originalSessionSaving) { + $this->sessionManager->enable(); + } + return $this; + } + + /** + * {@inheritdoc} + */ public function id() { return $this->getAccount()->id(); } @@ -185,6 +260,4 @@ public function getTimeZone() { public function getLastAccessedTime() { return $this->getAccount()->getLastAccessedTime(); } - } - diff --git a/core/lib/Drupal/Core/Session/AccountProxyInterface.php b/core/lib/Drupal/Core/Session/AccountProxyInterface.php index 11eee58..3c3f072 100644 --- a/core/lib/Drupal/Core/Session/AccountProxyInterface.php +++ b/core/lib/Drupal/Core/Session/AccountProxyInterface.php @@ -33,5 +33,34 @@ public function setAccount(AccountInterface $account); */ public function getAccount(); + /** + * Sets the currently wrapped account to impersonate another account. + * + * Always remember to call AccountProxyInterface::revertAccount() after this + * call! + * + * @param \Drupal\Core\Session\AccountInterface + * $this. + */ + public function impersonateAccount(AccountInterface $account); + + /** + * Reverts from impersonating another account. + * + * @return \Drupal\Core\Session\AccountInterface + * $this. + */ + public function revertAccount(); + + /** + * Reverts all account impersonations to original account. + * + * This is useful for when there is a failure or an exception and there has + * been possibility of multiple impersonations. + * + * @return \Drupal\Core\Session\AccountInterface + * $this. + */ + public function revertAll(); } diff --git a/core/lib/Drupal/Core/Session/NullAccountProxy.php b/core/lib/Drupal/Core/Session/NullAccountProxy.php new file mode 100644 index 0000000..499fecb --- /dev/null +++ b/core/lib/Drupal/Core/Session/NullAccountProxy.php @@ -0,0 +1,45 @@ +account = new AnonymousUserSession(); + } + + /** + * {@inheritdoc} + */ + public function impersonateAccount(AccountInterface $account) { + // Push the previous account. + $this->accountStack[] = $this->account; + $this->setAccount($account); + return $this->account; + } + + /** + * {@inheritdoc} + */ + public function revertAccount() { + // Restore the previous account from the stack. + if (!empty($this->accountStack)) { + $this->account = array_pop($this->accountStack); + } + return $this->account; + } +} diff --git a/core/modules/simpletest/src/TestBase.php b/core/modules/simpletest/src/TestBase.php index b8bed09..45e91fb 100644 --- a/core/modules/simpletest/src/TestBase.php +++ b/core/modules/simpletest/src/TestBase.php @@ -17,7 +17,6 @@ use Drupal\Core\Database\ConnectionNotDefinedException; use Drupal\Core\Config\StorageInterface; use Drupal\Core\Language\Language; -use Drupal\Core\Session\AccountProxy; use Drupal\Core\Session\AnonymousUserSession; use Drupal\Core\Site\Settings; use Drupal\Core\StreamWrapper\PublicStream; @@ -986,7 +985,6 @@ protected function beforePrepareEnvironment() { * @see TestBase::beforePrepareEnvironment() */ private function prepareEnvironment() { - $user = \Drupal::currentUser(); // Allow (base) test classes to backup global state information. $this->beforePrepareEnvironment(); @@ -1019,7 +1017,6 @@ private function prepareEnvironment() { // simpletest directory if a test is executed within a test. $this->originalFileDirectory = Settings::get('file_public_path', conf_path() . '/files'); $this->originalProfile = drupal_get_profile(); - $this->originalUser = isset($user) ? clone $user : NULL; // Prevent that session data is leaked into the UI test runner by closing // the session and then setting the session-name (i.e. the name of the diff --git a/core/modules/simpletest/src/WebTestBase.php b/core/modules/simpletest/src/WebTestBase.php index 98f68fc..fb74beb 100644 --- a/core/modules/simpletest/src/WebTestBase.php +++ b/core/modules/simpletest/src/WebTestBase.php @@ -101,13 +101,6 @@ protected $additionalCurlOptions = array(); /** - * The original user, before it was changed to a clean uid = 1 for testing. - * - * @var object - */ - protected $originalUser = NULL; - - /** * The original shutdown handlers array, before it was cleaned for testing. * * @var array diff --git a/core/modules/system/src/Tests/Session/UserImpersonatingUserTest.php b/core/modules/system/src/Tests/Session/UserImpersonatingUserTest.php new file mode 100644 index 0000000..2222e58 --- /dev/null +++ b/core/modules/system/src/Tests/Session/UserImpersonatingUserTest.php @@ -0,0 +1,91 @@ + 'Impersonate users', + 'description' => 'Temporarily impersonate another user account, and then restore the original account.', + 'group' => 'Session', + ); + } + + function testUserImpersonateUser() { + $session_manager = $this->container->get('session_manager'); + $user = $this->container->get('current_user'); + $original_user = clone $user; + $original_session_saving = $session_manager->isEnabled(); + + // If not currently logged in, use AccountProxy::impersonateAccount() to switch to + // user 1. If logged in, switch to the anonymous user instead. + if ($user->isAnonymous()) { + $user->impersonateAccount(new UserSession(array('uid' => 1))); + } + else { + $user->impersonateAccount(new AnonymousUserSession()); + } + + // Verify that the active user has changed, and that session saving is + // disabled. + $this->assertEqual($user->id(), ($original_user->id() == 0 ? 1 : 0), 'User switched'); + $this->assertFalse($session_manager->isEnabled(), 'Session saving is disabled.'); + + // Enable session saving for the purpose of this test. + $session_manager->enable(); + + // Perform a second (nested) impersonation. + $user->impersonateAccount(new UserSession(array('uid' => 2))); + $this->assertEqual($user->id(), 2, 'User switched.'); + + // Revert to the user which was active between the first and second + // impersonation attempt. + $user->revertAccount(); + + // Since we are still impersonating the user from the first attempt, + // session handling still needs to be disabled. + $this->assertEqual($user->id(), ($original_user->id() == 0 ? 1 : 0), 'User switched.'); + $this->assertFalse($session_manager->isEnabled(), 'Session saving is disabled.'); + + // Revert to the original user which was active before the first + // impersonation attempt. + $user->revertAccount(); + + // Assert that the original user is the active user again, and that session + // saving has been re-enabled. + $this->assertEqual($user->id(), $original_user->id(), 'Original user correctly restored.'); + $this->assertEqual($session_manager->isEnabled(), $original_session_saving, 'Original session saving correctly restored.'); + + // Verify that AccountProxy::revertAccount and AccountProxy::revertAll() + // will throw exceptions if there is none left in the stack. + try { + $ex1 = new \RuntimeException(); + $user->revertAccount(); + } + catch (\RuntimeException $e) { + $ex1 = $e; + } + $this->assertEqual($ex1->getMessage(), 'No more account impersonations to revert.', 'Revert account throws exception if called without previous impersonation.'); + try { + $ex2 = new \RuntimeException(); + $user->revertAll(); + } + catch (\RuntimeException $e) { + $ex2 = $e; + } + $this->assertEqual($ex2->getMessage(), 'No more account impersonations to revert.', 'Revert all throws exception if called without previous impersonation.'); + } +}