diff --git a/api/controllers/game/get-game.js b/api/controllers/game/get-game.js index a44cd07c6..a8dba14b8 100644 --- a/api/controllers/game/get-game.js +++ b/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); diff --git a/api/controllers/game/spectate/join.js b/api/controllers/game/spectate/join.js index b13173532..c4714df56 100644 --- a/api/controllers/game/spectate/join.js +++ b/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( diff --git a/tests/e2e/specs/in-game/game-over/rematch.spec.js b/tests/e2e/specs/in-game/game-over/rematch.spec.js index ed74c513f..df6b6645a 100644 --- a/tests/e2e/specs/in-game/game-over/rematch.spec.js +++ b/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(); diff --git a/tests/unit/specs/sails/game-perspectives.spec.js b/tests/unit/specs/sails/game-perspectives.spec.js new file mode 100644 index 000000000..398f85fb5 --- /dev/null +++ b/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(); + }); +});