Uploader Bugs (round 2) (#1264)

oscartbeaumont · web-flow · commit f60cd3af285d · 2025-10-22T11:21:07.000+08:00
* fix deleting no parts

* fix

* maybe fix multipart initiate

* fix auth check for multipart initiate

* fix auth policy

* bump URL expiry

* fixes

* fixes

* fixes

* fixes

* format
diff --git a/apps/web/actions/organization/delete-space.ts b/apps/web/actions/organization/delete-space.ts
@@ -77,7 +77,7 @@ export async function deleteSpace(
 					prefix: `organizations/${user.activeOrganizationId}/spaces/${spaceId}/`,
 				});
 
-				if (listedObjects.Contents?.length) {
+				if (listedObjects.Contents) {
 					yield* bucket.deleteObjects(
 						listedObjects.Contents.map((content) => ({
 							Key: content.Key,
diff --git a/apps/web/app/api/desktop/[...route]/video.ts b/apps/web/app/api/desktop/[...route]/video.ts
@@ -20,11 +20,7 @@ import { Effect, Option } from "effect";
 import { Hono } from "hono";
 import { z } from "zod";
 import { runPromise } from "@/lib/server";
-import {
-	isAtLeastSemver,
-	isFromDesktopSemver,
-	UPLOAD_PROGRESS_VERSION,
-} from "@/utils/desktop";
+import { isFromDesktopSemver, UPLOAD_PROGRESS_VERSION } from "@/utils/desktop";
 import { stringOrNumberOptional } from "@/utils/zod";
 import { withAuth } from "../../utils";
 
@@ -295,7 +291,7 @@ app.delete(
 					prefix: `${user.id}/${videoId}/`,
 				});
 
-				if (listedObjects.Contents?.length)
+				if (listedObjects.Contents)
 					yield* bucket.deleteObjects(
 						listedObjects.Contents.map((content: any) => ({
 							Key: content.Key,
diff --git a/apps/web/app/api/playlist/route.ts b/apps/web/app/api/playlist/route.ts
@@ -47,13 +47,15 @@ const ApiLive = HttpApiBuilder.api(Api).pipe(
 
 				return handlers.handle("getVideoSrc", ({ urlParams }) =>
 					Effect.gen(function* () {
-						const [video] = yield* videos.getById(urlParams.videoId).pipe(
-							Effect.flatten,
-							Effect.catchTag(
-								"NoSuchElementException",
-								() => new HttpApiError.NotFound(),
-							),
-						);
+						const [video] = yield* videos
+							.getByIdForViewing(urlParams.videoId)
+							.pipe(
+								Effect.flatten,
+								Effect.catchTag(
+									"NoSuchElementException",
+									() => new HttpApiError.NotFound(),
+								),
+							);
 
 						return yield* getPlaylistResponse(video, urlParams);
 					}).pipe(
diff --git a/apps/web/app/api/upload/[...route]/multipart.ts b/apps/web/app/api/upload/[...route]/multipart.ts
@@ -2,7 +2,7 @@ import {
 	CloudFrontClient,
 	CreateInvalidationCommand,
 } from "@aws-sdk/client-cloudfront";
-import { db, updateIfDefined } from "@cap/database";
+import { updateIfDefined } from "@cap/database";
 import * as Db from "@cap/database/schema";
 import { serverEnv } from "@cap/env";
 import {
@@ -11,8 +11,10 @@ import {
 	provideOptionalAuth,
 	S3Buckets,
 	Videos,
+	VideosPolicy,
+	VideosRepo,
 } from "@cap/web-backend";
-import { Video } from "@cap/web-domain";
+import { CurrentUser, Policy, Video } from "@cap/web-domain";
 import { zValidator } from "@hono/zod-validator";
 import { and, eq } from "drizzle-orm";
 import { Effect, Option, Schedule } from "effect";
@@ -47,14 +49,18 @@ app.post(
 		});
 
 		const videoIdFromFileKey = fileKey.split("/")[1];
-		const videoId = "videoId" in body ? body.videoId : videoIdFromFileKey;
-		if (!videoId) throw new Error("Video ID is required");
+		const videoIdRaw = "videoId" in body ? body.videoId : videoIdFromFileKey;
+		if (!videoIdRaw) return c.text("Video id not found", 400);
+		const videoId = Video.VideoId.make(videoIdRaw);
 
 		const resp = await Effect.gen(function* () {
-			const videos = yield* Videos;
+			const repo = yield* VideosRepo;
+			const policy = yield* VideosPolicy;
 			const db = yield* Database;
 
-			const video = yield* videos.getById(Video.VideoId.make(videoId));
+			const video = yield* repo
+				.getById(videoId)
+				.pipe(Policy.withPolicy(policy.isOwner(videoId)));
 			if (Option.isNone(video)) return yield* new Video.NotFoundError();
 
 			yield* db.use((db) =>
@@ -74,6 +80,7 @@ app.post(
 					c.json({ error: "Error initiating multipart upload" }, 500),
 				);
 			}),
+			Effect.provideService(CurrentUser, user),
 			runPromise,
 		);
 		if (resp) return resp;
@@ -230,24 +237,28 @@ app.post(
 				]),
 			),
 	),
-	(c) =>
-		Effect.gen(function* () {
-			const videos = yield* Videos;
-			const db = yield* Database;
+	(c) => {
+		const { uploadId, parts, ...body } = c.req.valid("json");
+		const user = c.get("user");
 
-			const { uploadId, parts, ...body } = c.req.valid("json");
-			const user = c.get("user");
+		return Effect.gen(function* () {
+			const repo = yield* VideosRepo;
+			const policy = yield* VideosPolicy;
+			const db = yield* Database;
 
 			const fileKey = parseVideoIdOrFileKey(user.id, {
 				...body,
 				subpath: "result.mp4",
 			});
 
 			const videoIdFromFileKey = fileKey.split("/")[1];
-			const videoId = "videoId" in body ? body.videoId : videoIdFromFileKey;
-			if (!videoId) throw new Error("Video ID is required");
+			const videoIdRaw = "videoId" in body ? body.videoId : videoIdFromFileKey;
+			if (!videoIdRaw) return c.text("Video id not found", 400);
+			const videoId = Video.VideoId.make(videoIdRaw);
 
-			const maybeVideo = yield* videos.getById(Video.VideoId.make(videoId));
+			const maybeVideo = yield* repo
+				.getById(videoId)
+				.pipe(Policy.withPolicy(policy.isOwner(videoId)));
 			if (Option.isNone(maybeVideo)) {
 				c.status(404);
 				return c.text(`Video '${encodeURIComponent(videoId)}' not found`);
@@ -467,5 +478,6 @@ app.post(
 					);
 				}),
 			);
-		}).pipe(provideOptionalAuth, runPromise),
+		}).pipe(Effect.provideService(CurrentUser, user), runPromise);
+	},
 );
diff --git a/apps/web/app/embed/[videoId]/page.tsx b/apps/web/app/embed/[videoId]/page.tsx
@@ -29,7 +29,7 @@ export async function generateMetadata(
 	const params = await props.params;
 	const videoId = params.videoId as Video.VideoId;
 
-	return Effect.flatMap(Videos, (v) => v.getById(videoId)).pipe(
+	return Effect.flatMap(Videos, (v) => v.getByIdForViewing(videoId)).pipe(
 		Effect.map(
 			Option.match({
 				onNone: () => notFound(),
diff --git a/apps/web/app/s/[videoId]/page.tsx b/apps/web/app/s/[videoId]/page.tsx
@@ -134,7 +134,7 @@ export async function generateMetadata(
 		referrer.includes(domain),
 	);
 
-	return Effect.flatMap(Videos, (v) => v.getById(videoId)).pipe(
+	return Effect.flatMap(Videos, (v) => v.getByIdForViewing(videoId)).pipe(
 		Effect.map(
 			Option.match({
 				onNone: () => notFound(),
diff --git a/apps/web/lib/server.ts b/apps/web/lib/server.ts
@@ -13,6 +13,7 @@ import {
 	SpacesPolicy,
 	Videos,
 	VideosPolicy,
+	VideosRepo,
 	Workflows,
 } from "@cap/web-backend";
 import { type HttpAuthMiddleware, Video } from "@cap/web-domain";
@@ -21,11 +22,10 @@ import {
 	Headers,
 	type HttpApi,
 	HttpApiBuilder,
-	HttpApiClient,
 	HttpMiddleware,
 	HttpServer,
 } from "@effect/platform";
-import { RpcClient, RpcMessage, RpcMiddleware } from "@effect/rpc";
+import { RpcClient, RpcMiddleware } from "@effect/rpc";
 import {
 	Cause,
 	Config,
@@ -100,6 +100,7 @@ export const Dependencies = Layer.mergeAll(
 	S3Buckets.Default,
 	Videos.Default,
 	VideosPolicy.Default,
+	VideosRepo.Default,
 	Folders.Default,
 	SpacesPolicy.Default,
 	OrganisationsPolicy.Default,
diff --git a/packages/web-backend/src/S3Buckets/S3BucketAccess.ts b/packages/web-backend/src/S3Buckets/S3BucketAccess.ts
@@ -181,7 +181,7 @@ export const createS3BucketAccess = Effect.gen(function* () {
 						),
 					),
 				),
-			),
+			).pipe(Effect.when(() => objects.length > 0)),
 		getPresignedPutUrl: (
 			key: string,
 			args?: Omit<S3.PutObjectRequest, "Key" | "Bucket">,
@@ -256,6 +256,7 @@ export const createS3BucketAccess = Effect.gen(function* () {
 									UploadId: uploadId,
 									PartNumber: partNumber,
 								}),
+								{ expiresIn: 3600 },
 							),
 						),
 					),
diff --git a/packages/web-backend/src/Videos/index.ts b/packages/web-backend/src/Videos/index.ts
@@ -16,7 +16,7 @@ export class Videos extends Effect.Service<Videos>()("Videos", {
 		const policy = yield* VideosPolicy;
 		const s3Buckets = yield* S3Buckets;
 
-		const getById = (id: Video.VideoId) =>
+		const getByIdForViewing = (id: Video.VideoId) =>
 			repo
 				.getById(id)
 				.pipe(
@@ -30,7 +30,7 @@ export class Videos extends Effect.Service<Videos>()("Videos", {
 			 */
 			// This is only for external use since it does an access check,
 			// internal use should prefer the repo directly
-			getById,
+			getByIdForViewing,
 
 			/*
 			 * Delete a video. Will fail if the user does not have access.
@@ -56,7 +56,7 @@ export class Videos extends Effect.Service<Videos>()("Videos", {
 
 				const listedObjects = yield* bucket.listObjects({ prefix });
 
-				if (listedObjects.Contents?.length) {
+				if (listedObjects.Contents) {
 					yield* bucket.deleteObjects(
 						listedObjects.Contents.map((content) => ({
 							Key: content.Key,
@@ -199,7 +199,7 @@ export class Videos extends Effect.Service<Videos>()("Videos", {
 			getAnalytics: Effect.fn("Videos.getAnalytics")(function* (
 				videoId: Video.VideoId,
 			) {
-				const [video] = yield* getById(videoId).pipe(
+				const [video] = yield* getByIdForViewing(videoId).pipe(
 					Effect.flatten,
 					Effect.catchTag(
 						"NoSuchElementException",
diff --git a/packages/web-backend/src/index.ts b/packages/web-backend/src/index.ts
@@ -11,4 +11,5 @@ export { Spaces } from "./Spaces/index.ts";
 export { SpacesPolicy } from "./Spaces/SpacesPolicy.ts";
 export { Videos } from "./Videos/index.ts";
 export { VideosPolicy } from "./Videos/VideosPolicy.ts";
+export { VideosRepo } from "./Videos/VideosRepo.ts";
 export * as Workflows from "./Workflows.ts";
