chore(server): Prepare access interfaces for bulk permission checks (#5223)

* chore(server): Prepare access interfaces for bulk permission checks

This change adds the `AccessCore.getAllowedIds` method, to evaluate
permissions in bulk, along with some other `getAllowedIds*` private
methods.

The added methods still calculate permissions by id, and are not
optimized to reduce the amount of queries and execution time, which will
be implemented in separate pull requests.

Services that were evaluating permissions in a loop have been refactored
to make use of the bulk approach.

* chore(server): Apply review suggestions

* chore(server): Make multiple-permission check more readable

server/src/domain/access/access.core.ts

@@ -68,40 +68,48 @@ export class AccessCore {
     return authUser;
   }
 
+  /**
+   * Check if user has access to all ids, for the given permission.
+   * Throws error if user does not have access to any of the ids.
+   */
   async requirePermission(authUser: AuthUserDto, permission: Permission, ids: string[] | string) {
-    const hasAccess = await this.hasPermission(authUser, permission, ids);
-    if (!hasAccess) {
+    ids = Array.isArray(ids) ? ids : [ids];
+    const allowedIds = await this.checkAccess(authUser, permission, ids);
+    if (new Set(ids).size !== allowedIds.size) {
       throw new BadRequestException(`Not found or no ${permission} access`);
     }
   }
 
-  async hasAny(authUser: AuthUserDto, permissions: Array<{ permission: Permission; id: string }>) {
-    for (const { permission, id } of permissions) {
-      const hasAccess = await this.hasPermission(authUser, permission, id);
-      if (hasAccess) {
-        return true;
-      }
+  /**
+   * Return ids that user has access to, for the given permission.
+   * Check is done for each id, and only allowed ids are returned.
+   *
+   * @returns Set<string>
+   */
+  async checkAccess(authUser: AuthUserDto, permission: Permission, ids: Set<string> | string[]) {
+    const idSet = Array.isArray(ids) ? new Set(ids) : ids;
+    if (idSet.size === 0) {
+      return new Set();
     }
-    return false;
-  }
-
-  async hasPermission(authUser: AuthUserDto, permission: Permission, ids: string[] | string) {
-    ids = Array.isArray(ids) ? ids : [ids];
 
     const isSharedLink = authUser.isPublicUser ?? false;
-
-    for (const id of ids) {
-      const hasAccess = isSharedLink
-        ? await this.hasSharedLinkAccess(authUser, permission, id)
-        : await this.hasOtherAccess(authUser, permission, id);
-      if (!hasAccess) {
-        return false;
-      }
-    }
-
-    return true;
+    return isSharedLink
+      ? await this.checkAccessSharedLink(authUser, permission, idSet)
+      : await this.checkAccessOther(authUser, permission, idSet);
   }
 
+  private async checkAccessSharedLink(authUser: AuthUserDto, permission: Permission, ids: Set<string>) {
+    const allowedIds = new Set();
+    for (const id of ids) {
+      const hasAccess = await this.hasSharedLinkAccess(authUser, permission, id);
+      if (hasAccess) {
+        allowedIds.add(id);
+      }
+    }
+    return allowedIds;
+  }
+
+  // TODO: Migrate logic to checkAccessSharedLink to evaluate permissions in bulk.
   private async hasSharedLinkAccess(authUser: AuthUserDto, permission: Permission, id: string) {
     const sharedLinkId = authUser.sharedLinkId;
     if (!sharedLinkId) {
@@ -136,6 +144,18 @@ export class AccessCore {
     }
   }
 
+  private async checkAccessOther(authUser: AuthUserDto, permission: Permission, ids: Set<string>) {
+    const allowedIds = new Set();
+    for (const id of ids) {
+      const hasAccess = await this.hasOtherAccess(authUser, permission, id);
+      if (hasAccess) {
+        allowedIds.add(id);
+      }
+    }
+    return allowedIds;
+  }
+
+  // TODO: Migrate logic to checkAccessOther to evaluate permissions in bulk.
   private async hasOtherAccess(authUser: AuthUserDto, permission: Permission, id: string) {
     switch (permission) {
       // uses album id

server/src/domain/album/album.service.ts

@@ -153,6 +153,8 @@ export class AlbumService {
     await this.access.requirePermission(authUser, Permission.ALBUM_READ, id);
 
     const existingAssetIds = await this.albumRepository.getAssetIds(id, dto.ids);
+    const notPresentAssetIds = dto.ids.filter((id) => !existingAssetIds.has(id));
+    const allowedAssetIds = await this.access.checkAccess(authUser, Permission.ASSET_SHARE, notPresentAssetIds);
 
     const results: BulkIdResponseDto[] = [];
     for (const assetId of dto.ids) {
@@ -162,7 +164,7 @@ export class AlbumService {
         continue;
       }
 
-      const hasAccess = await this.access.hasPermission(authUser, Permission.ASSET_SHARE, assetId);
+      const hasAccess = allowedAssetIds.has(assetId);
       if (!hasAccess) {
         results.push({ id: assetId, success: false, error: BulkIdErrorReason.NO_PERMISSION });
         continue;
@@ -190,6 +192,9 @@ export class AlbumService {
     await this.access.requirePermission(authUser, Permission.ALBUM_READ, id);
 
     const existingAssetIds = await this.albumRepository.getAssetIds(id, dto.ids);
+    const canRemove = await this.access.checkAccess(authUser, Permission.ALBUM_REMOVE_ASSET, existingAssetIds);
+    const canShare = await this.access.checkAccess(authUser, Permission.ASSET_SHARE, existingAssetIds);
+    const allowedAssetIds = new Set([...canRemove, ...canShare]);
 
     const results: BulkIdResponseDto[] = [];
     for (const assetId of dto.ids) {
@@ -199,10 +204,7 @@ export class AlbumService {
         continue;
       }
 
-      const hasAccess = await this.access.hasAny(authUser, [
-        { permission: Permission.ALBUM_REMOVE_ASSET, id: assetId },
-        { permission: Permission.ASSET_SHARE, id: assetId },
-      ]);
+      const hasAccess = allowedAssetIds.has(assetId);
       if (!hasAccess) {
         results.push({ id: assetId, success: false, error: BulkIdErrorReason.NO_PERMISSION });
         continue;

server/src/domain/person/person.service.ts

@@ -375,10 +375,11 @@ export class PersonService {
 
     const results: BulkIdResponseDto[] = [];
 
-    for (const mergeId of mergeIds) {
-      const hasPermission = await this.access.hasPermission(authUser, Permission.PERSON_MERGE, mergeId);
+    const allowedIds = await this.access.checkAccess(authUser, Permission.PERSON_MERGE, mergeIds);
 
-      if (!hasPermission) {
+    for (const mergeId of mergeIds) {
+      const hasAccess = allowedIds.has(mergeId);
+      if (!hasAccess) {
         results.push({ id: mergeId, success: false, error: BulkIdErrorReason.NO_PERMISSION });
         continue;
       }

server/src/domain/shared-link/shared-link.service.ts

@@ -119,15 +119,19 @@ export class SharedLinkService {
       throw new BadRequestException('Invalid shared link type');
     }
 
+    const existingAssetIds = new Set(sharedLink.assets.map((asset) => asset.id));
+    const notPresentAssetIds = dto.assetIds.filter((assetId) => !existingAssetIds.has(assetId));
+    const allowedAssetIds = await this.access.checkAccess(authUser, Permission.ASSET_SHARE, notPresentAssetIds);
+
     const results: AssetIdsResponseDto[] = [];
     for (const assetId of dto.assetIds) {
-      const hasAsset = sharedLink.assets.find((asset) => asset.id === assetId);
+      const hasAsset = existingAssetIds.has(assetId);
       if (hasAsset) {
         results.push({ assetId, success: false, error: AssetIdErrorReason.DUPLICATE });
         continue;
       }
 
-      const hasAccess = await this.access.hasPermission(authUser, Permission.ASSET_SHARE, assetId);
+      const hasAccess = allowedAssetIds.has(assetId);
       if (!hasAccess) {
         results.push({ assetId, success: false, error: AssetIdErrorReason.NO_PERMISSION });
         continue;