1.9.79: a new password ends the user's other sessions

202-account/account.php

@@ -731,11 +731,12 @@
 		$new_hash = hash_user_pass($newPass);
 		$passSaved = false;
         $passError = '';
+        $generation = null;
         try {
-            // The password and every remember-me key of the user, this
-            // browser's too (a stolen cookie is a copy of one), in one
-            // transaction; the session goes on.
-            \Prosper202\User\PasswordChange::store(
+            // The password, every remember-me key of the user (this
+            // browser's too: a stolen cookie is a copy of one) and every
+            // session of theirs, in one transaction.
+            $generation = \Prosper202\User\PasswordChange::store(
                 new \Prosper202\Database\Connection($db),
                 (int) ($_SESSION['user_own_id'] ?? 0),
                 $new_hash,
@@ -749,9 +750,12 @@
             prosper_log('account', 'Failed to update password: ' . $passError);
 			$pageFlashes[] = ['kind' => 'bad', 'text' => 'Your password could not be changed. Your old password still works; try again.'];
 		} else {
-			// Its key is gone with the rest: the cookie would only be refused.
+			// This session goes on, at the generation the change made; every
+			// other one ends at its next request. Its key is gone with the
+			// rest: the cookie would only be refused.
+			AUTH::keepSessionAfterPasswordChange((int) $generation);
 			AUTH::forgetRememberMeCookie();
-			p202_account_flash('ok', 'Your password is changed. Browsers kept signed in now need it to sign in again; this one stays signed in.');
+			p202_account_flash('ok', 'Your password is changed. Every other browser signed in as you is signed out, and needs it to sign in again; this one stays signed in.');
 			p202_account_redirect('202-account/account.php#password');
 		}
 	}

202-config/Database/Tables/UserTables.php

@@ -32,6 +32,12 @@ public static function getDefinitions(): array
         ];
     }
 
