From 51c564649c3900c3da05bbc25f59ad1af4355ffc Mon Sep 17 00:00:00 2001 From: Shun Miyazawa Date: Mon, 26 Aug 2024 07:09:09 +0000 Subject: [PATCH 1/4] imprv --- apps/app/src/server/models/page.ts | 17 ++++++++++++++--- apps/app/src/server/routes/apiv3/pages/index.js | 6 ++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/apps/app/src/server/models/page.ts b/apps/app/src/server/models/page.ts index 881029ddca0..46cd523ad7d 100644 --- a/apps/app/src/server/models/page.ts +++ b/apps/app/src/server/models/page.ts @@ -414,19 +414,30 @@ export class PageQueryBuilder { } // add viewer condition to PageQueryBuilder instance - async addViewerCondition(user, userGroups = null, includeAnyoneWithTheLink = false): Promise { + async addViewerCondition( + user, + userGroups = null, + includeAnyoneWithTheLink = false, + showPagesRestrictedByOwner = false, + showPagesRestrictedByGroup = false, + ): Promise { const relatedUserGroups = (user != null && userGroups == null) ? [ ...(await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user)), ...(await ExternalUserGroupRelation.findAllUserGroupIdsRelatedToUser(user)), ] : userGroups; - this.addConditionToFilteringByViewer(user, relatedUserGroups, includeAnyoneWithTheLink); + this.addConditionToFilteringByViewer(user, relatedUserGroups, includeAnyoneWithTheLink, showPagesRestrictedByOwner, showPagesRestrictedByGroup); return this; } addConditionToFilteringByViewer( user, userGroups: ObjectIdLike[] | null, includeAnyoneWithTheLink = false, showPagesRestrictedByOwner = false, showPagesRestrictedByGroup = false, ): PageQueryBuilder { + + console.log('showPagesRestrictedByOwner:', showPagesRestrictedByOwner); + console.log('showPagesRestrictedByGroup:', showPagesRestrictedByGroup); + + const condition = generateGrantCondition(user, userGroups, includeAnyoneWithTheLink, showPagesRestrictedByOwner, showPagesRestrictedByGroup); this.query = this.query @@ -690,7 +701,7 @@ schema.statics.findRecentUpdatedPages = async function( queryBuilder.addConditionToListWithDescendants(path, options); queryBuilder.populateDataToList(User.USER_FIELDS_EXCEPT_CONFIDENTIAL); - await queryBuilder.addViewerCondition(user); + await queryBuilder.addViewerCondition(user, undefined, undefined, !options.hideRestrictedByOwner, !options.hideRestrictedByGroup); const pages = await Page.paginate(queryBuilder.query.clone(), { lean: true, sort: sortOpt, offset: options.offset, limit: options.limit, }); diff --git a/apps/app/src/server/routes/apiv3/pages/index.js b/apps/app/src/server/routes/apiv3/pages/index.js index 8e76a934959..a46c4a5e49d 100644 --- a/apps/app/src/server/routes/apiv3/pages/index.js +++ b/apps/app/src/server/routes/apiv3/pages/index.js @@ -226,6 +226,9 @@ module.exports = (crowi) => { const offset = parseInt(req.query.offset) || 0; const includeWipPage = req.query.includeWipPage === 'true'; // Need validation using express-validator + const hideRestrictedByOwner = await crowi.configManager.getConfig('crowi', 'security:list-policy:hideRestrictedByOwner'); + const hideRestrictedByGroup = await crowi.configManager.getConfig('crowi', 'security:list-policy:hideRestrictedByGroup'); + const queryOptions = { offset, limit, @@ -234,7 +237,10 @@ module.exports = (crowi) => { isRegExpEscapedFromPath: true, sort: 'updatedAt', desc: -1, + hideRestrictedByOwner, + hideRestrictedByGroup, }; + try { const result = await Page.findRecentUpdatedPages('/', req.user, queryOptions); if (result.pages.length > limit) { From 30626d6b4e75ae025797f3d23454e28c55792af7 Mon Sep 17 00:00:00 2001 From: Shun Miyazawa Date: Mon, 26 Aug 2024 07:35:41 +0000 Subject: [PATCH 2/4] rm debug log --- apps/app/src/server/models/page.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/apps/app/src/server/models/page.ts b/apps/app/src/server/models/page.ts index 46cd523ad7d..2d9aea13180 100644 --- a/apps/app/src/server/models/page.ts +++ b/apps/app/src/server/models/page.ts @@ -433,11 +433,6 @@ export class PageQueryBuilder { addConditionToFilteringByViewer( user, userGroups: ObjectIdLike[] | null, includeAnyoneWithTheLink = false, showPagesRestrictedByOwner = false, showPagesRestrictedByGroup = false, ): PageQueryBuilder { - - console.log('showPagesRestrictedByOwner:', showPagesRestrictedByOwner); - console.log('showPagesRestrictedByGroup:', showPagesRestrictedByGroup); - - const condition = generateGrantCondition(user, userGroups, includeAnyoneWithTheLink, showPagesRestrictedByOwner, showPagesRestrictedByGroup); this.query = this.query From c57e00dda69d336b9806609b5331947057062999 Mon Sep 17 00:00:00 2001 From: Shun Miyazawa Date: Mon, 26 Aug 2024 08:47:41 +0000 Subject: [PATCH 3/4] Also supported by pageTree --- apps/app/src/server/routes/apiv3/page-listing.ts | 9 ++++++++- apps/app/src/server/service/page/index.ts | 11 ++++++++--- apps/app/src/server/service/page/page-service.ts | 4 +++- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/apps/app/src/server/routes/apiv3/page-listing.ts b/apps/app/src/server/routes/apiv3/page-listing.ts index 2c082c94b1a..caa3a65dcf1 100644 --- a/apps/app/src/server/routes/apiv3/page-listing.ts +++ b/apps/app/src/server/routes/apiv3/page-listing.ts @@ -9,6 +9,7 @@ import { query, oneOf } from 'express-validator'; import type { HydratedDocument } from 'mongoose'; import mongoose from 'mongoose'; +import { configManager } from '~/server/service/config-manager'; import type { IPageGrantService } from '~/server/service/page-grant'; import loggerFactory from '~/utils/logger'; @@ -18,6 +19,7 @@ import type { PageDocument, PageModel } from '../../models/page'; import type { ApiV3Response } from './interfaces/apiv3-response'; + const logger = loggerFactory('growi:routes:apiv3:page-tree'); /* @@ -103,8 +105,13 @@ const routerFactory = (crowi: Crowi): Router => { const pageService = crowi.pageService; + const hideRestrictedByOwner = await configManager.getConfig('crowi', 'security:list-policy:hideRestrictedByOwner'); + const hideRestrictedByGroup = await configManager.getConfig('crowi', 'security:list-policy:hideRestrictedByGroup'); + try { - const pages = await pageService.findChildrenByParentPathOrIdAndViewer((id || path)as string, req.user); + const pages = await pageService.findChildrenByParentPathOrIdAndViewer( + (id || path)as string, req.user, undefined, !hideRestrictedByOwner, !hideRestrictedByGroup, + ); return res.apiv3({ children: pages }); } catch (err) { diff --git a/apps/app/src/server/service/page/index.ts b/apps/app/src/server/service/page/index.ts index 3849d587cee..c984553bc79 100644 --- a/apps/app/src/server/service/page/index.ts +++ b/apps/app/src/server/service/page/index.ts @@ -4332,8 +4332,13 @@ class PageService implements IPageService { /* * Find all children by parent's path or id. Using id should be prioritized */ - async findChildrenByParentPathOrIdAndViewer(parentPathOrId: string, user, userGroups = null) - : Promise<(HydratedDocument & { processData?: IPageOperationProcessData })[]> { + async findChildrenByParentPathOrIdAndViewer( + parentPathOrId: string, + user, + userGroups = null, + showPagesRestrictedByOwner = false, + showPagesRestrictedByGroup = false, + ): Promise<(HydratedDocument & { processData?: IPageOperationProcessData })[]> { const Page = mongoose.model, PageModel>('Page'); let queryBuilder: PageQueryBuilder; if (hasSlash(parentPathOrId)) { @@ -4346,7 +4351,7 @@ class PageService implements IPageService { // Use $eq for user-controlled sources. see: https://codeql.github.com/codeql-query-help/javascript/js-sql-injection/#recommendation queryBuilder = new PageQueryBuilder(Page.find({ parent: { $eq: parentId } } as any), true); // TODO: improve type } - await queryBuilder.addViewerCondition(user, userGroups); + await queryBuilder.addViewerCondition(user, userGroups, undefined, showPagesRestrictedByOwner, showPagesRestrictedByGroup); const pages: HydratedDocument[] = await queryBuilder .addConditionToSortPagesByAscPath() diff --git a/apps/app/src/server/service/page/page-service.ts b/apps/app/src/server/service/page/page-service.ts index 21d6c28df91..90e80a517a4 100644 --- a/apps/app/src/server/service/page/page-service.ts +++ b/apps/app/src/server/service/page/page-service.ts @@ -21,7 +21,9 @@ export interface IPageService { getEventEmitter: () => EventEmitter, deleteMultipleCompletely: (pages: ObjectIdLike[], user: IUser | undefined) => Promise, findAncestorsChildrenByPathAndViewer(path: string, user, userGroups?): Promise>, - findChildrenByParentPathOrIdAndViewer(parentPathOrId: string, user, userGroups?): Promise, + findChildrenByParentPathOrIdAndViewer( + parentPathOrId: string, user, userGroups?, showPagesRestrictedByOwner?: boolean, showPagesRestrictedByGroup?: boolean, + ): Promise, shortBodiesMapByPageIds(pageIds?: Types.ObjectId[], user?): Promise>, constructBasicPageInfo(page: PageDocument, isGuestUser?: boolean): IPageInfo | Omit, normalizeAllPublicPages(): Promise, From fe24ca1b35aca69d416f4b674e3b6c3b564bf073 Mon Sep 17 00:00:00 2001 From: Shun Miyazawa Date: Mon, 26 Aug 2024 11:24:23 +0000 Subject: [PATCH 4/4] Add type to queryOptions --- apps/app/src/server/models/page.ts | 16 ++++++++++++++-- apps/app/src/server/routes/apiv3/pages/index.js | 3 +++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/apps/app/src/server/models/page.ts b/apps/app/src/server/models/page.ts index 2d9aea13180..98fb82559ad 100644 --- a/apps/app/src/server/models/page.ts +++ b/apps/app/src/server/models/page.ts @@ -65,6 +65,18 @@ type PaginatedPages = { offset: number } +export type FindRecentUpdatedPagesOption = { + offset: number, + limit: number, + includeWipPage: boolean, + includeTrashed: boolean, + isRegExpEscapedFromPath: boolean, + sort: 'updatedAt' + desc: number + hideRestrictedByOwner: boolean, + hideRestrictedByGroup: boolean, +} + export type CreateMethod = (path: string, body: string, user, options: IOptionsForCreate) => Promise> export interface PageModel extends Model { @@ -79,7 +91,7 @@ export interface PageModel extends Model { countByPathAndViewer(path: string | null, user, userGroups?, includeEmpty?:boolean): Promise findParentByPath(path: string | null): Promise | null> findTargetAndAncestorsByPathOrId(pathOrId: string): Promise - findRecentUpdatedPages(path: string, user, option, includeEmpty?: boolean): Promise + findRecentUpdatedPages(path: string, user, option: FindRecentUpdatedPagesOption, includeEmpty?: boolean): Promise generateGrantCondition( user, userGroups: ObjectIdLike[] | null, includeAnyoneWithTheLink?: boolean, showPagesRestrictedByOwner?: boolean, showPagesRestrictedByGroup?: boolean, ): { $or: any[] } @@ -670,7 +682,7 @@ schema.statics.countByPathAndViewer = async function(path: string | null, user, }; schema.statics.findRecentUpdatedPages = async function( - path: string, user, options, includeEmpty = false, + path: string, user, options: FindRecentUpdatedPagesOption, includeEmpty = false, ): Promise { const sortOpt = {}; diff --git a/apps/app/src/server/routes/apiv3/pages/index.js b/apps/app/src/server/routes/apiv3/pages/index.js index a46c4a5e49d..92dab4fde09 100644 --- a/apps/app/src/server/routes/apiv3/pages/index.js +++ b/apps/app/src/server/routes/apiv3/pages/index.js @@ -229,6 +229,9 @@ module.exports = (crowi) => { const hideRestrictedByOwner = await crowi.configManager.getConfig('crowi', 'security:list-policy:hideRestrictedByOwner'); const hideRestrictedByGroup = await crowi.configManager.getConfig('crowi', 'security:list-policy:hideRestrictedByGroup'); + /** + * @type {import('~/server/models/page').FindRecentUpdatedPagesOption} + */ const queryOptions = { offset, limit,