bug: harden website chat sends (auth, 2000-char cap, no relay of rejected messages)

src/chat/chat.gateway.spec.ts

@@ -0,0 +1,278 @@
+import { ChatGateway } from "./chat.gateway";
+import { ChatService } from "./chat.service";
+import { ChatErrorCode } from "./enums/ChatErrorCode";
+import { ChatLobbyType } from "./enums/ChatLobbyTypes";
+
+describe("ChatGateway lobby:chat", () => {
+  let chat: { sendMessageToChat: jest.Mock; sendChatToServer: jest.Mock };
+  let gateway: ChatGateway;
+
+  const client = (user: any = { steam_id: "1", name: "Luke", role: "user" }) =>
+    ({ id: "client-1", user, send: jest.fn() }) as any;
+
+  const sent = (socket: { send: jest.Mock }) =>
+    socket.send.mock.calls.map(([raw]) => JSON.parse(raw));
+
+  beforeEach(() => {
+    chat = {
+      sendMessageToChat: jest
+        .fn()
+        .mockResolvedValue({ accepted: true, messageId: "msg-1" }),
+      sendChatToServer: jest.fn(),
+    };
+    gateway = new ChatGateway(chat as any);
+  });
+
+  describe("input", () => {
+    it("ignores a socket that has not signed in", async () => {
+      const socket = client(null);
+
+      await gateway.lobby(
+        { id: "m-1", type: ChatLobbyType.Match, message: "hi" },
+        socket,
+      );
+
+      expect(chat.sendMessageToChat).not.toHaveBeenCalled();
+      expect(chat.sendChatToServer).not.toHaveBeenCalled();
+    });
+
+    it.each([
+      ["a number", 5],
+      ["an object", { toString: "x" }],
+      ["an array", ["hi"]],
+      ["null", null],
+      ["missing", undefined],
+      ["only whitespace", "   \n  "],
+    ])("ignores a message that is %s", async (_, message) => {
+      const socket = client();
+
+      await expect(
+        gateway.lobby(
+          { id: "m-1", type: ChatLobbyType.Match, message },
+          socket,
+        ),
+      ).resolves.toBeUndefined();
+
+      expect(chat.sendMessageToChat).not.toHaveBeenCalled();
+      expect(socket.send).not.toHaveBeenCalled();
+    });
+
+    it("ignores a lobby type it does not know", async () => {
+      await gateway.lobby(
+        { id: "m-1", type: "global" as ChatLobbyType, message: "hi" },
+        client(),
+      );
+
+      expect(chat.sendMessageToChat).not.toHaveBeenCalled();
+    });
+
+    it("ignores a room id that is not a string", async () => {
+      await gateway.lobby(
+        { id: { $ne: 1 } as any, type: ChatLobbyType.Match, message: "hi" },
+        client(),
+      );
+
+      expect(chat.sendMessageToChat).not.toHaveBeenCalled();
+    });
+
+    it("ignores a missing payload", async () => {
+      await expect(
+        gateway.lobby(undefined as any, client()),
+      ).resolves.toBeUndefined();
+
+      expect(chat.sendMessageToChat).not.toHaveBeenCalled();
+    });
+
+    it("sends the trimmed text", async () => {
+      await gateway.lobby(
+        { id: "t-1", type: ChatLobbyType.Tournament, message: "  hello  " },
+        client(),
+      );
+
+      expect(chat.sendMessageToChat).toHaveBeenCalledWith(
+        ChatLobbyType.Tournament,
+        "t-1",
+        expect.objectContaining({ steam_id: "1" }),
+        "hello",
+      );
+    });
+  });
+
+  describe("length", () => {
+    it("tells the sender a message is too long and never sends it", async () => {
+      const socket = client();
+
+      await gateway.lobby(
+        {
+          id: "m-1",
+          type: ChatLobbyType.Match,
+          message: "a".repeat(ChatService.MAX_MESSAGE_LENGTH + 1),
+          requestId: "r-1",
+        },
+        socket,
+      );
+
+      expect(chat.sendMessageToChat).not.toHaveBeenCalled();
+      expect(chat.sendChatToServer).not.toHaveBeenCalled();
+      expect(sent(socket)).toEqual([
+        {
+          event: "chat:error",
+          data: { code: ChatErrorCode.TooLong, max: 2000, requestId: "r-1" },
+        },
+      ]);
+    });
+
+    it("leaves requestId out when the client sent none", async () => {
+      const socket = client();
+
+      await gateway.lobby(
+        {
+          id: "m-1",
+          type: ChatLobbyType.Match,
+          message: "a".repeat(ChatService.MAX_MESSAGE_LENGTH + 1),
+        },
+        socket,
+      );
+
+      expect(sent(socket)).toEqual([
+        { event: "chat:error", data: { code: "too_long", max: 2000 } },
+      ]);
+    });
+
+    it("accepts exactly the limit", async () => {
+      const message = "a".repeat(ChatService.MAX_MESSAGE_LENGTH);
+
+      await gateway.lobby(
+        { id: "m-1", type: ChatLobbyType.Match, message },
+        client(),
+      );
+
+      expect(chat.sendMessageToChat).toHaveBeenCalledWith(
+        ChatLobbyType.Match,
+        "m-1",
+        expect.anything(),
+        message,
+      );
+    });
+  });
+
+  describe("relaying to the game server", () => {
+    it("never relays a send the room refused", async () => {
+      // The relay has no membership check of its own: this is the only thing
+      // stopping any signed-in socket printing into any live match.
+      chat.sendMessageToChat.mockResolvedValue({
+        accepted: false,
+        code: ChatErrorCode.NotAllowed,
+      });
+      const socket = client();
+
+      await gateway.lobby(
+        {
+          id: "someone-elses-match",
+          type: ChatLobbyType.Match,
+          message: "gg",
+          requestId: "r-2",
+        },
+        socket,
+      );
+
+      expect(chat.sendChatToServer).not.toHaveBeenCalled();
+      expect(sent(socket)).toEqual([
+        {
+          event: "chat:error",
+          data: { code: ChatErrorCode.NotAllowed, requestId: "r-2" },
+        },
+      ]);
+    });
+
+    it("says nothing about a refusal that carries no code", async () => {
+      chat.sendMessageToChat.mockResolvedValue({ accepted: false });
+      const socket = client();
+
+      await gateway.lobby(
+        { id: "m-1", type: ChatLobbyType.Match, message: "gg" },
+        socket,
+      );
+
+      expect(chat.sendChatToServer).not.toHaveBeenCalled();
+      expect(socket.send).not.toHaveBeenCalled();
+    });
+
+    it("relays an accepted match message", async () => {
+      await gateway.lobby(
+        { id: "m-1", type: ChatLobbyType.Match, message: 'say "gg"' },
+        client(),
+      );
+
+      expect(chat.sendChatToServer).toHaveBeenCalledWith(
+        "m-1",
+        "Luke: say 'gg'",
+      );
+    });
+
+    it("marks an organizer's relayed message", async () => {
+      await gateway.lobby(
+        { id: "m-1", type: ChatLobbyType.Match, message: "pause please" },
+        client({ steam_id: "1", name: "Luke", role: "administrator" }),
+      );
+
+      expect(chat.sendChatToServer).toHaveBeenCalledWith(
+        "m-1",
+        "[organizer] Luke: pause please",
+      );
+    });
+
+    it("never relays a team room", async () => {
+      await gateway.lobby(
+        { id: "m-1:l-1", type: ChatLobbyType.MatchTeam, message: "rush b" },
+        client(),
+      );
+
+      expect(chat.sendMessageToChat).toHaveBeenCalled();
+      expect(chat.sendChatToServer).not.toHaveBeenCalled();
+    });
+
+    it.each([
+      ChatLobbyType.Direct,
+      ChatLobbyType.Tournament,
+      ChatLobbyType.Draft,
+      ChatLobbyType.MatchMaking,
+      ChatLobbyType.Organizer,
+    ])("never relays a %s room", async (type) => {
+      await gateway.lobby({ id: "x", type, message: "hi" }, client());
+
+      expect(chat.sendChatToServer).not.toHaveBeenCalled();
+    });
+  });
+
+  describe("acknowledgement", () => {
+    it("acks an accepted send that carried a requestId", async () => {
+      const socket = client();
+
+      await gateway.lobby(
+        {
+          id: "t-1",
+          type: ChatLobbyType.Tournament,
+          message: "hi",
+          requestId: "r-3",
+        },
+        socket,
+      );
+
+      expect(sent(socket)).toEqual([
+        { event: "chat:ack", data: { requestId: "r-3", messageId: "msg-1" } },
+      ]);
+    });
+
+    it("stays quiet for an accepted send without one", async () => {
+      const socket = client();
+
+      await gateway.lobby(
+        { id: "t-1", type: ChatLobbyType.Tournament, message: "hi" },
+        socket,
+      );
+
+      expect(socket.send).not.toHaveBeenCalled();
+    });
+  });
+});

