WIP 5

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(

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,

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);
   }