fix: remove partner assets from existing memories

server/src/schema/migrations/1781012182488-DeleteMismatchedMemoryAssets.ts

@@ -0,0 +1,16 @@
+import { Kysely, sql } from 'kysely';
+
+export async function up(db: Kysely<any>): Promise<void> {
+    // Delete cross-owner memory assets
+  await sql`
+    DELETE FROM memory_asset
+    USING memory, asset
+    WHERE memory_asset."memoriesId" = memory.id
+      AND memory_asset."assetId" = asset.id
+      AND memory."ownerId" != asset."ownerId"
+  `.execute(db);
+}
+
+export async function down(): Promise<void> {
+  // Not implemented: the deleted rows were cross-owner entries
+}

server/src/services/album.service.ts

@@ -175,7 +175,7 @@ export class AlbumService extends BaseService {
     const results = await addAssets(
       auth,
       { access: this.accessRepository, bulk: this.albumRepository },
-      { parentId: id, assetIds: dto.ids },
+      { parentId: id, assetIds: dto.ids, permission: Permission.AssetShare },
     );
 
     const { id: firstNewAssetId } = results.find(({ success }) => success) || {};

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

@@ -134,6 +134,27 @@ describe(MemoryService.name, () => {
       );
     });
 
+    it('should not link a partner asset', async () => {
+      const [assetId, userId] = newUuids();
+      const memory = MemoryFactory.create({ ownerId: userId });
+
+      mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set());
+      mocks.access.asset.checkPartnerAccess.mockResolvedValue(new Set([assetId]));
+      mocks.memory.create.mockResolvedValue(getForMemory(memory));
+
+      await expect(
+        sut.create(factory.auth({ user: { id: userId } }), {
+          type: memory.type,
+          data: memory.data as OnThisDayData,
+          memoryAt: memory.memoryAt,
+          assetIds: [assetId],
+        }),
+      ).resolves.toMatchObject({ assets: [] });
+
+      expect(mocks.memory.create).toHaveBeenCalledWith(expect.objectContaining({ ownerId: userId }), new Set());
+      expect(mocks.access.asset.checkPartnerAccess).not.toHaveBeenCalled();
+    });
+
     it('should create a memory without assets', async () => {
       const memory = MemoryFactory.create();
 
@@ -230,6 +251,24 @@ describe(MemoryService.name, () => {
       expect(mocks.memory.addAssetIds).not.toHaveBeenCalled();
     });
 
+    it('should not link a partner asset', async () => {
+      const assetId = newUuid();
+      const memory = MemoryFactory.create();
+
+      mocks.access.memory.checkOwnerAccess.mockResolvedValue(new Set([memory.id]));
+      mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set());
+      mocks.access.asset.checkPartnerAccess.mockResolvedValue(new Set([assetId]));
+      mocks.memory.get.mockResolvedValue(getForMemory(memory));
+      mocks.memory.getAssetIds.mockResolvedValue(new Set());
+
+      await expect(sut.addAssets(factory.auth(), memory.id, { ids: [assetId] })).resolves.toEqual([
+        { error: 'no_permission', id: assetId, success: false },
+      ]);
+
+      expect(mocks.memory.addAssetIds).not.toHaveBeenCalled();
+      expect(mocks.access.asset.checkPartnerAccess).not.toHaveBeenCalled();
+    });
+
     it('should add assets', async () => {
       const assetId = newUuid();
       const memory = MemoryFactory.create();

server/src/services/memory.service.ts

@@ -93,7 +93,7 @@ export class MemoryService extends BaseService {
     const assetIds = dto.assetIds || [];
     const allowedAssetIds = await this.checkAccess({
       auth,
-      permission: Permission.AssetShare,
+      permission: Permission.AssetUpdate,
       ids: assetIds,
     });
     const memory = await this.memoryRepository.create(
@@ -134,7 +134,11 @@ export class MemoryService extends BaseService {
     await this.requireAccess({ auth, permission: Permission.MemoryRead, ids: [id] });
 
     const repos = { access: this.accessRepository, bulk: this.memoryRepository };
-    const results = await addAssets(auth, repos, { parentId: id, assetIds: dto.ids });
+    const results = await addAssets(auth, repos, {
+      parentId: id,
+      assetIds: dto.ids,
+      permission: Permission.AssetUpdate,
+    });
 
     const hasSuccess = results.find(({ success }) => success);
     if (hasSuccess) {

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

@@ -275,6 +275,19 @@ describe(TagService.name, () => {
       expect(mocks.tag.getAssetIds).toHaveBeenCalledWith('tag-1', ['asset-1', 'asset-2']);
       expect(mocks.tag.addAssetIds).toHaveBeenCalledWith('tag-1', ['asset-2']);
     });
+
+    it('should not tag a partner asset', async () => {
+      mocks.tag.getAssetIds.mockResolvedValue(new Set());
+      mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set());
+      mocks.access.asset.checkPartnerAccess.mockResolvedValue(new Set(['asset-1']));
+
+      await expect(sut.addAssets(authStub.admin, 'tag-1', { ids: ['asset-1'] })).resolves.toEqual([
+        { id: 'asset-1', success: false, error: BulkIdErrorReason.NO_PERMISSION },
+      ]);
+
+      expect(mocks.tag.addAssetIds).not.toHaveBeenCalled();
+      expect(mocks.access.asset.checkPartnerAccess).not.toHaveBeenCalled();
+    });
   });
 
   describe('removeAssets', () => {

server/src/services/tag.service.ts

@@ -104,7 +104,7 @@ export class TagService extends BaseService {
     const results = await addAssets(
       auth,
       { access: this.accessRepository, bulk: this.tagRepository },
-      { parentId: id, assetIds: dto.ids },
+      { parentId: id, assetIds: dto.ids, permission: Permission.AssetUpdate },
     );
 
     for (const { id: assetId, success } of results) {

server/src/utils/asset.util.ts

@@ -33,14 +33,14 @@ export const getAssetFiles = (files: AssetFile[]) => ({
 export const addAssets = async (
   auth: AuthDto,
   repositories: { access: AccessRepository; bulk: IBulkAsset },
-  dto: { parentId: string; assetIds: string[] },
+  dto: { parentId: string; assetIds: string[]; permission: Permission },
 ) => {
   const { access, bulk } = repositories;
   const existingAssetIds = await bulk.getAssetIds(dto.parentId, dto.assetIds);
   const notPresentAssetIds = dto.assetIds.filter((id) => !existingAssetIds.has(id));
   const allowedAssetIds = await checkAccess(access, {
     auth,
-    permission: Permission.AssetShare,
+    permission: dto.permission,
     ids: notPresentAssetIds,
   });