Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 55 additions & 12 deletions server/src/domain/access/access.core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,33 +75,64 @@ export class AccessCore {
}
}

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) {
/**
* Check if user has any of the given permissions, for any of the given ids.
* It returns true if the user has access to any of the ids, for any of the given permissions.
*
* @returns boolean
*/
async hasAny(authUser: AuthUserDto, permissions: Array<{ permission: Permission; ids: Set<string> }>) {
for (const { permission, ids } of permissions) {
const allowedIds = await this.getAllowedIds(authUser, permission, ids);
if (allowedIds.size > 0) {
return true;
}
}
return false;
}

/**
* Check if user has access to all ids, for the given permission.
* It only returns true if user has access to all specified ids.
*
* @returns boolean
*/
async hasPermission(authUser: AuthUserDto, permission: Permission, ids: string[] | string) {
ids = Array.isArray(ids) ? ids : [ids];
const idSet = Array.isArray(ids) ? new Set(ids) : new Set([ids]);
const allowedIds = await this.getAllowedIds(authUser, permission, idSet);
return idSet.size === allowedIds.size;
}

/**
* 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 getAllowedIds(authUser: AuthUserDto, permission: Permission, ids: Set<string>) {
if (ids.size === 0) {
return new Set();
}

const isSharedLink = authUser.isPublicUser ?? false;

return isSharedLink
? await this.getAllowedIdsSharedLinkAccess(authUser, permission, ids)
: await this.getAllowedIdsOtherAccess(authUser, permission, ids);
}

private async getAllowedIdsSharedLinkAccess(authUser: AuthUserDto, permission: Permission, ids: Set<string>) {
const allowedIds = new Set();
for (const id of ids) {
const hasAccess = isSharedLink
? await this.hasSharedLinkAccess(authUser, permission, id)
: await this.hasOtherAccess(authUser, permission, id);
if (!hasAccess) {
return false;
const hasAccess = await this.hasSharedLinkAccess(authUser, permission, id);
if (hasAccess) {
allowedIds.add(id);
}
}

return true;
return allowedIds;
}

// TODO: Migrate logic to getAllowedIdsSharedLinkAccess to evaluate permissions in bulk.
private async hasSharedLinkAccess(authUser: AuthUserDto, permission: Permission, id: string) {
const sharedLinkId = authUser.sharedLinkId;
if (!sharedLinkId) {
Expand Down Expand Up @@ -136,6 +167,18 @@ export class AccessCore {
}
}

private async getAllowedIdsOtherAccess(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 getAllowedIdsOtherAccess to evaluate permissions in bulk.
private async hasOtherAccess(authUser: AuthUserDto, permission: Permission, id: string) {
switch (permission) {
// uses album id
Expand Down
15 changes: 10 additions & 5 deletions server/src/domain/album/album.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ export class AlbumService {
await this.access.requirePermission(authUser, Permission.ALBUM_READ, id);

const existingAssetIds = await this.albumRepository.getAssetIds(id, dto.ids);
const notPresentAssetIds = new Set(dto.ids.filter((id) => !existingAssetIds.has(id)));
const allowedAssetIds = await this.access.getAllowedIds(authUser, Permission.ASSET_SHARE, notPresentAssetIds);

const results: BulkIdResponseDto[] = [];
for (const assetId of dto.ids) {
Expand All @@ -153,7 +155,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;
Expand Down Expand Up @@ -181,6 +183,12 @@ export class AlbumService {
await this.access.requirePermission(authUser, Permission.ALBUM_READ, id);

const existingAssetIds = await this.albumRepository.getAssetIds(id, dto.ids);
const assetIdsWithRemoveAccess = await this.access.getAllowedIds(
authUser,
Permission.ALBUM_REMOVE_ASSET,
existingAssetIds,
);
const assetIdsWithShareAccess = await this.access.getAllowedIds(authUser, Permission.ASSET_SHARE, existingAssetIds);

const results: BulkIdResponseDto[] = [];
for (const assetId of dto.ids) {
Expand All @@ -190,10 +198,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 = assetIdsWithRemoveAccess.has(assetId) || assetIdsWithShareAccess.has(assetId);
if (!hasAccess) {
results.push({ id: assetId, success: false, error: BulkIdErrorReason.NO_PERMISSION });
continue;
Expand Down
7 changes: 4 additions & 3 deletions server/src/domain/person/person.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.getAllowedIds(authUser, Permission.PERSON_MERGE, new Set(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;
}
Expand Down
8 changes: 6 additions & 2 deletions server/src/domain/shared-link/shared-link.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = new Set(dto.assetIds.filter((assetId) => !existingAssetIds.has(assetId)));
const allowedAssetIds = await this.access.getAllowedIds(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;
Expand Down
Loading