bug: socket pong, ban-only teammate notices, tighter player delete, support notification text

hasura/metadata/databases/default/tables/public_players.yaml

@@ -633,7 +633,7 @@ update_permissions:
           _eq: X-Hasura-User-Id
     comment: ""
 delete_permissions:
-  - role: match_organizer
+  - role: administrator
     permission:
       filter: {}
     comment: ""

src/hasura/metadata-permissions.spec.ts

@@ -137,6 +137,18 @@ describe("hasura table metadata", () => {
     expect(problems).toEqual([]);
   });
 
+  // A player row is the account itself: every match, stat and sanction hangs
+  // off it, so removing one is not a match organizer's call.
+  it("only lets an administrator delete a player", () => {
+    const players = tables.find(({ file }) => file === "public_players.yaml");
+
+    expect(
+      (players?.metadata?.delete_permissions ?? []).map(
+        (entry: { role: string }) => entry.role,
+      ),
+    ).toEqual(["administrator"]);
+  });
+
   it("never lists the same name as both a column and a computed field", () => {
     const problems: Array<string> = [];
 

src/matches/matches.controller.spec.ts

@@ -13,25 +13,30 @@ describe("MatchesController", () => {
     isOrganizer: jest.Mock;
     rebootOnDemandServer: jest.Mock;
   };
+  let hasura: { query: jest.Mock };
+  let notifications: { send: jest.Mock };
 
   beforeEach(() => {
     matchAssistant = {
       isOrganizer: jest.fn(),
       rebootOnDemandServer: jest.fn(),
     };
+    hasura = { query: jest.fn() };
+    notifications = { send: jest.fn().mockResolvedValue(undefined) };
 
     controller = new MatchesController(
       {} as any,
-      {} as any,
+      hasura as any,
       {} as any,
       {
-        get: jest.fn(() => ({})),
+        get: jest.fn(() => ({ webDomain: "https://5stack.test" })),
       } as any,
       {} as any,
       matchAssistant as any,
       {} as any,
       {} as any,
       {} as any,
+      notifications as any,
       {} as any,
       {} as any,
       {} as any,
@@ -52,7 +57,6 @@ describe("MatchesController", () => {
       {} as any,
       {} as any,
       {} as any,
-      {} as any
     );
   });
 
@@ -82,4 +86,92 @@ describe("MatchesController", () => {
 
     expect(matchAssistant.rebootOnDemandServer).toHaveBeenCalledWith("match-1");
   });
+
+  describe("callForOrganizer", () => {
+    const matchId = "00000000-0000-0000-0000-000000000001";
+
+    const callForOrganizer = () =>
+      controller.callForOrganizer({
+        match_id: matchId,
+        user: { steam_id: "76561198000000001", name: "signed-in-as" } as any,
+      });
+
+    const respond = (
+      match: Record<string, boolean>,
+      player: { name: string } | null = { name: "keith" },
+    ) =>
+      hasura.query.mockResolvedValue({
+        matches_by_pk: match,
+        players_by_pk: player,
+      });
+
+    const sent = () => {
+      const [type, notification] = notifications.send.mock.calls[0];
+      return { type, ...notification };
+    };
+
+    beforeEach(() => {
+      respond({ is_in_lineup: true, requested_organizer: false });
+    });
+
+    it("names the requester without linking them", async () => {
+      respond(
+        { is_in_lineup: true, requested_organizer: false },
+        { name: "<b>keith</b>" },
+      );
+
+      await callForOrganizer();
+
+      const notification = sent();
+      expect(notification.type).toBe("MatchSupport");
+      expect(notification.message).toContain(
+        "<b>&lt;b&gt;keith&lt;/b&gt;</b> requested assistance in match",
+      );
+      expect(notification.message.match(/href="([^"]+)"/)?.[1]).toBe(
+        `https://5stack.test/matches/${matchId}`,
+      );
+    });
+
+    it("uses the current name rather than the one the session signed in with", async () => {
+      await callForOrganizer();
+
+      expect(sent().message).toContain("<b>keith</b> requested assistance");
+      expect(sent().message).not.toContain("signed-in-as");
+    });
+
+    it("falls back to the steam id when the player row is missing", async () => {
+      respond({ is_in_lineup: true, requested_organizer: false }, null);
+
+      await callForOrganizer();
+
+      expect(sent().message).toContain(
+        "<b>76561198000000001</b> requested assistance",
+      );
+    });
+
+    it("titles the notification without the old typo", async () => {
+      await callForOrganizer();
+
+      expect(sent().title).toBe("Match Assistance Required");
+      expect(sent().message).not.toContain("Assistanced");
+    });
+
+    it("rejects someone who is not playing in the match", async () => {
+      respond({ is_in_lineup: false, requested_organizer: false });
+
+      await expect(callForOrganizer()).rejects.toThrow(
+        "only players in this match can contact support",
+      );
+      expect(notifications.send).not.toHaveBeenCalled();
+    });
+
+    it("does not ask twice while a request is still open", async () => {
+      respond({ is_in_lineup: true, requested_organizer: true });
+
+      await expect(callForOrganizer()).resolves.toEqual({
+        success: true,
+      });
+      expect(notifications.send).not.toHaveBeenCalled();
+    });
+  });
 });

src/matches/matches.controller.ts

@@ -2518,30 +2518,48 @@ export class MatchesController {
 
   @HasuraAction()
   public async callForOrganizer(data: { user: User; match_id: string }) {
-    const { matches_by_pk: match } = await this.hasura.query(
-      {
-        matches_by_pk: {
-          __args: {
-            id: data.match_id,
+    const { matches_by_pk: match, players_by_pk: requester } =
+      await this.hasura.query(
+        {
+          matches_by_pk: {
+            __args: {
+              id: data.match_id,
+            },
+            is_in_lineup: true,
+            requested_organizer: true,
+          },
+          players_by_pk: {
+            __args: {
+              steam_id: data.user.steam_id,
+            },
+            name: true,
           },
-          is_in_lineup: true,
-          requested_organizer: true,
         },
-      },
-      data.user.steam_id,
-    );
+        data.user.steam_id,
+      );
 
     if (!match || match.requested_organizer) {
       return {
         success: true,
       };
     }
 
+    if (!match.is_in_lineup) {
+      throw Error("only players in this match can contact support");
+    }
+
+    // The requester stays plain text: notificationUrl takes the first href as
+    // where the push lands, and that has to be the match. The name is read from
+    // the row because the session keeps whatever it was at sign-in.
+    const requesterName = NotificationsService.escapeHtml(
+      requester?.name ?? data.user.steam_id,
+    );
+
     void this.notifications.send(
       "MatchSupport",
       {
-        message: `Match Assistanced Required <a href="${this.appConfig.webDomain}/matches/${data.match_id}">${data.match_id}</a>`,
-        title: "Match Assistanced Required",
+        message: `<b>${requesterName}</b> requested assistance in match <a href="${this.appConfig.webDomain}/matches/${data.match_id}">${data.match_id}</a>`,
+        title: "Match Assistance Required",
         role: "match_organizer",
         entity_id: data.match_id,
       },

src/notifications/notifications.service.spec.ts

@@ -174,3 +174,82 @@ describe("CS2 build notices", () => {
     expect(NotificationsService.truncateDiscord("short")).toBe("short");
   });
 });
+
+describe("NotificationsService", () => {
+  const webDomain = "https://5stack.test";
+  let service: NotificationsService;
+  let postgres: { query: jest.Mock };
+  let hasura: { query: jest.Mock; mutation: jest.Mock };
+  let notifyPlayers: jest.SpyInstance;
+
+  const sanction = (type: string) => ({
+    sanctionId: "sanction-1",
+    steamId: "76561198000000001",
+    type,
+    reason: "cheating",
+  });
+
+  beforeEach(() => {
+    postgres = {
+      query: jest.fn().mockResolvedValue([{ steam_id: "76561198000000002" }]),
+    };
+    hasura = {
+      query: jest.fn().mockResolvedValue({ players_by_pk: { name: "keith" } }),
+      mutation: jest.fn().mockResolvedValue({}),
+    };
+
+    service = new NotificationsService(
+      hasura as any,
+      postgres as any,
+      { log: jest.fn(), warn: jest.fn(), error: jest.fn() } as any,
+      { get: jest.fn(() => ({ webDomain })) } as any,
+      {} as any,
+      {} as any,
+      {} as any,
+      {} as any,
+    );
+
+    notifyPlayers = jest.spyOn(service, "notifyPlayers").mockResolvedValue(1);
+  });
+
+  describe("playerProfileLink", () => {
+    it("links to the absolute profile url", () => {
+      expect(service.playerProfileLink("76561198000000001", "keith")).toBe(
+        `<a href="${webDomain}/players/76561198000000001">keith</a>`,
+      );
+    });
+
+    it("escapes the name and encodes the steam id", () => {
+      expect(
+        service.playerProfileLink('1"><x', `<img src=x onerror="alert('1')">`),
+      ).toBe(
+        `<a href="${webDomain}/players/1%22%3E%3Cx">` +
+          `&lt;img src=x onerror=&quot;alert(&#39;1&#39;)&quot;&gt;</a>`,
+      );
+    });
+  });
+
+  describe("notifyMatchPlayersOfSanction", () => {
+    it.each(["mute", "gag", "silence"])(
+      "keeps a %s between the player and staff",
+      async (type) => {
+        await service.notifyMatchPlayersOfSanction(sanction(type));
+
+        expect(postgres.query).not.toHaveBeenCalled();
+        expect(notifyPlayers).not.toHaveBeenCalled();
+      },
+    );
+
+    it("tells recent team-mates about a ban", async () => {
+      await service.notifyMatchPlayersOfSanction(sanction("ban"));
+
+      expect(notifyPlayers).toHaveBeenCalledTimes(1);
+      const [type, notification] = notifyPlayers.mock.calls[0];
+      expect(type).toBe("PlayerSanctioned");
+      expect(notification.steamIds).toEqual(["76561198000000002"]);
+      expect(notification.message).toContain(
+        `<a href="${webDomain}/players/76561198000000001">keith</a>, was banned. (cheating)`,
+      );
+    });
+  });
+});

src/notifications/notifications.service.ts

@@ -114,19 +114,24 @@ export class NotificationsService {
       .replace(/'/g, "'");
   }
 
-  private static readonly SANCTION_VERBS: Record<string, string> = {
-    ban: "banned",
-    mute: "muted",
-    gag: "gagged",
-    silence: "silenced",
-  };
+  public playerProfileLink(steamId: string, name: string): string {
+    return `<a href="${this.appConfig.webDomain}/players/${encodeURIComponent(
+      steamId,
+    )}">${NotificationsService.escapeHtml(name)}</a>`;
+  }
 
   async notifyMatchPlayersOfSanction(sanction: {
     sanctionId: string;
     steamId: string;
     type: string;
     reason?: string | null;
   }): Promise<void> {
+    // A mute or gag is chat moderation, and "was muted" sent to six months of
+    // team-mates reads as a ban to every one of them.
+    if (sanction.type !== "ban") {
+      return;
+    }
+
     const recipients = await this.postgres.query<Array<{ steam_id: string }>>(
       `SELECT DISTINCT other_p.steam_id::text AS steam_id
          FROM public.matches m
@@ -154,19 +159,12 @@ export class NotificationsService {
     });
     const name = players_by_pk?.name ?? `Player ${sanction.steamId}`;
 
-    const verb =
-      NotificationsService.SANCTION_VERBS[sanction.type] ?? "sanctioned";
-    const safeName = NotificationsService.escapeHtml(name);
-    const profileUrl = `${this.appConfig.webDomain}/players/${encodeURIComponent(
-      sanction.steamId,
-    )}`;
-    const reasonSuffix =
-      sanction.type === "ban" && sanction.reason
-        ? ` (${NotificationsService.escapeHtml(sanction.reason)})`
-        : "";
+    const reasonSuffix = sanction.reason
+      ? ` (${NotificationsService.escapeHtml(sanction.reason)})`
+      : "";
     const message =
       `A player you recently played with, ` +
-      `<a href="${profileUrl}">${safeName}</a>, was ${verb}.${reasonSuffix}`;
+      `${this.playerProfileLink(sanction.steamId, name)}, was banned.${reasonSuffix}`;
 
     // Through notifyPlayers rather than the raw insert this used to be. Six
     // months of team-mates is routinely hundreds of rows and the event trigger