From bc79e384e30d90c493b1a6db1c3f256dbb12175d Mon Sep 17 00:00:00 2001 From: Luke Policinski Date: Mon, 28 Sep 2026 14:38:31 -0400 Subject: [PATCH 1/3] bug: stop roster admins taking over team ownership Any roster Admin could write teams.owner_steam_id (to themselves or to someone off the team) and could demote the owner's own roster row, locking the owner out of roster edits. tbu_teams now lets only the owner or staff move ownership and only to a roster member, tau_teams makes the new owner a roster Admin, and tbu_team_roster keeps the owner's row an Admin. A backfill restores owners already demoted. tbiu_team_roster_status also stops re-checking caps on updates that leave status and team alone, so role edits work on an over-cap roster. --- .../up.sql | 12 + hasura/triggers/team_roster.sql | 36 +++ hasura/triggers/teams.sql | 52 ++++ test/team-rosters.spec.ts | 227 ++++++++++++++++++ 4 files changed, 327 insertions(+) create mode 100644 hasura/migrations/default/1888000000000_team_owner_roster_admin/up.sql diff --git a/hasura/migrations/default/1888000000000_team_owner_roster_admin/up.sql b/hasura/migrations/default/1888000000000_team_owner_roster_admin/up.sql new file mode 100644 index 000000000..6204b4786 --- /dev/null +++ b/hasura/migrations/default/1888000000000_team_owner_roster_admin/up.sql @@ -0,0 +1,12 @@ +-- Owners demoted on their own roster before tbu_team_roster existed. The +-- rebalancing GUC keeps the roster cap trigger out of a role-only update. +SELECT set_config('fivestack.rebalancing', 'true', true); + +UPDATE public.team_roster tr + SET role = 'Admin' + FROM public.teams t + WHERE t.id = tr.team_id + AND tr.player_steam_id = t.owner_steam_id + AND tr.role <> 'Admin'; + +SELECT set_config('fivestack.rebalancing', 'false', true); diff --git a/hasura/triggers/team_roster.sql b/hasura/triggers/team_roster.sql index d72c71a40..f712e9165 100644 --- a/hasura/triggers/team_roster.sql +++ b/hasura/triggers/team_roster.sql @@ -55,6 +55,32 @@ $$; DROP TRIGGER IF EXISTS tbd_team_roster ON public.team_roster; CREATE TRIGGER tbd_team_roster BEFORE DELETE ON public.team_roster FOR EACH ROW EXECUTE FUNCTION public.tbd_team_roster(); +-- Same reasoning as tbd_team_roster: the roster update permission is granted by +-- the Admin role, so an owner demoted by another Admin could no longer manage +-- their own team. Staff are held to it too; ownership moves first. +CREATE OR REPLACE FUNCTION public.tbu_team_roster() RETURNS TRIGGER + LANGUAGE plpgsql + AS $$ +BEGIN + IF NEW.role IS DISTINCT FROM OLD.role + AND NEW.role <> 'Admin' + AND EXISTS ( + SELECT 1 + FROM teams t + WHERE t.id = NEW.team_id + AND t.owner_steam_id = NEW.player_steam_id + ) THEN + RAISE EXCEPTION USING ERRCODE = '22000', + MESSAGE = 'The team owner must stay an Admin; transfer ownership first'; + END IF; + + RETURN NEW; +END; +$$; + +DROP TRIGGER IF EXISTS tbu_team_roster ON public.team_roster; +CREATE TRIGGER tbu_team_roster BEFORE UPDATE ON public.team_roster FOR EACH ROW EXECUTE FUNCTION public.tbu_team_roster(); + CREATE OR REPLACE FUNCTION public.tad_team_roster() RETURNS TRIGGER LANGUAGE plpgsql AS $$ @@ -128,6 +154,16 @@ BEGIN RETURN NEW; END IF; + -- The caps count rows by status alone (coaches included), so an update that + -- keeps the row's status and team cannot change any tier's count. Skipping + -- it keeps role and coach edits working on a roster that is already over a + -- cap, e.g. after team_max_subs() was lowered. + IF TG_OP = 'UPDATE' + AND NEW.status IS NOT DISTINCT FROM OLD.status + AND NEW.team_id = OLD.team_id THEN + RETURN NEW; + END IF; + IF NEW.status = 'Starter' THEN _max := 5; SELECT COUNT(*) INTO _count FROM public.team_roster diff --git a/hasura/triggers/teams.sql b/hasura/triggers/teams.sql index 62b798d48..d5e470dc9 100644 --- a/hasura/triggers/teams.sql +++ b/hasura/triggers/teams.sql @@ -14,9 +14,16 @@ $$; DROP TRIGGER IF EXISTS tai_teams ON public.teams; CREATE TRIGGER tai_teams AFTER INSERT ON public.teams FOR EACH ROW EXECUTE FUNCTION public.tai_teams(); +-- The user update permission on teams lets any roster Admin write +-- owner_steam_id, so ownership transfer is gated here: only the current owner +-- or staff may hand it over. A session with no role is an internal write and +-- stays unrestricted. CREATE OR REPLACE FUNCTION public.tbu_teams() RETURNS TRIGGER LANGUAGE plpgsql AS $$ +DECLARE + _session json; + _role text; BEGIN IF NEW.captain_steam_id IS NOT NULL AND NEW.captain_steam_id IS DISTINCT FROM OLD.captain_steam_id @@ -29,9 +36,54 @@ BEGIN RAISE EXCEPTION 'Team captain must be a team member' USING ERRCODE = '22000'; END IF; + IF NEW.owner_steam_id IS DISTINCT FROM OLD.owner_steam_id THEN + _session := nullif(current_setting('hasura.user', true), '')::json; + _role := _session ->> 'x-hasura-role'; + + IF _role IS NOT NULL + AND _role NOT IN ('admin', 'administrator', 'tournament_organizer') + AND nullif(_session ->> 'x-hasura-user-id', '')::bigint IS DISTINCT FROM OLD.owner_steam_id THEN + RAISE EXCEPTION USING ERRCODE = '22000', + MESSAGE = 'Only the team owner can transfer ownership'; + END IF; + + IF NEW.owner_steam_id IS NOT NULL + AND NOT EXISTS ( + SELECT 1 + FROM team_roster tr + WHERE tr.team_id = NEW.id + AND tr.player_steam_id = NEW.owner_steam_id + ) THEN + RAISE EXCEPTION USING ERRCODE = '22000', + MESSAGE = 'The new team owner must be a team member'; + END IF; + END IF; + RETURN NEW; END; $$; DROP TRIGGER IF EXISTS tbu_teams ON public.teams; CREATE TRIGGER tbu_teams BEFORE UPDATE ON public.teams FOR EACH ROW EXECUTE FUNCTION public.tbu_teams(); + +-- can_change_team_role trusts owner_steam_id, but the roster write permission +-- trusts the roster role, so a new owner is made an Admin on the roster too. +CREATE OR REPLACE FUNCTION public.tau_teams() RETURNS TRIGGER + LANGUAGE plpgsql + AS $$ +BEGIN + UPDATE team_roster + SET role = 'Admin' + WHERE team_id = NEW.id + AND player_steam_id = NEW.owner_steam_id + AND role <> 'Admin'; + + RETURN NEW; +END; +$$; + +DROP TRIGGER IF EXISTS tau_teams ON public.teams; +CREATE TRIGGER tau_teams AFTER UPDATE ON public.teams + FOR EACH ROW + WHEN (NEW.owner_steam_id IS DISTINCT FROM OLD.owner_steam_id) + EXECUTE FUNCTION public.tau_teams(); diff --git a/test/team-rosters.spec.ts b/test/team-rosters.spec.ts index b70d6f6fa..44b8edc59 100644 --- a/test/team-rosters.spec.ts +++ b/test/team-rosters.spec.ts @@ -214,6 +214,233 @@ describe("teams, rosters and lineup membership (SQL-driven)", () => { ); expect(roster).toHaveLength(0); }); + + describe("ownership", () => { + const addMember = async ( + teamId: string, + owner: string, + steamId: string, + role: "Admin" | "Member" = "Member", + ) => { + await asUser(owner, "admin", (query) => + query( + "INSERT INTO team_roster (team_id, player_steam_id) VALUES ($1, $2)", + [teamId, steamId], + ), + ); + if (role !== "Member") { + await postgres.query( + "UPDATE team_roster SET role = $1 WHERE team_id = $2 AND player_steam_id = $3", + [role, teamId, steamId], + ); + } + }; + + const getTeamOwner = async (teamId: string) => { + const [team] = await postgres.query>( + "SELECT owner_steam_id FROM teams WHERE id = $1", + [teamId], + ); + return team.owner_steam_id; + }; + + it("rejects a roster Admin making themselves owner", async () => { + const owner = await seedPlayer(); + const admin = await seedPlayer(); + const teamId = await createTeam(owner); + await addMember(teamId, owner, admin, "Admin"); + + await expect( + asUser(admin, "user", (query) => + query("UPDATE teams SET owner_steam_id = $1 WHERE id = $2", [ + admin, + teamId, + ]), + ), + ).rejects.toThrow(/only the team owner/i); + + expect(await getTeamOwner(teamId)).toBe(owner); + }); + + it("lets the owner hand the team to a member, who becomes an Admin", async () => { + const owner = await seedPlayer(); + const member = await seedPlayer(); + const teamId = await createTeam(owner); + await addMember(teamId, owner, member); + + await asUser(owner, "user", (query) => + query("UPDATE teams SET owner_steam_id = $1 WHERE id = $2", [ + member, + teamId, + ]), + ); + + expect(await getTeamOwner(teamId)).toBe(member); + expect((await rosterRow(teamId, member))?.role).toBe("Admin"); + expect((await rosterRow(teamId, owner))?.role).toBe("Admin"); + }); + + it("rejects handing the team to someone off the roster", async () => { + const owner = await seedPlayer(); + const outsider = await seedPlayer(); + const teamId = await createTeam(owner); + + await expect( + asUser(owner, "user", (query) => + query("UPDATE teams SET owner_steam_id = $1 WHERE id = $2", [ + outsider, + teamId, + ]), + ), + ).rejects.toThrow(/must be a team member/i); + + expect(await getTeamOwner(teamId)).toBe(owner); + }); + + it("lets a tournament organizer transfer ownership", async () => { + const owner = await seedPlayer(); + const member = await seedPlayer(); + const organizer = await seedPlayer(); + const teamId = await createTeam(owner); + await addMember(teamId, owner, member); + + await asUser(organizer, "tournament_organizer", (query) => + query("UPDATE teams SET owner_steam_id = $1 WHERE id = $2", [ + member, + teamId, + ]), + ); + + expect(await getTeamOwner(teamId)).toBe(member); + expect((await rosterRow(teamId, member))?.role).toBe("Admin"); + }); + + it("rejects a roster Admin demoting the owner", async () => { + const owner = await seedPlayer(); + const admin = await seedPlayer(); + const teamId = await createTeam(owner); + await addMember(teamId, owner, admin, "Admin"); + + await expect( + asUser(admin, "user", (query) => + query( + "UPDATE team_roster SET role = 'Member' WHERE team_id = $1 AND player_steam_id = $2", + [teamId, owner], + ), + ), + ).rejects.toThrow(/owner must stay an Admin/i); + + expect((await rosterRow(teamId, owner))?.role).toBe("Admin"); + }); + + it("lets the new owner demote the old one after a transfer", async () => { + const owner = await seedPlayer(); + const heir = await seedPlayer(); + const teamId = await createTeam(owner); + await addMember(teamId, owner, heir); + + await asUser(owner, "user", (query) => + query("UPDATE teams SET owner_steam_id = $1 WHERE id = $2", [ + heir, + teamId, + ]), + ); + await asUser(heir, "user", (query) => + query( + "UPDATE team_roster SET role = 'Member' WHERE team_id = $1 AND player_steam_id = $2", + [teamId, owner], + ), + ); + + expect((await rosterRow(teamId, owner))?.role).toBe("Member"); + }); + + it("lets the old owner step down to Member in the same mutation as the transfer", async () => { + const owner = await seedPlayer(); + const heir = await seedPlayer(); + const teamId = await createTeam(owner); + await addMember(teamId, owner, heir); + + await asUser(owner, "user", async (query) => { + await query("UPDATE teams SET owner_steam_id = $1 WHERE id = $2", [ + heir, + teamId, + ]); + await query( + "UPDATE team_roster SET role = 'Member' WHERE team_id = $1 AND player_steam_id = $2", + [teamId, owner], + ); + }); + + expect((await rosterRow(teamId, owner))?.role).toBe("Member"); + expect((await rosterRow(teamId, heir))?.role).toBe("Admin"); + }); + + it("lets a roster Admin demote another Admin", async () => { + const owner = await seedPlayer(); + const admin = await seedPlayer(); + const otherAdmin = await seedPlayer(); + const teamId = await createTeam(owner); + await addMember(teamId, owner, admin, "Admin"); + await addMember(teamId, owner, otherAdmin, "Admin"); + + await asUser(admin, "user", (query) => + query( + "UPDATE team_roster SET role = 'Member' WHERE team_id = $1 AND player_steam_id = $2", + [teamId, otherAdmin], + ), + ); + + expect((await rosterRow(teamId, otherAdmin))?.role).toBe("Member"); + }); + + it("lets a roster Admin rename the team", async () => { + const owner = await seedPlayer(); + const admin = await seedPlayer(); + const teamId = await createTeam(owner); + await addMember(teamId, owner, admin, "Admin"); + + await asUser(admin, "user", (query) => + query( + "UPDATE teams SET name = 'Renamed', short_name = 'RNM', owner_steam_id = $1 WHERE id = $2", + [owner, teamId], + ), + ); + + const [team] = await postgres.query>( + "SELECT name FROM teams WHERE id = $1", + [teamId], + ); + expect(team.name).toBe("Renamed"); + expect(await getTeamOwner(teamId)).toBe(owner); + }); + + it("lets roles change on a roster that is already over the starter cap", async () => { + const owner = await seedPlayer(); + const teamId = await createTeam(owner); + const starters = await fx.players(5); + await asUser(owner, "admin", async (query) => { + await query( + "SELECT set_config('fivestack.rebalancing', 'true', true)", + ); + for (const steamId of starters) { + await query( + "INSERT INTO team_roster (team_id, player_steam_id, status) VALUES ($1, $2, 'Starter')", + [teamId, steamId], + ); + } + }); + + await asUser(owner, "user", (query) => + query( + "UPDATE team_roster SET role = 'Admin' WHERE team_id = $1 AND player_steam_id = $2", + [teamId, starters[0]], + ), + ); + + expect((await rosterRow(teamId, starters[0]))?.role).toBe("Admin"); + }); + }); }); describe("match lineup membership", () => { From 32afd0f41b3664637564b25c2656f784aaa14c19 Mon Sep 17 00:00:00 2001 From: Luke Policinski Date: Mon, 28 Sep 2026 14:51:06 -0400 Subject: [PATCH 2/3] test: cover administrator transfers and staff owner demotion --- test/team-rosters.spec.ts | 54 +++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/test/team-rosters.spec.ts b/test/team-rosters.spec.ts index 44b8edc59..76c99555c 100644 --- a/test/team-rosters.spec.ts +++ b/test/team-rosters.spec.ts @@ -297,23 +297,26 @@ describe("teams, rosters and lineup membership (SQL-driven)", () => { expect(await getTeamOwner(teamId)).toBe(owner); }); - it("lets a tournament organizer transfer ownership", async () => { - const owner = await seedPlayer(); - const member = await seedPlayer(); - const organizer = await seedPlayer(); - const teamId = await createTeam(owner); - await addMember(teamId, owner, member); - - await asUser(organizer, "tournament_organizer", (query) => - query("UPDATE teams SET owner_steam_id = $1 WHERE id = $2", [ - member, - teamId, - ]), - ); + it.each(["tournament_organizer", "administrator"])( + "lets a %s transfer ownership", + async (role) => { + const owner = await seedPlayer(); + const member = await seedPlayer(); + const staff = await seedPlayer(); + const teamId = await createTeam(owner); + await addMember(teamId, owner, member); + + await asUser(staff, role, (query) => + query("UPDATE teams SET owner_steam_id = $1 WHERE id = $2", [ + member, + teamId, + ]), + ); - expect(await getTeamOwner(teamId)).toBe(member); - expect((await rosterRow(teamId, member))?.role).toBe("Admin"); - }); + expect(await getTeamOwner(teamId)).toBe(member); + expect((await rosterRow(teamId, member))?.role).toBe("Admin"); + }, + ); it("rejects a roster Admin demoting the owner", async () => { const owner = await seedPlayer(); @@ -333,6 +336,25 @@ describe("teams, rosters and lineup membership (SQL-driven)", () => { expect((await rosterRow(teamId, owner))?.role).toBe("Admin"); }); + it("holds staff and internal writes to the owner staying an Admin", async () => { + const owner = await seedPlayer(); + const organizer = await seedPlayer(); + const teamId = await createTeam(owner); + const demoteOwner = + "UPDATE team_roster SET role = 'Member' WHERE team_id = $1 AND player_steam_id = $2"; + + await expect( + asUser(organizer, "tournament_organizer", (query) => + query(demoteOwner, [teamId, owner]), + ), + ).rejects.toThrow(/owner must stay an Admin/i); + await expect( + postgres.query(demoteOwner, [teamId, owner]), + ).rejects.toThrow(/owner must stay an Admin/i); + + expect((await rosterRow(teamId, owner))?.role).toBe("Admin"); + }); + it("lets the new owner demote the old one after a transfer", async () => { const owner = await seedPlayer(); const heir = await seedPlayer(); From 54add0fced561edb0e25b26861e9482199c02399 Mon Sep 17 00:00:00 2001 From: Luke Policinski Date: Mon, 28 Sep 2026 20:24:15 -0400 Subject: [PATCH 3/3] test: prove the owner roster backfill re-promotes demoted owners --- test/team-rosters.spec.ts | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/team-rosters.spec.ts b/test/team-rosters.spec.ts index 76c99555c..f27174cf1 100644 --- a/test/team-rosters.spec.ts +++ b/test/team-rosters.spec.ts @@ -1,3 +1,5 @@ +import { readFileSync } from "fs"; +import { join } from "path"; import { PostgresService } from "./../src/postgres/postgres.service"; import { Fixtures } from "./utils/fixtures"; import { @@ -462,6 +464,39 @@ describe("teams, rosters and lineup membership (SQL-driven)", () => { expect((await rosterRow(teamId, starters[0]))?.role).toBe("Admin"); }); + + it("backfills owners demoted before the guard existed back to Admin", async () => { + const owner = await seedPlayer(); + const member = await seedPlayer(); + const teamId = await createTeam(owner); + await addMember(teamId, owner, member); + await postgres.transaction(async (client) => { + await client.query( + "ALTER TABLE team_roster DISABLE TRIGGER tbu_team_roster", + ); + await client.query( + "UPDATE team_roster SET role = 'Member' WHERE team_id = $1 AND player_steam_id = $2", + [teamId, owner], + ); + await client.query( + "ALTER TABLE team_roster ENABLE TRIGGER tbu_team_roster", + ); + }); + expect((await rosterRow(teamId, owner))?.role).toBe("Member"); + + await postgres.query( + readFileSync( + join( + __dirname, + "../hasura/migrations/default/1888000000000_team_owner_roster_admin/up.sql", + ), + "utf8", + ), + ); + + expect((await rosterRow(teamId, owner))?.role).toBe("Admin"); + expect((await rosterRow(teamId, member))?.role).toBe("Member"); + }); }); });