fix(games): don't let unverified players join games as a substitute (#2609)

garrappachc · web-flow · commit 0237149b1df8 · 2023-09-06T11:56:30.000+02:00
diff --git a/src/games/gateways/games.gateway.spec.ts b/src/games/gateways/games.gateway.spec.ts
@@ -8,9 +8,17 @@ import { Player } from '@/players/models/player';
 import { Types } from 'mongoose';
 import { PlayerId } from '@/players/types/player-id';
 import { GameId } from '../game-id';
+import { PlayersService } from '@/players/services/players.service';
+import { ConfigurationService } from '@/configuration/services/configuration.service';
+import { PlayerBansService } from '@/players/services/player-bans.service';
 
 jest.mock('../services/player-substitution.service');
 jest.mock('socket.io');
+jest.mock('@/players/services/players.service', () => ({
+  PlayersService: jest.fn().mockImplementation(() => ({})),
+}));
+jest.mock('@/configuration/services/configuration.service');
+jest.mock('@/players/services/player-bans.service');
 
 const mockGame = {
   id: 'FAKE_GAME_ID',
@@ -25,7 +33,14 @@ describe('GamesGateway', () => {
 
   beforeEach(async () => {
     const module: TestingModule = await Test.createTestingModule({
-      providers: [GamesGateway, PlayerSubstitutionService, Events],
+      providers: [
+        GamesGateway,
+        PlayerSubstitutionService,
+        Events,
+        PlayersService,
+        ConfigurationService,
+        PlayerBansService,
+      ],
     }).compile();
 
     gateway = module.get<GamesGateway>(GamesGateway);
diff --git a/src/games/gateways/games.gateway.ts b/src/games/gateways/games.gateway.ts
@@ -7,6 +7,7 @@ import {
   forwardRef,
   OnModuleInit,
   UseInterceptors,
+  UseGuards,
 } from '@nestjs/common';
 import { Events } from '@/events/events';
 import { WebsocketEvent } from '@/websocket-event';
@@ -17,6 +18,7 @@ import { WebsocketEventEmitter } from '@/shared/websocket-event-emitter';
 import { Types } from 'mongoose';
 import { GameId } from '../game-id';
 import { PlayerId } from '@/players/types/player-id';
+import { CanReplacePlayerGuard } from '../guards/can-replace-player.guard';
 
 @WebSocketGateway()
 export class GamesGateway
@@ -32,6 +34,7 @@ export class GamesGateway
   }
 
   @WsAuthorized()
+  @UseGuards(CanReplacePlayerGuard)
   @SubscribeMessage('replace player')
   @UseInterceptors(SerializerInterceptor)
   async replacePlayer(
diff --git a/src/games/guards/can-replace-player.guard.spec.ts b/src/games/guards/can-replace-player.guard.spec.ts
@@ -0,0 +1,158 @@
+import { PlayersService } from '@/players/services/players.service';
+import { CanReplacePlayerGuard } from './can-replace-player.guard';
+import { PlayerBansService } from '@/players/services/player-bans.service';
+import { ConfigurationService } from '@/configuration/services/configuration.service';
+import { Player } from '@/players/models/player';
+import { ExecutionContext } from '@nestjs/common';
+import { WsException } from '@nestjs/websockets';
+import { PlayerDeniedError } from '@/shared/errors/player-denied.error';
+import { PlayerBan } from '@/players/models/player-ban';
+
+jest.mock('@/configuration/services/configuration.service');
+jest.mock('@/players/services/player-bans.service');
+jest.mock('@/players/services/players.service');
+
+describe('CanReplacePlayerGuard', () => {
+  let guard: CanReplacePlayerGuard;
+  let playersService: jest.Mocked<PlayersService>;
+  let playerBansService: jest.Mocked<PlayerBansService>;
+  let configurationService: jest.Mocked<ConfigurationService>;
+
+  beforeEach(() => {
+    playersService = new PlayersService(
+      // @ts-expect-error
+      null,
+      null,
+      null,
+      null,
+      null,
+      null,
+      null,
+    ) as jest.Mocked<PlayersService>;
+    playerBansService = new PlayerBansService(
+      // @ts-ignore
+      null,
+      null,
+      null,
+      null,
+    ) as jest.Mocked<PlayerBansService>;
+    configurationService = new ConfigurationService(
+      // @ts-ignore
+      null,
+      null,
+    ) as jest.Mocked<ConfigurationService>;
+    guard = new CanReplacePlayerGuard(
+      playersService,
+      configurationService,
+      playerBansService,
+    );
+  });
+
+  beforeEach(() => {
+    configurationService.get.mockImplementation((key: string) =>
+      Promise.resolve(
+        {
+          'queue.deny_players_with_no_skill_assigned': false,
+        }[key],
+      ),
+    );
+    playerBansService.getPlayerActiveBans.mockResolvedValue([]);
+  });
+
+  it('should be defined', () => {
+    expect(guard).toBeDefined();
+  });
+
+  describe('#canActivate()', () => {
+    let client: { user?: Player };
+    let context: jest.Mocked<ExecutionContext>;
+
+    beforeEach(() => {
+      client = {};
+      context = {
+        switchToWs: jest.fn().mockImplementation(() => ({
+          getClient: jest.fn().mockReturnValue(client),
+          getData: jest.fn(),
+        })),
+      } as unknown as jest.Mocked<ExecutionContext>;
+    });
+
+    describe('when not logged in', () => {
+      it('should deny', async () => {
+        await expect(guard.canActivate(context)).rejects.toThrow(WsException);
+      });
+    });
+
+    describe('when the player has not accepted rules', () => {
+      let player: Player;
+
+      beforeEach(() => {
+        player = new Player();
+        player.hasAcceptedRules = false;
+        client.user = player;
+        playersService.getById.mockResolvedValue(player);
+      });
+
+      it('should deny', async () => {
+        await expect(guard.canActivate(context)).rejects.toThrow(
+          PlayerDeniedError,
+        );
+      });
+    });
+
+    describe('when the player has no skill assigned', () => {
+      let player: Player;
+
+      beforeEach(() => {
+        player = new Player();
+        player.hasAcceptedRules = true;
+        client.user = player;
+        playersService.getById.mockResolvedValue(player);
+      });
+
+      describe('and he should be denied', () => {
+        beforeEach(() => {
+          configurationService.get.mockImplementation((key: string) =>
+            Promise.resolve(
+              {
+                'queue.deny_players_with_no_skill_assigned': true,
+              }[key],
+            ),
+          );
+        });
+
+        it('should deny', async () => {
+          await expect(guard.canActivate(context)).rejects.toThrow(
+            PlayerDeniedError,
+          );
+        });
+      });
+
+      describe("but it's ok", () => {
+        it('should allow', async () => {
+          expect(await guard.canActivate(context)).toBe(true);
+        });
+      });
+    });
+
+    describe('when the player is banned', () => {
+      let player: Player;
+
+      beforeEach(() => {
+        player = new Player();
+        player.hasAcceptedRules = true;
+        client.user = player;
+        playersService.getById.mockResolvedValue(player);
+
+        const ban = new PlayerBan();
+        playerBansService.getPlayerActiveBans.mockResolvedValue([ban]);
+      });
+
+      it('should deny', async () => {
+        await expect(guard.canActivate(context)).rejects.toThrow(
+          PlayerDeniedError,
+        );
+      });
+    });
+  });
+});
diff --git a/src/games/guards/can-replace-player.guard.ts b/src/games/guards/can-replace-player.guard.ts
@@ -0,0 +1,47 @@
+import { ConfigurationService } from '@/configuration/services/configuration.service';
+import { PlayerBansService } from '@/players/services/player-bans.service';
+import { PlayersService } from '@/players/services/players.service';
+import {
+  DenyReason,
+  PlayerDeniedError,
+} from '@/shared/errors/player-denied.error';
+import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
+import { WsException } from '@nestjs/websockets';
+import { Socket } from 'socket.io';
+
+@Injectable()
+export class CanReplacePlayerGuard implements CanActivate {
+  constructor(
+    private readonly playersService: PlayersService,
+    private readonly configurationService: ConfigurationService,
+    private readonly playerBansService: PlayerBansService,
+  ) {}
+
+  async canActivate(context: ExecutionContext): Promise<boolean> {
+    const socket = context.switchToWs().getClient<Socket>();
+    if (!socket.user) {
+      throw new WsException('not logged in');
+    }
+
+    const player = await this.playersService.getById(socket.user._id);
+    if (!player.hasAcceptedRules) {
+      throw new PlayerDeniedError(player, DenyReason.playerHasNotAcceptedRules);
+    }
+
+    if (
+      !player.skill &&
+      (await this.configurationService.get<boolean>(
+        'queue.deny_players_with_no_skill_assigned',
+      ))
+    ) {
+      throw new PlayerDeniedError(player, DenyReason.noSkillAssigned);
+    }
+
+    const bans = await this.playerBansService.getPlayerActiveBans(player._id);
+    if (bans.length > 0) {
+      throw new PlayerDeniedError(player, DenyReason.playerIsBanned);
+    }
+
+    return true;
+  }
+}
diff --git a/src/games/services/player-substitution.service.spec.ts b/src/games/services/player-substitution.service.spec.ts
@@ -25,6 +25,7 @@ import { GameId } from '../game-id';
 import { GameEventType } from '../models/game-event-type';
 import { SubstituteRequested } from '../models/events/substitute-requested';
 import { PlayerReplaced } from '../models/events/player-replaced';
+import { PlayerDeniedError } from '@/shared/errors/player-denied.error';
 
 jest.mock('@/players/services/players.service');
 jest.mock('./games.service');
@@ -325,28 +326,6 @@ describe('PlayerSubstitutionService', () => {
       expect(event?._id.equals(mockGame._id)).toBe(true);
     });
 
-    describe('when the replacement player is banned', () => {
-      beforeEach(() => {
-        const end = new Date();
-        end.setHours(end.getHours() + 1);
-        playerBansService.getPlayerActiveBans = () =>
-          Promise.resolve([
-            {
-              player: 'FAKE_PLAYERID',
-              admin: 'FAKE_ADMINID',
-              start: new Date(),
-              end,
-            } as any,
-          ]);
-      });
-
-      it('should reject', async () => {
-        await expect(
-          service.replacePlayer(mockGame._id, player1._id, player3._id),
-        ).rejects.toThrow('player is banned');
-      });
-    });
-
     describe('when replacing an active player', () => {
       it('should reject', async () => {
         await expect(
@@ -406,7 +385,7 @@ describe('PlayerSubstitutionService', () => {
       it('should reject', async () => {
         await expect(
           service.replacePlayer(mockGame._id, player1._id, player3._id),
-        ).rejects.toThrow('player is involved in a currently running game');
+        ).rejects.toThrow(PlayerDeniedError);
       });
     });
 
diff --git a/src/games/services/player-substitution.service.ts b/src/games/services/player-substitution.service.ts
@@ -23,6 +23,10 @@ import { Mutex } from 'async-mutex';
 import { GameId } from '../game-id';
 import { PlayerId } from '@/players/types/player-id';
 import { GameEventType } from '../models/game-event-type';
+import {
+  DenyReason,
+  PlayerDeniedError,
+} from '@/shared/errors/player-denied.error';
 
 /**
  * A service that handles player substitution logic.
@@ -175,14 +179,6 @@ export class PlayerSubstitutionService implements OnModuleInit {
   ) {
     return await this.mutex.runExclusive(async () => {
       const replacement = await this.playersService.getById(replacementId);
-
-      if (
-        (await this.playerBansService.getPlayerActiveBans(replacementId))
-          .length > 0
-      ) {
-        throw new Error('player is banned');
-      }
-
       const game = await this.gamesService.getById(gameId);
       const slot = game.slots.find(
         (slot) =>
@@ -236,7 +232,10 @@ export class PlayerSubstitutionService implements OnModuleInit {
       }
 
       if (replacement.activeGame) {
-        throw new Error('player is involved in a currently running game');
+        throw new PlayerDeniedError(
+          replacement,
+          DenyReason.playerIsInvolvedInGame,
+        );
       }
 
       if (game.findPlayerSlot(replacementId)) {
diff --git a/src/queue/guards/can-join-queue.guard.spec.ts b/src/queue/guards/can-join-queue.guard.spec.ts
@@ -7,7 +7,7 @@ import { PlayersService } from '@/players/services/players.service';
 import { ExecutionContext } from '@nestjs/common';
 import { WsException } from '@nestjs/websockets';
 import { Types } from 'mongoose';
-import { PlayerDeniedError } from '../errors/player-denied.error';
+import { PlayerDeniedError } from '../../shared/errors/player-denied.error';
 import { CanJoinQueueGuard } from './can-join-queue.guard';
 
 jest.mock('@/configuration/services/configuration.service');
diff --git a/src/queue/guards/can-join-queue.guard.ts b/src/queue/guards/can-join-queue.guard.ts
@@ -4,7 +4,10 @@ import { PlayersService } from '@/players/services/players.service';
 import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
 import { WsException } from '@nestjs/websockets';
 import { Socket } from 'socket.io';
-import { DenyReason, PlayerDeniedError } from '../errors/player-denied.error';
+import {
+  DenyReason,
+  PlayerDeniedError,
+} from '../../shared/errors/player-denied.error';
 
 @Injectable()
 export class CanJoinQueueGuard implements CanActivate {
diff --git a/src/shared/errors/player-denied.error.ts b/src/shared/errors/player-denied.error.ts
diff --git a/src/shared/filters/all-exceptions.filter.spec.ts b/src/shared/filters/all-exceptions.filter.spec.ts
@@ -2,7 +2,7 @@ import { Player } from '@/players/models/player';
 import {
   DenyReason,
   PlayerDeniedError,
-} from '@/queue/errors/player-denied.error';
+} from '@/shared/errors/player-denied.error';
 import { AllExceptionsFilter } from './all-exceptions.filter';
 
 describe('AllExceptionsFilter', () => {
