diff --git a/server/src/domain/access/access.core.ts b/server/src/domain/access/access.core.ts index 16527de989..30086f5d26 100644 --- a/server/src/domain/access/access.core.ts +++ b/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 + */ + async checkAccess(authUser: AuthUserDto, permission: Permission, ids: Set | 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) { + 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) { + 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 diff --git a/server/src/domain/album/album.service.ts b/server/src/domain/album/album.service.ts index 0fb0391ef4..986cdb8911 100644 --- a/server/src/domain/album/album.service.ts +++ b/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; diff --git a/server/src/domain/person/person.service.ts b/server/src/domain/person/person.service.ts index 49329a452a..3452807f66 100644 --- a/server/src/domain/person/person.service.ts +++ b/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; } diff --git a/server/src/domain/shared-link/shared-link.service.ts b/server/src/domain/shared-link/shared-link.service.ts index d3fd89661b..bd9aaea6a3 100644 --- a/server/src/domain/shared-link/shared-link.service.ts +++ b/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;