+    /**
+     * user_session_generation counts the changes that end every session of
+     * the user (a new password, a switch-off, a removal): a session holds the
+     * value it began with, and connect.php ends one that holds another
+     * (Prosper202\User\SessionGeneration). From 1.9.79, added by its rung.
+     */
     public static function users(): SchemaDefinition
     {
         return SchemaBuilder::fromRawSql(
@@ -62,6 +68,7 @@ public static function users(): SchemaDefinition
                 `secret_key` char(48) DEFAULT NULL,
                 `user_mods_lb` tinyint(1) unsigned NOT NULL DEFAULT '0',
                 `p202_customer_api_key` char(60) DEFAULT NULL,
+                `user_session_generation` int(10) unsigned NOT NULL DEFAULT '0',
                 PRIMARY KEY (`user_id`),
                 UNIQUE KEY `user_name_2` (`user_name`),
                 KEY `user_name` (`user_name`,`user_pass`),

202-config/User.class.php

@@ -11,6 +11,9 @@ class User
     /** Whether that user may act, read with the row; null when no row was read. */
     private ?\Prosper202\User\UserStanding $standing = null;
 
+    /** The user's session generation, read with the row; null when there is none to compare. */
+    private ?int $generation = null;
+
     public function __construct($user_id)
     {
         try {
@@ -47,7 +50,7 @@ public function __construct($user_id)
         // installed, so no account (index.php and the setup pages load
         // connect.php before the tables exist).
         try {
-            $standing = \Prosper202\User\UserStanding::read(self::$db, $id);
+            [$standing, $generation] = \Prosper202\User\UserStanding::readWithGeneration(self::$db, $id);
         } catch (\Prosper202\Database\Exceptions\QueryException $e) {
             if ($e->getCode() !== 1146) {
                 throw $e;
@@ -56,6 +59,7 @@ public function __construct($user_id)
         }
         $this->userId = $id;
         $this->standing = $standing;
+        $this->generation = $generation;
         // A user who may not act holds no permission, whatever their roles.
         if ($standing->canAct()) {
             $this->loadRoles($id);
@@ -73,6 +77,16 @@ public function standingOf(int $userId): ?\Prosper202\User\UserStanding
         return $this->userId === $userId ? $this->standing : null;
     }
 
+    /**
+     * The user's session generation, as read when this object was built, for
+     * that user only; null when none was read (see standingOf()), or the
+     * database has no such column yet (before the 1.9.79 upgrade).
+     */
+    public function generationOf(int $userId): ?int
+    {
+        return $this->userId === $userId ? $this->generation : null;
+    }
+
     protected function loadRoles($user_id)
     {
         // Ensure $db is a valid mysqli object

202-config/User/MysqlUserRepository.php

@@ -110,7 +110,8 @@ public function update(int $id, array $data): void
             || (array_key_exists('user_active', $data) && !UserStanding::isActive($data['user_active']));
 
         // Switched off, or given a new password, the user's remember-me keys
-        // go in the same transaction (RememberMeKeys).
+        // and open sessions end in the same transaction (RememberMeKeys,
+        // SessionGeneration).
         $this->conn->transaction(function () use ($sets, $types, $values, $id, $revokes): void {
             $stmt = $this->conn->prepareWrite(
                 'UPDATE 202_users SET ' . implode(', ', $sets) . ' WHERE user_id = ?'
@@ -120,6 +121,7 @@ public function update(int $id, array $data): void
             $stmt->close();
             if ($revokes) {
                 RememberMeKeys::revokeAll($this->conn, $id);
+                SessionGeneration::bump($this->conn, $id);
             }
         });
     }

202-config/User/PasswordChange.php

@@ -9,8 +9,9 @@
 
 /**
  * A new password for a user, from Personal settings (the user's own) or the
- * reset link (202-pass-reset.php): the password and the end of every
- * remember-me key of the user, in one transaction.
+ * reset link (202-pass-reset.php): the password, the end of every remember-me
+ * key of the user and of every session they have open (SessionGeneration),
+ * in one transaction.
  *
  * The password changed and the keys stayed, so a stolen remember-me cookie
  * went on signing in after its owner changed the password to lock the thief
@@ -25,19 +26,22 @@ private function __construct()
     }
 
     /**
-     * Store $hash as the user's password and delete every remember-me key of
-     * theirs; with $consumesResetKey, also clear the reset key that allowed it.
-     * The caller holds no transaction; nothing is kept unless all of it is.
+     * Store $hash as the user's password, delete every remember-me key of
+     * theirs and end every session they have open; with $consumesResetKey,
+     * also clear the reset key that allowed it. The caller holds no
+     * transaction; nothing is kept unless all of it is. Answers the user's new
+     * session generation, which the browser that made the change takes to keep
+     * its own session (AUTH::keepSessionAfterPasswordChange()).
      *
      * @throws RuntimeException when nothing was stored (a QueryException for a
      *   failed statement, RuntimeException for a user who is not there)
      */
-    public static function store(Connection $conn, int $userId, string $hash, bool $consumesResetKey): void
+    public static function store(Connection $conn, int $userId, string $hash, bool $consumesResetKey): int
     {
         $sql = $consumesResetKey
             ? 'UPDATE 202_users SET user_pass = ?, user_pass_key = NULL, user_pass_time = 0 WHERE user_id = ?'
             : 'UPDATE 202_users SET user_pass = ? WHERE user_id = ?';
-        $conn->transaction(static function () use ($conn, $sql, $userId, $hash): void {
+        return $conn->transaction(static function () use ($conn, $sql, $userId, $hash): int {
             $stmt = $conn->prepareWrite($sql);
             $conn->bind($stmt, 'si', [$hash, $userId]);
             if ($conn->executeUpdate($stmt) !== 1) {
@@ -46,6 +50,8 @@ public static function store(Connection $conn, int $userId, string $hash, bool $
                 throw new RuntimeException('Storing the password of user ' . $userId . ' changed no row');
             }
             RememberMeKeys::revokeAll($conn, $userId);
+
+            return SessionGeneration::bump($conn, $userId);
         });
     }
 }

202-config/User/SessionGeneration.php

@@ -0,0 +1,74 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Prosper202\User;
+
+use Prosper202\Database\Connection;
+use Prosper202\Database\Exceptions\QueryException;
+use RuntimeException;
+
+/**
+ * The count of changes that end every session of a user
+ * (202_users.user_session_generation): a new password, a switch-off, a
+ * removal. A session holds the value it began with (AUTH::begin_user_session());
+ * connect.php reads the row's with the user it already reads (UserStanding)
+ * and ends a session that holds another, so a browser signed in before the
+ * change is signed out at its next request. A session from before 1.9.79
+ * holds none, which is read as 0, the value every user starts with.
+ *
+ * A counter, not a time: a time compared across PHP's clock and the
+ * database's, or within one second, can call a session begun before the
+ * change one begun after it.
+ */
+final class SessionGeneration
+{
+    public const string BUMP
+        = 'UPDATE 202_users SET user_session_generation = user_session_generation + 1 WHERE user_id = ?';
+
+    public const string READ = 'SELECT user_session_generation FROM 202_users WHERE user_id = ?';
+
+    private function __construct()
+    {
+    }
+
+    /**
+     * Count up the user's generation, on $conn (in the caller's transaction,
+     * with the change that ends the sessions), and answer the new value. A
+     * write that fails or finds no user throws.
+     *
+     * @throws QueryException|RuntimeException
+     */
+    public static function bump(Connection $conn, int $userId): int
+    {
+        try {
+            $stmt = $conn->prepareWrite(self::BUMP);
+            $conn->bind($stmt, 'i', [$userId]);
+            if ($conn->executeUpdate($stmt) !== 1) {
+                throw new RuntimeException('Ending the sessions of user ' . $userId . ' changed no row');
+            }
+            $stmt = $conn->prepareWrite(self::READ);
+            $conn->bind($stmt, 'i', [$userId]);
+            $row = $conn->fetchOne($stmt);
+        } catch (QueryException $e) {
+            throw new QueryException(
+                'Ending the sessions of user ' . $userId . ' failed: ' . $e->getMessage(),
+                (int) $e->getCode(),
+                $e
+            );
+        } catch (\mysqli_sql_exception $e) {
+            throw new QueryException(
+                'Ending the sessions of user ' . $userId . ' failed: ' . $e->getMessage()
+                    . ' [errno ' . (int) $e->getCode() . ']',
+                (int) $e->getCode(),
+                $e
+            );
+        }
+        $generation = $row['user_session_generation'] ?? null;
+        if (!is_numeric($generation)) {
+            throw new RuntimeException('Ending the sessions of user ' . $userId . ': the new generation was not read');
+        }
+
+        return (int) $generation;
+    }
+}

202-config/User/UserDataPurge.php

@@ -261,7 +261,13 @@ public function deleteUser(int $userId): void
                     $this->run($sql, $userId);
                 }
                 $unlinked = (new AppDataPurge($this->db))->purgeUser($userId);
-                $this->run('UPDATE 202_users SET user_deleted = 1 WHERE user_id = ?', $userId);
+                // Every open session of the user ends with the delete
+                // (SessionGeneration), so a row restored by hand stays signed out.
+                $this->run(
+                    'UPDATE 202_users SET user_deleted = 1,'
+                        . ' user_session_generation = user_session_generation + 1 WHERE user_id = ?',
+                    $userId
+                );
 
                 return [$exportFileNames, $unlinked];
             });

202-config/User/UserStanding.php

@@ -26,8 +26,20 @@ enum UserStanding
     case Deleted;
     case Missing;
 
-    /** The row of the user, read from the primary: a replica's copy can predate the switch-off. */
-    public const string READ = 'SELECT user_active, user_deleted FROM 202_users WHERE user_id = ? LIMIT 1';
+    /**
+     * The row of the user, read from the primary: a replica's copy can predate
+     * the switch-off. With the session generation (SessionGeneration), in the
+     * same read.
+     */
+    public const string READ = 'SELECT user_active, user_deleted, user_session_generation'
+        . ' FROM 202_users WHERE user_id = ? LIMIT 1';
+
+    /**
+     * The same, on a database whose upgrade has not run yet: the files are
+     * deployed first, and 202_users has no user_session_generation until the
+     * 1.9.79 rung adds it.
+     */
+    public const string READ_BEFORE_UPGRADE = 'SELECT user_active, user_deleted FROM 202_users WHERE user_id = ? LIMIT 1';
 
     /** @param array<string, mixed>|null $row the user's row (user_active, user_deleted), null for none */
     public static function ofRow(?array $row): self
@@ -59,16 +71,51 @@ private static function is(mixed $value, int $want): bool
      * @throws QueryException with the server's errno as its code where it has one
      */
     public static function read(\mysqli $db, int $userId): self
+    {
+        return self::readWithGeneration($db, $userId)[0];
+    }
+
+    /**
+     * The user's standing and session generation, in one read. The generation
+     * is null when there is none to compare: no row, or a database the 1.9.79
+     * upgrade has not reached yet (then one more read, without the column,
+     * on that request only). A read that fails otherwise throws.
+     *
+     * @return array{self, ?int}
+     * @throws QueryException with the server's errno as its code where it has one
+     */
+    public static function readWithGeneration(\mysqli $db, int $userId): array
     {
         if ($userId <= 0) {
-            return self::Missing;
+            return [self::Missing, null];
+        }
+        try {
+            $row = self::fetch($db, self::READ, $userId);
+        } catch (QueryException $e) {
+            // 1054: unknown column. Only this one is read without it.
+            if ($e->getCode() !== 1054 || !str_contains($e->getMessage(), 'user_session_generation')) {
+                throw $e;
+            }
+            return [self::ofRow(self::fetch($db, self::READ_BEFORE_UPGRADE, $userId)), null];
         }
+        $generation = $row['user_session_generation'] ?? null;
+
+        return [self::ofRow($row), is_numeric($generation) ? (int) $generation : null];
+    }
+
+    /**
+     * @return array<string, mixed>|null
+     * @throws QueryException
+     */
+    private static function fetch(\mysqli $db, string $sql, int $userId): ?array
+    {
         $failed = 'Reading whether user ' . $userId . ' may act failed';
         try {
             $conn = new Connection($db);
-            $stmt = $conn->prepareWrite(self::READ);
+            $stmt = $conn->prepareWrite($sql);
             $conn->bind($stmt, 'i', [$userId]);
-            $row = $conn->fetchOne($stmt);
+
+            return $conn->fetchOne($stmt);
         } catch (QueryException $e) {
             throw new QueryException($failed . ': ' . $e->getMessage(), self::errno($e), $e);
         } catch (\mysqli_sql_exception $e) {
@@ -79,8 +126,6 @@ public static function read(\mysqli $db, int $userId): self
                 $e
             );
         }
-
-        return self::ofRow($row);
     }
 
     /** Whether a value written to user_active leaves the user active. */

202-config/changelogs.json

@@ -73,11 +73,12 @@
       "Fixed: Security. A user switched off or removed in My Account, Users stayed signed in on every browser that already was. Their next request now signs them out, and nothing it asked for is done",
       "Fixed: Security. Signing out left the browser's remember-me key valid, so a copy of its cookie still signed in for up to 14 days, and switching a user off left all of theirs valid, so a remembered browser signed back in once the user was switched on again. Signing out now deletes that browser's key (the user's other remembered browsers stay signed in), and switching a user off in My Account, Users or through the API deletes every one of theirs",
       "Fixed: A sync writes a redirector's rules on the other install whole or not at all. When the other install refused one (since this version, a rule with no criterion), its redirector was left with its own rules removed and only the synced rules before the refused one",
-      "Fixed: Security. Changing a password left every browser kept signed in able to sign in again by its cookie, a stolen copy included. A new password (Personal settings, the reset link, My Account, Users, or the API) now ends them all; the browser making the change on Personal settings stays signed in. A browser already signed in stays signed in until it signs out or its session ends",
       "Fixed: The first page opened with only a remember-me cookie (Keep me signed in) ran without the user's roles, so a page such as My Account, Users sent the user away. It has them now",
       "Fixed: Setup › Traffic Sources refuses an account edit whose link names the account with more than its id (12abc), where it saved the change to account 12",
       "Fixed: p202 report losers and winners with --aff-campaign-id priced every click at a payout of 0 when they could not read the campaign's payout (a server error, a refused key, an unreachable server); they now fail with the error and what to do next. An answer with no payout in it is an error in every command that reads one, and a campaign whose payout is 0 is still priced at 0",
-      "Fixed: The Redirector page no longer drops every destination but the first of a rule made through the API with several destinations and Split test off when you save; it shows the rule as the split test the redirect links make of it, with each destination's weight"
+      "Fixed: The Redirector page no longer drops every destination but the first of a rule made through the API with several destinations and Split test off when you save; it shows the rule as the split test the redirect links make of it, with each destination's weight",
+      "Fixed: Security. Changing a password left every browser kept signed in able to sign in again by its cookie, a stolen copy included. A new password (Personal settings, the reset link, My Account, Users, or the API) now ends them all; the browser making the change on Personal settings stays signed in. A browser already signed in is signed out too",
+      "Fixed: Security. Changing or resetting a password now signs out every other browser signed in as the user, not only those kept signed in; the browser making the change on Personal settings stays signed in. Switching a user off ends their sessions for good, also one that was idle meanwhile"
     ]
   },
   "1.9.78": {

202-config/functions-account-ui.php

@@ -343,9 +343,10 @@ function p202_account_save_currency(\Prosper202\Database\Connection $conn, int $
  * without them; it must not open a transaction of its own.
  *
  * An edit that switches the user off, or sets their password, deletes their
- * remember-me keys in the same transaction (RememberMeKeys): a browser kept
- * signed in must not sign back in when they are switched on again, nor a
- * stolen cookie outlive the password an admin reset.
+ * remember-me keys and ends their open sessions in the same transaction
+ * (RememberMeKeys, SessionGeneration): a browser kept signed in must not sign
+ * back in when they are switched on again, nor a stolen cookie or session
+ * outlive the password an admin reset.
  *
  * @param array<string, string|int|null> $userSet  202_users column => value; the
  *   column names are the caller's own literals, never the request's
@@ -385,6 +386,7 @@ function p202_account_save_user(\Prosper202\Database\Connection $conn, ?int $edi
             $userId = $editUserId;
             if ($revokes) {
                 \Prosper202\User\RememberMeKeys::revokeAll($conn, $userId);
+                \Prosper202\User\SessionGeneration::bump($conn, $userId);
             }
             $stmt = $conn->prepareWrite('DELETE FROM `202_user_role` WHERE `user_id` = ?');
             $conn->bind($stmt, 'i', [$userId]);