fix: hide opponent hand when rematching a reopened finished game

api/controllers/game/get-game.js

@@ -15,27 +15,23 @@ module.exports = async function(req, res) {
       throw new NotFoundError(`Can't find Game ${ gameId }`);
     }
 
-    let userRelationship;
-
-    // Check if user is p0, p1, or a spectator
+    // Room membership follows identity even when a finished game reveals cards.
+    let userRelationship = 'spectator';
     const gameIsFinished = [ GameStatus.FINISHED, GameStatus.ARCHIVED ].includes(game.status);
 
-    if (!gameIsFinished && req.session.usr === game.p0.id) {
+    if (req.session.usr === game.p0?.id) {
       userRelationship = 'p0';
-    } else if (!gameIsFinished && req.session.usr === game.p1.id) {
+    } else if (req.session.usr === game.p1?.id) {
       userRelationship = 'p1';
     } else {
       // Check if user is already spectating this game
       const existingSpectator = game.spectatingUsers?.find(usg => usg.spectator === req.session.usr);
-      if (existingSpectator) {
-        userRelationship = 'spectator';
-      } else {
+      if (!existingSpectator) {
         // Make user a spectator by creating spectatingUsers row
         await UserSpectatingGame.create({ 
           gameSpectated: game.id, 
           spectator: req.session.usr 
         });
-        userRelationship = 'spectator';
       }
     }
 
@@ -55,9 +51,7 @@ module.exports = async function(req, res) {
 
     let gameStateIndex = Number(req.query.gameStateIndex);
     const isValidGameStateIndex = Number.isInteger(gameStateIndex) && gameStateIndex >= -1;
-    gameStateIndex = isValidGameStateIndex ? gameStateIndex :
-      [ GameStatus.FINISHED, GameStatus.ARCHIVED ]
-        .includes(game.status) ? 0 : -1;
+    gameStateIndex = isValidGameStateIndex ? gameStateIndex : gameIsFinished ? 0 : -1;
     const packedGameState = game.gameStates.at(gameStateIndex);
 
     if (!packedGameState) {
@@ -67,8 +61,10 @@ module.exports = async function(req, res) {
     const gameState = unpackGamestate(packedGameState);
     const socketEvents = await createSocketEvents(game, gameState);
 
-    // Return the appropriate state based on user relationship
-    const response = socketEvents[`${userRelationship}State`];
+    // Finished games reveal all cards, but players must stay in their player
+    // rooms so a rematch sends them only their own perspective of the new game.
+    const statePerspective = gameIsFinished ? 'spectator' : userRelationship;
+    const response = socketEvents[`${statePerspective}State`];
 
     return res.ok(response);
 

api/controllers/game/spectate/join.js

@@ -36,8 +36,15 @@ module.exports = async function (req, res) {
       return res.badRequest({ message: 'home.snackbar.spectateNoGamestates' });
     }
 
-    // Subscribe socket to game as spectator
-    sails.sockets.join(req, `game_${gameId}_spectator`);
+    // Players may view their finished games with full visibility, but must
+    // receive rematch events through their player room to keep new hands private.
+    let userRelationship = 'spectator';
+    if (spectator.id === p0.id) {
+      userRelationship = 'p0';
+    } else if (spectator.id === p1.id) {
+      userRelationship = 'p1';
+    }
+    sails.sockets.join(req, `game_${gameId}_${userRelationship}`);
 
     // Add spectating users to table
     const spectatorAlreadyExisted = await UserSpectatingGame.updateOne(

tests/e2e/specs/in-game/game-over/rematch.spec.js

@@ -266,6 +266,71 @@ describe('Creating And Updating Casual Games With Rematch', () => {
     cy.setupGameAsP0(true, false);
   });
 
+  describe('Rematching a reopened finished game', () => {
+    beforeEach(() => {
+      concedePlayer();
+      assertLoss({ wins: 0, losses: 1, stalemates: 0 });
+      cy.location('pathname').as('finishedGamePath', { type: 'static' });
+      cy.get('[data-cy=gameover-go-home]').click();
+      cy.location('pathname').should('equal', '/');
+    });
+
+    function assertRematchHidesOpponentHand(oldGamePath) {
+      const oldGameId = Number(oldGamePath.split('/').pop());
+
+      cy.location('pathname').should('equal', oldGamePath);
+      cy.get('#game-over-dialog').should('be.visible');
+      cy.window().its('cuttle.gameStore.opponent.hand')
+        .should((hand) => {
+          expect(hand.length).to.be.greaterThan(0);
+          expect(hand.every((card) => !card.isHidden && card.rank && card.suit != null)).to.eq(true);
+        });
+
+      cy.rematchOpponent({ gameId: oldGameId, rematch: true });
+      cy.get('[data-cy=gameover-rematch]').click();
+
+      cy.location('pathname').should('match', /^\/game\/\d+$/)
+        .should('not.equal', oldGamePath);
+      cy.get('#game-over-dialog').should('not.exist');
+
+      // Check the actual client state as well as the card backs: private card data
+      // must not carry over from the finished game's spectator perspective.
+      cy.window().its('cuttle.gameStore')
+        .should((game) => {
+          expect(game.id).not.to.eq(oldGameId);
+          expect(game.opponent.hand).to.have.length(5);
+          expect(game.opponent.hand.every(
+            (card) => card.isHidden && !card.id && !card.rank && card.suit == null,
+          )).to.eq(true);
+        });
+      cy.get('[data-opponent-hand-card]').should('have.length', 5)
+        .find('img.opponent-card-back')
+        .should('have.length', 5);
+
+      // The returning player must also receive updates from the new player room.
+      cy.drawCardOpponent();
+      cy.get('[data-opponent-hand-card]').should('have.length', 6)
+        .find('img.opponent-card-back')
+        .should('have.length', 6);
+    }
+
+    it('Hides opponent hand after returning via URL', () => {
+      cy.get('@finishedGamePath').then((oldGamePath) => {
+        cy.visit(oldGamePath);
+        assertRematchHidesOpponentHand(oldGamePath);
+      });
+    });
+
+    it('Hides opponent hand after returning via browser Back', () => {
+      // Browser Back keeps the SPA document and its socket connection alive.
+      cy.window().then((previousWindow) => {
+        cy.go('back');
+        cy.window().should('equal', previousWindow);
+      });
+      cy.get('@finishedGamePath').then(assertRematchHidesOpponentHand);
+    });
+  });
+
   it('Unranked games with rematch', function () {
     // Game 1: Opponent concedes
     cy.concedeOpponent();

tests/unit/specs/sails/game-perspectives.spec.js

@@ -0,0 +1,161 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { createRequire } from 'node:module';
+import { gameFixture } from '../../fixtures/Game';
+
+const require = createRequire(import.meta.url);
+const getGame = require('../../../../api/controllers/game/get-game');
+const joinSpectate = require('../../../../api/controllers/game/spectate/join');
+const GameStatus = require('../../../../utils/GameStatus.json');
+
+// Waterline queries are chainable and awaitable. Keep database access out of
+// these controller tests; socket delivery is exercised by the Cypress regression.
+function gameQuery(game) {
+  return {
+    populate: vi.fn().mockReturnThis(),
+    then: (resolve, reject) => Promise.resolve(game).then(resolve, reject),
+  };
+}
+
+describe('Game visibility and socket room membership', () => {
+  let game;
+  let req;
+  let res;
+  let socketEvents;
+
+  beforeEach(() => {
+    game = _.cloneDeep(gameFixture);
+    game.gameStates = [ { id: 101 }, { id: 102 } ];
+    req = { params: { gameId: String(game.id) }, session: { usr: game.p0.id }, query: {} };
+    res = {
+      ok: vi.fn(),
+      badRequest: vi.fn(),
+      serverError: vi.fn(),
+      status: vi.fn().mockReturnThis(),
+      json: vi.fn(),
+    };
+    socketEvents = {
+      p0State: { game: { perspective: 'p0' } },
+      p1State: { game: { perspective: 'p1' } },
+      spectatorState: { game: { perspective: 'spectator' } },
+    };
+
+    vi.spyOn(Game, 'findOne').mockReturnValue(gameQuery(game));
+    vi.spyOn(User, 'findOne').mockImplementation(async ({ id }) => ({ id, username: 'viewer' }));
+    vi.spyOn(UserSpectatingGame, 'create').mockResolvedValue(undefined);
+    vi.spyOn(UserSpectatingGame, 'updateOne').mockReturnValue({
+      set: vi.fn().mockResolvedValue({ id: 1 }),
+    });
+    vi.spyOn(sails.sockets, 'join').mockReturnValue(true);
+    vi.spyOn(sails.helpers.gameStates, 'unpackGamestate').mockReturnValue({ id: 'unpacked' });
+    vi.spyOn(sails.helpers.gameStates, 'createSocketEvents').mockResolvedValue(socketEvents);
+    vi.spyOn(sails.helpers, 'broadcastGameEvent').mockReturnValue(undefined);
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  for (const status of [ GameStatus.STARTED, GameStatus.FINISHED, GameStatus.ARCHIVED ]) {
+    for (const role of [ 'p0', 'p1' ]) {
+      it(`Keeps ${role} in their player room when loading a game with status ${status}`, async () => {
+        game.status = status;
+        req.session.usr = game[role].id;
+
+        await getGame(req, res);
+
+        const perspective = status === GameStatus.STARTED ? role : 'spectator';
+        expect(sails.sockets.join).toHaveBeenCalledExactlyOnceWith(req, `game_${game.id}_${role}`);
+        expect(res.ok).toHaveBeenCalledExactlyOnceWith(socketEvents[`${perspective}State`]);
+        expect(UserSpectatingGame.create).not.toHaveBeenCalled();
+        const expectedState = game.gameStates[status === GameStatus.STARTED ? 1 : 0];
+        expect(sails.helpers.gameStates.unpackGamestate).toHaveBeenCalledWith(expectedState);
+      });
+    }
+  }
+
+  it('Registers a new non-player spectator and returns full visibility', async () => {
+    req.session.usr = 99;
+
+    await getGame(req, res);
+
+    expect(sails.sockets.join).toHaveBeenCalledExactlyOnceWith(req, `game_${game.id}_spectator`);
+    expect(UserSpectatingGame.create).toHaveBeenCalledExactlyOnceWith({
+      gameSpectated: game.id, spectator: 99,
+    });
+    expect(res.ok).toHaveBeenCalledWith(socketEvents.spectatorState);
+  });
+
+  it('Does not duplicate an existing spectator registration', async () => {
+    req.session.usr = 99;
+    game.spectatingUsers = [ { spectator: 99 } ];
+
+    await getGame(req, res);
+
+    expect(UserSpectatingGame.create).not.toHaveBeenCalled();
+    expect(res.ok).toHaveBeenCalledWith(socketEvents.spectatorState);
+  });
+
+  it('Allows selecting the last state of a finished game', async () => {
+    game.status = GameStatus.FINISHED;
+    req.query.gameStateIndex = '-1';
+
+    await getGame(req, res);
+
+    expect(sails.helpers.gameStates.unpackGamestate).toHaveBeenCalledWith(game.gameStates[1]);
+    expect(res.ok).toHaveBeenCalledWith(socketEvents.spectatorState);
+  });
+
+  it('Returns 404 for a missing game', async () => {
+    Game.findOne.mockReturnValue(gameQuery(null));
+
+    await getGame(req, res);
+
+    expect(res.status).toHaveBeenCalledWith(404);
+    expect(sails.sockets.join).not.toHaveBeenCalled();
+  });
+
+  for (const status of [ GameStatus.FINISHED, GameStatus.ARCHIVED ]) {
+    for (const role of [ 'p0', 'p1', 'spectator' ]) {
+      it(`Uses the ${role} room when replaying a game with status ${status}`, async () => {
+        game.status = status;
+        req.session.usr = role === 'spectator' ? 99 : game[role].id;
+
+        await joinSpectate(req, res);
+
+        expect(sails.sockets.join).toHaveBeenCalledExactlyOnceWith(req, `game_${game.id}_${role}`);
+        expect(res.ok).toHaveBeenCalledExactlyOnceWith(socketEvents.spectatorState);
+      });
+    }
+  }
+
+  for (const role of [ 'p0', 'p1' ]) {
+    it(`Prevents ${role} from spectating their live game`, async () => {
+      req.session.usr = game[role].id;
+
+      await joinSpectate(req, res);
+
+      expect(res.status).toHaveBeenCalledWith(409);
+      expect(sails.sockets.join).not.toHaveBeenCalled();
+      expect(sails.helpers.gameStates.createSocketEvents).not.toHaveBeenCalled();
+    });
+  }
+
+  it('Allows a non-player to spectate a live game', async () => {
+    req.session.usr = 99;
+
+    await joinSpectate(req, res);
+
+    expect(sails.sockets.join).toHaveBeenCalledExactlyOnceWith(req, `game_${game.id}_spectator`);
+    expect(res.ok).toHaveBeenCalledWith(socketEvents.spectatorState);
+  });
+
+  it('Rejects spectating a game with no saved states', async () => {
+    game.status = GameStatus.FINISHED;
+    game.gameStates = [];
+
+    await joinSpectate(req, res);
+
+    expect(res.badRequest).toHaveBeenCalledWith({ message: 'home.snackbar.spectateNoGamestates' });
+    expect(sails.sockets.join).not.toHaveBeenCalled();
+  });
+});