fix(server): scope duplicate group resolution to the caller's assets (#32603)

(cherry picked from commit 7bb91afbe15e82502f8f6c92b61edb3865ce76f3)

server/src/queries/duplicate.repository.sql

@@ -121,7 +121,8 @@ from
   ) as "asset2" on true
 where
   "asset"."visibility" in ('archive', 'timeline')
-  and "asset"."duplicateId" = $1::uuid
+  and "asset"."ownerId" = $1::uuid
+  and "asset"."duplicateId" = $2::uuid
   and "asset"."deletedAt" is null
   and "asset"."stackId" is null
 group by

server/src/repositories/duplicate.repository.ts

@@ -112,7 +112,7 @@ export class DuplicateRepository {
   }
 
   @GenerateSql({ params: [DummyValue.UUID, DummyValue.UUID] })
-  async get(duplicateId: string): Promise<{ duplicateId: string; assets: MapAsset[] } | undefined> {
+  async get(userId: string, duplicateId: string): Promise<{ duplicateId: string; assets: MapAsset[] } | undefined> {
     const result = await this.db
       .selectFrom('asset')
       .$call(withDefaultVisibility)
@@ -141,6 +141,7 @@ export class DuplicateRepository {
       )
       .select('asset.duplicateId')
       .select((eb) => eb.fn.jsonAgg('asset2').orderBy('asset.localDateTime', 'asc').$castTo<MapAsset[]>().as('assets'))
+      .where('asset.ownerId', '=', asUuid(userId))
       .where('asset.duplicateId', '=', asUuid(duplicateId))
       .where('asset.deletedAt', 'is', null)
       .where('asset.stackId', 'is', null)

server/src/services/duplicate.service.spec.ts

@@ -233,6 +233,28 @@ describe(DuplicateService.name, () => {
       ]);
     });
 
+    it('should only load the assets owned by the user', async () => {
+      const asset = AssetFactory.create();
+      mocks.access.duplicate.checkOwnerAccess.mockResolvedValue(new Set(['group-1']));
+      mocks.duplicateRepository.get.mockResolvedValue({
+        duplicateId: 'group-1',
+        assets: [asset as unknown as MapAsset],
+      });
+
+      mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([asset.id]));
+
+      const result = await sut.resolve(authStub.admin, {
+        groups: [{ duplicateId: 'group-1', keepAssetIds: ['other-user-asset'], trashAssetIds: [asset.id] }],
+      });
+
+      expect(mocks.duplicateRepository.get).toHaveBeenCalledWith(authStub.admin.user.id, 'group-1');
+      expect(result[0].success).toBe(true);
+      expect(mocks.asset.updateAllExif).not.toHaveBeenCalled();
+      for (const [ids] of mocks.asset.updateAll.mock.calls) {
+        expect(ids).toEqual([asset.id]);
+      }
+    });
+
     it('should skip when keepAssetIds contains non-member', async () => {
       const asset = AssetFactory.create();
       mocks.access.duplicate.checkOwnerAccess.mockResolvedValue(new Set(['group-1']));

server/src/services/duplicate.service.ts

@@ -112,7 +112,7 @@ export class DuplicateService extends BaseService {
   private async resolveGroup(auth: AuthDto, group: DuplicateResolveGroupDto): Promise<BulkIdResponseDto> {
     const { duplicateId, keepAssetIds, trashAssetIds } = group;
 
-    const duplicateGroup = await this.duplicateRepository.get(duplicateId);
+    const duplicateGroup = await this.duplicateRepository.get(auth.user.id, duplicateId);
     if (!duplicateGroup) {
       return { id: duplicateId, success: false, error: BulkIdErrorReason.NOT_FOUND };
     }

server/test/medium/specs/services/duplicate.service.spec.ts

@@ -289,7 +289,7 @@ describe(DuplicateService.name, () => {
       }
     });
 
-    it('should fail without permission to delete an asset owned by someone else', async () => {
+    it('should ignore an asset owned by someone else', async () => {
       const { sut, ctx } = setup();
       const { user } = await ctx.newUser();
       const { user: other } = await ctx.newUser();
@@ -301,16 +301,12 @@ describe(DuplicateService.name, () => {
       const auth = factory.auth({ user: { id: user.id } });
       await expect(
         sut.resolve(auth, { groups: [{ duplicateId, keepAssetIds: [mine.id], trashAssetIds: [theirs.id] }] }),
-      ).resolves.toEqual([
+      ).resolves.toEqual([{ id: duplicateId, success: true }]);
-        {
-          id: duplicateId,
-          success: false,
-          error: BulkIdErrorReason.NO_PERMISSION,
-          errorMessage: 'No permission to delete assets',
-        },
-      ]);
 
-      await expect(ctx.get(AssetRepository).getById(theirs.id)).resolves.toMatchObject({ deletedAt: null });
+      await expect(ctx.get(AssetRepository).getById(theirs.id)).resolves.toMatchObject({
+        deletedAt: null,
+        duplicateId,
+      });
     });
 
     it('should resolve several groups in one request', async () => {
@@ -475,6 +471,28 @@ describe(DuplicateService.name, () => {
         expect(result?.exifInfo?.description?.split('\n').sort()).toEqual(['from keeper', 'from trashed']);
       });
 
+      it('should not merge metadata into an asset owned by someone else', async () => {
+        const { sut, ctx } = setup();
+        const { user } = await ctx.newUser();
+        const { user: other } = await ctx.newUser();
+        const duplicateId = factory.uuid();
+
+        const theirs = await newDuplicateAsset(ctx, { ownerId: other.id, duplicateId }, { description: 'theirs' });
+        const mine = await newDuplicateAsset(ctx, { ownerId: user.id, duplicateId }, { description: 'mine' });
+
+        const auth = factory.auth({ user: { id: user.id } });
+        expectSuccess(
+          await sut.resolve(auth, {
+            groups: [{ duplicateId, keepAssetIds: [theirs.id], trashAssetIds: [mine.id] }],
+          }),
+          duplicateId,
+        );
+
+        const result = await ctx.get(AssetRepository).getById(theirs.id, { exifInfo: true });
+        expect(result).toMatchObject({ duplicateId, deletedAt: null });
+        expect(result?.exifInfo?.description).toBe('theirs');
+      });
+
       it('should merge a location that both assets agree on', async () => {
         const { sut, ctx } = setup();
         const { user } = await ctx.newUser();