Correct handling of deleted sessions after SID change

A deleted session only exists for 30 more seconds.

devel/manual/sessions.md

@@ -38,15 +38,17 @@ encoding](https://www.php.net/manual/en/function.session-encode.php).
     the email address of the first account to log in, `us[1]` the second, and
     so forth. A URL like `https://conf.hotcrp.com/u/N/` accesses the given
     conference using account `N`. If `us` is not set but `u` is, then `us`
-    defaults to the one-element list `[u]`.
+    defaults to the one-element list `[u]`. If an account is removed from an
+    active session, but the session remains active, then that entry in `us`
+    is set to an empty string.
 
 * `uchoice` (associative array): Account choice
 
     When a user is signed in to multiple accounts, HotCRP tries to remember
     the most relevant account for each conference. On receiving a request for
     a plain conference URL (e.g., `https://conf.hotcrp.com/` rather than
     `https://conf.hotcrp.com/u/1/`), HotCRP looks up the best user for that
-    conference using `uchoice` and redirect appropriately.
+    conference using `uchoice` and redirects appropriately.
 
     `uchoice` is keyed by conference session key. The value for a conference
     is a two-element list `[USERINDEX, USETIME]`. `USERINDEX` is the most

lib/qsession.php

@@ -9,7 +9,7 @@ class Qsession {
     /** @var bool */
     protected $sopen = false;
     /** @var 0|1|2 */
-    protected $opentype = 0;
+    private $opentype = 0;
 
     function maybe_open() {
         if (!$this->sopen && isset($_COOKIE[session_name()])) {
@@ -34,10 +34,10 @@ function reopen() {
     }
 
     function handle_open() {
-        if ($this->sopen && $this->opentype !== 1) {
+        if ($this->opentype !== 1 && $this->sopen) {
             return;
         }
-        if ($this->sid !== null && $this->opentype === 2) {
+        if ($this->opentype === 2 && $this->sid !== null) {
             $sid = $this->start($this->sid);
             assert($sid === $this->sid);
             return;
@@ -58,16 +58,55 @@ function handle_open() {
             $this->sopen = true;
         }
 
-        // if reopened, empty [session fixation], or old, reset session
-        $curv = $this->all();
-        if (empty($curv)
-            || ($curv["deletedat"] ?? Conf::$now) < Conf::$now - 30) {
-            $this->opentype = 1;
+        // reset session while reopened, empty [session fixation], or deleted
+        if ($this->sid === $cookie_sid) {
+            $this->check_reopen();
+            if ($this->sid === null) {
+                return;
+            }
         }
-        if ($this->sid === $cookie_sid && $this->opentype === 1) {
-            $nsid = $this->new_sid();
-            if (!isset($curv["deletedat"])) {
+
+        // maybe update session format
+        if (($this->get("v") ?? 0) < 2) {
+            if (empty($this->all())) {
+                $this->set("v", 2);
+            } else {
+                UpdateSession::run($this);
+            }
+        }
+        if ($this->get("u") || $this->sid !== $cookie_sid) {
+            $this->refresh();
+        }
+    }
+
+    private function check_reopen() {
+        $tries = 0;
+        while (true) {
+            $curv = $this->all();
+            if (($this->opentype !== 1 || $tries > 0)
+                && (!empty($curv) || $tries > 0)
+                && !isset($curv["deletedat"])) {
+                return;
+            }
+            ++$tries;
+
+            $nsid = null;
+            if (isset($curv["deletedat"])) {
+                $transfer = false;
+                if ($curv["deletedat"] >= Conf::$now - 30
+                    && isset($curv["new_sid"])
+                    && is_string($curv["new_sid"])
+                    && $tries < 10) {
+                    $nsid = $curv["new_sid"];
+                }
+            } else {
+                $transfer = !empty($curv);
+            }
+
+            $nsid = $nsid ?? $this->new_sid();
+            if ($transfer) {
                 $this->set("deletedat", Conf::$now);
+                $this->set("new_sid", $nsid);
             }
             $this->commit();
 
@@ -77,23 +116,16 @@ function handle_open() {
                 return;
             }
             $this->sopen = true;
-            unset($curv["deletedat"]);
-            foreach ($curv as $k => $v) {
-                $this->set($k, $v);
-            }
-        }
 
-        // maybe update session format
-        if (empty($this->all())) {
-            $this->set("v", 2);
-        } else {
-            if (($this->get("v") ?? 0) < 2) {
-                UpdateSession::run($this);
+            if ($transfer) {
+                // `unset` should be a no-op, because we never transfer data
+                // from a deleted session:
+                unset($curv["deletedat"], $curv["new_sid"]);
+                foreach ($curv as $k => $v) {
+                    $this->set($k, $v);
+                }
             }
         }
-        if ($this->get("u") || $cookie_sid !== $this->sid) {
-            $this->refresh();
-        }
     }
 
 

src/updatesession.php

@@ -146,6 +146,7 @@ static function usec_query(Qrequest $qreq, $email, $type, $reason, $bound = 0) {
      * @param 0|1 $reason
      * @param bool $success */
     static function usec_add(Qrequest $qreq, $email, $type, $reason, $success) {
+        // See `etc/devel/sessions.md` for format information
         $uindex = Contact::session_index_by_email($qreq, $email);
         $old_usec = $qreq->gsession("usec") ?? [];
         $nold_usec = count($old_usec);