src/chat/chat.gateway.ts

@@ -7,6 +7,7 @@ import {
 import { ChatService } from "./chat.service";
 import { FiveStackWebSocketClient } from "src/sockets/types/FiveStackWebSocketClient";
 import { ChatLobbyType } from "./enums/ChatLobbyTypes";
+import { ChatErrorCode } from "./enums/ChatErrorCode";
 import { isRoleAbove } from "@utilities/isRoleAbove";
 
 @WebSocketGateway({
@@ -90,38 +91,99 @@ export class ChatGateway {
     @MessageBody()
     data: {
       id: string;
-      message: string;
+      message: unknown;
       type: ChatLobbyType;
+      requestId?: string;
     },
     @ConnectedSocket() client: FiveStackWebSocketClient,
   ) {
-    if (!data.message) {
+    if (!client.user) {
+      return;
+    }
+
+    if (!ChatGateway.isLobbyType(data?.type) || typeof data.id !== "string") {
       return;
     }
 
-    data.message = data.message.trim();
+    const requestId =
+      typeof data.requestId === "string" ? data.requestId : undefined;
 
-    if (data.message.length === 0) {
+    const parsed = ChatService.messageText(data.message);
+
+    if ("error" in parsed) {
+      if (parsed.error === ChatErrorCode.TooLong) {
+        this.sendError(client, parsed.error, requestId);
+      }
       return;
     }
 
-    await this.chat.sendMessageToChat(
+    const result = await this.chat.sendMessageToChat(
       data.type,
       data.id,
       client.user,
-      data.message,
+      parsed.text,
     );
 
+    // Only a message the room accepted may reach the game server: the relay
+    // does no membership check of its own, so relaying regardless would let
+    // any signed-in socket print into any live match.
+    if (result.accepted === false) {
+      if (result.code) {
+        this.sendError(client, result.code, requestId);
+      }
+      return;
+    }
+
+    if (requestId) {
+      this.sendAck(client, requestId, result.messageId);
+    }
+
     if (data.type !== ChatLobbyType.Match) {
       return;
     }
 
     await this.chat.sendChatToServer(
       data.id,
-      `${isRoleAbove(client.user.role, "match_organizer") ? `[organizer] ` : ""}${client.user.name}: ${data.message}`.replaceAll(
+      `${isRoleAbove(client.user.role, "match_organizer") ? `[organizer] ` : ""}${client.user.name}: ${parsed.text}`.replaceAll(
         `"`,
         `'`,
       ),
     );
   }
+
+  private static isLobbyType(value: unknown): value is ChatLobbyType {
+    return Object.values(ChatLobbyType).includes(value as ChatLobbyType);
+  }
+
+  private sendError(
+    client: FiveStackWebSocketClient,
+    code: ChatErrorCode,
+    requestId?: string,
+  ) {
+    client.send(
+      JSON.stringify({
+        event: "chat:error",
+        data: {
+          code,
+          ...(code === ChatErrorCode.TooLong
+            ? { max: ChatService.MAX_MESSAGE_LENGTH }
+            : {}),
+          ...(requestId ? { requestId } : {}),
+        },
+      }),
+    );
+  }
+
+  private sendAck(
+    client: FiveStackWebSocketClient,
+    requestId: string,
+    messageId: string,
+  ) {
+    client.send(
+      JSON.stringify({
+        event: "chat:ack",
+        data: { requestId, messageId },
+      }),
+    );
+  }
 }