From c2e722ee5c270fe0e876892dc6041f63d25c26cf Mon Sep 17 00:00:00 2001 From: Philipinho <16838612+Philipinho@users.noreply.github.com> Date: Wed, 24 Dec 2025 00:27:25 +0000 Subject: [PATCH] WIP 3 --- .../extensions/authentication.extension.ts | 29 +++++++ .../core/attachment/attachment.controller.ts | 24 +++--- .../src/core/attachment/attachment.module.ts | 3 +- .../src/core/comment/comment.controller.ts | 73 ++++++++---------- .../server/src/core/comment/comment.module.ts | 3 +- apps/server/src/core/page/page.controller.ts | 76 +++++++++++++------ .../src/core/page/services/page.service.ts | 23 +++++- apps/server/src/core/search/search.service.ts | 28 ++++++- .../src/database/repos/page/page.repo.ts | 56 ++++++++++++++ 9 files changed, 229 insertions(+), 86 deletions(-) diff --git a/apps/server/src/collaboration/extensions/authentication.extension.ts b/apps/server/src/collaboration/extensions/authentication.extension.ts index 1a42bd97..1309c5f8 100644 --- a/apps/server/src/collaboration/extensions/authentication.extension.ts +++ b/apps/server/src/collaboration/extensions/authentication.extension.ts @@ -9,6 +9,7 @@ import { TokenService } from '../../core/auth/services/token.service'; import { UserRepo } from '@docmost/db/repos/user/user.repo'; import { PageRepo } from '@docmost/db/repos/page/page.repo'; import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo'; +import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo'; import { findHighestUserSpaceRole } from '@docmost/db/repos/space/utils'; import { SpaceRole } from '../../common/helpers/types/permission'; import { getPageId } from '../collaboration.util'; @@ -23,6 +24,7 @@ export class AuthenticationExtension implements Extension { private userRepo: UserRepo, private pageRepo: PageRepo, private readonly spaceMemberRepo: SpaceMemberRepo, + private readonly pagePermissionRepo: PagePermissionRepo, ) {} async onAuthenticate(data: onAuthenticatePayload) { @@ -73,6 +75,33 @@ export class AuthenticationExtension implements Extension { this.logger.debug(`User granted readonly access to page: ${pageId}`); } + // Check page-level permissions (in addition to space permissions) + const canAccessPage = await this.pagePermissionRepo.canUserAccessPage( + user.id, + page.id, + ); + + if (!canAccessPage) { + this.logger.warn( + `User ${user.id} denied page-level access to page: ${pageId}`, + ); + throw new UnauthorizedException(); + } + + // Check if user can edit (has writer role on all restricted ancestors) + const canEditPage = await this.pagePermissionRepo.canUserEditPage( + user.id, + page.id, + ); + + // If user has space edit permission but lacks page-level write permission, force readonly + if (!canEditPage && !data.connection.readOnly) { + data.connection.readOnly = true; + this.logger.debug( + `User ${user.id} granted readonly access to restricted page: ${pageId}`, + ); + } + this.logger.debug(`Authenticated user ${user.id} on page ${pageId}`); return { diff --git a/apps/server/src/core/attachment/attachment.controller.ts b/apps/server/src/core/attachment/attachment.controller.ts index fdf17523..b3ba841c 100644 --- a/apps/server/src/core/attachment/attachment.controller.ts +++ b/apps/server/src/core/attachment/attachment.controller.ts @@ -53,6 +53,7 @@ import { TokenService } from '../auth/services/token.service'; import { JwtAttachmentPayload, JwtType } from '../auth/dto/jwt-payload'; import * as path from 'path'; import { RemoveIconDto } from './dto/attachment.dto'; +import { PagePermissionService } from '../page/services/page-permission.service'; @Controller() export class AttachmentController { @@ -67,6 +68,7 @@ export class AttachmentController { private readonly attachmentRepo: AttachmentRepo, private readonly environmentService: EnvironmentService, private readonly tokenService: TokenService, + private readonly pagePermissionService: PagePermissionService, ) {} @UseGuards(JwtAuthGuard) @@ -111,13 +113,8 @@ export class AttachmentController { throw new NotFoundException('Page not found'); } - const spaceAbility = await this.spaceAbility.createForUser( - user, - page.spaceId, - ); - if (spaceAbility.cannot(SpaceCaslAction.Manage, SpaceCaslSubject.Page)) { - throw new ForbiddenException(); - } + // Checks both space-level and page-level edit permissions + await this.pagePermissionService.validateCanEdit(page, user); const spaceId = page.spaceId; @@ -171,15 +168,14 @@ export class AttachmentController { throw new NotFoundException(); } - const spaceAbility = await this.spaceAbility.createForUser( - user, - attachment.spaceId, - ); - - if (spaceAbility.cannot(SpaceCaslAction.Read, SpaceCaslSubject.Page)) { - throw new ForbiddenException(); + const page = await this.pageRepo.findById(attachment.pageId); + if (!page) { + throw new NotFoundException(); } + // Checks both space-level and page-level view permissions + await this.pagePermissionService.validateCanView(page, user); + try { const fileStream = await this.storageService.read(attachment.filePath); res.headers({ diff --git a/apps/server/src/core/attachment/attachment.module.ts b/apps/server/src/core/attachment/attachment.module.ts index f80a2eb7..b068b7dc 100644 --- a/apps/server/src/core/attachment/attachment.module.ts +++ b/apps/server/src/core/attachment/attachment.module.ts @@ -6,9 +6,10 @@ import { UserModule } from '../user/user.module'; import { WorkspaceModule } from '../workspace/workspace.module'; import { AttachmentProcessor } from './processors/attachment.processor'; import { TokenModule } from '../auth/token.module'; +import { PageModule } from '../page/page.module'; @Module({ - imports: [StorageModule, UserModule, WorkspaceModule, TokenModule], + imports: [StorageModule, UserModule, WorkspaceModule, TokenModule, PageModule], controllers: [AttachmentController], providers: [AttachmentService, AttachmentProcessor], }) diff --git a/apps/server/src/core/comment/comment.controller.ts b/apps/server/src/core/comment/comment.controller.ts index 5ced1656..99d6aa46 100644 --- a/apps/server/src/core/comment/comment.controller.ts +++ b/apps/server/src/core/comment/comment.controller.ts @@ -24,6 +24,7 @@ import { SpaceCaslSubject, } from '../casl/interfaces/space-ability.type'; import { CommentRepo } from '@docmost/db/repos/comment/comment.repo'; +import { PagePermissionService } from '../page/services/page-permission.service'; @UseGuards(JwtAuthGuard) @Controller('comments') @@ -33,6 +34,7 @@ export class CommentController { private readonly commentRepo: CommentRepo, private readonly pageRepo: PageRepo, private readonly spaceAbility: SpaceAbilityFactory, + private readonly pagePermissionService: PagePermissionService, ) {} @HttpCode(HttpStatus.OK) @@ -52,6 +54,9 @@ export class CommentController { throw new ForbiddenException(); } + // Check page-level edit permission (comments require edit access) + await this.pagePermissionService.validateCanEdit(page, user); + return this.commentService.create( { userId: user.id, @@ -75,10 +80,11 @@ export class CommentController { throw new NotFoundException('Page not found'); } - const ability = await this.spaceAbility.createForUser(user, page.spaceId); - if (ability.cannot(SpaceCaslAction.Read, SpaceCaslSubject.Page)) { - throw new ForbiddenException(); - } + // + + // Checks both space-level and page-level permissions + await this.pagePermissionService.validateCanView(page, user); + return this.commentService.findByPageId(page.id, pagination); } @@ -90,13 +96,14 @@ export class CommentController { throw new NotFoundException('Comment not found'); } - const ability = await this.spaceAbility.createForUser( - user, - comment.spaceId, - ); - if (ability.cannot(SpaceCaslAction.Read, SpaceCaslSubject.Page)) { - throw new ForbiddenException(); + const page = await this.pageRepo.findById(comment.pageId); + if (!page) { + throw new NotFoundException('Page not found'); } + + // Checks both space-level and page-level permissions + await this.pagePermissionService.validateCanView(page, user); + return comment; } @@ -108,18 +115,14 @@ export class CommentController { throw new NotFoundException('Comment not found'); } - const ability = await this.spaceAbility.createForUser( - user, - comment.spaceId, - ); - - // must be a space member with edit permission - if (ability.cannot(SpaceCaslAction.Edit, SpaceCaslSubject.Page)) { - throw new ForbiddenException( - 'You must have space edit permission to edit comments', - ); + const page = await this.pageRepo.findById(comment.pageId); + if (!page) { + throw new NotFoundException('Page not found'); } + // Checks both space-level and page-level edit permissions + await this.pagePermissionService.validateCanEdit(page, user); + return this.commentService.update(comment, dto, user); } @@ -131,37 +134,23 @@ export class CommentController { throw new NotFoundException('Comment not found'); } + const page = await this.pageRepo.findById(comment.pageId); + if (!page) { + throw new NotFoundException('Page not found'); + } + + // Check page-level edit permission first + await this.pagePermissionService.validateCanEdit(page, user); + const ability = await this.spaceAbility.createForUser( user, comment.spaceId, ); - // must be a space member with edit permission - if (ability.cannot(SpaceCaslAction.Edit, SpaceCaslSubject.Page)) { - throw new ForbiddenException(); - } - // Check if user is the comment owner const isOwner = comment.creatorId === user.id; if (isOwner) { - /* - // Check if comment has children from other users - const hasChildrenFromOthers = - await this.commentRepo.hasChildrenFromOtherUsers(comment.id, user.id); - - // Owner can delete if no children from other users - if (!hasChildrenFromOthers) { - await this.commentRepo.deleteComment(comment.id); - return; - } - - // If has children from others, only space admin can delete - if (ability.cannot(SpaceCaslAction.Manage, SpaceCaslSubject.Settings)) { - throw new ForbiddenException( - 'Only space admins can delete comments with replies from other users', - ); - }*/ await this.commentRepo.deleteComment(comment.id); return; } diff --git a/apps/server/src/core/comment/comment.module.ts b/apps/server/src/core/comment/comment.module.ts index 60a577e8..a32b2fde 100644 --- a/apps/server/src/core/comment/comment.module.ts +++ b/apps/server/src/core/comment/comment.module.ts @@ -1,9 +1,10 @@ import { Module } from '@nestjs/common'; import { CommentService } from './comment.service'; import { CommentController } from './comment.controller'; +import { PageModule } from '../page/page.module'; @Module({ - imports: [], + imports: [PageModule], controllers: [CommentController], providers: [CommentService], exports: [CommentService], diff --git a/apps/server/src/core/page/page.controller.ts b/apps/server/src/core/page/page.controller.ts index 97275440..5bb5d871 100644 --- a/apps/server/src/core/page/page.controller.ts +++ b/apps/server/src/core/page/page.controller.ts @@ -10,6 +10,7 @@ import { UseGuards, } from '@nestjs/common'; import { PageService } from './services/page.service'; +import { PagePermissionService } from './services/page-permission.service'; import { CreatePageDto } from './dto/create-page.dto'; import { UpdatePageDto } from './dto/update-page.dto'; import { MovePageDto, MovePageToSpaceDto } from './dto/move-page.dto'; @@ -44,6 +45,7 @@ export class PageController { private readonly pageRepo: PageRepo, private readonly pageHistoryService: PageHistoryService, private readonly spaceAbility: SpaceAbilityFactory, + private readonly pagePermissionService: PagePermissionService, ) {} @HttpCode(HttpStatus.OK) @@ -61,10 +63,8 @@ export class PageController { throw new NotFoundException('Page not found'); } - const ability = await this.spaceAbility.createForUser(user, page.spaceId); - if (ability.cannot(SpaceCaslAction.Read, SpaceCaslSubject.Page)) { - throw new ForbiddenException(); - } + // Checks both space-level and page-level permissions + await this.pagePermissionService.validateCanView(page, user); return page; } @@ -84,6 +84,14 @@ export class PageController { throw new ForbiddenException(); } + // If creating under a parent page, check page-level edit permission + if (createPageDto.parentPageId) { + const parentPage = await this.pageRepo.findById(createPageDto.parentPageId); + if (parentPage) { + await this.pagePermissionService.validateCanEdit(parentPage, user); + } + } + return this.pageService.create(user.id, workspace.id, createPageDto); } @@ -96,10 +104,8 @@ export class PageController { throw new NotFoundException('Page not found'); } - const ability = await this.spaceAbility.createForUser(user, page.spaceId); - if (ability.cannot(SpaceCaslAction.Edit, SpaceCaslSubject.Page)) { - throw new ForbiddenException(); - } + // Checks both space-level and page-level permissions + await this.pagePermissionService.validateCanEdit(page, user); return this.pageService.update(page, updatePageDto, user.id); } @@ -128,10 +134,13 @@ export class PageController { } await this.pageService.forceDelete(deletePageDto.pageId, workspace.id); } else { - // Soft delete requires page manage permissions + // Soft delete requires page manage permissions at space level if (ability.cannot(SpaceCaslAction.Manage, SpaceCaslSubject.Page)) { throw new ForbiddenException(); } + // Also check page-level edit permission + await this.pagePermissionService.validateCanEdit(page, user); + await this.pageService.removePage( deletePageDto.pageId, user.id, @@ -158,6 +167,9 @@ export class PageController { throw new ForbiddenException(); } + // Check page-level edit permission (if restoring to a restricted ancestor) + await this.pagePermissionService.validateCanEdit(page, user); + await this.pageRepo.restorePage(pageIdDto.pageId, workspace.id); return this.pageRepo.findById(pageIdDto.pageId, { @@ -228,10 +240,8 @@ export class PageController { throw new NotFoundException('Page not found'); } - const ability = await this.spaceAbility.createForUser(user, page.spaceId); - if (ability.cannot(SpaceCaslAction.Read, SpaceCaslSubject.Page)) { - throw new ForbiddenException(); - } + // Checks both space-level and page-level permissions + await this.pagePermissionService.validateCanView(page, user); return this.pageHistoryService.findHistoryByPageId(page.id, pagination); } @@ -247,13 +257,15 @@ export class PageController { throw new NotFoundException('Page history not found'); } - const ability = await this.spaceAbility.createForUser( - user, - history.spaceId, - ); - if (ability.cannot(SpaceCaslAction.Read, SpaceCaslSubject.Page)) { - throw new ForbiddenException(); + // Get the page to check permissions + const page = await this.pageRepo.findById(history.pageId); + if (!page) { + throw new NotFoundException('Page not found'); } + + // Checks both space-level and page-level permissions + await this.pagePermissionService.validateCanView(page, user); + return history; } @@ -285,7 +297,7 @@ export class PageController { throw new ForbiddenException(); } - return this.pageService.getSidebarPages(spaceId, pagination, dto.pageId); + return this.pageService.getSidebarPages(spaceId, pagination, dto.pageId, user.id); } @HttpCode(HttpStatus.OK) @@ -315,6 +327,9 @@ export class PageController { throw new ForbiddenException(); } + // Check page-level edit permission on the source page + await this.pagePermissionService.validateCanEdit(movedPage, user); + return this.pageService.movePageToSpace(movedPage, dto.spaceId); } @@ -326,6 +341,9 @@ export class PageController { throw new NotFoundException('Page to copy not found'); } + // Check page-level view permission on the source page (need to read to copy) + await this.pagePermissionService.validateCanView(copiedPage, user); + // If spaceId is provided, it's a copy to different space if (dto.spaceId) { const abilities = await Promise.all([ @@ -372,6 +390,17 @@ export class PageController { throw new ForbiddenException(); } + // Check page-level edit permission + await this.pagePermissionService.validateCanEdit(movedPage, user); + + // If moving to a new parent, check permission on the target parent + if (dto.parentPageId && dto.parentPageId !== movedPage.parentPageId) { + const targetParent = await this.pageRepo.findById(dto.parentPageId); + if (targetParent) { + await this.pagePermissionService.validateCanEdit(targetParent, user); + } + } + return this.pageService.movePage(dto, movedPage); } @@ -383,10 +412,9 @@ export class PageController { throw new NotFoundException('Page not found'); } - const ability = await this.spaceAbility.createForUser(user, page.spaceId); - if (ability.cannot(SpaceCaslAction.Read, SpaceCaslSubject.Page)) { - throw new ForbiddenException(); - } + // Checks both space-level and page-level permissions + await this.pagePermissionService.validateCanView(page, user); + return this.pageService.getPageBreadCrumbs(page.id); } } diff --git a/apps/server/src/core/page/services/page.service.ts b/apps/server/src/core/page/services/page.service.ts index 9bfb5e1c..b1b01eab 100644 --- a/apps/server/src/core/page/services/page.service.ts +++ b/apps/server/src/core/page/services/page.service.ts @@ -7,6 +7,7 @@ import { import { CreatePageDto } from '../dto/create-page.dto'; import { UpdatePageDto } from '../dto/update-page.dto'; import { PageRepo } from '@docmost/db/repos/page/page.repo'; +import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo'; import { InsertablePage, Page, User } from '@docmost/db/types/entity.types'; import { PaginationOptions } from '@docmost/db/pagination/pagination-options'; import { @@ -47,6 +48,7 @@ export class PageService { constructor( private pageRepo: PageRepo, + private pagePermissionRepo: PagePermissionRepo, private attachmentRepo: AttachmentRepo, @InjectKysely() private readonly db: KyselyDB, private readonly storageService: StorageService, @@ -180,6 +182,7 @@ export class PageService { spaceId: string, pagination: PaginationOptions, pageId?: string, + userId?: string, ): Promise { let query = this.db .selectFrom('pages') @@ -194,7 +197,12 @@ export class PageService { 'creatorId', 'deletedAt', ]) - .select((eb) => this.pageRepo.withHasChildren(eb)) + .$if(Boolean(userId), (qb) => + qb.select((eb) => this.pageRepo.withHasChildrenV2(eb, userId)), + ) + //.$if(!userId, (qb) => + // qb.select((eb) => this.pageRepo.withHasChildren(eb)), + // ) .orderBy('position', (ob) => ob.collate('C').asc()) .where('deletedAt', 'is', null) .where('spaceId', '=', spaceId); @@ -205,11 +213,22 @@ export class PageService { query = query.where('parentPageId', 'is', null); } - const result = executeWithPagination(query, { + const result = await executeWithPagination(query, { page: pagination.page, perPage: 250, }); + // Filter by page-level permissions + if (userId && result.items.length > 0) { + const pageIds = result.items.map((p: any) => p.id); + const accessiblePageIds = await this.pagePermissionRepo.filterAccessiblePageIds( + pageIds, + userId, + ); + const accessibleSet = new Set(accessiblePageIds); + result.items = result.items.filter((p: any) => accessibleSet.has(p.id)); + } + return result; } diff --git a/apps/server/src/core/search/search.service.ts b/apps/server/src/core/search/search.service.ts index 29508797..26879c51 100644 --- a/apps/server/src/core/search/search.service.ts +++ b/apps/server/src/core/search/search.service.ts @@ -7,6 +7,7 @@ import { sql } from 'kysely'; import { PageRepo } from '@docmost/db/repos/page/page.repo'; import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo'; import { ShareRepo } from '@docmost/db/repos/share/share.repo'; +import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo'; // eslint-disable-next-line @typescript-eslint/no-require-imports const tsquery = require('pg-tsquery')(); @@ -18,6 +19,7 @@ export class SearchService { private pageRepo: PageRepo, private shareRepo: ShareRepo, private spaceMemberRepo: SpaceMemberRepo, + private pagePermissionRepo: PagePermissionRepo, ) {} async searchPage( @@ -118,10 +120,21 @@ export class SearchService { } //@ts-ignore - queryResults = await queryResults.execute(); + let results: any[] = await queryResults.execute(); + + // Filter results by page-level permissions (if user is authenticated) + if (opts.userId && results.length > 0) { + const pageIds = results.map((r: any) => r.id); + const accessiblePageIds = await this.pagePermissionRepo.filterAccessiblePageIds( + pageIds, + opts.userId, + ); + const accessibleSet = new Set(accessiblePageIds); + results = results.filter((r: any) => accessibleSet.has(r.id)); + } //@ts-ignore - const searchResults = queryResults.map((result: SearchResponseDto) => { + const searchResults = results.map((result: SearchResponseDto) => { if (result.highlight) { result.highlight = result.highlight .replace(/\r\n|\r|\n/g, ' ') @@ -210,6 +223,17 @@ export class SearchService { pageSearch = pageSearch.where('spaceId', 'in', userSpaceIds); pages = await pageSearch.execute(); } + + // Filter by page-level permissions + if (pages.length > 0) { + const pageIds = pages.map((p) => p.id); + const accessiblePageIds = await this.pagePermissionRepo.filterAccessiblePageIds( + pageIds, + userId, + ); + const accessibleSet = new Set(accessiblePageIds); + pages = pages.filter((p) => accessibleSet.has(p.id)); + } } return { users, groups, pages }; diff --git a/apps/server/src/database/repos/page/page.repo.ts b/apps/server/src/database/repos/page/page.repo.ts index 3b948a48..cc42ba7d 100644 --- a/apps/server/src/database/repos/page/page.repo.ts +++ b/apps/server/src/database/repos/page/page.repo.ts @@ -411,6 +411,62 @@ export class PageRepo { .as('hasChildren'); } + /** + * Permission-aware version of withHasChildren. + * Returns true only if there are children the user can access. + * Uses page_hierarchy closure table to check all restricted ancestors. + */ + withHasChildrenV2(eb: ExpressionBuilder, userId: string) { + return eb + .selectFrom('pages as child') + .select((eb) => + eb + .case() + .when(eb.fn.countAll(), '>', 0) + .then(true) + .else(false) + .end() + .as('count'), + ) + .whereRef('child.parentPageId', '=', 'pages.id') + .where('child.deletedAt', 'is', null) + // Only count children that the user can access + .where(({ not, exists, selectFrom }) => + not( + exists( + selectFrom('pageHierarchy') + .innerJoin( + 'pageAccess', + 'pageAccess.pageId', + 'pageHierarchy.ancestorId', + ) + .leftJoin('pagePermissions', (join) => + join + .onRef('pagePermissions.pageAccessId', '=', 'pageAccess.id') + .on((eb) => + eb.or([ + eb('pagePermissions.userId', '=', userId), + eb( + 'pagePermissions.groupId', + 'in', + eb + .selectFrom('groupUsers') + .select('groupUsers.groupId') + .where('groupUsers.userId', '=', userId), + ), + ]), + ), + ) + .select('pageAccess.pageId') + .whereRef('pageHierarchy.descendantId', '=', 'child.id') + .where('pagePermissions.id', 'is', null), + ), + ), + ) + .limit(1) + .as('hasChildren'); + } + async getPageAndDescendants( parentPageId: string, opts: { includeContent: boolean },