fix: asset upload metadata validation (#21594)

server/src/controllers/asset-media.controller.spec.ts

@@ -1,4 +1,6 @@
 import { AssetMediaController } from 'src/controllers/asset-media.controller';
+import { AssetMediaStatus } from 'src/dtos/asset-media-response.dto';
+import { AssetMetadataKey } from 'src/enum';
 import { LoggingRepository } from 'src/repositories/logging.repository';
 import { AssetMediaService } from 'src/services/asset-media.service';
 import request from 'supertest';
@@ -11,7 +13,7 @@ const makeUploadDto = (options?: { omit: string }): Record<string, any> => {
     deviceId: 'TEST',
     fileCreatedAt: new Date().toISOString(),
     fileModifiedAt: new Date().toISOString(),
-    isFavorite: 'testing',
+    isFavorite: 'false',
     duration: '0:00:00.000000',
   };
 
@@ -27,16 +29,20 @@ describe(AssetMediaController.name, () => {
   let ctx: ControllerContext;
   const assetData = Buffer.from('123');
   const filename = 'example.png';
+  const service = mockBaseService(AssetMediaService);
 
   beforeAll(async () => {
     ctx = await controllerSetup(AssetMediaController, [
       { provide: LoggingRepository, useValue: automock(LoggingRepository, { strict: false }) },
-      { provide: AssetMediaService, useValue: mockBaseService(AssetMediaService) },
+      { provide: AssetMediaService, useValue: service },
     ]);
     return () => ctx.close();
   });
 
   beforeEach(() => {
+    service.resetAllMocks();
+    service.uploadAsset.mockResolvedValue({ status: AssetMediaStatus.DUPLICATE, id: factory.uuid() });
+
     ctx.reset();
   });
 
@@ -46,13 +52,61 @@ describe(AssetMediaController.name, () => {
       expect(ctx.authenticate).toHaveBeenCalled();
     });
 
+    it('should accept metadata', async () => {
+      const mobileMetadata = { key: AssetMetadataKey.MobileApp, value: { iCloudId: '123' } };
+      const { status } = await request(ctx.getHttpServer())
+        .post('/assets')
+        .attach('assetData', assetData, filename)
+        .field({
+          ...makeUploadDto(),
+          metadata: JSON.stringify([mobileMetadata]),
+        });
+
+      expect(service.uploadAsset).toHaveBeenCalledWith(
+        undefined,
+        expect.objectContaining({ metadata: [mobileMetadata] }),
+        expect.objectContaining({ originalName: 'example.png' }),
+        undefined,
+      );
+
+      expect(status).toBe(200);
+    });
+
+    it('should handle invalid metadata json', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/assets')
+        .attach('assetData', assetData, filename)
+        .field({
+          ...makeUploadDto(),
+          metadata: 'not-a-string-string',
+        });
+
+      expect(status).toBe(400);
+      expect(body).toEqual(factory.responses.badRequest(['metadata must be valid JSON']));
+    });
+
+    it('should validate iCloudId is a string', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/assets')
+        .attach('assetData', assetData, filename)
+        .field({
+          ...makeUploadDto(),
+          metadata: JSON.stringify([{ key: AssetMetadataKey.MobileApp, value: { iCloudId: 123 } }]),
+        });
+
+      expect(status).toBe(400);
+      expect(body).toEqual(factory.responses.badRequest(['metadata.0.value.iCloudId must be a string']));
+    });
+
     it('should require `deviceAssetId`', async () => {
       const { status, body } = await request(ctx.getHttpServer())
         .post('/assets')
         .attach('assetData', assetData, filename)
         .field({ ...makeUploadDto({ omit: 'deviceAssetId' }) });
       expect(status).toBe(400);
-      expect(body).toEqual(factory.responses.badRequest());
+      expect(body).toEqual(
+        factory.responses.badRequest(['deviceAssetId must be a string', 'deviceAssetId should not be empty']),
+      );
     });
 
     it('should require `deviceId`', async () => {
@@ -61,7 +115,7 @@ describe(AssetMediaController.name, () => {
         .attach('assetData', assetData, filename)
         .field({ ...makeUploadDto({ omit: 'deviceId' }) });
       expect(status).toBe(400);
-      expect(body).toEqual(factory.responses.badRequest());
+      expect(body).toEqual(factory.responses.badRequest(['deviceId must be a string', 'deviceId should not be empty']));
     });
 
     it('should require `fileCreatedAt`', async () => {
@@ -70,25 +124,20 @@ describe(AssetMediaController.name, () => {
         .attach('assetData', assetData, filename)
         .field({ ...makeUploadDto({ omit: 'fileCreatedAt' }) });
       expect(status).toBe(400);
-      expect(body).toEqual(factory.responses.badRequest());
+      expect(body).toEqual(
+        factory.responses.badRequest(['fileCreatedAt must be a Date instance', 'fileCreatedAt should not be empty']),
+      );
     });
 
     it('should require `fileModifiedAt`', async () => {
       const { status, body } = await request(ctx.getHttpServer())
         .post('/assets')
         .attach('assetData', assetData, filename)
-        .field({ ...makeUploadDto({ omit: 'fileModifiedAt' }) });
+        .field(makeUploadDto({ omit: 'fileModifiedAt' }));
       expect(status).toBe(400);
-      expect(body).toEqual(factory.responses.badRequest());
+      expect(body).toEqual(
-    });
+        factory.responses.badRequest(['fileModifiedAt must be a Date instance', 'fileModifiedAt should not be empty']),
-
+      );
-    it('should require `duration`', async () => {
-      const { status, body } = await request(ctx.getHttpServer())
-        .post('/assets')
-        .attach('assetData', assetData, filename)
-        .field({ ...makeUploadDto({ omit: 'duration' }) });
-      expect(status).toBe(400);
-      expect(body).toEqual(factory.responses.badRequest());
     });
 
     it('should throw if `isFavorite` is not a boolean', async () => {
@@ -97,16 +146,18 @@ describe(AssetMediaController.name, () => {
         .attach('assetData', assetData, filename)
         .field({ ...makeUploadDto(), isFavorite: 'not-a-boolean' });
       expect(status).toBe(400);
-      expect(body).toEqual(factory.responses.badRequest());
+      expect(body).toEqual(factory.responses.badRequest(['isFavorite must be a boolean value']));
     });
 
     it('should throw if `visibility` is not an enum', async () => {
       const { status, body } = await request(ctx.getHttpServer())
         .post('/assets')
         .attach('assetData', assetData, filename)
-        .field({ ...makeUploadDto(), visibility: 'not-a-boolean' });
+        .field({ ...makeUploadDto(), visibility: 'not-an-option' });
       expect(status).toBe(400);
-      expect(body).toEqual(factory.responses.badRequest());
+      expect(body).toEqual(
+        factory.responses.badRequest([expect.stringContaining('visibility must be one of the following values:')]),
+      );
     });
 
     // TODO figure out how to deal with `sendFile`

server/src/dtos/asset-media.dto.ts

@@ -1,5 +1,6 @@
+import { BadRequestException } from '@nestjs/common';
 import { ApiProperty } from '@nestjs/swagger';
-import { Type } from 'class-transformer';
+import { plainToInstance, Transform, Type } from 'class-transformer';
 import { ArrayNotEmpty, IsArray, IsNotEmpty, IsString, ValidateNested } from 'class-validator';
 import { AssetMetadataUpsertItemDto } from 'src/dtos/asset.dto';
 import { AssetVisibility } from 'src/enum';
@@ -65,10 +66,18 @@ export class AssetMediaCreateDto extends AssetMediaBase {
   @ValidateUUID({ optional: true })
   livePhotoVideoId?: string;
 
+  @Transform(({ value }) => {
+    try {
+      const json = JSON.parse(value);
+      const items = Array.isArray(json) ? json : [json];
+      return items.map((item) => plainToInstance(AssetMetadataUpsertItemDto, item));
+    } catch {
+      throw new BadRequestException(['metadata must be valid JSON']);
+    }
+  })
   @Optional()
-  @IsArray()
   @ValidateNested({ each: true })
-  @Type(() => AssetMetadataUpsertItemDto)
+  @IsArray()
   metadata!: AssetMetadataUpsertItemDto[];
 
   @ApiProperty({ type: 'string', format: 'binary', required: false })

server/test/utils.ts

@@ -1,12 +1,16 @@
-import { CallHandler, Provider, ValidationPipe } from '@nestjs/common';
+import { CallHandler, ExecutionContext, Provider, ValidationPipe } from '@nestjs/common';
 import { APP_GUARD, APP_PIPE } from '@nestjs/core';
+import { transformException } from '@nestjs/platform-express/multer/multer/multer.utils';
 import { Test } from '@nestjs/testing';
 import { ClassConstructor } from 'class-transformer';
+import { NextFunction } from 'express';
 import { Kysely } from 'kysely';
+import multer from 'multer';
 import { ChildProcessWithoutNullStreams } from 'node:child_process';
 import { Readable, Writable } from 'node:stream';
 import { PNG } from 'pngjs';
 import postgres from 'postgres';
+import { UploadFieldName } from 'src/dtos/asset-media.dto';
 import { AssetUploadInterceptor } from 'src/middleware/asset-upload.interceptor';
 import { AuthGuard } from 'src/middleware/auth.guard';
 import { FileUploadInterceptor } from 'src/middleware/file-upload.interceptor';
@@ -82,6 +86,24 @@ export type ControllerContext = {
 
 export const controllerSetup = async (controller: ClassConstructor<unknown>, providers: Provider[]) => {
   const noopInterceptor = { intercept: (ctx: never, next: CallHandler<unknown>) => next.handle() };
+  const upload = multer({ storage: multer.memoryStorage() });
+  const memoryFileInterceptor = {
+    intercept: async (ctx: ExecutionContext, next: CallHandler<unknown>) => {
+      const context = ctx.switchToHttp();
+      const handler = upload.fields([
+        { name: UploadFieldName.ASSET_DATA, maxCount: 1 },
+        { name: UploadFieldName.SIDECAR_DATA, maxCount: 1 },
+      ]);
+
+      await new Promise<void>((resolve, reject) => {
+        const next: NextFunction = (error) => (error ? reject(transformException(error)) : resolve());
+        const maybePromise = handler(context.getRequest(), context.getResponse(), next);
+        Promise.resolve(maybePromise).catch((error) => reject(error));
+      });
+
+      return next.handle();
+    },
+  };
   const moduleRef = await Test.createTestingModule({
     controllers: [controller],
     providers: [
@@ -93,7 +115,7 @@ export const controllerSetup = async (controller: ClassConstructor<unknown>, pro
     ],
   })
     .overrideInterceptor(FileUploadInterceptor)
-    .useValue(noopInterceptor)
+    .useValue(memoryFileInterceptor)
     .overrideInterceptor(AssetUploadInterceptor)
     .useValue(noopInterceptor)
     .compile();