fix page permissions management

apps/server/src/core/page/dto/page-permission.dto.ts

@@ -43,12 +43,22 @@ export class AddPagePermissionDto extends PageIdDto {
 
 export class RemovePagePermissionDto extends PageIdDto {
   @IsOptional()
-  @IsUUID()
-  userId?: string;
+  @IsArray()
+  @ArrayMaxSize(25, {
+    message: 'userIds must be an array with no more than 25 elements',
+  })
+  @ArrayMinSize(1)
+  @IsUUID('all', { each: true })
+  userIds?: string[];
 
   @IsOptional()
-  @IsUUID()
-  groupId?: string;
+  @IsArray()
+  @ArrayMaxSize(25, {
+    message: 'groupIds must be an array with no more than 25 elements',
+  })
+  @ArrayMinSize(1)
+  @IsUUID('all', { each: true })
+  groupIds?: string[];
 }
 
 export class UpdatePagePermissionRoleDto extends PageIdDto {

apps/server/src/core/page/page-permission.controller.ts

@@ -25,9 +25,7 @@ import { User, Workspace } from '@docmost/db/types/entity.types';
 @UseGuards(JwtAuthGuard)
 @Controller('pages/permissions')
 export class PagePermissionController {
-  constructor(
-    private readonly pagePermissionService: PagePermissionService,
-  ) {}
+  constructor(private readonly pagePermissionService: PagePermissionService) {}
 
   @HttpCode(HttpStatus.OK)
   @Post('restrict')
@@ -36,41 +34,42 @@ export class PagePermissionController {
     @AuthUser() user: User,
     @AuthWorkspace() workspace: Workspace,
   ) {
-    await this.pagePermissionService.restrictPage(dto.pageId, user, workspace.id);
+    await this.pagePermissionService.restrictPage(
+      dto.pageId,
+      user,
+      workspace.id,
+    );
   }
 
   @HttpCode(HttpStatus.OK)
-  @Post('add')
+  @Post('add-members')
   async addPagePermission(
     @Body() dto: AddPagePermissionDto,
     @AuthUser() user: User,
     @AuthWorkspace() workspace: Workspace,
   ) {
-    if (
-      (!dto.userIds || dto.userIds.length === 0) &&
-      (!dto.groupIds || dto.groupIds.length === 0)
-    ) {
-      throw new BadRequestException('userIds or groupIds is required');
-    }
+    validateMemberIds(dto);
 
-    await this.pagePermissionService.addPagePermissions(dto, user, workspace.id);
+    await this.pagePermissionService.addPagePermissions(
+      dto,
+      user,
+      workspace.id,
+    );
   }
 
   @HttpCode(HttpStatus.OK)
-  @Post('remove')
-  async removePagePermission(
+  @Post('remove-members')
+  async removePagePermissions(
     @Body() dto: RemovePagePermissionDto,
     @AuthUser() user: User,
   ) {
-    if (!dto.userId && !dto.groupId) {
-      throw new BadRequestException('userId or groupId is required');
-    }
+    validateMemberIds(dto);
 
-    await this.pagePermissionService.removePagePermission(dto, user);
+    await this.pagePermissionService.removePagePermissions(dto, user);
   }
 
   @HttpCode(HttpStatus.OK)
-  @Post('update-role')
+  @Post('change-role')
   async updatePagePermissionRole(
     @Body() dto: UpdatePagePermissionRoleDto,
     @AuthUser() user: User,
@@ -105,3 +104,12 @@ export class PagePermissionController {
     );
   }
 }
+
+function validateMemberIds(dto: { userIds?: string[]; groupIds?: string[] }) {
+  if (
+    (!dto.userIds || dto.userIds.length === 0) &&
+    (!dto.groupIds || dto.groupIds.length === 0)
+  ) {
+    throw new BadRequestException('userIds or groupIds is required');
+  }
+}

apps/server/src/core/page/services/page-permission.service.ts

@@ -45,13 +45,7 @@ export class PagePermissionService {
       throw new NotFoundException('Page not found');
     }
 
-    const ability = await this.spaceAbility.createForUser(authUser, page.spaceId);
-    if (ability.cannot(SpaceCaslAction.Edit, SpaceCaslSubject.Page)) {
-      throw new ForbiddenException();
-    }
-
-    // TODO: does this check if any of the page's ancestor's is restricted and the user don't have access to it?
-    // to have access to this page, they must already have access to the page if any of it's ancestor's is restricted
+    await this.validateWriteAccess(page, authUser);
 
     const existingAccess =
       await this.pagePermissionRepo.findPageAccessByPageId(pageId);
@@ -171,7 +165,7 @@ export class PagePermissionService {
     }
   }
 
-  async removePagePermission(
+  async removePagePermissions(
     dto: RemovePagePermissionDto,
     authUser: User,
   ): Promise<void> {
@@ -189,44 +183,28 @@ export class PagePermissionService {
       throw new BadRequestException('Page is not restricted');
     }
 
-    if (!dto.userId && !dto.groupId) {
-      throw new BadRequestException('Please provide a userId or groupId');
+    const userIds = dto.userIds ?? [];
+    const groupIds = dto.groupIds ?? [];
+
+    if (userIds.length > 0) {
+      await this.pagePermissionRepo.deletePagePermissionsByUserIds(
+        pageAccess.id,
+        userIds,
+      );
     }
 
-    if (dto.userId) {
-      const permission = await this.pagePermissionRepo.findPagePermissionByUserId(
+    if (groupIds.length > 0) {
+      await this.pagePermissionRepo.deletePagePermissionsByGroupIds(
         pageAccess.id,
-        dto.userId,
+        groupIds,
       );
-      if (!permission) {
-        throw new NotFoundException('Permission not found');
-      }
+    }
 
-      if (permission.role === PagePermissionRole.WRITER) {
-        await this.validateLastWriter(pageAccess.id);
-      }
-
-      await this.pagePermissionRepo.deletePagePermissionByUserId(
-        pageAccess.id,
-        dto.userId,
-      );
-    } else if (dto.groupId) {
-      const permission =
-        await this.pagePermissionRepo.findPagePermissionByGroupId(
-          pageAccess.id,
-          dto.groupId,
-        );
-      if (!permission) {
-        throw new NotFoundException('Permission not found');
-      }
-
-      if (permission.role === PagePermissionRole.WRITER) {
-        await this.validateLastWriter(pageAccess.id);
-      }
-
-      await this.pagePermissionRepo.deletePagePermissionByGroupId(
-        pageAccess.id,
-        dto.groupId,
+    const writerCount =
+      await this.pagePermissionRepo.countWritersByPageAccessId(pageAccess.id);
+    if (writerCount < 1) {
+      throw new BadRequestException(
+        'There must be at least one user with "Can edit" permission',
       );
     }
   }
@@ -329,21 +307,29 @@ export class PagePermissionService {
       throw new NotFoundException('Page not found');
     }
 
-    const ability = await this.spaceAbility.createForUser(authUser, page.spaceId);
+    const ability = await this.spaceAbility.createForUser(
+      authUser,
+      page.spaceId,
+    );
+    // user must be a space member
     if (ability.cannot(SpaceCaslAction.Read, SpaceCaslSubject.Page)) {
       throw new ForbiddenException();
     }
 
+    // user must not have any restriction to view this page
+    const canView = await this.canViewPage(authUser.id, pageId);
+    if (!canView) {
+      throw new ForbiddenException();
+    }
+
     const pageAccess =
       await this.pagePermissionRepo.findPageAccessByPageId(pageId);
     if (!pageAccess) {
       return {
         items: [],
-        pagination: {
+        meta: {
+          limit: pagination.limit,
           page: 1,
-          perPage: pagination.limit,
-          totalItems: 0,
-          totalPages: 0,
           hasNextPage: false,
           hasPrevPage: false,
         },
@@ -367,36 +353,38 @@ export class PagePermissionService {
   }
 
   /**
-   * Check if user has writer permission on ALL restricted ancestors of a page.
-   * Used for permission management operations.
+   * Validate if user can manage page permissions (restrict, add/remove members, etc.)
+   *
+   * Requirements:
+   * 1. User must have space-level Edit permission (minimum baseline)
+   * 2. For restricted pages, user must have one of:
+   *    - Page-level Writer permission on all restricted ancestors
+   *    - Space Admin role + at least page-level Reader permission (admin elevates)
    */
-  async hasWritePermission(userId: string, pageId: string): Promise<boolean> {
-    const hasRestriction =
-      await this.pagePermissionRepo.hasRestrictedAncestor(pageId);
+  async validateWriteAccess(page: Page, user: User): Promise<void> {
+    const ability = await this.spaceAbility.createForUser(user, page.spaceId);
 
-    if (!hasRestriction) {
-      return false; // no restrictions, defer to space permissions
+    if (ability.cannot(SpaceCaslAction.Edit, SpaceCaslSubject.Page)) {
+      throw new ForbiddenException();
     }
 
-    return this.pagePermissionRepo.canUserEditPage(userId, pageId);
-  }
-
-  async hasPageAccess(pageId: string): Promise<boolean> {
-    const pageAccess =
-      await this.pagePermissionRepo.findPageAccessByPageId(pageId);
-    return !!pageAccess;
-  }
-
-  async validateWriteAccess(page: Page, user: User): Promise<void> {
-    const hasWritePermission = await this.hasWritePermission(user.id, page.id);
-    if (hasWritePermission) {
+    const canEdit = await this.canEditPage(user.id, page.id);
+    if (canEdit) {
       return;
     }
 
-    const ability = await this.spaceAbility.createForUser(user, page.spaceId);
-    if (ability.cannot(SpaceCaslAction.Manage, SpaceCaslSubject.Page)) {
-      throw new ForbiddenException();
+    const isSpaceAdmin = ability.can(
+      SpaceCaslAction.Manage,
+      SpaceCaslSubject.Page,
+    );
+    if (isSpaceAdmin) {
+      const canView = await this.canViewPage(user.id, page.id);
+      if (canView) {
+        return;
+      }
     }
+
+    throw new ForbiddenException();
   }
 
   /**
@@ -422,17 +410,23 @@ export class PagePermissionService {
   }
 
   /**
-   * Filter page IDs to only those the user can access.
+   * Check if user has writer permission on ALL restricted ancestors of a page.
+   * Used for permission management operations.
    */
-  async filterAccessiblePages(
-    pageIds: string[],
-    userId: string,
-  ): Promise<string[]> {
-    const results =
-      await this.pagePermissionRepo.filterAccessiblePageIdsWithPermissions(
-        pageIds,
-        userId,
-      );
-    return results.map((r) => r.id);
+  async hasWritePermission(userId: string, pageId: string): Promise<boolean> {
+    const hasRestriction =
+      await this.pagePermissionRepo.hasRestrictedAncestor(pageId);
+
+    if (!hasRestriction) {
+      return false; // no restrictions, defer to space permissions
+    }
+
+    return this.pagePermissionRepo.canUserEditPage(userId, pageId);
+  }
+
+  async hasPageAccess(pageId: string): Promise<boolean> {
+    const pageAccess =
+      await this.pagePermissionRepo.findPageAccessByPageId(pageId);
+    return !!pageAccess;
   }
 }

apps/server/src/database/repos/page/page-permission.repo.ts

@@ -117,6 +117,34 @@ export class PagePermissionRepo {
       .execute();
   }
 
+  async deletePagePermissionsByUserIds(
+    pageAccessId: string,
+    userIds: string[],
+    trx?: KyselyTransaction,
+  ): Promise<void> {
+    if (userIds.length === 0) return;
+    const db = dbOrTx(this.db, trx);
+    await db
+      .deleteFrom('pagePermissions')
+      .where('pageAccessId', '=', pageAccessId)
+      .where('userId', 'in', userIds)
+      .execute();
+  }
+
+  async deletePagePermissionsByGroupIds(
+    pageAccessId: string,
+    groupIds: string[],
+    trx?: KyselyTransaction,
+  ): Promise<void> {
+    if (groupIds.length === 0) return;
+    const db = dbOrTx(this.db, trx);
+    await db
+      .deleteFrom('pagePermissions')
+      .where('pageAccessId', '=', pageAccessId)
+      .where('groupId', 'in', groupIds)
+      .execute();
+  }
+
   async updatePagePermissionRole(
     pageAccessId: string,
     role: string,