From fbecc16e2d767c9fcbde74aa8f91af8826905fd2 Mon Sep 17 00:00:00 2001 From: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> Date: Tue, 9 Jun 2026 19:40:15 +0530 Subject: [PATCH] fix: remove partner assets from existing memories --- ...1012182488-DeleteMismatchedMemoryAssets.ts | 16 ++++++++ server/src/services/album.service.ts | 2 +- server/src/services/memory.service.spec.ts | 39 +++++++++++++++++++ server/src/services/memory.service.ts | 8 +++- server/src/services/tag.service.spec.ts | 13 +++++++ server/src/services/tag.service.ts | 2 +- server/src/utils/asset.util.ts | 4 +- 7 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 server/src/schema/migrations/1781012182488-DeleteMismatchedMemoryAssets.ts diff --git a/server/src/schema/migrations/1781012182488-DeleteMismatchedMemoryAssets.ts b/server/src/schema/migrations/1781012182488-DeleteMismatchedMemoryAssets.ts new file mode 100644 index 0000000000..16efe3871a --- /dev/null +++ b/server/src/schema/migrations/1781012182488-DeleteMismatchedMemoryAssets.ts @@ -0,0 +1,16 @@ +import { Kysely, sql } from 'kysely'; + +export async function up(db: Kysely): Promise { + // 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 { + // Not implemented: the deleted rows were cross-owner entries +} diff --git a/server/src/services/album.service.ts b/server/src/services/album.service.ts index 5f4bc56d98..83ccd251e2 100644 --- a/server/src/services/album.service.ts +++ b/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) || {}; diff --git a/server/src/services/memory.service.spec.ts b/server/src/services/memory.service.spec.ts index 9976189e39..6b122b6c0e 100644 --- a/server/src/services/memory.service.spec.ts +++ b/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(); diff --git a/server/src/services/memory.service.ts b/server/src/services/memory.service.ts index ac8f88ad87..f13a36175b 100644 --- a/server/src/services/memory.service.ts +++ b/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) { diff --git a/server/src/services/tag.service.spec.ts b/server/src/services/tag.service.spec.ts index 0c748fded8..34e4077855 100644 --- a/server/src/services/tag.service.spec.ts +++ b/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', () => { diff --git a/server/src/services/tag.service.ts b/server/src/services/tag.service.ts index 8b92d3abf8..d34b9d3719 100644 --- a/server/src/services/tag.service.ts +++ b/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) { diff --git a/server/src/utils/asset.util.ts b/server/src/utils/asset.util.ts index 5420e60361..cca61dab11 100644 --- a/server/src/utils/asset.util.ts +++ b/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, });