Stabilize room host ownership across rejoins

README.md

@@ -50,6 +50,9 @@ yarn start:client
 
 For more on the internals and contributing, check out the [wiki](https://github.com/coder13/LetsCube/wiki)
 
+The server's owner/admin handoff and reconnect guarantees are documented in
+[Room ownership and administration](docs/room-ownership.md).
+
 ## Metrics
 
 The server stores pseudonymous room and authentication events in both the

docs/room-ownership.md

@@ -0,0 +1,66 @@
+# Room ownership and administration
+
+Rooms keep two separate host roles:
+
+- `owner` is the permanent creator. It does not change when the creator leaves,
+  and the owner may delete the room while absent.
+- `admin` is the active participant who currently has room configuration and
+  moderation controls.
+
+The active admin is selected deterministically after a membership change:
+
+1. The owner is admin whenever they are in the room.
+2. Otherwise, the existing admin remains in control while they are in the room.
+3. If that admin leaves, the first active participant in the room's stored user
+   order takes control.
+4. An empty room has no admin. Its owner remains unchanged, and a normal room
+   enters the existing stale-room expiry window.
+
+There is no user-facing manual transfer operation. If a future operation sets
+an active participant as admin while the owner is absent, membership changes
+preserve that transfer until the admin leaves or the owner returns.
+
+## Disconnect and rejoin behavior
+
+A transient disconnect does not immediately change membership or admin. The
+owner or current admin keeps control during the configured reconnect grace
+period. Reconnecting within that period cancels departure cleanup. After the
+grace period, departure uses the same admin handoff as an explicit leave.
+
+Presence is per user, not per socket. Closing one tab does not hand off admin
+while another socket for the same user remains in the room. Explicitly leaving
+from the last tab finalizes the departure immediately: each leaving tab removes
+itself from the per-user socket group before the remaining-tab check, so two
+simultaneous explicit leaves cannot strand membership. If the owner later
+rejoins, they reclaim admin before the join is acknowledged.
+
+The final persisted departure is a MongoDB compare-and-set keyed by room
+membership and the active user's presence revision. This applies across
+Socket.IO processes: exactly one process can claim a leave, emit its room
+events, and record its metrics. Every authenticated duplicate join advances
+that user's durable presence revision before acknowledgment, so an older leave
+is terminal and cannot overwrite an active new tab.
+
+An empty room persists `admin: null`, but the established `UPDATE_ADMIN`
+Socket.IO event is emitted only for a non-null active admin. Spectators and
+other clients therefore never receive a null admin payload.
+
+Grand Prix rooms use the same role selection when Grand Prix is enabled. When
+it is disabled, create and join requests are rejected before membership or
+roles change.
+
+## Authorization contract
+
+The server refreshes canonical room state before processing room socket events.
+Admin actions must authorize against the current `admin`, not a client-provided
+role or an older socket snapshot. Owner-only operations authorize against the
+unchanging `owner`.
+
+Admins cannot kick or ban themselves through raw socket events; they must use
+the ordinary leave operation so membership, admin handoff, and reconnect
+semantics remain coherent.
+
+Private-room invitations and access approvals may treat the active owner or
+current admin as a host, but must also independently require that host to be an
+active room participant. This prevents an absent owner from granting access
+solely because owner deletion authority survives departure.

server/models/room.js

@@ -7,6 +7,33 @@ const { mirrorRoomChanges } = require('../postgres/dualWrite');
 const PASSWORD_SALT_ROUNDS = 10;
 const STALE_ROOM_LIFETIME_MS = 10 * 60 * 1000;
 
+const sameUser = (left, right) => left && right
+  && String(left.id) === String(right.id);
+
+const selectRoomAdmin = ({ usersInRoom, owner, admin }) => {
+  if (usersInRoom.length === 0) {
+    return null;
+  }
+
+  const activeOwner = usersInRoom.find((user) => sameUser(user, owner));
+  if (activeOwner) {
+    return activeOwner;
+  }
+
+  return usersInRoom.find((user) => sameUser(user, admin)) || usersInRoom[0];
+};
+
+const presenceRevisionFor = (room, userKey) => (
+  room.presenceRevision ? room.presenceRevision.get(userKey) || 0 : 0
+);
+
+const advancePresenceRevisionFor = (room, userKey) => {
+  if (!room.presenceRevision) {
+    room.presenceRevision = new Map();
+  }
+  room.presenceRevision.set(userKey, presenceRevisionFor(room, userKey) + 1);
+};
+
 // const lengths = {
 
 // };
@@ -87,6 +114,17 @@ const Room = new mongoose.Schema({
     of: Boolean,
     default: {},
   },
+  // Incremented for every join or departure so a conditional departure cannot
+  // overwrite a reconnect that raced it on another Socket.IO process.
+  membershipRevision: {
+    type: Number,
+    default: 0,
+  },
+  presenceRevision: {
+    type: Map,
+    of: Number,
+    default: {},
+  },
   admin: { type: mongoose.Schema.Types.ObjectId, ref: 'User' },
   owner: { type: mongoose.Schema.Types.ObjectId, ref: 'User' },
   type: {
@@ -187,6 +225,8 @@ Room.methods.addUser = async function (user, spectating, updateAdmin) {
   }
 
   this.inRoom.set(user.id.toString(), true);
+  this.membershipRevision = (this.membershipRevision || 0) + 1;
+  advancePresenceRevisionFor(this, user.id.toString());
 
   if (this.waitingForCount === 0) {
     this.waitingFor.set(user.id.toString(), true);
@@ -210,10 +250,10 @@ Room.methods.addUser = async function (user, spectating, updateAdmin) {
 Room.methods.dropUser = async function (user, updateAdmin) {
   this.inRoom.set(user.id.toString(), false);
   this.waitingFor.set(user.id.toString(), false);
+  this.membershipRevision = (this.membershipRevision || 0) + 1;
+  advancePresenceRevisionFor(this, user.id.toString());
 
-  if (updateAdmin) {
-    await this.updateAdminIfNeeded(updateAdmin);
-  }
+  await this.updateAdminIfNeeded(updateAdmin);
 
   if (this.usersInRoom.length === 0 && this.type === 'normal') {
     await this.updateStale(true);
@@ -222,6 +262,105 @@ Room.methods.dropUser = async function (user, updateAdmin) {
   return this.save();
 };
 
+Room.methods.dropUserAtomically = async function (
+  user,
+  expectedMembershipRevision = this.membershipRevision || 0,
+  expectedPresenceRevision = presenceRevisionFor(this, user.id.toString()),
+) {
+  const userKey = user.id.toString();
+  const membershipRevision = this.membershipRevision || 0;
+  const presenceRevision = presenceRevisionFor(this, userKey);
+  if (!this.inRoom.get(userKey)
+    || expectedMembershipRevision !== membershipRevision
+    || expectedPresenceRevision !== presenceRevision) {
+    return null;
+  }
+
+  const usersInRoom = this.users.filter((candidate) => (
+    candidate.id.toString() !== userKey
+      && this.inRoom.get(candidate.id.toString())
+  ));
+  const nextAdmin = selectRoomAdmin({
+    usersInRoom,
+    owner: this.owner,
+    admin: this.admin,
+  });
+  const adminChanged = !sameUser(this.admin, nextAdmin);
+  const condition = {
+    _id: this._id,
+    [`inRoom.${userKey}`]: true,
+  };
+  const revisionConditions = [];
+  if (membershipRevision > 0) {
+    condition.membershipRevision = membershipRevision;
+  } else {
+    revisionConditions.push({
+      $or: [
+        { membershipRevision: 0 },
+        { membershipRevision: { $exists: false } },
+      ],
+    });
+  }
+  if (presenceRevision > 0) {
+    condition[`presenceRevision.${userKey}`] = presenceRevision;
+  } else {
+    revisionConditions.push({
+      $or: [
+        { [`presenceRevision.${userKey}`]: 0 },
+        { [`presenceRevision.${userKey}`]: { $exists: false } },
+      ],
+    });
+  }
+  if (revisionConditions.length > 0) {
+    condition.$and = revisionConditions;
+  }
+
+  const update = {
+    $set: {
+      [`inRoom.${userKey}`]: false,
+      [`waitingFor.${userKey}`]: false,
+      admin: nextAdmin ? nextAdmin._id || nextAdmin : null,
+    },
+    $inc: {
+      membershipRevision: 1,
+      [`presenceRevision.${userKey}`]: 1,
+    },
+  };
+  if (usersInRoom.length === 0 && this.type === 'normal') {
+    update.$set.expireAt = new Date(Date.now() + STALE_ROOM_LIFETIME_MS);
+  }
+
+  const updatedRoom = await this.constructor.findOneAndUpdate(condition, update, {
+    new: true,
+  }).populate('users').populate('admin').populate('owner');
+  if (!updatedRoom) {
+    return null;
+  }
+
+  await mirrorRoomChanges(updatedRoom, {
+    attempts: [],
+    participantUserIds: [userKey],
+    syncAllParticipants: false,
+    syncRoomOwners: adminChanged,
+  });
+
+  return { room: updatedRoom, adminChanged };
+};
+
+Room.methods.advancePresenceRevision = function (userId) {
+  const userKey = userId.toString();
+  return this.constructor.findOneAndUpdate({
+    _id: this._id,
+    [`inRoom.${userKey}`]: true,
+  }, {
+    $inc: {
+      [`presenceRevision.${userKey}`]: 1,
+    },
+  }, {
+    new: true,
+  }).populate('users').populate('admin').populate('owner');
+};
+
 Room.methods.banUser = async function (userId) {
   this.banned.set(userId.toString(), true);
   return this.dropUser({ id: userId });
@@ -316,25 +455,20 @@ Room.methods.edit = async function (options) {
   return this.save();
 };
 
-Room.methods.updateAdminIfNeeded = function (cb) {
-  if (this.usersInRoom.length === 0) {
-    this.admin = null;
-    return this.save();
+Room.methods.updateAdminIfNeeded = async function (cb) {
+  const nextAdmin = selectRoomAdmin(this);
+  if (sameUser(this.admin, nextAdmin) || (!this.admin && !nextAdmin)) {
+    return this;
   }
 
-  const findOwner = this.usersInRoom.find((user) => user.id === this.owner.id);
-  if (findOwner && this.admin && this.admin.id !== findOwner.id) {
-    this.admin = findOwner;
-    return this.save().then(cb);
-  }
-
-  if (!this.admin || this.admin.id !== this.usersInRoom[0].id) {
-    const { usersInRoom } = this;
-
-    // eslint-disable-next-line prefer-destructuring
-    this.admin = usersInRoom[0];
-    return this.save().then(cb);
+  this.admin = nextAdmin;
+  const room = await this.save();
+  // UPDATE_ADMIN has always carried a user. Empty rooms persist null without
+  // notifying clients that still expect a populated admin object.
+  if (cb && room.admin) {
+    cb(room);
   }
+  return room;
 };
 
 const collectPostgresChanges = (room) => {
@@ -411,3 +545,4 @@ Room.post('save', async (room) => {
 module.exports.Attempt = Attempt;
 module.exports.collectPostgresChanges = collectPostgresChanges;
 module.exports.Room = Room;
+module.exports.selectRoomAdmin = selectRoomAdmin;