Kirby vulnerable to Insufficient Session Expiration after a password change

@@ -279,18 +279,39 @@ public function currentUserFromSession($session = null)
 
 		$id = $session->data()->get('kirby.userId');
 
+		// if no user is logged in, return immediately
 		if (is_string($id) !== true) {
 			return null;
 		}
 
-		if ($user = $this->kirby->users()->find($id)) {
-			// in case the session needs to be updated, do it now
-			// for better performance
-			$session->commit();
-			return $user;
+		// a user is logged in, ensure it exists
+		$user = $this->kirby->users()->find($id);
+		if ($user === null) {
+			return null;
 		}
 
-		return null;
+		if ($passwordTimestamp = $user->passwordTimestamp()) {
+			$loginTimestamp = $session->data()->get('kirby.loginTimestamp');
+			if (is_int($loginTimestamp) !== true) {
+				// session that was created before Kirby
+				// 3.5.8.3, 3.6.6.3, 3.7.5.2, 3.8.4.1 or 3.9.6
+				// or when the user didn't have a password set
+				$user->logout();
+				return null;
+			}
+
+			// invalidate the session if the password
+			// changed since the login
+			if ($loginTimestamp < $passwordTimestamp) {
+				$user->logout();
+				return null;
+			}
+		}
+
+		// in case the session needs to be updated, do it now
+		// for better performance
+		$session->commit();
+		return $user;
 	}
 
 	/**

@@ -118,6 +118,13 @@ public function changePassword(
 			// update the users collection
 			$user->kirby()->users()->set($user->id(), $user);
 
+			// keep the user logged in to the current browser
+			// if they changed their own password
+			// (regenerate the session token, update the login timestamp)
+			if ($user->isLoggedIn() === true) {
+				$user->loginPasswordless();
+			}
+
 			return $user;
 		});
 	}
@@ -323,7 +330,7 @@ protected function readCredentials(): array
 	 */
 	protected function readPassword()
 	{
-		return F::read($this->root() . '/.htpasswd');
+		return F::read($this->passwordFile());
 	}
 
 	/**
@@ -384,6 +391,6 @@ protected function writePassword(
 		#[SensitiveParameter]
 		string $password = null
 	): bool {
-		return F::write($this->root() . '/.htpasswd', $password);
+		return F::write($this->passwordFile(), $password);
 	}
 }

@@ -441,6 +441,9 @@ public function loginPasswordless($session = null): void
 
 		$session->regenerateToken(); // privilege change
 		$session->data()->set('kirby.userId', $this->id());
+		if ($this->passwordTimestamp() !== null) {
+			$session->data()->set('kirby.loginTimestamp', time());
+		}
 		$this->kirby()->auth()->setUser($this);
 
 		$kirby->trigger('user.login:after', ['user' => $this, 'session' => $session]);
@@ -461,6 +464,7 @@ public function logout($session = null): void
 
 		// remove the user from the session for future requests
 		$session->data()->remove('kirby.userId');
+		$session->data()->remove('kirby.loginTimestamp');
 
 		// clear the cached user object from the app state of the current request
 		$this->kirby()->auth()->flush();
@@ -607,6 +611,26 @@ public function password(): string|null
 		return $this->password = $this->readPassword();
 	}
 
+	/**
+	 * Returns the timestamp when the password
+	 * was last changed
+	 */
+	public function passwordTimestamp(): int|null
+	{
+		$file = $this->passwordFile();
+
+		// ensure we have the latest information
+		// to prevent cache attacks
+		clearstatcache();
+
+		// user does not have a password
+		if (is_file($file) === false) {
+			return null;
+		}
+
+		return filemtime($file);
+	}
+
 	/**
 	 * @return \Kirby\Cms\UserPermissions
 	 */
@@ -881,4 +905,12 @@ public function validatePassword(
 
 		return true;
 	}
+
+	/**
+	 * Returns the path to the password file
+	 */
+	protected function passwordFile(): string
+	{
+		return $this->root() . '/.htpasswd';
+	}
 }

@@ -5,6 +5,7 @@
 use Kirby\Exception\NotFoundException;
 use Kirby\Exception\PermissionException;
 use Kirby\Filesystem\Dir;
