From 12763a146ebc550963b5d368633539701e7884ec Mon Sep 17 00:00:00 2001
From: Bizley <pawel@positive.codes>
Date: Wed, 29 Dec 2021 10:45:13 +0100
Subject: [PATCH] Fix #19067: Fix constant session regeneration (#19113)

* Fix constant session regeneration
* Add tests for session ID, always regenerating session ID for switching identity
---
 framework/web/User.php           | 11 +++----
 tests/framework/web/UserTest.php | 53 +++++++++++++++++++++++++++++---
 2 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/framework/web/User.php b/framework/web/User.php
index 863b6d4b428..010987e1351 100644
--- a/framework/web/User.php
+++ b/framework/web/User.php
@@ -655,9 +655,7 @@ public function switchIdentity($identity, $duration = 0)
         }
 
         $session = Yii::$app->getSession();
-        if (!YII_ENV_TEST) {
-            $session->regenerateID(true);
-        }
+        $session->regenerateID(true);
         $session->remove($this->idParam);
         $session->remove($this->authTimeoutParam);
         $session->remove($this->authKeyParam);
@@ -698,6 +696,9 @@ protected function renewAuthStatus()
             /* @var $class IdentityInterface */
             $class = $this->identityClass;
             $identity = $class::findIdentity($id);
+            if ($identity === null) {
+                $this->switchIdentity(null);
+            }
         }
 
         if ($identity !== null) {
@@ -728,10 +729,6 @@ protected function renewAuthStatus()
                 $this->renewIdentityCookie();
             }
         }
