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.

Sign up for GitHub

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

Jump to bottom

chore(server): Check album permissions in bulk #5290

Merged
merged 3 commits into from
Nov 25, 2023
Merged

chore(server): Check album permissions in bulk #5290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5de01d2
chore(server): Check album permissions in bulk
adamantike Nov 24, 2023
d210315
chore(server): Add set utils, avoid double queries for same ids
adamantike Nov 25, 2023
a6c5c5c
chore(server): Review feedback
adamantike Nov 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 45 additions & 33 deletions server/src/domain/access/access.core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { BadRequestException, UnauthorizedException } from '@nestjs/common';
import { AuthUserDto } from '../auth';
import { setDifference, setUnion } from '../domain.util';
import { IAccessRepository } from '../repositories';

export enum Permission {
Expand Down Expand Up @@ -99,6 +100,24 @@ export class AccessCore {
}

private async checkAccessSharedLink(authUser: AuthUserDto, permission: Permission, ids: Set<string>) {
const sharedLinkId = authUser.sharedLinkId;
if (!sharedLinkId) {
return new Set();
}

switch (permission) {
case Permission.ASSET_UPLOAD:
return authUser.isAllowUpload ? ids : new Set();

case Permission.ALBUM_READ:
return await this.repository.album.checkSharedLinkAccess(sharedLinkId, ids);

case Permission.ALBUM_DOWNLOAD:
return !!authUser.isAllowDownload
? await this.repository.album.checkSharedLinkAccess(sharedLinkId, ids)
: new Set();
}

const allowedIds = new Set();
for (const id of ids) {
const hasAccess = await this.hasSharedLinkAccess(authUser, permission, id);
Expand Down Expand Up @@ -126,25 +145,42 @@ export class AccessCore {
case Permission.ASSET_DOWNLOAD:
return !!authUser.isAllowDownload && (await this.repository.asset.hasSharedLinkAccess(sharedLinkId, id));

case Permission.ASSET_UPLOAD:
return authUser.isAllowUpload;

case Permission.ASSET_SHARE:
// TODO: fix this to not use authUser.id for shared link access control
return this.repository.asset.hasOwnerAccess(authUser.id, id);

case Permission.ALBUM_READ:
return this.repository.album.hasSharedLinkAccess(sharedLinkId, id);

case Permission.ALBUM_DOWNLOAD:
return !!authUser.isAllowDownload && (await this.repository.album.hasSharedLinkAccess(sharedLinkId, id));

default:
return false;
}
}

private async checkAccessOther(authUser: AuthUserDto, permission: Permission, ids: Set<string>) {
switch (permission) {
case Permission.ALBUM_READ: {
const isOwner = await this.repository.album.checkOwnerAccess(authUser.id, ids);
const isShared = await this.repository.album.checkSharedAlbumAccess(authUser.id, setDifference(ids, isOwner));
jrasm91 marked this conversation as resolved.
Show resolved Hide resolved
return setUnion(isOwner, isShared);
}

case Permission.ALBUM_UPDATE:
return this.repository.album.checkOwnerAccess(authUser.id, ids);

case Permission.ALBUM_DELETE:
return this.repository.album.checkOwnerAccess(authUser.id, ids);

case Permission.ALBUM_SHARE:
return this.repository.album.checkOwnerAccess(authUser.id, ids);

case Permission.ALBUM_DOWNLOAD: {
const isOwner = await this.repository.album.checkOwnerAccess(authUser.id, ids);
const isShared = await this.repository.album.checkSharedAlbumAccess(authUser.id, setDifference(ids, isOwner));
return setUnion(isOwner, isShared);
}

case Permission.ALBUM_REMOVE_ASSET:
return this.repository.album.checkOwnerAccess(authUser.id, ids);
}

const allowedIds = new Set();
for (const id of ids) {
const hasAccess = await this.hasOtherAccess(authUser, permission, id);
Expand Down Expand Up @@ -204,33 +240,9 @@ export class AccessCore {
(await this.repository.asset.hasPartnerAccess(authUser.id, id))
);

case Permission.ALBUM_READ:
return (
(await this.repository.album.hasOwnerAccess(authUser.id, id)) ||
(await this.repository.album.hasSharedAlbumAccess(authUser.id, id))
);

case Permission.ALBUM_UPDATE:
return this.repository.album.hasOwnerAccess(authUser.id, id);

case Permission.ALBUM_DELETE:
return this.repository.album.hasOwnerAccess(authUser.id, id);

case Permission.ALBUM_SHARE:
return this.repository.album.hasOwnerAccess(authUser.id, id);

case Permission.ALBUM_DOWNLOAD:
return (
(await this.repository.album.hasOwnerAccess(authUser.id, id)) ||
(await this.repository.album.hasSharedAlbumAccess(authUser.id, id))
);

case Permission.ASSET_UPLOAD:
return this.repository.library.hasOwnerAccess(authUser.id, id);

case Permission.ALBUM_REMOVE_ASSET:
return this.repository.album.hasOwnerAccess(authUser.id, id);

case Permission.ARCHIVE_READ:
return authUser.id === id;

Expand Down
13 changes: 6 additions & 7 deletions server/src/domain/activity/activity.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe(ActivityService.name, () => {

describe('getAll', () => {
it('should get all', async () => {
accessMock.album.hasOwnerAccess.mockResolvedValue(true);
accessMock.album.checkOwnerAccess.mockResolvedValue(new Set(['album-id']));
activityMock.search.mockResolvedValue([]);

await expect(sut.getAll(authStub.admin, { assetId: 'asset-id', albumId: 'album-id' })).resolves.toEqual([]);
Expand All @@ -37,7 +37,7 @@ describe(ActivityService.name, () => {
});

it('should filter by type=like', async () => {
accessMock.album.hasOwnerAccess.mockResolvedValue(true);
accessMock.album.checkOwnerAccess.mockResolvedValue(new Set(['album-id']));
activityMock.search.mockResolvedValue([]);

await expect(
Expand All @@ -52,7 +52,7 @@ describe(ActivityService.name, () => {
});

it('should filter by type=comment', async () => {
accessMock.album.hasOwnerAccess.mockResolvedValue(true);
accessMock.album.checkOwnerAccess.mockResolvedValue(new Set(['album-id']));
activityMock.search.mockResolvedValue([]);

await expect(
Expand All @@ -70,7 +70,7 @@ describe(ActivityService.name, () => {
describe('getStatistics', () => {
it('should get the comment count', async () => {
activityMock.getStatistics.mockResolvedValue(1);
accessMock.album.hasOwnerAccess.mockResolvedValue(true);
accessMock.album.checkOwnerAccess.mockResolvedValue(new Set([activityStub.oneComment.albumId]));
await expect(
sut.getStatistics(authStub.admin, {
assetId: 'asset-id',
Expand All @@ -82,7 +82,6 @@ describe(ActivityService.name, () => {

describe('addComment', () => {
it('should require access to the album', async () => {
accessMock.album.hasOwnerAccess.mockResolvedValue(false);
await expect(
sut.create(authStub.admin, {
albumId: 'album-id',
Expand Down Expand Up @@ -114,7 +113,7 @@ describe(ActivityService.name, () => {
});

it('should fail because activity is disabled for the album', async () => {
accessMock.album.hasOwnerAccess.mockResolvedValue(true);
accessMock.album.checkOwnerAccess.mockResolvedValue(new Set(['album-id']));
accessMock.activity.hasCreateAccess.mockResolvedValue(false);
activityMock.create.mockResolvedValue(activityStub.oneComment);

Expand Down Expand Up @@ -148,7 +147,7 @@ describe(ActivityService.name, () => {
});

it('should skip if like exists', async () => {
accessMock.album.hasOwnerAccess.mockResolvedValue(true);
accessMock.album.checkOwnerAccess.mockResolvedValue(new Set(['album-id']));
accessMock.activity.hasCreateAccess.mockResolvedValue(true);
activityMock.search.mockResolvedValue([activityStub.liked]);

Expand Down