Преглед изворни кода

Merge pull request #8311 from weseek/feat/136137-136537-page-grant-update-for-multiple-group-grant

page grant update for multiple group grant
Futa Arai пре 2 година
родитељ
комит
a4d4a63fa1

+ 4 - 3
apps/app/src/server/crowi/index.js

@@ -712,12 +712,13 @@ Crowi.prototype.setupGrowiPluginService = async function() {
 };
 };
 
 
 Crowi.prototype.setupPageService = async function() {
 Crowi.prototype.setupPageService = async function() {
-  if (this.pageService == null) {
-    this.pageService = new PageService(this);
-  }
   if (this.pageGrantService == null) {
   if (this.pageGrantService == null) {
     this.pageGrantService = new PageGrantService(this);
     this.pageGrantService = new PageGrantService(this);
   }
   }
+  // initialize after pageGrantService since pageService uses pageGrantService in constructor
+  if (this.pageService == null) {
+    this.pageService = new PageService(this);
+  }
   if (this.pageOperationService == null) {
   if (this.pageOperationService == null) {
     this.pageOperationService = new PageOperationService(this);
     this.pageOperationService = new PageOperationService(this);
     await this.pageOperationService.init();
     await this.pageOperationService.init();

+ 0 - 5
apps/app/src/server/routes/page.js

@@ -454,11 +454,6 @@ module.exports = function(crowi, app) {
     const isSlackEnabled = !!req.body.isSlackEnabled; // cast to boolean
     const isSlackEnabled = !!req.body.isSlackEnabled; // cast to boolean
     const slackChannels = req.body.slackChannels || null;
     const slackChannels = req.body.slackChannels || null;
 
 
-    // TODO: remove in https://redmine.weseek.co.jp/issues/136140
-    if (grantUserGroupIds != null && grantUserGroupIds.length > 1) {
-      return res.apiv3Err('Cannot grant multiple groups to page at the moment');
-    }
-
     if (pageId === null || pageBody === null || revisionId === null) {
     if (pageId === null || pageBody === null || revisionId === null) {
       return res.json(ApiResponse.error('page_id, body and revision_id are required.'));
       return res.json(ApiResponse.error('page_id, body and revision_id are required.'));
     }
     }

+ 90 - 15
apps/app/src/server/service/page-grant.ts

@@ -1,19 +1,18 @@
 import {
 import {
   type IGrantedGroup,
   type IGrantedGroup,
-  PageGrant, type PageGrantCanBeOnTree, GroupType,
+  PageGrant, GroupType, getIdForRef,
 } from '@growi/core';
 } from '@growi/core';
 import {
 import {
   pagePathUtils, pathUtils, pageUtils,
   pagePathUtils, pathUtils, pageUtils,
 } from '@growi/core/dist/utils';
 } from '@growi/core/dist/utils';
-import { et } from 'date-fns/locale';
 import escapeStringRegexp from 'escape-string-regexp';
 import escapeStringRegexp from 'escape-string-regexp';
 import mongoose from 'mongoose';
 import mongoose from 'mongoose';
 
 
-import ExternalUserGroup, { ExternalUserGroupDocument } from '~/features/external-user-group/server/models/external-user-group';
+import ExternalUserGroup from '~/features/external-user-group/server/models/external-user-group';
 import ExternalUserGroupRelation from '~/features/external-user-group/server/models/external-user-group-relation';
 import ExternalUserGroupRelation from '~/features/external-user-group/server/models/external-user-group-relation';
-import { IRecordApplicableGrant } from '~/interfaces/page-grant';
+import { IRecordApplicableGrant, PopulatedGrantedGroup } from '~/interfaces/page-grant';
 import { PageDocument, PageModel } from '~/server/models/page';
 import { PageDocument, PageModel } from '~/server/models/page';
-import UserGroup, { UserGroupDocument } from '~/server/models/user-group';
+import UserGroup from '~/server/models/user-group';
 import { includesObjectIds, excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
 import { includesObjectIds, excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
 
 
 import { ObjectIdLike } from '../interfaces/mongoose-utils';
 import { ObjectIdLike } from '../interfaces/mongoose-utils';
@@ -26,7 +25,7 @@ const { isTopPage } = pagePathUtils;
 const LIMIT_FOR_MULTIPLE_PAGE_OP = 20;
 const LIMIT_FOR_MULTIPLE_PAGE_OP = 20;
 
 
 type ComparableTarget = {
 type ComparableTarget = {
-  grant: number,
+  grant?: number,
   grantedUserIds?: ObjectIdLike[],
   grantedUserIds?: ObjectIdLike[],
   grantedGroupIds?: IGrantedGroup[],
   grantedGroupIds?: IGrantedGroup[],
   applicableUserIds?: ObjectIdLike[],
   applicableUserIds?: ObjectIdLike[],
@@ -78,7 +77,26 @@ type OperatorGrantInfo = {
   userGroupIds: Set<ObjectIdLike>,
   userGroupIds: Set<ObjectIdLike>,
 };
 };
 
 
-class PageGrantService {
+export interface IPageGrantService {
+  isGrantNormalized: (
+    user,
+    targetPath: string,
+    grant?: PageGrant,
+    grantedUserIds?: ObjectIdLike[],
+    grantedGroupIds?: IGrantedGroup[],
+    shouldCheckDescendants?: boolean,
+    includeNotMigratedPages?: boolean,
+    previousGrantedGroupIds?: IGrantedGroup[]
+  ) => Promise<boolean>,
+  separateNormalizableAndNotNormalizablePages: (user, pages) => Promise<[(PageDocument & { _id: any })[], (PageDocument & { _id: any })[]]>,
+  generateUpdateGrantInfoToOverwriteDescendants: (
+    operator, updateGrant?: PageGrant, grantGroupIds?: IGrantedGroup[],
+  ) => Promise<UpdateGrantInfo>,
+  canOverwriteDescendants: (targetPath: string, operator: { _id: ObjectIdLike }, updateGrantInfo: UpdateGrantInfo) => Promise<boolean>,
+  validateGrantChange: (user, previousGrantedGroupIds: IGrantedGroup[], grant?: PageGrant, grantedGroupIds?: IGrantedGroup[]) => Promise<boolean>
+}
+
+class PageGrantService implements IPageGrantService {
 
 
   crowi!: any;
   crowi!: any;
 
 
@@ -103,7 +121,10 @@ class PageGrantService {
    * About the rule of validation, see: https://dev.growi.org/61b2cdabaa330ce7d8152844
    * About the rule of validation, see: https://dev.growi.org/61b2cdabaa330ce7d8152844
    * @returns boolean
    * @returns boolean
    */
    */
-  private processValidation(target: ComparableTarget, ancestor: ComparableAncestor, descendants?: ComparableDescendants): boolean {
+  private validateGrant(target: ComparableTarget, ancestor: ComparableAncestor, descendants?: ComparableDescendants): boolean {
+    /*
+     * the page itself
+     */
     this.validateComparableTarget(target);
     this.validateComparableTarget(target);
 
 
     const Page = mongoose.model('Page') as unknown as PageModel;
     const Page = mongoose.model('Page') as unknown as PageModel;
@@ -209,12 +230,41 @@ class PageGrantService {
     return true;
     return true;
   }
   }
 
 
+  /**
+   * Validate if page grant can be changed from prior grant to specified grant.
+   * Necessary for pages with multiple group grant.
+   * see: https://dev.growi.org/656745fa52eafe1cf1879508#%E3%83%9A%E3%83%BC%E3%82%B8%E3%81%AE-grant-%E3%81%AE%E6%9B%B4%E6%96%B0
+   * @param user The user who is changing the grant
+   * @param previousGrantedGroupIds The groups that were granted priorly
+   * @param grant The grant to be changed to
+   * @param grantedGroupIds The groups to be granted
+   */
+  async validateGrantChange(user, previousGrantedGroupIds: IGrantedGroup[], grant?: PageGrant, grantedGroupIds?: IGrantedGroup[]): Promise<boolean> {
+    const userRelatedGroupIds = (await this.getUserRelatedGroups(user)).map(g => g.item._id);
+    const userBelongsToAllPreviousGrantedGroups = excludeTestIdsFromTargetIds(
+      previousGrantedGroupIds.map(g => getIdForRef(g.item)),
+      userRelatedGroupIds,
+    ).length === 0;
+
+    if (!userBelongsToAllPreviousGrantedGroups) {
+      if (grant !== PageGrant.GRANT_USER_GROUP) {
+        return false;
+      }
+      const pageGrantIncludesUserRelatedGroup = includesObjectIds(grantedGroupIds?.map(g => getIdForRef(g.item)) || [], userRelatedGroupIds);
+      if (!pageGrantIncludesUserRelatedGroup) {
+        return false;
+      }
+    }
+
+    return true;
+  }
+
   /**
   /**
    * Prepare ComparableTarget
    * Prepare ComparableTarget
    * @returns Promise<ComparableAncestor>
    * @returns Promise<ComparableAncestor>
    */
    */
   private async generateComparableTarget(
   private async generateComparableTarget(
-      grant, grantedUserIds: ObjectIdLike[] | undefined, grantedGroupIds: IGrantedGroup[] | undefined, includeApplicable: boolean,
+      grant: PageGrant | undefined, grantedUserIds: ObjectIdLike[] | undefined, grantedGroupIds: IGrantedGroup[] | undefined, includeApplicable: boolean,
   ): Promise<ComparableTarget> {
   ): Promise<ComparableTarget> {
     if (includeApplicable) {
     if (includeApplicable) {
       const Page = mongoose.model('Page') as unknown as PageModel;
       const Page = mongoose.model('Page') as unknown as PageModel;
@@ -415,27 +465,51 @@ class PageGrantService {
    * Only v5 schema pages will be used to compare by default (Set includeNotMigratedPages to true to include v4 schema pages as well).
    * Only v5 schema pages will be used to compare by default (Set includeNotMigratedPages to true to include v4 schema pages as well).
    * When comparing, it will use path regex to collect pages instead of using parent attribute of the Page model. This is reasonable since
    * When comparing, it will use path regex to collect pages instead of using parent attribute of the Page model. This is reasonable since
    * using the path attribute is safer than using the parent attribute in this case. 2022.02.13 -- Taichi Masuyama
    * using the path attribute is safer than using the parent attribute in this case. 2022.02.13 -- Taichi Masuyama
+   * @param user The user responsible for execution
+   * @param targetPath Path of page which grant will be validated
+   * @param grant Type of the grant to be validated
+   * @param grantedUserIds Users of grant to be validated
+   * @param grantedGroupIds Groups of grant to be validated
+   * @param shouldCheckDescendants Whether or not to use descendant grant for validation
+   * @param includeNotMigratedPages Whether or not to use unmigrated pages for validation
+   * @param previousGrantedGroupIds
+   *   Previously granted groups of the page. Specific validation is required when previous grant is multiple group grant.
+   *   Apply when page grant change needs to be validated.
+   *   see: https://dev.growi.org/656745fa52eafe1cf1879508#%E3%83%9A%E3%83%BC%E3%82%B8%E3%81%AE-grant-%E3%81%AE%E6%9B%B4%E6%96%B0
    * @returns Promise<boolean>
    * @returns Promise<boolean>
    */
    */
   async isGrantNormalized(
   async isGrantNormalized(
-      // eslint-disable-next-line max-len
-      user, targetPath: string, grant, grantedUserIds?: ObjectIdLike[], grantedGroupIds?: IGrantedGroup[], shouldCheckDescendants = false, includeNotMigratedPages = false,
+      user,
+      targetPath: string,
+      grant?: PageGrant,
+      grantedUserIds?: ObjectIdLike[],
+      grantedGroupIds?: IGrantedGroup[],
+      shouldCheckDescendants = false,
+      includeNotMigratedPages = false,
+      previousGrantedGroupIds?: IGrantedGroup[],
   ): Promise<boolean> {
   ): Promise<boolean> {
     if (isTopPage(targetPath)) {
     if (isTopPage(targetPath)) {
       return true;
       return true;
     }
     }
 
 
+    if (previousGrantedGroupIds != null) {
+      const isGrantChangeable = await this.validateGrantChange(user, previousGrantedGroupIds, grant, grantedGroupIds);
+      if (!isGrantChangeable) {
+        return false;
+      }
+    }
+
     const comparableAncestor = await this.generateComparableAncestor(targetPath, includeNotMigratedPages);
     const comparableAncestor = await this.generateComparableAncestor(targetPath, includeNotMigratedPages);
 
 
     if (!shouldCheckDescendants) { // checking the parent is enough
     if (!shouldCheckDescendants) { // checking the parent is enough
       const comparableTarget = await this.generateComparableTarget(grant, grantedUserIds, grantedGroupIds, false);
       const comparableTarget = await this.generateComparableTarget(grant, grantedUserIds, grantedGroupIds, false);
-      return this.processValidation(comparableTarget, comparableAncestor);
+      return this.validateGrant(comparableTarget, comparableAncestor);
     }
     }
 
 
     const comparableTarget = await this.generateComparableTarget(grant, grantedUserIds, grantedGroupIds, true);
     const comparableTarget = await this.generateComparableTarget(grant, grantedUserIds, grantedGroupIds, true);
     const comparableDescendants = await this.generateComparableDescendants(targetPath, user, includeNotMigratedPages);
     const comparableDescendants = await this.generateComparableDescendants(targetPath, user, includeNotMigratedPages);
 
 
-    return this.processValidation(comparableTarget, comparableAncestor, comparableDescendants);
+    return this.validateGrant(comparableTarget, comparableAncestor, comparableDescendants);
   }
   }
 
 
   /**
   /**
@@ -568,7 +642,7 @@ class PageGrantService {
     return data;
     return data;
   }
   }
 
 
-  async getUserRelatedGroups(user) {
+  async getUserRelatedGroups(user): Promise<PopulatedGrantedGroup[]> {
     const userRelatedUserGroups = await UserGroupRelation.findAllGroupsForUser(user);
     const userRelatedUserGroups = await UserGroupRelation.findAllGroupsForUser(user);
     const userRelatedExternalUserGroups = await ExternalUserGroupRelation.findAllGroupsForUser(user);
     const userRelatedExternalUserGroups = await ExternalUserGroupRelation.findAllGroupsForUser(user);
     return [
     return [
@@ -622,7 +696,7 @@ class PageGrantService {
   }
   }
 
 
   async generateUpdateGrantInfoToOverwriteDescendants(
   async generateUpdateGrantInfoToOverwriteDescendants(
-      operator, updateGrant: PageGrantCanBeOnTree, grantGroupIds?: IGrantedGroup[],
+      operator, updateGrant?: PageGrant, grantGroupIds?: IGrantedGroup[],
   ): Promise<UpdateGrantInfo> {
   ): Promise<UpdateGrantInfo> {
     let updateGrantInfo: UpdateGrantInfo | null = null;
     let updateGrantInfo: UpdateGrantInfo | null = null;
 
 
@@ -666,6 +740,7 @@ class PageGrantService {
     }
     }
 
 
     if (updateGrantInfo == null) {
     if (updateGrantInfo == null) {
+      // Neither pages with grant `GRANT_RESTRICTED` nor `GRANT_SPECIFIED` can be on a page tree.
       throw Error('The parameter `updateGrant` must be 1, 4, or 5');
       throw Error('The parameter `updateGrant` must be 1, 4, or 5');
     }
     }
 
 

+ 29 - 24
apps/app/src/server/service/page.ts

@@ -46,6 +46,7 @@ import { V5ConversionError } from '../models/vo/v5-conversion-error';
 import { divideByType } from '../util/granted-group';
 import { divideByType } from '../util/granted-group';
 
 
 import { configManager } from './config-manager';
 import { configManager } from './config-manager';
+import { IPageGrantService } from './page-grant';
 import { preNotifyService } from './pre-notify';
 import { preNotifyService } from './pre-notify';
 
 
 const debug = require('debug')('growi:services:page');
 const debug = require('debug')('growi:services:page');
@@ -152,11 +153,14 @@ class PageService {
 
 
   activityEvent: any;
   activityEvent: any;
 
 
+  pageGrantService: IPageGrantService;
+
   constructor(crowi) {
   constructor(crowi) {
     this.crowi = crowi;
     this.crowi = crowi;
     this.pageEvent = crowi.event('page');
     this.pageEvent = crowi.event('page');
     this.tagEvent = crowi.event('tag');
     this.tagEvent = crowi.event('tag');
     this.activityEvent = crowi.event('activity');
     this.activityEvent = crowi.event('activity');
+    this.pageGrantService = crowi.pageGrantService;
 
 
     // init
     // init
     this.initPageEvent();
     this.initPageEvent();
@@ -544,7 +548,7 @@ class PageService {
     if (grant !== Page.GRANT_RESTRICTED) {
     if (grant !== Page.GRANT_RESTRICTED) {
       let isGrantNormalized = false;
       let isGrantNormalized = false;
       try {
       try {
-        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, newPagePath, grant, grantedUserIds, grantedGroupIds, false);
+        isGrantNormalized = await this.pageGrantService.isGrantNormalized(user, newPagePath, grant, grantedUserIds, grantedGroupIds, false);
       }
       }
       catch (err) {
       catch (err) {
         logger.error(`Failed to validate grant of page at "${newPagePath}" when renaming`, err);
         logger.error(`Failed to validate grant of page at "${newPagePath}" when renaming`, err);
@@ -1054,7 +1058,7 @@ class PageService {
     if (grant !== Page.GRANT_RESTRICTED) {
     if (grant !== Page.GRANT_RESTRICTED) {
       let isGrantNormalized = false;
       let isGrantNormalized = false;
       try {
       try {
-        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, newPagePath, grant, grantedUserIds, grantedGroupIds, false);
+        isGrantNormalized = await this.pageGrantService.isGrantNormalized(user, newPagePath, grant, grantedUserIds, grantedGroupIds, false);
       }
       }
       catch (err) {
       catch (err) {
         logger.error(`Failed to validate grant of page at "${newPagePath}" when duplicating`, err);
         logger.error(`Failed to validate grant of page at "${newPagePath}" when duplicating`, err);
@@ -1188,7 +1192,7 @@ class PageService {
 
 
     newPagePath = this.crowi.xss.process(newPagePath); // eslint-disable-line no-param-reassign
     newPagePath = this.crowi.xss.process(newPagePath); // eslint-disable-line no-param-reassign
 
 
-    const createdPage = await this.crowi.pageService.create(
+    const createdPage = await this.create(
       newPagePath, page.revision.body, user, options,
       newPagePath, page.revision.body, user, options,
     );
     );
     this.pageEvent.emit('duplicate', page, user);
     this.pageEvent.emit('duplicate', page, user);
@@ -1464,10 +1468,10 @@ class PageService {
     }
     }
 
 
     if (pagePathUtils.isUsersHomepage(page.path)) {
     if (pagePathUtils.isUsersHomepage(page.path)) {
-      if (!this.crowi.pageService.canDeleteUserHomepageByConfig()) {
+      if (!this.canDeleteUserHomepageByConfig()) {
         throw new Error('User Homepage is not deletable.');
         throw new Error('User Homepage is not deletable.');
       }
       }
-      if (!await this.crowi.pageService.isUsersHomepageOwnerAbsent(page.path)) {
+      if (!await this.isUsersHomepageOwnerAbsent(page.path)) {
         throw new Error('User Homepage is not deletable.');
         throw new Error('User Homepage is not deletable.');
       }
       }
     }
     }
@@ -2680,7 +2684,7 @@ class PageService {
     try {
     try {
       const shouldCheckDescendants = true;
       const shouldCheckDescendants = true;
 
 
-      isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
+      isGrantNormalized = await this.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
     }
     }
     catch (err) {
     catch (err) {
       logger.error(`Failed to validate grant of page at "${path}"`, err);
       logger.error(`Failed to validate grant of page at "${path}"`, err);
@@ -2797,7 +2801,7 @@ class PageService {
       try {
       try {
         const shouldCheckDescendants = true;
         const shouldCheckDescendants = true;
 
 
-        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
+        isGrantNormalized = await this.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
       }
       }
       catch (err) {
       catch (err) {
         logger.error(`Failed to validate grant of page at "${path}"`, err);
         logger.error(`Failed to validate grant of page at "${path}"`, err);
@@ -2843,7 +2847,7 @@ class PageService {
     let normalizablePages;
     let normalizablePages;
     let nonNormalizablePages;
     let nonNormalizablePages;
     try {
     try {
-      [normalizablePages, nonNormalizablePages] = await this.crowi.pageGrantService.separateNormalizableAndNotNormalizablePages(user, pagesToNormalize);
+      [normalizablePages, nonNormalizablePages] = await this.pageGrantService.separateNormalizableAndNotNormalizablePages(user, pagesToNormalize);
     }
     }
     catch (err) {
     catch (err) {
       socket.emit(SocketEventName.PageMigrationError);
       socket.emit(SocketEventName.PageMigrationError);
@@ -3666,7 +3670,7 @@ class PageService {
   private async canProcessCreate(
   private async canProcessCreate(
       path: string,
       path: string,
       grantData: {
       grantData: {
-        grant: number,
+        grant?: PageGrant,
         grantedUserIds?: ObjectIdLike[],
         grantedUserIds?: ObjectIdLike[],
         grantUserGroupIds?: IGrantedGroup[],
         grantUserGroupIds?: IGrantedGroup[],
       },
       },
@@ -3703,7 +3707,7 @@ class PageService {
         const isEmptyPageAlreadyExist = await Page.count({ path, isEmpty: true }) > 0;
         const isEmptyPageAlreadyExist = await Page.count({ path, isEmpty: true }) > 0;
         const shouldCheckDescendants = isEmptyPageAlreadyExist && !options?.overwriteScopesOfDescendants;
         const shouldCheckDescendants = isEmptyPageAlreadyExist && !options?.overwriteScopesOfDescendants;
 
 
-        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantUserGroupIds, shouldCheckDescendants);
+        isGrantNormalized = await this.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantUserGroupIds, shouldCheckDescendants);
       }
       }
       catch (err) {
       catch (err) {
         logger.error(`Failed to validate grant of page at "${path}" of grant ${grant}:`, err);
         logger.error(`Failed to validate grant of page at "${path}" of grant ${grant}:`, err);
@@ -3714,8 +3718,8 @@ class PageService {
       }
       }
 
 
       if (options?.overwriteScopesOfDescendants) {
       if (options?.overwriteScopesOfDescendants) {
-        const updateGrantInfo = await this.crowi.pageGrantService.generateUpdateGrantInfoToOverwriteDescendants(user, grant, options.grantUserGroupIds);
-        const canOverwriteDescendants = await this.crowi.pageGrantService.canOverwriteDescendants(path, user, updateGrantInfo);
+        const updateGrantInfo = await this.pageGrantService.generateUpdateGrantInfoToOverwriteDescendants(user, grant, options.grantUserGroupIds);
+        const canOverwriteDescendants = await this.pageGrantService.canOverwriteDescendants(path, user, updateGrantInfo);
 
 
         if (!canOverwriteDescendants) {
         if (!canOverwriteDescendants) {
           throw Error('Cannot overwrite scopes of descendants.');
           throw Error('Cannot overwrite scopes of descendants.');
@@ -3745,14 +3749,14 @@ class PageService {
     const {
     const {
       format = 'markdown', grantUserGroupIds,
       format = 'markdown', grantUserGroupIds,
     } = options;
     } = options;
-    const grant = isTopPage(path) ? Page.GRANT_PUBLIC : options.grant;
+    const grant = isTopPage(path) ? PageGrant.GRANT_PUBLIC : options.grant;
     const grantData = {
     const grantData = {
       grant,
       grant,
-      grantedUserIds: grant === Page.GRANT_OWNER ? [user._id] : undefined,
+      grantedUserIds: grant === PageGrant.GRANT_OWNER ? [user._id] : undefined,
       grantUserGroupIds,
       grantUserGroupIds,
     };
     };
 
 
-    const isGrantRestricted = grant === Page.GRANT_RESTRICTED;
+    const isGrantRestricted = grant === PageGrant.GRANT_RESTRICTED;
 
 
     // Validate
     // Validate
     const shouldValidateGrant = !isGrantRestricted;
     const shouldValidateGrant = !isGrantRestricted;
@@ -3901,7 +3905,7 @@ class PageService {
   private async canProcessForceCreateBySystem(
   private async canProcessForceCreateBySystem(
       path: string,
       path: string,
       grantData: {
       grantData: {
-        grant: number,
+        grant: PageGrant,
         grantedUserIds?: ObjectIdLike[],
         grantedUserIds?: ObjectIdLike[],
         grantUserGroupId?: ObjectIdLike,
         grantUserGroupId?: ObjectIdLike,
       },
       },
@@ -4052,7 +4056,7 @@ class PageService {
   }
   }
 
 
   async updatePage(
   async updatePage(
-      pageData,
+      pageData: PageDocument,
       body: string | null,
       body: string | null,
       previousBody: string | null,
       previousBody: string | null,
       user,
       user,
@@ -4081,14 +4085,12 @@ class PageService {
     const shouldBeOnTree = grant !== PageGrant.GRANT_RESTRICTED;
     const shouldBeOnTree = grant !== PageGrant.GRANT_RESTRICTED;
     const isChildrenExist = await Page.count({ path: new RegExp(`^${escapeStringRegexp(addTrailingSlash(clonedPageData.path))}`), parent: { $ne: null } });
     const isChildrenExist = await Page.count({ path: new RegExp(`^${escapeStringRegexp(addTrailingSlash(clonedPageData.path))}`), parent: { $ne: null } });
 
 
-    const { pageService, pageGrantService } = this.crowi;
-
     if (shouldBeOnTree) {
     if (shouldBeOnTree) {
       let isGrantNormalized = false;
       let isGrantNormalized = false;
       try {
       try {
         const shouldCheckDescendants = !options.overwriteScopesOfDescendants;
         const shouldCheckDescendants = !options.overwriteScopesOfDescendants;
         // eslint-disable-next-line max-len
         // eslint-disable-next-line max-len
-        isGrantNormalized = await pageGrantService.isGrantNormalized(user, clonedPageData.path, grant, grantedUserIds, grantUserGroupIds, shouldCheckDescendants);
+        isGrantNormalized = await this.pageGrantService.isGrantNormalized(user, clonedPageData.path, grant, grantedUserIds, grantUserGroupIds, shouldCheckDescendants, false, pageData.grantedGroups);
       }
       }
       catch (err) {
       catch (err) {
         logger.error(`Failed to validate grant of page at "${clonedPageData.path}" of grant ${grant}:`, err);
         logger.error(`Failed to validate grant of page at "${clonedPageData.path}" of grant ${grant}:`, err);
@@ -4099,8 +4101,8 @@ class PageService {
       }
       }
 
 
       if (options.overwriteScopesOfDescendants) {
       if (options.overwriteScopesOfDescendants) {
-        const updateGrantInfo = await pageGrantService.generateUpdateGrantInfoToOverwriteDescendants(user, grant, options.grantUserGroupIds);
-        const canOverwriteDescendants = await pageGrantService.canOverwriteDescendants(clonedPageData.path, user, updateGrantInfo);
+        const updateGrantInfo = await this.pageGrantService.generateUpdateGrantInfoToOverwriteDescendants(user, grant, options.grantUserGroupIds);
+        const canOverwriteDescendants = await this.pageGrantService.canOverwriteDescendants(clonedPageData.path, user, updateGrantInfo);
 
 
         if (!canOverwriteDescendants) {
         if (!canOverwriteDescendants) {
           throw Error('Cannot overwrite scopes of descendants.');
           throw Error('Cannot overwrite scopes of descendants.');
@@ -4108,7 +4110,7 @@ class PageService {
       }
       }
 
 
       if (!wasOnTree) {
       if (!wasOnTree) {
-        const newParent = await pageService.getParentAndFillAncestorsByUser(user, newPageData.path);
+        const newParent = await this.getParentAndFillAncestorsByUser(user, newPageData.path);
         newPageData.parent = newParent._id;
         newPageData.parent = newParent._id;
       }
       }
     }
     }
@@ -4184,7 +4186,7 @@ class PageService {
   }
   }
 
 
 
 
-  async updatePageV4(pageData, body, previousBody, user, options: IOptionsForUpdate = {}): Promise<PageDocument> {
+  async updatePageV4(pageData: PageDocument, body, previousBody, user, options: IOptionsForUpdate = {}): Promise<PageDocument> {
     const Page = mongoose.model('Page') as unknown as PageModel;
     const Page = mongoose.model('Page') as unknown as PageModel;
     const Revision = mongoose.model('Revision') as any; // TODO: TypeScriptize model
     const Revision = mongoose.model('Revision') as any; // TODO: TypeScriptize model
 
 
@@ -4192,6 +4194,9 @@ class PageService {
     const grantUserGroupIds = options.grantUserGroupIds || pageData.grantUserGroupIds; // use the previous data if absence
     const grantUserGroupIds = options.grantUserGroupIds || pageData.grantUserGroupIds; // use the previous data if absence
     const isSyncRevisionToHackmd = options.isSyncRevisionToHackmd;
     const isSyncRevisionToHackmd = options.isSyncRevisionToHackmd;
 
 
+    // TODO 136137: validate multiple group grant before save using pageData and options
+    await this.pageGrantService.validateGrantChange(user, pageData.grantedGroups, grant, grantUserGroupIds);
+
     await this.validateAppliedScope(user, grant, grantUserGroupIds);
     await this.validateAppliedScope(user, grant, grantUserGroupIds);
     pageData.applyScope(user, grant, grantUserGroupIds);
     pageData.applyScope(user, grant, grantUserGroupIds);
 
 

+ 68 - 1
apps/app/test/integration/service/page-grant.test.js

@@ -33,6 +33,7 @@ describe('PageGrantService', () => {
 
 
   let groupParent;
   let groupParent;
   let groupChild;
   let groupChild;
+  let differentTreeGroup;
 
 
   let externalGroupParent;
   let externalGroupParent;
   let externalGroupChild;
   let externalGroupChild;
@@ -51,10 +52,13 @@ describe('PageGrantService', () => {
   const emptyPagePath2 = '/E2';
   const emptyPagePath2 = '/E2';
   const emptyPagePath3 = '/E3';
   const emptyPagePath3 = '/E3';
 
 
+  let multipleGroupTreesAndUsersPage;
+
   let pageRootPublic;
   let pageRootPublic;
   let pageRootGroupParent;
   let pageRootGroupParent;
   const pageRootPublicPath = '/Public';
   const pageRootPublicPath = '/Public';
   const pageRootGroupParentPath = '/GroupParent';
   const pageRootGroupParentPath = '/GroupParent';
+  const pageMultipleGroupTreesAndUsersPath = '/MultipleGroupTreesAndUsers';
 
 
   const v4PageRootOnlyMePagePath = '/v4OnlyMe';
   const v4PageRootOnlyMePagePath = '/v4OnlyMe';
   const v4PageRootAnyoneWithTheLinkPagePath = '/v4AnyoneWithTheLink';
   const v4PageRootAnyoneWithTheLinkPagePath = '/v4AnyoneWithTheLink';
@@ -102,10 +106,15 @@ describe('PageGrantService', () => {
         name: 'GroupChild',
         name: 'GroupChild',
         parent: userGroupIdParent,
         parent: userGroupIdParent,
       },
       },
+      {
+        name: 'DifferentTreeGroup',
+        parent: null,
+      },
     ]);
     ]);
 
 
     groupParent = await UserGroup.findOne({ name: 'GroupParent' });
     groupParent = await UserGroup.findOne({ name: 'GroupParent' });
     groupChild = await UserGroup.findOne({ name: 'GroupChild' });
     groupChild = await UserGroup.findOne({ name: 'GroupChild' });
+    differentTreeGroup = await UserGroup.findOne({ name: 'DifferentTreeGroup' });
 
 
     // UserGroupRelations
     // UserGroupRelations
     await UserGroupRelation.insertMany([
     await UserGroupRelation.insertMany([
@@ -121,6 +130,10 @@ describe('PageGrantService', () => {
         relatedGroup: groupChild._id,
         relatedGroup: groupChild._id,
         relatedUser: user1._id,
         relatedUser: user1._id,
       },
       },
+      {
+        relatedGroup: differentTreeGroup._id,
+        relatedUser: user1._id,
+      },
     ]);
     ]);
 
 
     await ExternalUserGroup.insertMany([
     await ExternalUserGroup.insertMany([
@@ -198,8 +211,19 @@ describe('PageGrantService', () => {
         grantedGroups: [{ item: groupParent._id, type: GroupType.userGroup }, { item: externalGroupParent._id, type: GroupType.externalUserGroup }],
         grantedGroups: [{ item: groupParent._id, type: GroupType.userGroup }, { item: externalGroupParent._id, type: GroupType.externalUserGroup }],
         parent: rootPage._id,
         parent: rootPage._id,
       },
       },
+      {
+        path: pageMultipleGroupTreesAndUsersPath,
+        grant: Page.GRANT_USER_GROUP,
+        creator: user1,
+        lastUpdateUser: user1,
+        grantedUsers: null,
+        grantedGroups: [{ item: groupParent._id, type: GroupType.userGroup }, { item: differentTreeGroup._id, type: GroupType.userGroup }],
+        parent: null,
+      },
     ]);
     ]);
 
 
+    multipleGroupTreesAndUsersPage = await Page.findOne({ path: pageMultipleGroupTreesAndUsersPath });
+
     await Page.insertMany([
     await Page.insertMany([
       // Root Page
       // Root Page
       {
       {
@@ -505,7 +529,7 @@ describe('PageGrantService', () => {
       expect(result).toBe(true);
       expect(result).toBe(true);
     });
     });
 
 
-    test('Should return false when Target: owned by UserA, Descendant: public', async() => {
+    test('Should return false when Target: owned by User1, Descendant: public', async() => {
       const targetPath = emptyPagePath1;
       const targetPath = emptyPagePath1;
       const grant = Page.GRANT_OWNER;
       const grant = Page.GRANT_OWNER;
       const grantedUserIds = [user1._id];
       const grantedUserIds = [user1._id];
@@ -518,6 +542,49 @@ describe('PageGrantService', () => {
     });
     });
   });
   });
 
 
+  describe('Test isGrantNormalized method with previousGrantedGroupIds given', () => {
+    test('Should return true when Target: completely owned by User1 (belongs to all groups)', async() => {
+      const targetPath = pageMultipleGroupTreesAndUsersPath;
+      const grant = Page.GRANT_PUBLIC;
+      const grantedUserIds = null;
+      const grantedGroupIds = null;
+      const shouldCheckDescendants = false;
+
+      const result = await pageGrantService.isGrantNormalized(
+        user1, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants, false, multipleGroupTreesAndUsersPage.grantedGroups,
+      );
+
+      expect(result).toBe(true);
+    });
+
+    test('Should return false when Target: partially owned by User2 (belongs to one of the groups), and change to public grant', async() => {
+      const targetPath = pageMultipleGroupTreesAndUsersPath;
+      const grant = Page.GRANT_PUBLIC;
+      const grantedUserIds = null;
+      const grantedGroupIds = null;
+      const shouldCheckDescendants = false;
+
+      const result = await pageGrantService.isGrantNormalized(
+        user2, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants, false, multipleGroupTreesAndUsersPage.grantedGroups,
+      );
+
+      expect(result).toBe(false);
+    });
+
+    test('Should return false when Target: partially owned by User2, and change to group grant without any groups of user2', async() => {
+      const targetPath = pageMultipleGroupTreesAndUsersPath;
+      const grant = Page.GRANT_USER_GROUP;
+      const grantedUserIds = null;
+      const grantedGroupIds = [{ item: differentTreeGroup._id, type: GroupType.userGroup }];
+      const shouldCheckDescendants = false;
+
+      const result = await pageGrantService.isGrantNormalized(
+        user2, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants, false, multipleGroupTreesAndUsersPage.grantedGroups,
+      );
+
+      expect(result).toBe(false);
+    });
+  });
 
 
   describe('Test for calcApplicableGrantData', () => {
   describe('Test for calcApplicableGrantData', () => {
     test('Only Public is Applicable in case of top page', async() => {
     test('Only Public is Applicable in case of top page', async() => {

+ 0 - 5
packages/core/src/interfaces/page.ts

@@ -64,11 +64,6 @@ export const PageGrant = {
 type UnionPageGrantKeys = keyof typeof PageGrant;
 type UnionPageGrantKeys = keyof typeof PageGrant;
 export type PageGrant = typeof PageGrant[UnionPageGrantKeys];
 export type PageGrant = typeof PageGrant[UnionPageGrantKeys];
 
 
-/**
- * Neither pages with grant `GRANT_RESTRICTED` nor `GRANT_SPECIFIED` can be on a page tree.
- */
-export type PageGrantCanBeOnTree = typeof PageGrant[Exclude<UnionPageGrantKeys, 'GRANT_RESTRICTED' | 'GRANT_SPECIFIED'>];
-
 export const PageStatus = {
 export const PageStatus = {
   STATUS_PUBLISHED: 'published',
   STATUS_PUBLISHED: 'published',
   STATUS_DELETED: 'deleted',
   STATUS_DELETED: 'deleted',