diff --git a/apps/web/__tests__/unit/video-download-permissions.test.ts b/apps/web/__tests__/unit/video-download-permissions.test.ts new file mode 100644 index 0000000000..898d344be5 --- /dev/null +++ b/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 = {}) => + ({ + userId: "user-b", + ownerId: "user-a", + videoId: "video-1", + ...overrides, + }) as Parameters[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); + }); +}); diff --git a/apps/web/actions/videos/download.ts b/apps/web/actions/videos/download.ts index 9a894f1341..5ced7ae8cf 100644 --- a/apps/web/actions/videos/download.ts +++ b/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) { diff --git a/apps/web/app/s/[videoId]/page.tsx b/apps/web/app/s/[videoId]/page.tsx index e4572e0f37..a19c9aa0b4 100644 --- a/apps/web/app/s/[videoId]/page.tsx +++ b/apps/web/app/s/[videoId]/page.tsx @@ -862,7 +862,6 @@ async function AuthorizedContent({ userId, ownerId: video.owner.id, videoId, - orgId: video.orgId, }) : false; diff --git a/apps/web/lib/video-download-permissions.ts b/apps/web/lib/video-download-permissions.ts index 0fc626bf64..825eea79ef 100644 --- a/apps/web/lib/video-download-permissions.ts +++ b/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 { 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 })