Skip to content

Commit e7d6859

Browse files
committed
Reauthorize recovered room joins
Validate the current room state after a departure wins the duplicate-join race, preventing bans or changed access rules from being bypassed.
1 parent af3602b commit e7d6859

2 files changed

Lines changed: 77 additions & 2 deletions

File tree

server/socket/namespaces/rooms.js

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,12 @@ module.exports = (io, middlewares) => {
386386
}
387387
}
388388

389-
async function joinRoom(room, cb, spectating) {
389+
async function joinRoom(
390+
room,
391+
cb,
392+
spectating,
393+
{ password, reauthorizeOnRecovery = false } = {},
394+
) {
390395
if (socket.roomId) {
391396
if (String(socket.roomId) === String(room._id)) {
392397
socket.room = socket.room || room;
@@ -449,6 +454,41 @@ module.exports = (io, middlewares) => {
449454
message: 'Room no longer exists',
450455
});
451456
}
457+
if (reauthorizeOnRecovery) {
458+
const rejectRecoveryJoin = (error) => {
459+
socket.leave(room.accessCode);
460+
socket.leave(encodeUserRoom(socket.userId, room._id));
461+
delete socket.room;
462+
delete socket.roomId;
463+
return cb(error);
464+
};
465+
const userKey = socket.userId.toString();
466+
if (!isRoomTypeEnabled(currentRoom.type)) {
467+
return rejectRecoveryJoin({
468+
statusCode: 403,
469+
message: 'Grand Prix rooms are disabled',
470+
});
471+
}
472+
if (currentRoom.private && (!password || !(await currentRoom.authenticate(password)))) {
473+
return rejectRecoveryJoin({
474+
statusCode: 403,
475+
message: 'Invalid password',
476+
});
477+
}
478+
if (currentRoom.banned.get(userKey)) {
479+
return rejectRecoveryJoin({
480+
statusCode: 401,
481+
message: 'Banned',
482+
banned: true,
483+
});
484+
}
485+
if (currentRoom.requireRevealedIdentity && !socket.user.showWCAID) {
486+
return rejectRecoveryJoin({
487+
statusCode: 403,
488+
message: 'Must be showing WCA Identity to join room.',
489+
});
490+
}
491+
}
452492
const r = await currentRoom.addUser(socket.user, spectating, sendAdminUpdate);
453493
if (!r) {
454494
const fencedRoom = await currentRoom.advancePresenceRevision(socket.userId);
@@ -564,7 +604,10 @@ module.exports = (io, middlewares) => {
564604
}, room);
565605
}
566606

567-
return await joinRoom(room, acknowledgment, spectating);
607+
return await joinRoom(room, acknowledgment, spectating, {
608+
password,
609+
reauthorizeOnRecovery: true,
610+
});
568611
} catch (e) {
569612
logger.error(e);
570613
return rejectJoin('internal_error', {

server/socket/namespaces/rooms.test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,38 @@ describe('room namespace departures and moderation', () => {
535535
expect(socket.room).toBe(rejoinedRoom);
536536
});
537537

538+
it('does not re-add a user banned while a duplicate join recovers from departure', async () => {
539+
const room = makeRoom({
540+
private: false,
541+
password: null,
542+
inRoom: new Map([['101', true]]),
543+
});
544+
room.advancePresenceRevision.mockResolvedValue(null);
545+
const departedAndBannedRoom = {
546+
...room,
547+
inRoom: new Map([['101', false]]),
548+
banned: new Map([['101', true]]),
549+
addUser: jest.fn(),
550+
};
551+
const connect = setup(new Map([[room._id, room]]));
552+
let fetchCount = 0;
553+
Room.findById.mockImplementation(() => {
554+
const fetchedRoom = fetchCount === 0 ? room : departedAndBannedRoom;
555+
fetchCount += 1;
556+
return queryResult(fetchedRoom);
557+
});
558+
const socket = await connect(makeSocket({ id: 'new-tab', session: {}, userId: 101 }));
559+
560+
const acknowledgment = await joinRoom(socket, { id: room._id });
561+
562+
expect(departedAndBannedRoom.addUser).not.toHaveBeenCalled();
563+
expect(acknowledgment).toHaveBeenCalledWith(expect.objectContaining({
564+
statusCode: 401,
565+
banned: true,
566+
}));
567+
expect(socket.roomId).toBeUndefined();
568+
});
569+
538570
it('finalizes one departure when two tabs explicitly leave together', async () => {
539571
const room = makeRoom({
540572
private: false,

0 commit comments

Comments
 (0)