-
-        if ($this->getIdentity(false) === null) {
-            $this->switchIdentity(null);
-        }
     }
 
     /**
diff --git a/tests/framework/web/UserTest.php b/tests/framework/web/UserTest.php
index b37eeed313e..3d1b323efd4 100644
--- a/tests/framework/web/UserTest.php
+++ b/tests/framework/web/UserTest.php
@@ -56,8 +56,8 @@ public function testLoginExpires()
                 'authManager' => [
                     'class' => PhpManager::className(),
                     'itemFile' => '@runtime/user_test_rbac_items.php',
-                     'assignmentFile' => '@runtime/user_test_rbac_assignments.php',
-                     'ruleFile' => '@runtime/user_test_rbac_rules.php',
+                    'assignmentFile' => '@runtime/user_test_rbac_assignments.php',
+                    'ruleFile' => '@runtime/user_test_rbac_rules.php',
                 ],
             ],
         ];
@@ -169,6 +169,7 @@ public function testCookieCleanup()
         ];
 
         $this->mockWebApplication($appConfig);
+        $id1 = Yii::$app->session->id;
         Yii::$app->session->removeAll();
 
         $cookie = new Cookie(Yii::$app->user->identityCookie);
@@ -176,16 +177,21 @@ public function testCookieCleanup()
         $cookiesMock->add($cookie);
         Yii::$app->user->getIdentity();
         $this->assertEquals(strlen($cookiesMock->getValue(Yii::$app->user->identityCookie['name'])), 0);
+        $this->assertSame($id1, Yii::$app->session->id);
 
         Yii::$app->user->login(UserIdentity::findIdentity('user1'), 3600);
         $this->assertFalse(Yii::$app->user->isGuest);
         $this->assertSame(Yii::$app->user->id, 'user1');
         $this->assertNotEquals(strlen($cookiesMock->getValue(Yii::$app->user->identityCookie['name'])), 0);
+        $id2 = Yii::$app->session->id;
+        $this->assertNotSame($id1, $id2);
 
         Yii::$app->user->login(UserIdentity::findIdentity('user2'), 0);
         $this->assertFalse(Yii::$app->user->isGuest);
         $this->assertSame(Yii::$app->user->id, 'user2');
         $this->assertEquals(strlen($cookiesMock->getValue(Yii::$app->user->identityCookie['name'])), 0);
+        $id3 = Yii::$app->session->id;
+        $this->assertNotSame($id2, $id3);
     }
 
     /**
@@ -225,7 +231,7 @@ public function testLoginRequired()
             ],
         ];
         $this->mockWebApplication($appConfig);
-
+        $id = Yii::$app->session->id;
 
         $user = Yii::$app->user;
 
@@ -234,6 +240,7 @@ public function testLoginRequired()
         $user->loginRequired();
         $this->assertEquals('normal', $user->getReturnUrl());
         $this->assertTrue(Yii::$app->response->getIsRedirection());
+        $this->assertSame($id, Yii::$app->session->id);
 
 
         $this->reset();
@@ -242,12 +249,13 @@ public function testLoginRequired()
 
         $user->loginRequired();
         $this->assertEquals(Yii::$app->getHomeUrl(), $user->getReturnUrl());
-        // AJAX requests don't update returnUrl but they do cause redirection.
+        // AJAX requests don't update returnUrl, but they do cause redirection.
         $this->assertTrue(Yii::$app->response->getIsRedirection());
 
         $user->loginRequired(false);
         $this->assertEquals('ajax', $user->getReturnUrl());
         $this->assertTrue(Yii::$app->response->getIsRedirection());
+        $this->assertSame($id, Yii::$app->session->id);
 
         $this->reset();
         Yii::$app->request->setUrl('json-only');
@@ -255,6 +263,7 @@ public function testLoginRequired()
         $user->loginRequired(true, false);
         $this->assertEquals('json-only', $user->getReturnUrl());
         $this->assertTrue(Yii::$app->response->getIsRedirection());
+        $this->assertSame($id, Yii::$app->session->id);
 
         $this->reset();
         Yii::$app->request->setUrl('json-only');
@@ -262,6 +271,7 @@ public function testLoginRequired()
         $user->loginRequired(true, false);
         $this->assertEquals('json-only', $user->getReturnUrl());
         $this->assertTrue(Yii::$app->response->getIsRedirection());
+        $this->assertSame($id, Yii::$app->session->id);
 
         $this->reset();
         Yii::$app->request->setUrl('accept-all');
@@ -269,6 +279,7 @@ public function testLoginRequired()
         $user->loginRequired();
         $this->assertEquals('accept-all', $user->getReturnUrl());
         $this->assertTrue(Yii::$app->response->getIsRedirection());
+        $this->assertSame($id, Yii::$app->session->id);
 
         $this->reset();
         Yii::$app->request->setUrl('json-and-accept-all');
@@ -278,6 +289,7 @@ public function testLoginRequired()
         } catch (ForbiddenHttpException $e) {
         }
         $this->assertFalse(Yii::$app->response->getIsRedirection());
+        $this->assertSame($id, Yii::$app->session->id);
 
         $this->reset();
         Yii::$app->request->setUrl('accept-html-json');
@@ -285,6 +297,7 @@ public function testLoginRequired()
         $user->loginRequired();
         $this->assertEquals('accept-html-json', $user->getReturnUrl());
         $this->assertTrue(Yii::$app->response->getIsRedirection());
+        $this->assertSame($id, Yii::$app->session->id);
 
         $this->reset();
         Yii::$app->request->setUrl('accept-html-json');
@@ -292,6 +305,7 @@ public function testLoginRequired()
         $user->loginRequired();
         $this->assertEquals('accept-html-json', $user->getReturnUrl());
         $this->assertTrue(Yii::$app->response->getIsRedirection());
+        $this->assertSame($id, Yii::$app->session->id);
 
         $this->reset();
         $_SERVER['REQUEST_METHOD'] = 'POST';
@@ -299,6 +313,7 @@ public function testLoginRequired()
         Yii::$app->getSession()->set($user->returnUrlParam, null);
         $user->loginRequired();
         $this->assertNull(Yii::$app->getSession()->get($user->returnUrlParam));
+        $this->assertSame($id, Yii::$app->session->id);
 
         $this->reset();
         $_SERVER['REQUEST_METHOD'] = 'GET';
@@ -306,6 +321,7 @@ public function testLoginRequired()
         Yii::$app->getSession()->set($user->returnUrlParam, null);
         $user->loginRequired();
         $this->assertEquals('set-return-url-on-get-request', Yii::$app->getSession()->get($user->returnUrlParam));
+        $this->assertSame($id, Yii::$app->session->id);
 
         // Confirm that returnUrl is not set.
         $this->reset();
@@ -316,11 +332,13 @@ public function testLoginRequired()
         } catch (ForbiddenHttpException $e) {
         }
         $this->assertNotEquals('json-only', $user->getReturnUrl());
+        $this->assertSame($id, Yii::$app->session->id);
 
         $this->reset();
         $_SERVER['HTTP_ACCEPT'] = 'text/json;q=0.1';
         $this->expectException('yii\\web\\ForbiddenHttpException');
         $user->loginRequired();
+        $this->assertSame($id, Yii::$app->session->id);
     }
 
     public function testLoginRequiredException1()
@@ -482,8 +500,10 @@ public function testSessionAuthWithMissingKey()
         $this->mockWebApplication($appConfig);
 
         Yii::$app->session->set('__id', 'user1');
+        $id = Yii::$app->session->id;
 
         $this->assertNotNull(Yii::$app->user->getIdentity());
+        $this->assertSame($id, Yii::$app->session->id);
     }
 
     public function testSessionAuthWithInvalidKey()
@@ -497,12 +517,13 @@ public function testSessionAuthWithInvalidKey()
         ];
 
         $this->mockWebApplication($appConfig);
+        $id = Yii::$app->session->id;
 
         Yii::$app->session->set('__id', 'user1');
         Yii::$app->session->set('__authKey', 'invalid');
 
-
         $this->assertNull(Yii::$app->user->getIdentity());
+        $this->assertSame($id, Yii::$app->session->id);
     }
 
     public function testSessionAuthWithValidKey()
@@ -516,11 +537,33 @@ public function testSessionAuthWithValidKey()
         ];
 
         $this->mockWebApplication($appConfig);
+        $id = Yii::$app->session->id;
 
         Yii::$app->session->set('__id', 'user1');
         Yii::$app->session->set('__authKey', 'ABCD1234');
 
         $this->assertNotNull(Yii::$app->user->getIdentity());
+        $this->assertSame($id, Yii::$app->session->id);
+    }
+
+    public function testSessionAuthWhenIdentityReturnsNull()
+    {
+        $appConfig = [
+            'components' => [
+                'user' => [
+                    'identityClass' => UserIdentity::className(),
+                ],
+            ],
+        ];
+
+        $this->mockWebApplication($appConfig);
+        $id = Yii::$app->session->id;
+
+        Yii::$app->session->set('__id', 'user999999');
+        Yii::$app->session->set('__authKey', 'ABCD1234');
+
+        $this->assertNull(Yii::$app->user->getIdentity());
+        $this->assertNotSame($id, Yii::$app->session->id);
     }
 }