fix(server): restrict individual shared link asset removal to owners (#26868)

* fix(server): restrict individual shared link asset removal to owners

* make open-api

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`)

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);
+  });
 });

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<Response> removeSharedLinkAssetsWithHttpInfo(String id, AssetIdsDto assetIdsDto, { String? key, String? slug, }) async {
+  Future<Response> 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 = <String, String>{};
     final formParams = <String, String>{};
 
-    if (key != null) {
-      queryParams.addAll(_queryParams('', 'key', key));
-    }
-    if (slug != null) {
-      queryParams.addAll(_queryParams('', 'slug', slug));
-    }
-
     const contentTypes = <String>['application/json'];
 
 
@@ -473,12 +462,8 @@ class SharedLinksApi {
   /// * [String] id (required):
   ///
   /// * [AssetIdsDto] assetIdsDto (required):
-  ///
-  /// * [String] key:
-  ///
-  /// * [String] slug:
-  Future<List<AssetIdsResponseDto>?> removeSharedLinkAssets(String id, AssetIdsDto assetIdsDto, { String? key, String? slug, }) async {
-    final response = await removeSharedLinkAssetsWithHttpInfo(id, assetIdsDto,  key: key, slug: slug, );
+  Future<List<AssetIdsResponseDto>?> removeSharedLinkAssets(String id, AssetIdsDto assetIdsDto,) async {
+    final response = await removeSharedLinkAssetsWithHttpInfo(id, assetIdsDto,);
     if (response.statusCode >= HttpStatus.badRequest) {
       throw ApiException(response.statusCode, await _decodeBodyBytes(response));
     }

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": {

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

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 }),
+        }),
+      );
+    });
+  });
 });

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:

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 },
     });