diff --git a/apps/server/src/core/comment/comment.controller.ts b/apps/server/src/core/comment/comment.controller.ts index e43051ec..f9b7c529 100644 --- a/apps/server/src/core/comment/comment.controller.ts +++ b/apps/server/src/core/comment/comment.controller.ts @@ -49,12 +49,6 @@ export class CommentController { throw new NotFoundException('Page not found'); } - const ability = await this.spaceAbility.createForUser(user, page.spaceId); - if (ability.cannot(SpaceCaslAction.Create, SpaceCaslSubject.Page)) { - throw new ForbiddenException(); - } - - // Check page-level edit permission (comments require edit access) await this.pageAccessService.validateCanEdit(page, user); return this.commentService.create( @@ -80,9 +74,6 @@ export class CommentController { throw new NotFoundException('Page not found'); } - // - - // Checks both space-level and page-level permissions await this.pageAccessService.validateCanView(page, user); return this.commentService.findByPageId(page.id, pagination); @@ -101,7 +92,6 @@ export class CommentController { throw new NotFoundException('Page not found'); } - // Checks both space-level and page-level permissions await this.pageAccessService.validateCanView(page, user); return comment; @@ -120,7 +110,6 @@ export class CommentController { throw new NotFoundException('Page not found'); } - // Checks both space-level and page-level edit permissions await this.pageAccessService.validateCanEdit(page, user); return this.commentService.update(comment, dto, user); @@ -142,11 +131,6 @@ export class CommentController { // Check page-level edit permission first await this.pageAccessService.validateCanEdit(page, user); - const ability = await this.spaceAbility.createForUser( - user, - comment.spaceId, - ); - // Check if user is the comment owner const isOwner = comment.creatorId === user.id; @@ -155,6 +139,11 @@ export class CommentController { return; } + const ability = await this.spaceAbility.createForUser( + user, + comment.spaceId, + ); + // Space admin can delete any comment if (ability.cannot(SpaceCaslAction.Manage, SpaceCaslSubject.Settings)) { throw new ForbiddenException( diff --git a/apps/server/src/core/page/page.controller.ts b/apps/server/src/core/page/page.controller.ts index 7c3df4ae..46388a2b 100644 --- a/apps/server/src/core/page/page.controller.ts +++ b/apps/server/src/core/page/page.controller.ts @@ -390,11 +390,6 @@ export class PageController { throw new NotFoundException('Moved page not found'); } - //TODO: CAN USERS MOVE PAGES IN PORTIONS WHERE THEY HAVE BEEN GRANTED ACCESS TO? - // WHAT HAPPENS IF A PAGE WHICH MODES THE PERMISSION IS MOVED TO A DIFFERENT ROOT? - // ALSO THE EDIT CHECK BELOW WILL NOT WORK FOR USERS GRANTED EDIT WHO INITIALLY HOLD SPACE LEVEL VIEW - // ALSO, SHOULD REALLY PUT IN MIND WHAT SUCH USERS CAN DO IN TERMS OF WHERE THEY MOVE THE PAGE TO - const ability = await this.spaceAbility.createForUser( user, movedPage.spaceId, diff --git a/apps/server/src/core/share/share.controller.ts b/apps/server/src/core/share/share.controller.ts index ef6e9b2a..cce95921 100644 --- a/apps/server/src/core/share/share.controller.ts +++ b/apps/server/src/core/share/share.controller.ts @@ -26,6 +26,7 @@ import { UpdateShareDto, } from './dto/share.dto'; import { PageRepo } from '@docmost/db/repos/page/page.repo'; +import { PageAccessService } from '../page-access/page-access.service'; import { JwtAuthGuard } from '../../common/guards/jwt-auth.guard'; import { Public } from '../../common/decorators/public.decorator'; import { ShareRepo } from '@docmost/db/repos/share/share.repo'; @@ -41,6 +42,7 @@ export class ShareController { private readonly spaceAbility: SpaceAbilityFactory, private readonly shareRepo: ShareRepo, private readonly pageRepo: PageRepo, + private readonly pageAccessService: PageAccessService, private readonly environmentService: EnvironmentService, ) {} @@ -96,6 +98,7 @@ export class ShareController { @AuthUser() user: User, @AuthWorkspace() workspace: Workspace, ) { + // TODO: look into permission const page = await this.pageRepo.findById(dto.pageId); if (!page) { throw new NotFoundException('Shared page not found'); @@ -122,10 +125,8 @@ export class ShareController { throw new NotFoundException('Page not found'); } - const ability = await this.spaceAbility.createForUser(user, page.spaceId); - if (ability.cannot(SpaceCaslAction.Create, SpaceCaslSubject.Share)) { - throw new ForbiddenException(); - } + // User must be able to edit the page to create a share + await this.pageAccessService.validateCanEdit(page, user); return this.shareService.createShare({ page, @@ -144,11 +145,14 @@ export class ShareController { throw new NotFoundException('Share not found'); } - const ability = await this.spaceAbility.createForUser(user, share.spaceId); - if (ability.cannot(SpaceCaslAction.Edit, SpaceCaslSubject.Share)) { - throw new ForbiddenException(); + const page = await this.pageRepo.findById(share.pageId); + if (!page) { + throw new NotFoundException('Page not found'); } + // User must be able to edit the page to update its share + await this.pageAccessService.validateCanEdit(page, user); + return this.shareService.updateShare(share.id, updateShareDto); } @@ -161,11 +165,14 @@ export class ShareController { throw new NotFoundException('Share not found'); } - const ability = await this.spaceAbility.createForUser(user, share.spaceId); - if (ability.cannot(SpaceCaslAction.Manage, SpaceCaslSubject.Share)) { - throw new ForbiddenException(); + const page = await this.pageRepo.findById(share.pageId); + if (!page) { + throw new NotFoundException('Page not found'); } + // User must be able to edit the page to delete its share + await this.pageAccessService.validateCanEdit(page, user); + await this.shareRepo.deleteShare(share.id); }