From 3fd24e2083e6d26ea7f1b0d0f8633146f3738f66 Mon Sep 17 00:00:00 2001 From: Michel Heusschen <59014050+michelheusschen@users.noreply.github.com> Date: Thu, 12 Mar 2026 19:48:00 +0100 Subject: [PATCH] fix(server): restrict individual shared link asset removal to owners (#26868) * fix(server): restrict individual shared link asset removal to owners * make open-api --- .../specs/server/api/shared-link.e2e-spec.ts | 10 ++++++++ e2e/src/specs/web/shared-link.e2e-spec.ts | 24 +++++++++++++++++++ mobile/openapi/lib/api/shared_links_api.dart | 21 +++------------- open-api/immich-openapi-specs.json | 17 +------------ open-api/typescript-sdk/src/fetch-client.ts | 9 ++----- .../shared-link.controller.spec.ts | 15 +++++++++++- .../src/controllers/shared-link.controller.ts | 2 +- web/src/lib/services/shared-link.service.ts | 2 -- 8 files changed, 55 insertions(+), 45 deletions(-) diff --git a/e2e/src/specs/server/api/shared-link.e2e-spec.ts b/e2e/src/specs/server/api/shared-link.e2e-spec.ts index 80232beb75..00c455d6cb 100644 --- a/e2e/src/specs/server/api/shared-link.e2e-spec.ts +++ b/e2e/src/specs/server/api/shared-link.e2e-spec.ts @@ -438,6 +438,16 @@ describe('/shared-links', () => { expect(body).toEqual(errorDto.badRequest('Invalid shared link type')); }); + it('should reject guests removing assets from an individual shared link', async () => { + const { status, body } = await request(app) + .delete(`/shared-links/${linkWithAssets.id}/assets`) + .query({ key: linkWithAssets.key }) + .send({ assetIds: [asset1.id] }); + + expect(status).toBe(403); + expect(body).toEqual(errorDto.forbidden); + }); + it('should remove assets from a shared link (individual)', async () => { const { status, body } = await request(app) .delete(`/shared-links/${linkWithAssets.id}/assets`) diff --git a/e2e/src/specs/web/shared-link.e2e-spec.ts b/e2e/src/specs/web/shared-link.e2e-spec.ts index f6d1ec98d4..8380840935 100644 --- a/e2e/src/specs/web/shared-link.e2e-spec.ts +++ b/e2e/src/specs/web/shared-link.e2e-spec.ts @@ -12,15 +12,18 @@ import { asBearerAuth, utils } from 'src/utils'; test.describe('Shared Links', () => { let admin: LoginResponseDto; let asset: AssetMediaResponseDto; + let asset2: AssetMediaResponseDto; let album: AlbumResponseDto; let sharedLink: SharedLinkResponseDto; let sharedLinkPassword: SharedLinkResponseDto; + let individualSharedLink: SharedLinkResponseDto; test.beforeAll(async () => { utils.initSdk(); await utils.resetDatabase(); admin = await utils.adminSetup(); asset = await utils.createAsset(admin.accessToken); + asset2 = await utils.createAsset(admin.accessToken); album = await createAlbum( { createAlbumDto: { @@ -39,6 +42,10 @@ test.describe('Shared Links', () => { albumId: album.id, password: 'test-password', }); + individualSharedLink = await utils.createSharedLink(admin.accessToken, { + type: SharedLinkType.Individual, + assetIds: [asset.id, asset2.id], + }); }); test('download from a shared link', async ({ page }) => { @@ -109,4 +116,21 @@ test.describe('Shared Links', () => { await page.waitForURL('/photos'); await page.locator(`[data-asset-id="${asset.id}"]`).waitFor(); }); + + test('owner can remove assets from an individual shared link', async ({ context, page }) => { + await utils.setAuthCookies(context, admin.accessToken); + + await page.goto(`/share/${individualSharedLink.key}`); + await page.locator(`[data-asset="${asset.id}"]`).waitFor(); + await expect(page.locator(`[data-asset]`)).toHaveCount(2); + + await page.locator(`[data-asset="${asset.id}"]`).hover(); + await page.locator(`[data-asset="${asset.id}"] [role="checkbox"]`).click(); + + await page.getByRole('button', { name: 'Remove from shared link' }).click(); + await page.getByRole('button', { name: 'Remove', exact: true }).click(); + + await expect(page.locator(`[data-asset="${asset.id}"]`)).toHaveCount(0); + await expect(page.locator(`[data-asset="${asset2.id}"]`)).toHaveCount(1); + }); }); diff --git a/mobile/openapi/lib/api/shared_links_api.dart b/mobile/openapi/lib/api/shared_links_api.dart index 37eeffcf46..084662ace8 100644 --- a/mobile/openapi/lib/api/shared_links_api.dart +++ b/mobile/openapi/lib/api/shared_links_api.dart @@ -427,11 +427,7 @@ class SharedLinksApi { /// * [String] id (required): /// /// * [AssetIdsDto] assetIdsDto (required): - /// - /// * [String] key: - /// - /// * [String] slug: - Future removeSharedLinkAssetsWithHttpInfo(String id, AssetIdsDto assetIdsDto, { String? key, String? slug, }) async { + Future removeSharedLinkAssetsWithHttpInfo(String id, AssetIdsDto assetIdsDto,) async { // ignore: prefer_const_declarations final apiPath = r'/shared-links/{id}/assets' .replaceAll('{id}', id); @@ -443,13 +439,6 @@ class SharedLinksApi { final headerParams = {}; final formParams = {}; - if (key != null) { - queryParams.addAll(_queryParams('', 'key', key)); - } - if (slug != null) { - queryParams.addAll(_queryParams('', 'slug', slug)); - } - const contentTypes = ['application/json']; @@ -473,12 +462,8 @@ class SharedLinksApi { /// * [String] id (required): /// /// * [AssetIdsDto] assetIdsDto (required): - /// - /// * [String] key: - /// - /// * [String] slug: - Future?> removeSharedLinkAssets(String id, AssetIdsDto assetIdsDto, { String? key, String? slug, }) async { - final response = await removeSharedLinkAssetsWithHttpInfo(id, assetIdsDto, key: key, slug: slug, ); + Future?> removeSharedLinkAssets(String id, AssetIdsDto assetIdsDto,) async { + final response = await removeSharedLinkAssetsWithHttpInfo(id, assetIdsDto,); if (response.statusCode >= HttpStatus.badRequest) { throw ApiException(response.statusCode, await _decodeBodyBytes(response)); } diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index d2eb322009..2227273535 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -11605,22 +11605,6 @@ "format": "uuid", "type": "string" } - }, - { - "name": "key", - "required": false, - "in": "query", - "schema": { - "type": "string" - } - }, - { - "name": "slug", - "required": false, - "in": "query", - "schema": { - "type": "string" - } } ], "requestBody": { @@ -11677,6 +11661,7 @@ "state": "Stable" } ], + "x-immich-permission": "sharedLink.update", "x-immich-state": "Stable" }, "put": { diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index 5c8ac6dbc1..5a47cf2707 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -5987,19 +5987,14 @@ export function updateSharedLink({ id, sharedLinkEditDto }: { /** * Remove assets from a shared link */ -export function removeSharedLinkAssets({ id, key, slug, assetIdsDto }: { +export function removeSharedLinkAssets({ id, assetIdsDto }: { id: string; - key?: string; - slug?: string; assetIdsDto: AssetIdsDto; }, opts?: Oazapfts.RequestOpts) { return oazapfts.ok(oazapfts.fetchJson<{ status: 200; data: AssetIdsResponseDto[]; - }>(`/shared-links/${encodeURIComponent(id)}/assets${QS.query(QS.explode({ - key, - slug - }))}`, oazapfts.json({ + }>(`/shared-links/${encodeURIComponent(id)}/assets`, oazapfts.json({ ...opts, method: "DELETE", body: assetIdsDto diff --git a/server/src/controllers/shared-link.controller.spec.ts b/server/src/controllers/shared-link.controller.spec.ts index 96c84040ca..d8b89d0029 100644 --- a/server/src/controllers/shared-link.controller.spec.ts +++ b/server/src/controllers/shared-link.controller.spec.ts @@ -1,7 +1,8 @@ import { SharedLinkController } from 'src/controllers/shared-link.controller'; -import { SharedLinkType } from 'src/enum'; +import { Permission, SharedLinkType } from 'src/enum'; import { SharedLinkService } from 'src/services/shared-link.service'; import request from 'supertest'; +import { factory } from 'test/small.factory'; import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils'; describe(SharedLinkController.name, () => { @@ -31,4 +32,16 @@ describe(SharedLinkController.name, () => { expect(service.create).toHaveBeenCalledWith(undefined, expect.objectContaining({ expiresAt: null })); }); }); + + describe('DELETE /shared-links/:id/assets', () => { + it('should require shared link update permission', async () => { + await request(ctx.getHttpServer()).delete(`/shared-links/${factory.uuid()}/assets`).send({ assetIds: [] }); + + expect(ctx.authenticate).toHaveBeenCalledWith( + expect.objectContaining({ + metadata: expect.objectContaining({ permission: Permission.SharedLinkUpdate, sharedLinkRoute: false }), + }), + ); + }); + }); }); diff --git a/server/src/controllers/shared-link.controller.ts b/server/src/controllers/shared-link.controller.ts index 1f91409e80..c7ba589a9f 100644 --- a/server/src/controllers/shared-link.controller.ts +++ b/server/src/controllers/shared-link.controller.ts @@ -180,7 +180,7 @@ export class SharedLinkController { } @Delete(':id/assets') - @Authenticated({ sharedLink: true }) + @Authenticated({ permission: Permission.SharedLinkUpdate }) @Endpoint({ summary: 'Remove assets from a shared link', description: diff --git a/web/src/lib/services/shared-link.service.ts b/web/src/lib/services/shared-link.service.ts index fc4bbe11c0..135c67b95a 100644 --- a/web/src/lib/services/shared-link.service.ts +++ b/web/src/lib/services/shared-link.service.ts @@ -1,5 +1,4 @@ import { goto } from '$app/navigation'; -import { authManager } from '$lib/managers/auth-manager.svelte'; import { eventManager } from '$lib/managers/event-manager.svelte'; import { serverConfigManager } from '$lib/managers/server-config-manager.svelte'; import QrCodeModal from '$lib/modals/QrCodeModal.svelte'; @@ -138,7 +137,6 @@ export const handleRemoveSharedLinkAssets = async (sharedLink: SharedLinkRespons try { const results = await removeSharedLinkAssets({ - ...authManager.params, id: sharedLink.id, assetIdsDto: { assetIds }, });