fix(session): correctly invalidate session on privilege elevation

engine/classes/ElggSession.php

@@ -65,6 +65,8 @@ public function boot(): void {
 
 		$this->start();
 
+		$migrate_session = false;
+
 		// test whether we have a user session
 		if ($this->has('guid')) {
 			$user = _elgg_services()->entityTable->get($this->get('guid'), 'user');
@@ -80,11 +82,12 @@ public function boot(): void {
 			$user = _elgg_services()->persistentLogin->bootSession();
 			if ($user instanceof ElggUser) {
 				_elgg_services()->persistentLogin->updateTokenUsage($user);
+				$migrate_session = true;
 			}
 		}
 
 		if ($user instanceof ElggUser) {
-			$this->setLoggedInUser($user);
+			$this->setLoggedInUser($user, $migrate_session);
 			$user->setLastAction();
 
 			// logout a user with open session who has been banned
@@ -121,7 +124,7 @@ public function start() {
 	 * @return boolean
 	 * @since 1.9
 	 */
-	public function migrate($destroy = false) {
+	public function migrate($destroy = true) {
 		return $this->storage->migrate($destroy);
 	}
 
@@ -275,7 +278,7 @@ public function login(\ElggUser $user, bool $persistent = false): void {
 
 		// #5933: set logged in user early so code in login event will be able to
 		// use elgg_get_logged_in_user_entity().
-		$this->setLoggedInUser($user);
+		$this->setLoggedInUser($user, true);
 		$this->setUserToken($user);
 
 		// re-register at least the core language file for users with language other than site default
@@ -286,9 +289,6 @@ public function login(\ElggUser $user, bool $persistent = false): void {
 			_elgg_services()->persistentLogin->makeLoginPersistent($user);
 		}
 
-		// User's privilege has been elevated, so change the session id (prevents session fixation)
-		$this->migrate();
-
 		// check before updating last login to determine first login
 		$first_login = empty($user->last_login);
 
@@ -334,13 +334,22 @@ public function logout(): bool {
 	/**
 	 * Sets the logged in user
 	 *
-	 * @param \ElggUser $user The user who is logged in
+	 * @param \ElggUser $user    The user who is logged in
+	 * @param bool|null $migrate Migrate the session (default: !\Elgg\Application::isCli())
 	 * @return void
 	 * @since 1.9
 	 */
-	public function setLoggedInUser(\ElggUser $user) {
+	public function setLoggedInUser(\ElggUser $user, bool $migrate = null) {
 		$current_user = $this->getLoggedInUser();
 		if ($current_user != $user) {
+			if (!isset($migrate)) {
+				$migrate = !\Elgg\Application::isCli();
+			}
+
+			if ($migrate) {
+				$this->migrate(true);
+			}
+
 			$this->set('guid', $user->guid);
 			$this->logged_in_user = $user;
 			_elgg_services()->sessionCache->clear();

engine/tests/phpunit/integration/Elgg/Actions/LoginIntegrationTest.php

@@ -38,6 +38,8 @@ public function testLoginWithUsernameAndPassword() {
 
 		$user->setValidationStatus(true, 'login_test');
 
+		$session_id = _elgg_services()->session->getID();
+
 		$response = $this->executeAction('login', [
 			'username' => $user->username,
 			'password' => 123456,
@@ -51,6 +53,9 @@ public function testLoginWithUsernameAndPassword() {
 		$this->assertEquals(elgg_echo('loginok', [], $user->language), array_shift($messages['success']));
 
 		$this->assertEquals($user, _elgg_services()->session->getLoggedInUser());
+
+		// validate that the session id was migrated
+		$this->assertNotEquals($session_id, _elgg_services()->session->getID());
 	}
 
 	public function testLoginWithEmailAndPassword() {
@@ -61,6 +66,8 @@ public function testLoginWithEmailAndPassword() {
 
 		$user->setValidationStatus(true, 'login_test');
 
+		$session_id = _elgg_services()->session->getID();
+
 		$response = $this->executeAction('login', [
 			'username' => $user->email,
 			'password' => 123456,
@@ -70,6 +77,9 @@ public function testLoginWithEmailAndPassword() {
 		$this->assertInstanceOf(OkResponse::class, $response);
 
 		$this->assertEquals($user, _elgg_services()->session->getLoggedInUser());
+
+		// validate that the session id was migrated
+		$this->assertNotEquals($session_id, _elgg_services()->session->getID());
 	}
 
 	public function testLoginFailsWithEmptyPassword() {

engine/tests/phpunit/unit/ElggSessionUnitTest.php

@@ -56,6 +56,32 @@ public function testCanSetLoggedInUser() {
 
 		$this->assertNull($session->getLoggedInUser());
 	}
+
+	public function testSetLoggedInUserChangesSessionID() {
+		$user = $this->createUser();
+
+		$session = \ElggSession::getMock();
+
+		$session->start();
+		$session_id = $session->getID();
+
+		$session->setLoggedInUser($user, true);
+
+		$this->assertNotEquals($session_id, $session->getID());
+	}
+
+	public function testSetLoggedInUserDoesntChangesSessionID() {
+		$user = $this->createUser();
+
+		$session = \ElggSession::getMock();
+
+		$session->start();
+		$session_id = $session->getID();
+
+		$session->setLoggedInUser($user, false);
+
+		$this->assertEquals($session_id, $session->getID());
+	}
 
 	public function testUserTokenValidation() {
 		$user = $this->createUser();