+use Kirby\Filesystem\F;
 use Kirby\Session\AutoSession;
 use Throwable;
 
@@ -15,13 +16,13 @@ class AuthTest extends TestCase
 {
 	protected $app;
 	protected $auth;
-	protected $fixtures;
+	protected $tmp;
 
 	public function setUp(): void
 	{
 		$this->app = new App([
 			'roots' => [
-				'index' => $this->fixtures = __DIR__ . '/fixtures/AuthTest'
+				'index' => $this->tmp = __DIR__ . '/tmp'
 			],
 			'options' => [
 				'api' => [
@@ -41,19 +42,21 @@ public function setUp(): void
 				[
 					'email'    => 'homer@simpsons.com',
 					'id'       => 'homer',
-					'password' => password_hash('springfield123', PASSWORD_DEFAULT)
+					'password' => $hash = password_hash('springfield123', PASSWORD_DEFAULT)
 				]
 			]
 		]);
-		Dir::make($this->fixtures . '/site/accounts');
+		Dir::make($this->tmp . '/site/accounts/homer');
+		F::write($this->tmp . '/site/accounts/homer/.htpasswd', $hash);
+		touch($this->tmp . '/site/accounts/homer/.htpasswd', 1337000000);
 
 		$this->auth = $this->app->auth();
 	}
 
 	public function tearDown(): void
 	{
 		$this->app->session()->destroy();
-		Dir::remove($this->fixtures);
+		Dir::remove($this->tmp);
 		App::destroy();
 	}
 
@@ -254,7 +257,7 @@ public function testTypeSession()
 	 * @covers ::status
 	 * @covers ::user
 	 */
-	public function testUserSession1()
+	public function testUserSession()
 	{
 		$session = $this->app->session();
 		$session->set('kirby.userId', 'marge');
@@ -286,10 +289,11 @@ public function testUserSession1()
 	 * @covers ::status
 	 * @covers ::user
 	 */
-	public function testUserSession2()
+	public function testUserSessionManualSession()
 	{
 		$session = (new AutoSession($this->app->root('sessions')))->createManually();
 		$session->set('kirby.userId', 'homer');
+		$session->set('kirby.loginTimestamp', 1337000000);
 
 		$user = $this->auth->user($session);
 		$this->assertSame('homer@simpsons.com', $user->email());
@@ -300,6 +304,47 @@ public function testUserSession2()
 		], $this->auth->status()->toArray());
 	}
 
+	/**
+	 * @covers ::status
+	 * @covers ::user
+	 */
+	public function testUserSessionOldTimestamp()
+	{
+		$session = $this->app->session();
+		$session->set('kirby.userId', 'homer');
+		$session->set('kirby.loginTimestamp', 1000000000);
+
+		$this->assertNull($this->auth->user());
+		$this->assertSame([
+			'challenge' => null,
+			'email'     => null,
+			'status'    => 'inactive'
+		], $this->auth->status()->toArray());
+
+		// user should be logged out completely
+		$this->assertSame([], $session->data()->get());
+	}
+
+	/**
+	 * @covers ::status
+	 * @covers ::user
+	 */
+	public function testUserSessionNoTimestamp()
+	{
+		$session = $this->app->session();
+		$session->set('kirby.userId', 'homer');
+
+		$this->assertNull($this->auth->user());
+		$this->assertSame([
+			'challenge' => null,
+			'email'     => null,
+			'status'    => 'inactive'
+		], $this->auth->status()->toArray());
+
+		// user should be logged out completely
+		$this->assertSame([], $session->data()->get());
+	}
+
 	/**
 	 * @covers ::status
 	 * @covers ::user

@@ -42,6 +42,7 @@ public function setUp(): void
 
 	public function tearDown(): void
 	{
+		$this->app->session()->destroy();
 		Dir::remove($this->tmp);
 	}
 
@@ -390,15 +391,53 @@ public function testChangePasswordHooks()
 				'user.changePassword:after' => function (User $newUser, User $oldUser) use ($phpunit, &$calls) {
 					$phpunit->assertTrue($newUser->validatePassword('topsecret2018'));
 					$phpunit->assertEmpty($oldUser->password());
-					$calls++;
-				}
+					$calls += 2;
+				},
+				'user.login:before' => function () use (&$calls) {
+					$calls += 4;
+				},
+				'user.login:after' => function () use (&$calls) {
+					$calls += 8;
+				},
 			]
 		]);
 
 		$user = $app->user('editor@domain.com');
 		$user->changePassword('topsecret2018');
 
-		$this->assertSame(2, $calls);
+		$this->assertSame(3, $calls);
+	}
+
+	public function testChangePasswordHooksCurrentUser()
+	{
+		$calls = 0;
+		$phpunit = $this;
+
+		$this->app = $this->app->clone([
+			'hooks' => [
+				'user.changePassword:before' => function (User $user, $password) use ($phpunit, &$calls) {
+					$phpunit->assertEmpty($user->password());
+					$phpunit->assertSame('topsecret2018', $password);
+					$calls++;
+				},
+				'user.changePassword:after' => function (User $newUser, User $oldUser) use ($phpunit, &$calls) {
+					$phpunit->assertTrue($newUser->validatePassword('topsecret2018'));
+					$phpunit->assertEmpty($oldUser->password());
+					$calls += 2;
+				},
+				'user.login:before' => function () use (&$calls) {
+					$calls += 4;
+				},
+				'user.login:after' => function () use (&$calls) {
+					$calls += 8;
+				},
+			]
+		]);
+
+		$user = $this->app->user('admin@domain.com');
+		$user->changePassword('topsecret2018');
+
+		$this->assertSame(15, $calls);
 	}
 
 	public function testChangeRoleHooks()

@@ -2,6 +2,8 @@
 
 namespace Kirby\Cms;
 
+use Kirby\Filesystem\F;
+
 class UserAuthTest extends TestCase
 {
 	protected $app;
@@ -19,6 +21,7 @@ public function setUp(): void
 			'users' => [
 				[
 					'email' => 'test@getkirby.com',
+					'id'    => 'testuser',
 					'role'  => 'admin'
 				]
 			]
@@ -96,4 +99,30 @@ public function testLoginLogoutHooks()
 		// each hook needs to be called exactly twice
 		$this->assertSame((1 + 2 + 4 + 8) * 2, $calls);
 	}
+
+	public function testSessionData()
+	{
+		$user    = $this->app->user('test@getkirby.com');
+		$session = $this->app->session();
+
+		$this->assertSame([], $session->data()->get());
+		$user->loginPasswordless();
+		$this->assertSame(['kirby.userId' => 'testuser'], $session->data()->get());
+		$user->logout();
+		$this->assertSame([], $session->data()->get());
+	}
+
+	public function testSessionDataWithPassword()
+	{
+		F::write($this->tmp . '/accounts/testuser/.htpasswd', 'a very secure hash');
+
+		$user    = $this->app->user('test@getkirby.com');
+		$session = $this->app->session();
+
+		$this->assertSame([], $session->data()->get());
+		$user->loginPasswordless();
+		$this->assertSame(['kirby.userId' => 'testuser', 'kirby.loginTimestamp' => 1337000000], $session->data()->get());
+		$user->logout();
+		$this->assertSame([], $session->data()->get());
+	}
 }

@@ -193,6 +193,32 @@ public function testModifiedSpecifyingLanguage()
 		Dir::remove($index);
 	}
 
+	public function testPasswordTimestamp()
+	{
+		$app = new App([
+			'roots' => [
+				'index'    => $this->tmp,
+				'accounts' => $this->tmp
+			]
+		]);
+
+		// create a user file
+		F::write($this->tmp . '/test/index.php', '<?php return [];');
+
+		$user = $app->user('test');
+		$this->assertNull($user->passwordTimestamp());
+
+		// create a password file
+		F::write($this->tmp . '/test/.htpasswd', 'a very secure hash');
+		touch($this->tmp . '/test/.htpasswd', 1337000000);
+
+		$this->assertSame(1337000000, $user->passwordTimestamp());
+
+		// timestamp is not cached
+		touch($this->tmp . '/test/.htpasswd', 1338000000);
+		$this->assertSame(1338000000, $user->passwordTimestamp());
+	}
+
 	public function passwordProvider()
 	{
 		return [