Check permissions for EXISTING image

app/db/seed/factories/TeamFactory.ts

@@ -23,6 +23,8 @@ type Options = {
 	mapModePreferences?: UserMapModePreferences;
 	/** Roles of the members, keyed by user id, saved as the roster page saves them. Members left out keep none. */
 	roles?: Record<number, MemberRole>;
+	/** Members who may edit the team like the owner does, saved as the roster page saves them. */
+	managerUserIds?: number[];
 };
 
 /** First of `memberUserIds` is the owner, the rest join like in production (within the non-patron team limit). */
@@ -50,17 +52,23 @@ export const { create } = defineFactory({
 	},
 	applyOptions: async (
 		team,
-		{ hasAvatar, avatarUrl, mapModePreferences, roles }: Options,
+		{
+			hasAvatar,
+			avatarUrl,
+			mapModePreferences,
+			roles,
+			managerUserIds,
+		}: Options,
 	) => {
-		if (roles) {
+		if (roles || managerUserIds) {
 			await TeamRepository.updateRoster({
 				teamId: team.id,
 				members: team.memberUserIds.map((userId, index) => ({
 					userId,
-					role: roles[userId] ?? null,
+					role: roles?.[userId] ?? null,
 					customRole: null,
 					roleType: null,
-					isManager: false,
+					isManager: managerUserIds?.includes(userId) ?? false,
 					order: index,
 				})),
 				kickedUserIds: [],

app/features/calendar/CalendarRepository.server.test.ts

@@ -1,5 +1,7 @@
 import { sub } from "date-fns";
 import { beforeEach, describe, expect, test } from "vitest";
+import * as CalendarEventFactory from "~/db/seed/factories/CalendarEventFactory";
+import * as ImageFactory from "~/db/seed/factories/ImageFactory";
 import * as TournamentFactory from "~/db/seed/factories/TournamentFactory";
 import * as TournamentOrganizationFactory from "~/db/seed/factories/TournamentOrganizationFactory";
 import * as UserFactory from "~/db/seed/factories/UserFactory";
@@ -188,3 +190,50 @@ describe("findRecentTournamentsByOrganizerUserId", () => {
 		expect(await recentTournamentNames()).toEqual(["Low Ink February"]);
 	});
 });
+
+describe("findAvatarImgIds", () => {
+	const authorId = () => users.id(1);
+
+	beforeEach(async () => {
+		await users.create(1);
+	});
+
+	test("returns the logos of the edited event and of the copied tournament's event", async () => {
+		const copiedLogo = await ImageFactory.create({
+			submitterUserId: authorId(),
+		});
+		const tournamentToCopy = await TournamentFactory.create({
+			authorId: authorId(),
+			avatarImgId: copiedLogo.id,
+		});
+		const editedLogo = await ImageFactory.create({
+			submitterUserId: authorId(),
+		});
+		const eventToEdit = await CalendarEventFactory.create({
+			authorId: authorId(),
+			avatarImgId: editedLogo.id,
+		});
+		await CalendarEventFactory.create({
+			authorId: authorId(),
+			hasAvatar: true,
+		});
+
+		const imgIds = await CalendarRepository.findAvatarImgIds({
+			eventId: eventToEdit.id,
+			tournamentId: tournamentToCopy.id,
+		});
+
+		expect(imgIds.toSorted((a, b) => a - b)).toEqual(
+			[copiedLogo.id, editedLogo.id].toSorted((a, b) => a - b),
+		);
+	});
+
+	test("returns nothing when neither an event nor a tournament is given", async () => {
+		await CalendarEventFactory.create({
+			authorId: authorId(),
+			hasAvatar: true,
+		});
+
+		expect(await CalendarRepository.findAvatarImgIds({})).toEqual([]);
+	});
+});

app/features/calendar/CalendarRepository.server.ts

@@ -359,6 +359,33 @@ export async function findById(
 	};
 }
 
+/** Logo image ids of the given event and of the given tournament's event: what the new event form may keep when editing or copying. */
+export async function findAvatarImgIds({
+	eventId,
+	tournamentId,
+}: {
+	eventId?: number;
+	tournamentId?: number;
+}) {
+	if (!eventId && !tournamentId) return [];
+
+	const rows = await db
+		.selectFrom("CalendarEvent")
+		.select("CalendarEvent.avatarImgId")
+		.where((eb) =>
+			eb.or([
+				...(eventId ? [eb("CalendarEvent.id", "=", eventId)] : []),
+				...(tournamentId
+					? [eb("CalendarEvent.tournamentId", "=", tournamentId)]
+					: []),
+			]),
+		)
+		.where("CalendarEvent.avatarImgId", "is not", null)
+		.execute();
+
+	return rows.flatMap((row) => (row.avatarImgId ? [row.avatarImgId] : []));
+}
+
 /**
  * Past year's tournaments the user organized (author, organization ADMIN/ORGANIZER or staff
  * ORGANIZER), newest first. Latest event per series only, the next newest filling spare spots.

app/features/calendar/actions/calendar.new.server.ts

@@ -42,6 +42,13 @@ export const action: ActionFunction = async ({ request }) => {
 	const result = await parseFormDataWithImages({
 		request,
 		schema: calendarNewSchemaServer,
+		isCurrentImgId: async (imgId, submitted) =>
+			(
+				await CalendarRepository.findAvatarImgIds({
+					eventId: submitted.eventToEditId,
+					tournamentId: submitted.tournamentToCopyId,
+				})
+			).includes(imgId),
 	});
 	if (!result.success) {
 		return { fieldErrors: result.fieldErrors };

app/features/img-upload/ImageRepository.server.ts

@@ -5,7 +5,7 @@ import { databaseTimestampNow } from "~/utils/dates";
 import { concatUserSubmittedImagePrefix } from "~/utils/kysely.server";
 import { IMAGES_TO_VALIDATE_AT_ONCE } from "./upload-constants";
 
-/** Unvalidated image with its calendar event data. */
+/** Unvalidated image's submitter with its calendar event data. */
 export function findById(id: number) {
 	return db
 		.selectFrom("UnvalidatedUserSubmittedImage")
@@ -14,7 +14,10 @@ export function findById(id: number) {
 			"CalendarEvent.avatarImgId",
 			"UnvalidatedUserSubmittedImage.id",
 		)
-		.select(["CalendarEvent.tournamentId"])
+		.select([
+			"UnvalidatedUserSubmittedImage.submitterUserId",
+			"CalendarEvent.tournamentId",
+		])
 		.where("UnvalidatedUserSubmittedImage.id", "=", id)
 		.executeTakeFirst();
 }

app/features/img-upload/image-field.server.test.ts

@@ -0,0 +1,91 @@
+import { beforeEach, describe, expect, test } from "vitest";
+import * as ImageFactory from "~/db/seed/factories/ImageFactory";
+import * as UserFactory from "~/db/seed/factories/UserFactory";
+import * as UserRepository from "~/features/user-page/UserRepository.server";
+import { invariant } from "~/utils/invariant";
+import { assertResponseErrored } from "~/utils/Test";
+import { imageFieldValueToImgId } from "./image-field.server";
+
+const users = UserFactory.pool();
+const editorId = () => users.id(1);
+const otherUserId = () => users.id(2);
+
+const existing = (imgId: number) => ({
+	type: "EXISTING" as const,
+	imgId,
+	url: "logo.webp",
+});
+
+async function editor() {
+	const user = await UserRepository.findLeanById(editorId());
+	invariant(user, "Expected the editor to exist");
+	return user;
+}
+
+async function thrownBy(promise: Promise<unknown>) {
+	return promise.then(
+		() => null,
+		(error: unknown) => error,
+	);
+}
+
+describe("imageFieldValueToImgId", () => {
+	beforeEach(async () => {
+		await users.create(2);
+	});
+
+	test("resolves a removed image to null", async () => {
+		expect(
+			await imageFieldValueToImgId({ value: null, user: await editor() }),
+		).toBeNull();
+	});
+
+	test("keeps the user's own upload", async () => {
+		const image = await ImageFactory.create({ submitterUserId: editorId() });
+
+		expect(
+			await imageFieldValueToImgId({
+				value: existing(image.id),
+				user: await editor(),
+			}),
+		).toBe(image.id);
+	});
+
+	test("keeps another user's upload the edited entity already holds", async () => {
+		const image = await ImageFactory.create({ submitterUserId: otherUserId() });
+
+		expect(
+			await imageFieldValueToImgId({
+				value: existing(image.id),
+				user: await editor(),
+				isCurrentImgId: (imgId) => imgId === image.id,
+			}),
+		).toBe(image.id);
+	});
+
+	test("rejects another user's upload the edited entity does not hold", async () => {
+		const image = await ImageFactory.create({ submitterUserId: otherUserId() });
+
+		const thrown = await thrownBy(
+			imageFieldValueToImgId({
+				value: existing(image.id),
+				user: await editor(),
+				isCurrentImgId: () => false,
+			}),
+		);
+
+		expect(thrown).toBeInstanceOf(Response);
+		assertResponseErrored(thrown as Response, "Image does not belong to you");
+	});
+
+	test("rejects an image id that does not exist", async () => {
+		const thrown = await thrownBy(
+			imageFieldValueToImgId({
+				value: existing(999_999),
+				user: await editor(),
+			}),
+		);
+
+		expect(thrown).toBeInstanceOf(Response);
+	});
+});

app/features/img-upload/image-field.server.ts

@@ -15,19 +15,37 @@ import { MAX_UNVALIDATED_IMG_COUNT } from "./upload-constants";
  * Resolves a SendouForm `image` field value to the image id the caller stores on its FK column:
  * `null` → `null`, `EXISTING` → the unchanged `imgId`, `NEW` → uploads to S3 and inserts an
  * unvalidated image row (auto-validated for supporters or when `autoValidate` is set).
+ *
+ * An `EXISTING` id is client-supplied, so it is only accepted when the user uploaded that image
+ * themselves or `isCurrentImgId` vouches for it; otherwise anyone could attach (and, via the
+ * entity's image cleanup, delete) another user's image.
  */
 export async function imageFieldValueToImgId({
 	value,
 	user,
 	autoValidate = false,
+	isCurrentImgId,
 }: {
 	value: ImageFieldValue;
 	user: AuthenticatedUser;
 	/** Bypass the moderator queue (e.g. trusted org logos). */
 	autoValidate?: boolean;
+	/** Whether the entity being edited already holds this image, letting co-editors keep one someone else uploaded. */
+	isCurrentImgId?: (imgId: number) => boolean | Promise<boolean>;
 }): Promise<number | null> {
 	if (!value) return null;
-	if (value.type === "EXISTING") return value.imgId;
+	if (value.type === "EXISTING") {
+		errorToastIfFalsy(
+			await canKeepExistingImage({
+				imgId: value.imgId,
+				userId: user.id,
+				isCurrentImgId,
+			}),
+			"Image does not belong to you",
+		);
+
+		return value.imgId;
+	}
 
 	const shouldAutoValidate = autoValidate || user.roles.includes("SUPPORTER");
 
@@ -62,3 +80,19 @@ export async function imageFieldValueToImgId({
 
 	return img.id;
 }
+
+async function canKeepExistingImage({
+	imgId,
+	userId,
+	isCurrentImgId,
+}: {
+	imgId: number;
+	userId: number;
+	isCurrentImgId?: (imgId: number) => boolean | Promise<boolean>;
+}) {
+	if (await isCurrentImgId?.(imgId)) return true;
+
+	const image = await ImageRepository.findById(imgId);
+
+	return image?.submitterUserId === userId;
+}

app/features/team/actions/t.$customUrl.edit.server.test.ts

@@ -1,12 +1,13 @@
 import { beforeEach, describe, expect, test } from "vitest";
 import { REGULAR_USER_TEST_ID } from "~/db/seed/constants";
+import * as ImageFactory from "~/db/seed/factories/ImageFactory";
 import * as TeamFactory from "~/db/seed/factories/TeamFactory";
 import * as UserFactory from "~/db/seed/factories/UserFactory";
 import * as ImageRepository from "~/features/img-upload/ImageRepository.server";
 import * as TeamRepository from "~/features/team/TeamRepository.server";
 import { invariant } from "~/utils/invariant";
 import { clampThemeToGamut } from "~/utils/oklch-gamut";
-import { wrappedAction } from "~/utils/Test";
+import { assertResponseErrored, wrappedAction } from "~/utils/Test";
 import type { editTeamActionSchema } from "../team-schemas";
 import { action as _editTeamProfileAction } from "./t.$customUrl.edit.server";
 
@@ -43,6 +44,16 @@ const VALID_CUSTOM_THEME = {
 const expectedStoredTheme = () =>
 	JSON.parse(JSON.stringify(clampThemeToGamut(VALID_CUSTOM_THEME)));
 
+const users = UserFactory.pool();
+const victimId = () => users.id(1);
+const managerId = () => users.id(2);
+
+const existingImage = (imgId: number) => ({
+	type: "EXISTING" as const,
+	imgId,
+	url: "https://example.com/test-avatar.jpg",
+});
+
 describe("team page editing", () => {
 	let customUrl: string;
 
@@ -198,4 +209,47 @@ describe("team page editing", () => {
 			expect(await imageExists(imageId)).toBe(true);
 		});
 	});
+
+	describe("keeping an image someone else uploaded", () => {
+		const imageExists = async (id: number) =>
+			Boolean(await ImageRepository.findById(id));
+
+		beforeEach(() => users.create(2));
+
+		test("rejects another user's image the team does not hold, leaving that image be", async () => {
+			await createTeam();
+			const victimImage = await ImageFactory.create({
+				submitterUserId: victimId(),
+			});
+
+			const response = await editTeamProfileAction(
+				{ ...DEFAULT_EDIT_FIELDS, logo: existingImage(victimImage.id) },
+				{ user: "regular", params: { customUrl } },
+			);
+
+			assertResponseErrored(response, "Image does not belong to you");
+			expect(await imageExists(victimImage.id)).toBe(true);
+			expect((await teamRow()).avatarImgId).toBeNull();
+		});
+
+		test("lets a manager keep the logo the owner uploaded", async () => {
+			const team = await TeamFactory.create(
+				{ name: "Team 1", memberUserIds: [REGULAR_USER_TEST_ID, managerId()] },
+				{ hasAvatar: true, managerUserIds: [managerId()] },
+			);
+			customUrl = team.customUrl;
+			const avatarImgId = (await teamRow()).avatarImgId;
+			invariant(avatarImgId, "The team was created without a logo");
+
+			const response = await editTeamProfileAction(
+				{ ...DEFAULT_EDIT_FIELDS, logo: existingImage(avatarImgId) },
+				{ user: managerId(), params: { customUrl } },
+			);
+
+			expect(response.status).toBe(302);
+			expect(response.headers.get("Location")).not.toContain("__error");
+			expect((await teamRow()).avatarImgId).toBe(avatarImgId);
+			expect(await imageExists(avatarImgId)).toBe(true);
+		});
+	});
 });

app/features/team/actions/t.$customUrl.edit.server.ts

@@ -26,6 +26,8 @@ export const action: ActionFunction = async ({ request, params }) => {
 	const result = await parseFormDataWithImages({
 		request,
 		schema: editTeamActionSchema,
+		isCurrentImgId: (imgId) =>
+			imgId === team.avatarImgId || imgId === team.bannerImgId,
 	});
 
 	if (!result.success) {

app/features/tournament-admin/actions/to.$id.admin.registration.server.ts

@@ -38,6 +38,8 @@ export const upsertRegistrationAction = async (
 	const result = await parseFormDataWithImages({
 		request,
 		schema: adminRegistrationFormSchemaServer({ tournament }),
+		// the team's own logo, or one imported along with a team of another tournament
+		isCurrentImgId: TournamentTeamRepository.isPickupAvatarImgId,
 	});
 	if (!result.success) {
 		return { fieldErrors: result.fieldErrors };

app/features/tournament-organization/actions/org.$slug.edit.server.ts

@@ -12,9 +12,14 @@ import { organizationFromParams } from "../tournament-organization-utils.server"
 
 export const action = async ({ request, params }: ActionFunctionArgs) => {
 	const user = requireUser();
+	const organization = await organizationFromParams(params);
+
+	requirePermission(organization, "EDIT");
+
 	const result = await parseFormDataWithImages({
 		request,
 		schema: organizationEditFormSchema,
+		isCurrentImgId: (imgId) => imgId === organization.avatarImgId,
 	});
 
 	if (!result.success) {
@@ -25,10 +30,6 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
 
 	const t = getServerTFunction(["org"]);
 
-	const organization = await organizationFromParams(params);
-
-	requirePermission(organization, "EDIT");
-
 	if (
 		!data.members.some(
 			(member) => member.userId === user.id && member.role === "ADMIN",

app/features/tournament/TournamentTeamRepository.server.test.ts

@@ -1,9 +1,11 @@
 import { beforeEach, describe, expect, test } from "vitest";
+import * as ImageFactory from "~/db/seed/factories/ImageFactory";
 import * as TournamentFactory from "~/db/seed/factories/TournamentFactory";
 import * as TournamentTeamFactory from "~/db/seed/factories/TournamentTeamFactory";
 import * as UserFactory from "~/db/seed/factories/UserFactory";
 import { db } from "~/db/sql";
 import type { TournamentSettings } from "~/db/tables-json";
+import { invariant } from "~/utils/invariant";
 import { withUserId } from "~/utils/Test";
 import * as TournamentTeamRepository from "./TournamentTeamRepository.server";
 
@@ -457,6 +459,37 @@ describe("TournamentTeamRepository", () => {
 			]);
 		});
 	});
+
+	describe("isPickupAvatarImgId", () => {
+		test("tells a team's pickup logo apart from an image no team uses", async () => {
+			const tournament = await TournamentFactory.create({
+				authorId: organizerId(),
+			});
+			await TournamentTeamFactory.create({
+				tournamentId: tournament.id,
+				memberUserIds: [ownerId()],
+				hasAvatar: true,
+			});
+			const unusedImage = await ImageFactory.create({
+				submitterUserId: ownerId(),
+			});
+			const pickupAvatarImgId = (
+				await db
+					.selectFrom("TournamentTeam")
+					.select("TournamentTeam.avatarImgId")
+					.where("TournamentTeam.tournamentId", "=", tournament.id)
+					.executeTakeFirstOrThrow()
+			).avatarImgId;
+			invariant(pickupAvatarImgId, "Expected the team to have a logo");
+
+			expect(
+				await TournamentTeamRepository.isPickupAvatarImgId(pickupAvatarImgId),
+			).toBe(true);
+			expect(
+				await TournamentTeamRepository.isPickupAvatarImgId(unusedImage.id),
+			).toBe(false);
+		});
+	});
 });
 
 const byId = (a: number, b: number) => a - b;

app/features/tournament/TournamentTeamRepository.server.ts

@@ -1037,6 +1037,17 @@ export async function findInviteCodeById(tournamentTeamId: number) {
 	return row?.inviteCode ?? null;
 }
 
+/** Whether some team of some tournament has this image as its pickup logo; organizers copy those when importing teams. */
+export async function isPickupAvatarImgId(imgId: number) {
+	const row = await db
+		.selectFrom("TournamentTeam")
+		.select("TournamentTeam.id")
+		.where("TournamentTeam.avatarImgId", "=", imgId)
+		.executeTakeFirst();
+
+	return Boolean(row);
+}
+
 export function findByInviteCode(inviteCode: string) {
 	return db
 		.selectFrom("TournamentTeam")

app/features/tournament/actions/to.$id.register.server.ts

@@ -14,6 +14,7 @@ import type { Tournament } from "~/features/tournament-bracket/core/Tournament";
 import {
 	clearTournamentDataCache,
 	tournamentFromParams,
+	tournamentTeamsFullCached,
 } from "~/features/tournament-bracket/core/Tournament.server";
 import * as TournamentLFGRepository from "~/features/tournament-lfg/TournamentLFGRepository.server";
 import * as UserRepository from "~/features/user-page/UserRepository.server";
@@ -44,6 +45,11 @@ export const action: ActionFunction = async ({ request, params }) => {
 	const result = await parseFormDataWithImages({
 		request,
 		schema: registerSchema({ tournament, ownTeamId: ownTeam?.id }),
+		isCurrentImgId: async (imgId) =>
+			Boolean(ownTeam) &&
+			(await tournamentTeamsFullCached({ tournamentId, user })).some(
+				(team) => team.id === ownTeam?.id && team.avatarImgId === imgId,
+			),
 	});
 	if (!result.success) {
 		return { fieldErrors: result.fieldErrors };

app/form/parse.server.ts

@@ -63,13 +63,22 @@ type ResolvedImages<T> = T extends unknown
  * {@link parseFormData} plus every `image()` field resolved to the image id for the FK column via
  * {@link imageFieldValueToImgId} (uploading new, keeping unchanged, clearing removed). The schema may be an
  * object or a union of objects (e.g. `_action` discriminated).
+ *
+ * A kept (`EXISTING`) image must be the user's own upload unless `isCurrentImgId` says the edited
+ * entity already holds it; forms that only ever keep the user's own images can leave it out.
  */
 export async function parseFormDataWithImages<T extends AnySchema>({
 	request,
 	schema,
+	isCurrentImgId,
 }: {
 	request: Request;
 	schema: T;
+	/** Whether the edited entity already holds this image (given the parsed form data to find the entity by). */
+	isCurrentImgId?: (
+		imgId: number,
+		data: v.InferOutput<T>,
+	) => boolean | Promise<boolean>;
 }): Promise<ParseResult<ResolvedImages<v.InferOutput<T>>>> {
 	const result = await parseFormData({ request, schema });
 	if (!result.success) return result;
@@ -83,6 +92,9 @@ export async function parseFormDataWithImages<T extends AnySchema>({
 				value: data[key] as ImageFieldValue,
 				user,
 				autoValidate,
+				isCurrentImgId: isCurrentImgId
+					? (imgId) => isCurrentImgId(imgId, result.data)
+					: undefined,
 			});
 		}
 	}

migrations/20260909052252-tournament-team-avatar-img-id-index.ts

@@ -0,0 +1,11 @@
+import type { Kysely } from "kysely";
+
+export async function up(db: Kysely<any>): Promise<void> {
+	// the pickup logo lookup of every registration upsert scanned every team, and most teams have no logo
+	await db.schema
+		.createIndex("tournament_team_avatar_img_id")
+		.on("TournamentTeam")
+		.column("avatarImgId")
+		.where("avatarImgId", "is not", null)
+		.execute();
+}

scripts/benchmark-db/cases.ts

@@ -269,6 +269,12 @@ export function buildCases(fx: Fixtures): {
 			includeBadgePrizes: true,
 		}),
 	);
+	add(
+		"CalendarRepository.findAvatarImgIds",
+		both(fx.heavyCalendarEventId, fx.heavyTournamentId),
+		([eventId, tournamentId]) =>
+			CalendarRepository.findAvatarImgIds({ eventId, tournamentId }),
+	);
 	add(
 		"CalendarRepository.findRecentTournamentsByOrganizerUserId",
 		fx.calendarAuthorId,
@@ -1255,6 +1261,9 @@ export function buildCases(fx: Fixtures): {
 		(tournamentTeamId) =>
 			TournamentTeamRepository.findInviteCodeById(tournamentTeamId),
 	);
+	add("TournamentTeamRepository.isPickupAvatarImgId", fx.imageId, (imageId) =>
+		TournamentTeamRepository.isPickupAvatarImgId(imageId),
+	);
 	add(
 		"TournamentTeamRepository.findRecentlyPlayedMapsByIds",
 		both(fx.tournamentTeamPair, fx.heavyTournamentMatchId),