fix(web): require an explicit share to download, matching the view policy

apps/web/__tests__/unit/video-download-permissions.test.ts

@@ -0,0 +1,128 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+/**
+ * `canUserDownloadVideo` issues its queries in a fixed order, so each test
+ * declares the rows every query returns, in order:
+ *
+ *   1. sharedVideos    - orgs this video was explicitly shared with
+ *   2. organizationMembers - the user's membership in one of those orgs (only when 1 is non-empty)
+ *   3. spaceVideos     - spaces this video sits in
+ *   4. spaceMembers    - the user's membership in one of those spaces (only when 3 is non-empty)
+ */
+const mocks = vi.hoisted(() => {
+	const queue: unknown[][] = [];
+	let selectCalls = 0;
+
+	const db = () => ({
+		select: () => {
+			selectCalls += 1;
+			const rows = queue.shift() ?? [];
+			// The production code awaits some chains directly and calls .limit()
+			// on others, so the tail satisfies both shapes.
+			const tail = {
+				limit: async () => rows,
+				then: (resolve: (value: unknown[]) => unknown) => resolve(rows),
+			};
+			return { from: () => ({ where: () => tail }) };
+		},
+	});
+
+	return {
+		db,
+		queue,
+		reset() {
+			queue.length = 0;
+			selectCalls = 0;
+		},
+		get selectCalls() {
+			return selectCalls;
+		},
+	};
+});
+
+vi.mock("@cap/database", () => ({ db: mocks.db }));
+
+vi.mock("@cap/database/schema", () => ({
+	organizationMembers: {
+		id: "organizationMembers.id",
+		userId: "organizationMembers.userId",
+		organizationId: "organizationMembers.organizationId",
+	},
+	sharedVideos: {
+		videoId: "sharedVideos.videoId",
+		organizationId: "sharedVideos.organizationId",
+	},
+	spaceMembers: {
+		id: "spaceMembers.id",
+		userId: "spaceMembers.userId",
+		spaceId: "spaceMembers.spaceId",
+	},
+	spaceVideos: {
+		videoId: "spaceVideos.videoId",
+		spaceId: "spaceVideos.spaceId",
+	},
+}));
+
+import { canUserDownloadVideo } from "@/lib/video-download-permissions";
+
+const request = (overrides: Record<string, string> = {}) =>
+	({
+		userId: "user-b",
+		ownerId: "user-a",
+		videoId: "video-1",
+		...overrides,
+	}) as Parameters<typeof canUserDownloadVideo>[0];
+
+beforeEach(() => {
+	mocks.reset();
+});
+
+describe("canUserDownloadVideo", () => {
+	it("allows the owner without querying anything", async () => {
+		await expect(
+			canUserDownloadVideo(request({ userId: "user-a" })),
+		).resolves.toBe(true);
+
+		expect(mocks.selectCalls).toBe(0);
+	});
+
+	it("refuses a colleague when the video was never shared", async () => {
+		// The video belongs to the owner's org, but no sharedVideos or
+		// spaceVideos row exists. Membership of the owner's org is not a grant:
+		// buildCanView requires an explicit share, and download must match it.
+		mocks.queue.push([], []);
+
+		await expect(canUserDownloadVideo(request())).resolves.toBe(false);
+
+		// With no share rows there is nothing to match a membership against, so
+		// neither membership lookup should run: 2 queries, not 4.
+		expect(mocks.selectCalls).toBe(2);
+	});
+
+	it("allows a member of an org the video was explicitly shared with", async () => {
+		mocks.queue.push(
+			[{ organizationId: "org-shared" }],
+			[{ id: "org-membership-1" }],
+		);
+
+		await expect(canUserDownloadVideo(request())).resolves.toBe(true);
+	});
+
+	it("refuses when shared to an org the user does not belong to", async () => {
+		mocks.queue.push([{ organizationId: "org-shared" }], [], []);
+
+		await expect(canUserDownloadVideo(request())).resolves.toBe(false);
+	});
+
+	it("allows a member of a space the video was shared into", async () => {
+		mocks.queue.push([], [{ spaceId: "space-1" }], [{ id: "space-membership-1" }]);
+
+		await expect(canUserDownloadVideo(request())).resolves.toBe(true);
+	});
+
+	it("refuses when the space is one the user does not belong to", async () => {
+		mocks.queue.push([], [{ spaceId: "space-1" }], []);
+
+		await expect(canUserDownloadVideo(request())).resolves.toBe(false);
+	});
+});

apps/web/actions/videos/download.ts

@@ -90,7 +90,6 @@ export async function getVideoDownloadInfo(
 		userId: user.id,
 		ownerId: video.ownerId,
 		videoId,
-		orgId: video.orgId,
 	});
 
 	if (!allowed) {

apps/web/app/s/[videoId]/page.tsx

@@ -862,7 +862,6 @@ async function AuthorizedContent({
 					userId,
 					ownerId: video.owner.id,
 					videoId,
-					orgId: video.orgId,
 				})
 			: false;
 

apps/web/lib/video-download-permissions.ts

@@ -5,19 +5,17 @@ import {
 	spaceMembers,
 	spaceVideos,
 } from "@cap/database/schema";
-import type { Organisation, User, Video } from "@cap/web-domain";
+import type { User, Video } from "@cap/web-domain";
 import { and, eq, inArray } from "drizzle-orm";
 
 export async function canUserDownloadVideo({
 	userId,
 	ownerId,
 	videoId,
-	orgId,
 }: {
 	userId: User.UserId;
 	ownerId: User.UserId;
 	videoId: Video.VideoId;
-	orgId: Organisation.OrganisationId;
 }): Promise<boolean> {
 	if (userId === ownerId) return true;
 
@@ -26,20 +24,27 @@ export async function canUserDownloadVideo({
 		.from(sharedVideos)
 		.where(eq(sharedVideos.videoId, videoId));
 
-	const orgIds = [orgId, ...sharedOrgs.map((org) => org.organizationId)];
-
-	const [orgMembership] = await db()
-		.select({ id: organizationMembers.id })
-		.from(organizationMembers)
-		.where(
-			and(
-				eq(organizationMembers.userId, userId),
-				inArray(organizationMembers.organizationId, orgIds),
-			),
-		)
-		.limit(1);
+	// Only orgs this video was explicitly shared with count. The video's own
+	// orgId is where it lives, not a grant: including it would let any member of
+	// the owner's org download a video they cannot even view, since buildCanView
+	// requires a sharedVideos row for the org (VideosPolicy.ts).
+	if (sharedOrgs.length > 0) {
+		const [orgMembership] = await db()
+			.select({ id: organizationMembers.id })
+			.from(organizationMembers)
+			.where(
+				and(
+					eq(organizationMembers.userId, userId),
+					inArray(
+						organizationMembers.organizationId,
+						sharedOrgs.map((org) => org.organizationId),
+					),
+				),
+			)
+			.limit(1);
 
-	if (orgMembership) return true;
+		if (orgMembership) return true;
+	}
 
 	const sharedSpaces = await db()
 		.select({ spaceId: spaceVideos.spaceId })