Procházet zdrojové kódy

Merge pull request #7928 from weseek/fix-page-service-and-page-grant-service

Fix page service and page grant service
Yuki Takei před 2 roky
rodič
revize
245b9718de

+ 1 - 1
apps/app/src/components/Sidebar/PageTree/Item.tsx

@@ -354,7 +354,7 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
         path: newPagePath,
         body: undefined,
         grant: page.grant,
-        grantUserGroupId: page.grantedGroup,
+        grantUserGroupIds: page.grantedGroups,
       });
 
       mutateChildren();

+ 4 - 0
apps/app/src/features/external-user-group/server/models/external-user-group-relation.ts

@@ -32,4 +32,8 @@ schema.statics.findAllRelation = UserGroupRelation.findAllRelation;
 
 schema.statics.removeAllInvalidRelations = UserGroupRelation.removeAllInvalidRelations;
 
+schema.statics.findGroupsWithDescendantsByGroupAndUser = UserGroupRelation.findGroupsWithDescendantsByGroupAndUser;
+
+schema.statics.countByGroupIdAndUser = UserGroupRelation.countByGroupIdAndUser;
+
 export default getOrCreateModel<ExternalUserGroupRelationDocument, ExternalUserGroupRelationModel>('ExternalUserGroupRelation', schema);

+ 2 - 0
apps/app/src/features/external-user-group/server/models/external-user-group.ts

@@ -57,4 +57,6 @@ schema.statics.findGroupsWithAncestorsRecursively = UserGroup.findGroupsWithAnce
 
 schema.statics.findGroupsWithDescendantsRecursively = UserGroup.findGroupsWithDescendantsRecursively;
 
+schema.statics.findGroupsWithDescendantsById = UserGroup.findGroupsWithDescendantsById;
+
 export default getOrCreateModel<ExternalUserGroupDocument, ExternalUserGroupModel>('ExternalUserGroup', schema);

+ 4 - 13
apps/app/src/server/models/interfaces/page-operation.ts

@@ -1,4 +1,4 @@
-import { GroupType } from '@growi/core';
+import { GrantedGroup } from '@growi/core';
 
 import { PageGrant } from '~/interfaces/page';
 
@@ -11,10 +11,7 @@ export type IPageForResuming = {
   parent?: ObjectIdLike,
   grant?: number,
   grantedUsers?: ObjectIdLike[],
-  grantedGroups: {
-    type: GroupType,
-    item: ObjectIdLike,
-  }[],
+  grantedGroups: GrantedGroup[],
   descendantCount: number,
   status?: number,
   revision?: ObjectIdLike,
@@ -28,20 +25,14 @@ export type IUserForResuming = {
 
 export type IOptionsForUpdate = {
   grant?: PageGrant,
-  grantUserGroupIds?: {
-    type: GroupType,
-    item: ObjectIdLike,
-  }[],
+  grantUserGroupIds?: GrantedGroup[],
   isSyncRevisionToHackmd?: boolean,
   overwriteScopesOfDescendants?: boolean,
 };
 
 export type IOptionsForCreate = {
   format?: string,
-  grantUserGroupIds?: {
-    type: GroupType,
-    item: ObjectIdLike,
-  }[],
+  grantUserGroupIds?: GrantedGroup[],
   grant?: PageGrant,
   overwriteScopesOfDescendants?: boolean,
   isSynchronously?: boolean,

+ 2 - 2
apps/app/src/server/models/obsolete-page.js

@@ -249,7 +249,7 @@ export const getPageSchema = (crowi) => {
     return this.populate('revision');
   };
 
-  pageSchema.methods.applyScope = function(user, grant, grantUserGroupId) {
+  pageSchema.methods.applyScope = function(user, grant, grantUserGroupIds) {
     // Reset
     this.grantedUsers = [];
     this.grantedGroup = null;
@@ -261,7 +261,7 @@ export const getPageSchema = (crowi) => {
     }
 
     if (grant === GRANT_USER_GROUP) {
-      this.grantedGroup = grantUserGroupId;
+      this.grantedGroups = grantUserGroupIds;
     }
   };
 

+ 5 - 2
apps/app/src/server/models/page-operation.ts

@@ -1,3 +1,4 @@
+import { GroupType } from '@growi/core';
 import { addSeconds } from 'date-fns';
 import mongoose, {
   Schema, Model, Document, QueryOptions, FilterQuery,
@@ -62,8 +63,9 @@ const pageSchemaForResuming = new Schema<IPageForResuming>({
   grantedGroups: [{
     type: {
       type: String,
-      enum: ['UserGroup', 'ExternalUserGroup'],
+      enum: Object.values(GroupType),
       required: true,
+      default: 'UserGroup',
     },
     item: {
       type: ObjectId, refPath: 'grantedGroups.type', required: true,
@@ -85,8 +87,9 @@ const optionsSchemaForResuming = new Schema<IOptionsForResuming>({
   grantUserGroupIds: [{
     type: {
       type: String,
-      enum: ['UserGroup', 'ExternalUserGroup'],
+      enum: Object.values(GroupType),
       required: true,
+      default: 'UserGroup',
     },
     item: {
       type: ObjectId, refPath: 'grantedGroups.type', required: true,

+ 7 - 3
apps/app/src/server/models/page.ts

@@ -2,7 +2,10 @@
 
 import nodePath from 'path';
 
-import { HasObjectId, pagePathUtils, pathUtils } from '@growi/core';
+import {
+  GrantedGroup,
+  GroupType, HasObjectId, pagePathUtils, pathUtils,
+} from '@growi/core';
 import { collectAncestorPaths } from '@growi/core/dist/utils/page-path-utils/collect-ancestor-paths';
 import escapeStringRegexp from 'escape-string-regexp';
 import mongoose, {
@@ -100,8 +103,9 @@ const schema = new Schema<PageDocument, PageModel>({
   grantedGroups: [{
     type: {
       type: String,
-      enum: ['UserGroup', 'ExternalUserGroup'],
+      enum: Object.values(GroupType),
       required: true,
+      default: 'UserGroup',
     },
     item: {
       type: ObjectId, refPath: 'grantedGroups.type', required: true, index: true,
@@ -971,7 +975,7 @@ schema.statics.findNonEmptyClosestAncestor = async function(path: string): Promi
 
 export type PageCreateOptions = {
   format?: string
-  grantUserGroupId?: ObjectIdLike
+  grantUserGroupIds?: GrantedGroup[],
   grant?: number
   overwriteScopesOfDescendants?: boolean
 }

+ 2 - 2
apps/app/src/server/models/user-group-relation.ts

@@ -310,7 +310,7 @@ schema.statics.createByGroupIdsAndUserIds = async function(groupIds, userIds) {
 /**
  * Recursively finds descendant groups by populating relations.
  * @static
- * @param {UserGroupDocument[]} groups
+ * @param {UserGroupDocument} group
  * @param {UserDocument} user
  * @returns UserGroupDocument[]
  */
@@ -328,7 +328,7 @@ schema.statics.findGroupsWithDescendantsByGroupAndUser = async function(group, u
       },
       {
         $lookup: {
-          from: 'usergroups',
+          from: this.collection.collectionName,
           localField: 'relatedGroup',
           foreignField: '_id',
           as: 'relatedGroup',

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

@@ -1,26 +1,31 @@
 import {
+  GrantedGroup,
   pagePathUtils, pathUtils, pageUtils,
   PageGrant, PageGrantCanBeOnTree,
 } from '@growi/core';
+import { et } from 'date-fns/locale';
 import escapeStringRegexp from 'escape-string-regexp';
 import mongoose from 'mongoose';
 
+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 { IRecordApplicableGrant } from '~/interfaces/page-grant';
 import { PageDocument, PageModel } from '~/server/models/page';
 import UserGroup from '~/server/models/user-group';
-import { isIncludesObjectId, excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
+import { includesObjectIds, excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
+
+import { ObjectIdLike } from '../interfaces/mongoose-utils';
+import { divideByType } from '../util/granted-group';
 
 const { addTrailingSlash } = pathUtils;
 const { isTopPage } = pagePathUtils;
 
 const LIMIT_FOR_MULTIPLE_PAGE_OP = 20;
 
-type ObjectIdLike = mongoose.Types.ObjectId | string;
-
 type ComparableTarget = {
   grant: number,
   grantedUserIds?: ObjectIdLike[],
-  grantedGroupId?: ObjectIdLike,
+  grantedGroupIds?: GrantedGroup[],
   applicableUserIds?: ObjectIdLike[],
   applicableGroupIds?: ObjectIdLike[],
 };
@@ -35,7 +40,7 @@ type ComparableAncestor = {
 type ComparableDescendants = {
   isPublicExist: boolean,
   grantedUserIds: ObjectIdLike[],
-  grantedGroupIds: ObjectIdLike[],
+  grantedGroupIds: GrantedGroup[],
 };
 
 /**
@@ -59,7 +64,7 @@ type UpdateGrantInfo = {
 type DescendantPagesGrantInfo = {
   grantSet: Set<number>,
   grantedUserIds: Set<ObjectIdLike>, // all only me users of descendant pages
-  grantedUserGroupIds: Set<ObjectIdLike>, // all user groups of descendant pages
+  grantedUserGroupIds: Set<GrantedGroup>, // all user groups of descendant pages
 };
 
 /**
@@ -82,13 +87,13 @@ class PageGrantService {
   private validateComparableTarget(comparable: ComparableTarget) {
     const Page = mongoose.model('Page') as unknown as PageModel;
 
-    const { grant, grantedUserIds, grantedGroupId } = comparable;
+    const { grant, grantedUserIds, grantedGroupIds } = comparable;
 
     if (grant === Page.GRANT_OWNER && (grantedUserIds == null || grantedUserIds.length !== 1)) {
       throw Error('grantedUserIds must not be null and must have 1 length');
     }
-    if (grant === Page.GRANT_USER_GROUP && grantedGroupId == null) {
-      throw Error('grantedGroupId is not specified');
+    if (grant === Page.GRANT_USER_GROUP && grantedGroupIds == null) {
+      throw Error('grantedGroupIds is not specified');
     }
   }
 
@@ -137,17 +142,17 @@ class PageGrantService {
           throw Error('grantedUserIds must have one user');
         }
 
-        if (!isIncludesObjectId(ancestor.applicableUserIds, target.grantedUserIds[0])) { // GRANT_OWNER pages under GRAND_USER_GROUP page must be owned by the member of the grantedGroup of the GRAND_USER_GROUP page
+        if (!includesObjectIds(ancestor.applicableUserIds, [target.grantedUserIds[0]])) { // GRANT_OWNER pages under GRAND_USER_GROUP page must be owned by the member of the grantedGroup of the GRAND_USER_GROUP page
           return false;
         }
       }
 
       if (target.grant === Page.GRANT_USER_GROUP) {
-        if (target.grantedGroupId == null) {
-          throw Error('grantedGroupId must not be null');
+        if (target.grantedGroupIds == null || target.grantedGroupIds.length === 0) {
+          throw Error('grantedGroupId must not be empty');
         }
-
-        if (!isIncludesObjectId(ancestor.applicableGroupIds, target.grantedGroupId)) { // only child groups or the same group can exist under GRANT_USER_GROUP page
+        const targetGrantedGroupStrIds = target.grantedGroupIds.map(e => (typeof e.item === 'string' ? e.item : e.item._id));
+        if (!includesObjectIds(ancestor.applicableGroupIds, targetGrantedGroupStrIds)) { // only child groups or the same group can exist under GRANT_USER_GROUP page
           return false;
         }
       }
@@ -192,7 +197,7 @@ class PageGrantService {
         return false;
       }
 
-      const shouldNotExistGroupIds = excludeTestIdsFromTargetIds(descendants.grantedGroupIds, target.applicableGroupIds);
+      const shouldNotExistGroupIds = excludeTestIdsFromTargetIds(descendants.grantedGroupIds.map(g => g.item), target.applicableGroupIds);
       const shouldNotExistUserIds = excludeTestIdsFromTargetIds(descendants.grantedUserIds, target.applicableUserIds);
       if (shouldNotExistGroupIds.length !== 0 || shouldNotExistUserIds.length !== 0) {
         return false;
@@ -207,7 +212,7 @@ class PageGrantService {
    * @returns Promise<ComparableAncestor>
    */
   private async generateComparableTarget(
-      grant, grantedUserIds: ObjectIdLike[] | undefined, grantedGroupId: ObjectIdLike | undefined, includeApplicable: boolean,
+      grant, grantedUserIds: ObjectIdLike[] | undefined, grantedGroupIds: GrantedGroup[] | undefined, includeApplicable: boolean,
   ): Promise<ComparableTarget> {
     if (includeApplicable) {
       const Page = mongoose.model('Page') as unknown as PageModel;
@@ -217,22 +222,34 @@ class PageGrantService {
       let applicableGroupIds: ObjectIdLike[] | undefined;
 
       if (grant === Page.GRANT_USER_GROUP) {
-        const targetUserGroup = await UserGroup.findOne({ _id: grantedGroupId });
-        if (targetUserGroup == null) {
-          throw Error('Target user group does not exist');
+        if (grantedGroupIds == null || grantedGroupIds.length === 0) {
+          throw Error('Target user group is not given');
         }
 
-        const relatedUsers = await UserGroupRelation.find({ relatedGroup: targetUserGroup._id });
-        applicableUserIds = relatedUsers.map(u => u.relatedUser);
+        const { grantedUserGroups: grantedUserGroupIds, grantedExternalUserGroups: grantedExternalUserGroupIds } = divideByType(grantedGroupIds);
+        const targetUserGroups = await UserGroup.find({ _id: { $in: grantedUserGroupIds } });
+        const targetExternalUserGroups = await ExternalUserGroup.find({ _id: { $in: grantedExternalUserGroupIds } });
+        if (targetUserGroups.length === 0 && targetExternalUserGroups.length === 0) {
+          throw Error('Target user group does not exist');
+        }
 
-        const applicableGroups = grantedGroupId != null ? await UserGroup.findGroupsWithDescendantsById(grantedGroupId) : null;
-        applicableGroupIds = applicableGroups?.map(g => g._id) || null;
+        const userGroupRelations = await UserGroupRelation.find({ relatedGroup: { $in: targetUserGroups.map(g => g._id) } });
+        const externalUserGroupRelations = await ExternalUserGroupRelation.find({ relatedGroup: { $in: targetExternalUserGroups.map(g => g._id) } });
+        applicableUserIds = Array.from(new Set([...userGroupRelations, ...externalUserGroupRelations].map(u => u.relatedUser)));
+
+        const applicableUserGroups = (await Promise.all(targetUserGroups.map((group) => {
+          return UserGroup.findGroupsWithDescendantsById(group._id);
+        }))).flat();
+        const applicableExternalUserGroups = (await Promise.all(targetExternalUserGroups.map((group) => {
+          return ExternalUserGroup.findGroupsWithDescendantsById(group._id);
+        }))).flat();
+        applicableGroupIds = [...applicableUserGroups, ...applicableExternalUserGroups].map(g => g._id);
       }
 
       return {
         grant,
         grantedUserIds,
-        grantedGroupId,
+        grantedGroupIds,
         applicableUserIds,
         applicableGroupIds,
       };
@@ -241,7 +258,7 @@ class PageGrantService {
     return {
       grant,
       grantedUserIds,
-      grantedGroupId,
+      grantedGroupIds,
     };
   }
 
@@ -277,10 +294,20 @@ class PageGrantService {
 
     if (testAncestor.grant === Page.GRANT_USER_GROUP) {
       // make a set of all users
-      const grantedRelations = await UserGroupRelation.find({ relatedGroup: testAncestor.grantedGroup }, { _id: 0, relatedUser: 1 });
-      const grantedGroups = await UserGroup.findGroupsWithDescendantsById(testAncestor.grantedGroup);
-      applicableGroupIds = grantedGroups.map(g => g._id);
-      applicableUserIds = Array.from(new Set(grantedRelations.map(r => r.relatedUser))) as ObjectIdLike[];
+      const { grantedUserGroups, grantedExternalUserGroups } = divideByType(testAncestor.grantedGroups);
+
+      const userGroupRelations = await UserGroupRelation.find({ relatedGroup: { $in: grantedUserGroups } }, { _id: 0, relatedUser: 1 });
+      const externalUserGroupRelations = await ExternalUserGroupRelation.find({ relatedGroup: { $in: grantedExternalUserGroups } }, { _id: 0, relatedUser: 1 });
+      applicableUserIds = Array.from(new Set([...userGroupRelations, ...externalUserGroupRelations].map(r => r.relatedUser)));
+
+      const applicableUserGroups = (await Promise.all(grantedUserGroups.map((groupId) => {
+        return UserGroup.findGroupsWithDescendantsById(groupId);
+      }))).flat();
+      const applicableExternalUserGroups = (await Promise.all(grantedExternalUserGroups.map((groupId) => {
+        return ExternalUserGroup.findGroupsWithDescendantsById(groupId);
+      }))).flat();
+      applicableGroupIds = [...applicableUserGroups, ...applicableExternalUserGroups].map(g => g._id);
+
     }
 
     return {
@@ -341,13 +368,13 @@ class PageGrantService {
           _id: 0,
           grant: 1,
           grantedUsers: 1,
-          grantedGroup: 1,
+          grantedGroups: 1,
         },
       },
       { // remove duplicates from pipeline
         $group: {
           _id: '$grant',
-          grantedGroupSet: { $addToSet: '$grantedGroup' },
+          grantedGroupsSet: { $addToSet: '$grantedGroups' },
           grantedUsersSet: { $addToSet: '$grantedUsers' },
         },
       },
@@ -356,6 +383,11 @@ class PageGrantService {
           path: '$grantedUsersSet',
         },
       },
+      { // flatten granted group set
+        $unwind: {
+          path: '$grantedGroupsSet',
+        },
+      },
     ]);
 
     // GRANT_PUBLIC group
@@ -365,7 +397,7 @@ class PageGrantService {
     const grantedUserIds: ObjectIdLike[] = grantOwnerResult?.grantedUsersSet ?? [];
     // GRANT_USER_GROUP group
     const grantUserGroupResult = result.filter(r => r._id === Page.GRANT_USER_GROUP)[0]; // users of GRANT_OWNER
-    const grantedGroupIds = grantUserGroupResult?.grantedGroupSet ?? [];
+    const grantedGroupIds = grantUserGroupResult?.grantedGroupsSet ?? [];
 
     return {
       isPublicExist,
@@ -383,7 +415,7 @@ class PageGrantService {
    */
   async isGrantNormalized(
       // eslint-disable-next-line max-len
-      user, targetPath: string, grant, grantedUserIds?: ObjectIdLike[], grantedGroupId?: ObjectIdLike, shouldCheckDescendants = false, includeNotMigratedPages = false,
+      user, targetPath: string, grant, grantedUserIds?: ObjectIdLike[], grantedGroupIds?: GrantedGroup[], shouldCheckDescendants = false, includeNotMigratedPages = false,
   ): Promise<boolean> {
     if (isTopPage(targetPath)) {
       return true;
@@ -392,11 +424,11 @@ class PageGrantService {
     const comparableAncestor = await this.generateComparableAncestor(targetPath, includeNotMigratedPages);
 
     if (!shouldCheckDescendants) { // checking the parent is enough
-      const comparableTarget = await this.generateComparableTarget(grant, grantedUserIds, grantedGroupId, false);
+      const comparableTarget = await this.generateComparableTarget(grant, grantedUserIds, grantedGroupIds, false);
       return this.processValidation(comparableTarget, comparableAncestor);
     }
 
-    const comparableTarget = await this.generateComparableTarget(grant, grantedUserIds, grantedGroupId, true);
+    const comparableTarget = await this.generateComparableTarget(grant, grantedUserIds, grantedGroupIds, true);
     const comparableDescendants = await this.generateComparableDescendants(targetPath, user, includeNotMigratedPages);
 
     return this.processValidation(comparableTarget, comparableAncestor, comparableDescendants);
@@ -421,7 +453,7 @@ class PageGrantService {
 
     for await (const page of pages) {
       const {
-        path, grant, grantedUsers: grantedUserIds, grantedGroup: grantedGroupId,
+        path, grant, grantedUsers: grantedUserIds, grantedGroups: grantedGroupIds,
       } = page;
 
       if (!pageUtils.isPageNormalized(page)) {
@@ -429,7 +461,7 @@ class PageGrantService {
         continue;
       }
 
-      if (await this.isGrantNormalized(user, path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants, shouldIncludeNotMigratedPages)) {
+      if (await this.isGrantNormalized(user, path, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants, shouldIncludeNotMigratedPages)) {
         normalizable.push(page);
       }
       else {
@@ -473,7 +505,7 @@ class PageGrantService {
     }
 
     const {
-      grant, grantedUsers, grantedGroup,
+      grant, grantedUsers, grantedGroups,
     } = parent;
 
     if (grant === PageGrant.GRANT_PUBLIC) {
@@ -491,14 +523,28 @@ class PageGrantService {
       }
     }
     else if (grant === PageGrant.GRANT_USER_GROUP) {
-      const group = await UserGroup.findById(grantedGroup);
-      if (group == null) {
+      const { grantedUserGroups: grantedUserGroupIds, grantedExternalUserGroups: grantedExternalUserGroupIds } = divideByType(grantedGroups);
+      const targetUserGroups = await UserGroup.find({ _id: { $in: grantedUserGroupIds } });
+      const targetExternalUserGroups = await ExternalUserGroup.find({ _id: { $in: grantedExternalUserGroupIds } });
+      if (targetUserGroups.length === 0 && targetExternalUserGroups.length === 0) {
         throw Error('Group not found to calculate grant data.');
       }
 
-      const applicableGroups = await UserGroupRelation.findGroupsWithDescendantsByGroupAndUser(group, user);
-
-      const isUserExistInGroup = await UserGroupRelation.countByGroupIdAndUser(group, user) > 0;
+      const applicableUserGroups = (await Promise.all(targetUserGroups.map((group) => {
+        return UserGroupRelation.findGroupsWithDescendantsByGroupAndUser(group, user);
+      }))).flat();
+      const applicableExternalUserGroups = (await Promise.all(targetExternalUserGroups.map((group) => {
+        return ExternalUserGroupRelation.findGroupsWithDescendantsByGroupAndUser(group, user);
+      }))).flat();
+      const applicableGroups = [...applicableUserGroups, ...applicableExternalUserGroups];
+
+      const isUserExistInUserGroup = (await Promise.all(targetUserGroups.map((group) => {
+        return UserGroupRelation.countByGroupIdAndUser(group, user);
+      }))).some(count => count > 0);
+      const isUserExistInExternalUserGroup = (await Promise.all(targetExternalUserGroups.map((group) => {
+        return ExternalUserGroupRelation.countByGroupIdAndUser(group, user);
+      }))).some(count => count > 0);
+      const isUserExistInGroup = isUserExistInUserGroup || isUserExistInExternalUserGroup;
 
       if (isUserExistInGroup) {
         data[PageGrant.GRANT_OWNER] = null;
@@ -589,7 +635,7 @@ class PageGrantService {
   private calcIsAllDescendantsGrantedByOperator(operatorGrantInfo: OperatorGrantInfo, descendantPagesGrantInfo: DescendantPagesGrantInfo): boolean {
     if (descendantPagesGrantInfo.grantSet.has(PageGrant.GRANT_OWNER)) {
       const isNonApplicableOwnerExist = descendantPagesGrantInfo.grantedUserIds.size >= 2
-        || !isIncludesObjectId([...descendantPagesGrantInfo.grantedUserIds], operatorGrantInfo.userId);
+        || !includesObjectIds([...descendantPagesGrantInfo.grantedUserIds], [operatorGrantInfo.userId]);
       if (isNonApplicableOwnerExist) {
         return false;
       }

+ 40 - 34
apps/app/src/server/service/page.ts

@@ -3,7 +3,7 @@ import { Readable, Writable } from 'stream';
 
 import type {
   Ref, HasObjectId, IUserHasId,
-  IPage, IPageInfo, IPageInfoAll, IPageInfoForEntity, IPageWithMeta,
+  IPage, IPageInfo, IPageInfoAll, IPageInfoForEntity, IPageWithMeta, GrantedGroup,
 } from '@growi/core';
 import {
   pagePathUtils, pathUtils, PageGrant, PageStatus,
@@ -452,7 +452,7 @@ class PageService {
     // use the parent's grant when target page is an empty page
     let grant;
     let grantedUserIds;
-    let grantedGroupId;
+    let grantedGroupIds;
     if (page.isEmpty) {
       const parent = await Page.findOne({ _id: page.parent });
       if (parent == null) {
@@ -460,18 +460,18 @@ class PageService {
       }
       grant = parent.grant;
       grantedUserIds = parent.grantedUsers;
-      grantedGroupId = parent.grantedGroup;
+      grantedGroupIds = parent.grantedGroups;
     }
     else {
       grant = page.grant;
       grantedUserIds = page.grantedUsers;
-      grantedGroupId = page.grantedGroup;
+      grantedGroupIds = page.grantedGroups;
     }
 
     if (grant !== Page.GRANT_RESTRICTED) {
       let isGrantNormalized = false;
       try {
-        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, newPagePath, grant, grantedUserIds, grantedGroupId, false);
+        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, newPagePath, grant, grantedUserIds, grantedGroupIds, false);
       }
       catch (err) {
         logger.error(`Failed to validate grant of page at "${newPagePath}" when renaming`, err);
@@ -959,7 +959,7 @@ class PageService {
     // use the parent's grant when target page is an empty page
     let grant;
     let grantedUserIds;
-    let grantedGroupId;
+    let grantedGroupIds;
     if (page.isEmpty) {
       const parent = await Page.findOne({ _id: page.parent });
       if (parent == null) {
@@ -967,18 +967,18 @@ class PageService {
       }
       grant = parent.grant;
       grantedUserIds = parent.grantedUsers;
-      grantedGroupId = parent.grantedGroup;
+      grantedGroupIds = parent.grantedGroups;
     }
     else {
       grant = page.grant;
       grantedUserIds = page.grantedUsers;
-      grantedGroupId = page.grantedGroup;
+      grantedGroupIds = page.grantedGroups;
     }
 
     if (grant !== Page.GRANT_RESTRICTED) {
       let isGrantNormalized = false;
       try {
-        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, newPagePath, grant, grantedUserIds, grantedGroupId, false);
+        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, newPagePath, grant, grantedUserIds, grantedGroupIds, false);
       }
       catch (err) {
         logger.error(`Failed to validate grant of page at "${newPagePath}" when duplicating`, err);
@@ -995,7 +995,7 @@ class PageService {
     // 3. Duplicate target
     const options: PageCreateOptions = {
       grant: page.grant,
-      grantUserGroupId: page.grantedGroup,
+      grantUserGroupIds: page.grantedGroups,
     };
     let duplicatedTarget;
     if (page.isEmpty) {
@@ -1107,7 +1107,7 @@ class PageService {
     // create option
     const options: any = { page };
     options.grant = page.grant;
-    options.grantUserGroupId = page.grantedGroup;
+    options.grantUserGroupIds = page.grantedGroups;
     options.grantedUserIds = page.grantedUsers;
 
     newPagePath = this.crowi.xss.process(newPagePath); // eslint-disable-line no-param-reassign
@@ -1208,7 +1208,7 @@ class PageService {
           path: newPagePath,
           creator: user._id,
           grant: page.grant,
-          grantedGroup: page.grantedGroup,
+          grantedGroups: page.grantedGroups,
           grantedUsers: page.grantedUsers,
           lastUpdateUser: user._id,
           revision: revisionId,
@@ -1254,7 +1254,7 @@ class PageService {
         path: newPagePath,
         creator: user._id,
         grant: page.grant,
-        grantedGroup: page.grantedGroup,
+        grantedGroups: page.grantedGroups,
         grantedUsers: page.grantedUsers,
         lastUpdateUser: user._id,
         revision: revisionId,
@@ -2268,7 +2268,13 @@ class PageService {
 
   async handlePrivatePagesForGroupsToDelete(groupsToDelete, action, transferToUserGroupId, user) {
     const Page = this.crowi.model('Page');
-    const pages = await Page.find({ grantedGroup: { $in: groupsToDelete } });
+    const pages = await Page.find({
+      grantedGroups: {
+        $elemMatch: {
+          item: { $in: groupsToDelete },
+        },
+      },
+    });
 
     switch (action) {
       case 'public':
@@ -2439,7 +2445,7 @@ class PageService {
 
       const options: PageCreateOptions & { grantedUsers?: ObjectIdLike[] | undefined } = {
         grant: notEmptyParent.grant,
-        grantUserGroupId: notEmptyParent.grantedGroup,
+        grantUserGroupIds: notEmptyParent.grantedGroups,
         grantedUsers: notEmptyParent.grantedUsers,
       };
 
@@ -2456,7 +2462,7 @@ class PageService {
 
     const grant = page.grant;
     const grantedUserIds = page.grantedUsers;
-    const grantedGroupId = page.grantedGroup;
+    const grantedGroupIds = page.grantedGroups;
 
     /*
      * UserGroup & Owner validation
@@ -2465,7 +2471,7 @@ class PageService {
     try {
       const shouldCheckDescendants = true;
 
-      isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
     }
     catch (err) {
       logger.error(`Failed to validate grant of page at "${path}"`, err);
@@ -2565,7 +2571,7 @@ class PageService {
     const Page = mongoose.model('Page') as unknown as PageModel;
 
     const {
-      path, grant, grantedUsers: grantedUserIds, grantedGroup: grantedGroupId,
+      path, grant, grantedUsers: grantedUserIds, grantedGroups: grantedGroupIds,
     } = page;
 
     // check if any page exists at target path already
@@ -2582,7 +2588,7 @@ class PageService {
       try {
         const shouldCheckDescendants = true;
 
-        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
       }
       catch (err) {
         logger.error(`Failed to validate grant of page at "${path}"`, err);
@@ -3454,7 +3460,7 @@ class PageService {
       grantData: {
         grant: number,
         grantedUserIds?: ObjectIdLike[],
-        grantUserGroupId?: ObjectIdLike,
+        grantUserGroupIds?: GrantedGroup[],
       },
       shouldValidateGrant: boolean,
       user?,
@@ -3477,7 +3483,7 @@ class PageService {
     }
 
     // UserGroup & Owner validation
-    const { grant, grantedUserIds, grantUserGroupId } = grantData;
+    const { grant, grantedUserIds, grantUserGroupIds } = grantData;
     if (shouldValidateGrant) {
       if (user == null) {
         throw Error('user is required to validate grant');
@@ -3489,7 +3495,7 @@ class PageService {
         const isEmptyPageAlreadyExist = await Page.count({ path, isEmpty: true }) > 0;
         const shouldCheckDescendants = isEmptyPageAlreadyExist && !options?.overwriteScopesOfDescendants;
 
-        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantUserGroupId, shouldCheckDescendants);
+        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantUserGroupIds, shouldCheckDescendants);
       }
       catch (err) {
         logger.error(`Failed to validate grant of page at "${path}" of grant ${grant}:`, err);
@@ -3500,7 +3506,7 @@ class PageService {
       }
 
       if (options?.overwriteScopesOfDescendants) {
-        const updateGrantInfo = await this.crowi.pageGrantService.generateUpdateGrantInfoToOverwriteDescendants(user, grant, options.grantUserGroupId);
+        const updateGrantInfo = await this.crowi.pageGrantService.generateUpdateGrantInfoToOverwriteDescendants(user, grant, options.grantUserGroupIds);
         const canOverwriteDescendants = await this.crowi.pageGrantService.canOverwriteDescendants(path, user, updateGrantInfo);
 
         if (!canOverwriteDescendants) {
@@ -3529,13 +3535,13 @@ class PageService {
     // eslint-disable-next-line no-param-reassign
     path = this.crowi.xss.process(path); // sanitize path
     const {
-      format = 'markdown', grantUserGroupId,
+      format = 'markdown', grantUserGroupIds,
     } = options;
     const grant = isTopPage(path) ? Page.GRANT_PUBLIC : options.grant;
     const grantData = {
       grant,
       grantedUserIds: grant === Page.GRANT_OWNER ? [user._id] : undefined,
-      grantUserGroupId,
+      grantUserGroupIds,
     };
 
     const isGrantRestricted = grant === Page.GRANT_RESTRICTED;
@@ -3555,7 +3561,7 @@ class PageService {
     this.setFieldExceptForGrantRevisionParent(page, path, user);
 
     // Apply scope
-    page.applyScope(user, grant, grantUserGroupId);
+    page.applyScope(user, grant, grantUserGroupIds);
 
     // Set parent
     if (isTopPage(path) || isGrantRestricted) { // set parent to null when GRANT_RESTRICTED
@@ -3716,7 +3722,7 @@ class PageService {
     path = this.crowi.xss.process(path); // sanitize path
 
     const {
-      format = 'markdown', grantUserGroupId, grantedUsers,
+      format = 'markdown', grantUserGroupIds, grantedUsers,
     } = options;
     const grant = isTopPage(path) ? Page.GRANT_PUBLIC : options.grant;
 
@@ -3726,7 +3732,7 @@ class PageService {
     const grantData = {
       grant,
       grantedUserIds: isGrantOwner ? grantedUsers : undefined,
-      grantUserGroupId,
+      grantUserGroupIds,
     };
 
     // Validate
@@ -3746,7 +3752,7 @@ class PageService {
     this.setFieldExceptForGrantRevisionParent(page, path);
 
     // Apply scope
-    page.applyScope({ _id: grantedUsers?.[0] }, grant, grantUserGroupId);
+    page.applyScope({ _id: grantedUsers?.[0] }, grant, grantUserGroupIds);
 
     // Set parent
     if (isTopPage(path) || isGrantRestricted) { // set parent to null when GRANT_RESTRICTED
@@ -3786,12 +3792,12 @@ class PageService {
    * @param {UserDocument} user
    * @param options
    */
-  async updateGrant(page, user, grantData: {grant: PageGrant, grantedGroup: ObjectIdLike}): Promise<PageDocument> {
-    const { grant, grantedGroup } = grantData;
+  async updateGrant(page, user, grantData: {grant: PageGrant, grantedGroups: GrantedGroup[]}): Promise<PageDocument> {
+    const { grant, grantedGroups } = grantData;
 
     const options = {
       grant,
-      grantUserGroupId: grantedGroup,
+      grantUserGroupIds: grantedGroups,
       isSyncRevisionToHackmd: false,
     };
 
@@ -3861,7 +3867,7 @@ class PageService {
     const newPageData = pageData;
 
     const grant = options.grant ?? clonedPageData.grant; // use the previous data if absence
-    const grantUserGroupId: undefined | ObjectIdLike = options.grantUserGroupId ?? clonedPageData.grantedGroup?._id.toString();
+    const grantUserGroupId: undefined | ObjectIdLike = options.grantUserGroupIds ?? clonedPageData.grantedGroup?._id.toString();
 
     const grantedUserIds = clonedPageData.grantedUserIds || [user._id];
     const shouldBeOnTree = grant !== PageGrant.GRANT_RESTRICTED;
@@ -3885,7 +3891,7 @@ class PageService {
       }
 
       if (options.overwriteScopesOfDescendants) {
-        const updateGrantInfo = await pageGrantService.generateUpdateGrantInfoToOverwriteDescendants(user, grant, options.grantUserGroupId);
+        const updateGrantInfo = await pageGrantService.generateUpdateGrantInfoToOverwriteDescendants(user, grant, options.grantUserGroupIds);
         const canOverwriteDescendants = await pageGrantService.canOverwriteDescendants(clonedPageData.path, user, updateGrantInfo);
 
         if (!canOverwriteDescendants) {

+ 2 - 2
apps/app/src/server/service/user-group.ts

@@ -3,7 +3,7 @@ import { Model } from 'mongoose';
 import { IUser } from '~/interfaces/user';
 import { ObjectIdLike } from '~/server/interfaces/mongoose-utils';
 import UserGroup, { UserGroupDocument, UserGroupModel } from '~/server/models/user-group';
-import { excludeTestIdsFromTargetIds, isIncludesObjectId } from '~/server/util/compare-objectId';
+import { excludeTestIdsFromTargetIds, includesObjectIds } from '~/server/util/compare-objectId';
 import loggerFactory from '~/utils/logger';
 
 import UserGroupRelation, { UserGroupRelationDocument, UserGroupRelationModel } from '../models/user-group-relation';
@@ -78,7 +78,7 @@ class UserGroupService {
 
     // throw if parent was in self and its descendants
     const descendantsWithTarget = await UserGroup.findGroupsWithDescendantsRecursively([userGroup]);
-    if (isIncludesObjectId(descendantsWithTarget.map(d => d._id), parent._id)) {
+    if (includesObjectIds(descendantsWithTarget.map(d => d._id), [parent._id])) {
       throw Error('It is not allowed to choose parent from descendant groups.');
     }
 

+ 28 - 0
apps/app/src/server/util/compare-objectId.spec.ts

@@ -0,0 +1,28 @@
+import { Types } from 'mongoose';
+
+import { includesObjectIds } from './compare-objectId';
+
+describe('includesObjectIds', () => {
+  const id1 = new Types.ObjectId();
+  const id2 = new Types.ObjectId();
+  const id3 = new Types.ObjectId();
+  const id4 = new Types.ObjectId();
+
+  describe('When subset of array given', () => {
+    const arr = [id1, id2, id3, id4];
+    const subset = [id1, id4];
+
+    it('returns true', () => {
+      expect(includesObjectIds(arr, subset)).toBe(true);
+    });
+  });
+
+  describe('When set that intersects with array given', () => {
+    const arr = [id1, id2, id3];
+    const subset = [id1, id4];
+
+    it('returns false', () => {
+      expect(includesObjectIds(arr, subset)).toBe(false);
+    });
+  });
+});

+ 9 - 3
apps/app/src/server/util/compare-objectId.ts

@@ -5,11 +5,17 @@ import { ObjectIdLike } from '~/server/interfaces/mongoose-utils';
 type IObjectId = mongoose.Types.ObjectId;
 const ObjectId = mongoose.Types.ObjectId;
 
-export const isIncludesObjectId = (arr: ObjectIdLike[], id: ObjectIdLike): boolean => {
+/**
+ * Check if array contains all specified ObjectIds
+ * @param arr array that potentially contains potentialSubset
+ * @param potentialSubset array that is potentially a subset of arr
+ * @returns Whether or not arr includes all elements of potentialSubset
+ */
+export const includesObjectIds = (arr: ObjectIdLike[], potentialSubset: ObjectIdLike[]): boolean => {
   const _arr = arr.map(i => i.toString());
-  const _id = id.toString();
+  const _potentialSubset = potentialSubset.map(i => i.toString());
 
-  return _arr.includes(_id);
+  return _potentialSubset.every(id => _arr.includes(id));
 };
 
 /**

+ 23 - 0
apps/app/src/server/util/granted-group.ts

@@ -0,0 +1,23 @@
+import { GrantedGroup, GroupType } from '@growi/core';
+
+import { ObjectIdLike } from '../interfaces/mongoose-utils';
+
+export const divideByType = (grantedGroups: GrantedGroup[]): {
+  grantedUserGroups: ObjectIdLike[];
+  grantedExternalUserGroups: ObjectIdLike[];
+} => {
+  const grantedUserGroups: ObjectIdLike[] = [];
+  const grantedExternalUserGroups: ObjectIdLike[] = [];
+
+  grantedGroups.forEach((group) => {
+    const id = typeof group.item === 'string' ? group.item : group.item._id;
+    if (group.type === GroupType.userGroup) {
+      grantedUserGroups.push(id);
+    }
+    else {
+      grantedExternalUserGroups.push(id);
+    }
+  });
+
+  return { grantedUserGroups, grantedExternalUserGroups };
+};

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

@@ -144,7 +144,7 @@ describe('PageGrantService', () => {
         creator: user1,
         lastUpdateUser: user1,
         grantedUsers: null,
-        grantedGroup: null,
+        grantedGroups: null,
         parent: rootPage._id,
       },
       {
@@ -153,7 +153,7 @@ describe('PageGrantService', () => {
         creator: user1,
         lastUpdateUser: user1,
         grantedUsers: null,
-        grantedGroup: groupParent._id,
+        grantedGroups: [{ item: groupParent._id, type: 'UserGroup' }],
         parent: rootPage._id,
       },
     ]);
@@ -183,7 +183,7 @@ describe('PageGrantService', () => {
         path: v4PageRootOnlyInsideTheGroupPagePath,
         grant: Page.GRANT_USER_GROUP,
         parent: null,
-        grantedGroup: groupParent._id,
+        grantedGroups: [{ item: groupParent._id, type: 'UserGroup' }],
       },
     ]);
 
@@ -262,7 +262,7 @@ describe('PageGrantService', () => {
         creator: user1,
         lastUpdateUser: user1,
         grantedUsers: null,
-        grantedGroup: null,
+        grantedGroups: null,
         parent: emptyPage1._id,
       },
       {
@@ -271,7 +271,7 @@ describe('PageGrantService', () => {
         creator: user1,
         lastUpdateUser: user1,
         grantedUsers: [user1._id],
-        grantedGroup: null,
+        grantedGroups: null,
         parent: emptyPage2._id,
       },
       {
@@ -280,7 +280,7 @@ describe('PageGrantService', () => {
         creator: user1,
         lastUpdateUser: user1,
         grantedUsers: null,
-        grantedGroup: groupParent._id,
+        grantedGroups: [{ item: groupParent._id, type: 'UserGroup' }],
         parent: emptyPage3._id,
       },
       {
@@ -289,7 +289,7 @@ describe('PageGrantService', () => {
         creator: user1,
         lastUpdateUser: user1,
         grantedUsers: null,
-        grantedGroup: groupChild._id,
+        grantedGroups: [{ item: groupChild._id, type: 'UserGroup' }],
         parent: emptyPage3._id,
       },
       {
@@ -298,7 +298,7 @@ describe('PageGrantService', () => {
         creator: user1,
         lastUpdateUser: user1,
         grantedUsers: [user1._id],
-        grantedGroup: null,
+        grantedGroups: null,
         parent: emptyPage3._id,
       },
     ]);
@@ -333,10 +333,10 @@ describe('PageGrantService', () => {
       const targetPath = '/NEW';
       const grant = Page.GRANT_PUBLIC;
       const grantedUserIds = null;
-      const grantedGroupId = null;
+      const grantedGroupIds = null;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -345,10 +345,10 @@ describe('PageGrantService', () => {
       const targetPath = '/NEW_GroupParent';
       const grant = Page.GRANT_USER_GROUP;
       const grantedUserIds = null;
-      const grantedGroupId = groupParent._id;
+      const grantedGroupIdš = [{ item: groupParent._id, type: 'UserGroup' }];
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupIdš, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -357,10 +357,10 @@ describe('PageGrantService', () => {
       const targetPath = `${pageRootPublicPath}/NEW`;
       const grant = Page.GRANT_PUBLIC;
       const grantedUserIds = null;
-      const grantedGroupId = null;
+      const grantedGroupIds = null;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -369,10 +369,10 @@ describe('PageGrantService', () => {
       const targetPath = `${pageRootGroupParentPath}/NEW`;
       const grant = Page.GRANT_USER_GROUP;
       const grantedUserIds = null;
-      const grantedGroupId = groupParent._id;
+      const grantedGroupIds = [{ item: groupParent._id, type: 'UserGroup' }];
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -381,10 +381,10 @@ describe('PageGrantService', () => {
       const targetPath = `${pageE1PublicPath}/NEW`;
       const grant = Page.GRANT_PUBLIC;
       const grantedUserIds = null;
-      const grantedGroupId = null;
+      const grantedGroupIds = null;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -393,10 +393,10 @@ describe('PageGrantService', () => {
       const targetPath = `${pageE2User1Path}/NEW`;
       const grant = Page.GRANT_OWNER;
       const grantedUserIds = [user1._id];
-      const grantedGroupId = null;
+      const grantedGroupIds = null;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -405,10 +405,10 @@ describe('PageGrantService', () => {
       const targetPath = `${pageE3GroupParentPath}/NEW`;
       const grant = Page.GRANT_PUBLIC;
       const grantedUserIds = null;
-      const grantedGroupId = null;
+      const grantedGroupIds = null;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
 
       expect(result).toBe(false);
     });
@@ -417,10 +417,10 @@ describe('PageGrantService', () => {
       const targetPath = `${pageE3GroupChildPath}/NEW`;
       const grant = Page.GRANT_USER_GROUP;
       const grantedUserIds = null;
-      const grantedGroupId = groupParent._id;
+      const grantedGroupIds = [{ item: groupParent._id, type: 'UserGroup' }];
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
 
       expect(result).toBe(false);
     });
@@ -431,10 +431,10 @@ describe('PageGrantService', () => {
       const targetPath = emptyPagePath1;
       const grant = Page.GRANT_PUBLIC;
       const grantedUserIds = null;
-      const grantedGroupId = null;
+      const grantedGroupIds = null;
       const shouldCheckDescendants = true;
 
-      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -443,10 +443,10 @@ describe('PageGrantService', () => {
       const targetPath = emptyPagePath2;
       const grant = Page.GRANT_OWNER;
       const grantedUserIds = [user1._id];
-      const grantedGroupId = null;
+      const grantedGroupIds = null;
       const shouldCheckDescendants = true;
 
-      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -455,10 +455,10 @@ describe('PageGrantService', () => {
       const targetPath = emptyPagePath3;
       const grant = Page.GRANT_USER_GROUP;
       const grantedUserIds = null;
-      const grantedGroupId = groupParent._id;
+      const grantedGroupIds = [{ item: groupParent._id, type: 'UserGroup' }];
       const shouldCheckDescendants = true;
 
-      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -467,10 +467,10 @@ describe('PageGrantService', () => {
       const targetPath = emptyPagePath1;
       const grant = Page.GRANT_OWNER;
       const grantedUserIds = [user1._id];
-      const grantedGroupId = null;
+      const grantedGroupIds = null;
       const shouldCheckDescendants = true;
 
-      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupIds, shouldCheckDescendants);
 
       expect(result).toBe(false);
     });

+ 35 - 21
apps/app/test/integration/service/v5.migration.test.js

@@ -37,8 +37,22 @@ describe('V5 page migration', () => {
   const onlyPublic = filter => ({ grant: Page.GRANT_PUBLIC, ...filter });
   const ownedByTestUser1 = filter => ({ grant: Page.GRANT_OWNER, grantedUsers: [testUser1._id], ...filter });
   const root = filter => ({ grantedUsers: [rootUser._id], ...filter });
-  const rootUserGroup = filter => ({ grantedGroup: rootUserGroupId, ...filter });
-  const testUser1Group = filter => ({ grantedGroup: testUser1GroupId, ...filter });
+  const rootUserGroup = filter => ({
+    grantedGroups: {
+      $elemMatch: {
+        item: rootUserGroupId,
+      },
+    },
+    ...filter,
+  });
+  const testUser1Group = filter => ({
+    grantedGroups: {
+      $elemMatch: {
+        item: testUser1GroupId,
+      },
+    },
+    ...filter,
+  });
 
   const normalized = { parent: { $ne: null } };
   const notNormalized = { parent: null };
@@ -160,14 +174,14 @@ describe('V5 page migration', () => {
         path: '/normalize_1/normalize_2',
         parent: pageId1,
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdB,
+        grantedGroups: [{ item: groupIdB, type: 'UserGroup' }],
         grantedUsers: [testUser1._id],
       },
       {
         _id: pageId3,
         path: '/normalize_1',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdA,
+        grantedGroups: [{ item: groupIdA, type: 'UserGroup' }],
         grantedUsers: [testUser1._id],
       },
       {
@@ -182,34 +196,34 @@ describe('V5 page migration', () => {
         path: '/normalize_4/normalize_5',
         parent: pageId4,
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdA,
+        grantedGroups: [{ item: groupIdA, type: 'UserGroup' }],
         grantedUsers: [testUser1._id],
       },
       {
         _id: pageId6,
         path: '/normalize_4',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdIsolate,
+        grantedGroups: [{ item: groupIdIsolate, type: 'UserGroup' }],
         grantedUsers: [testUser1._id],
       },
       {
         path: '/normalize_7/normalize_8_gA',
         grant: Page.GRANT_USER_GROUP,
         creator: testUser1,
-        grantedGroup: groupIdA,
+        grantedGroups: [{ item: groupIdA, type: 'UserGroup' }],
         grantedUsers: [testUser1._id],
       },
       {
         path: '/normalize_7/normalize_8_gA/normalize_9_gB',
         grant: Page.GRANT_USER_GROUP,
         creator: testUser1,
-        grantedGroup: groupIdB,
+        grantedGroups: [{ item: groupIdB, type: 'UserGroup' }],
         grantedUsers: [testUser1._id],
       },
       {
         path: '/normalize_7/normalize_8_gC',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdC,
+        grantedGroups: [{ item: groupIdC, type: 'UserGroup' }],
         grantedUsers: [testUser1._id],
       },
       {
@@ -231,13 +245,13 @@ describe('V5 page migration', () => {
         _id: pageId9, // not v5
         path: '/normalize_10/normalize_11_gA',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdA,
+        grantedGroups: [{ item: groupIdA, type: 'UserGroup' }],
       },
       {
         _id: pageId10,
         path: '/normalize_10/normalize_11_gA/normalize_11_gB',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdB,
+        grantedGroups: [{ item: groupIdB, type: 'UserGroup' }],
         parent: pageId8,
         descendantCount: 0,
       },
@@ -245,7 +259,7 @@ describe('V5 page migration', () => {
         _id: pageId11,
         path: '/normalize_10/normalize_12_gC',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdC,
+        grantedGroups: [{ item: groupIdC, type: 'UserGroup' }],
         grantedUsers: [testUser1._id],
         parent: pageId7,
         descendantCount: 0,
@@ -431,13 +445,13 @@ describe('V5 page migration', () => {
         {
           path: '/deep_path/normalize_a',
           grant: Page.GRANT_USER_GROUP,
-          grantedGroup: testUser1GroupId,
+          grantedGroups: [{ item: testUser1GroupId, type: 'UserGroup' }],
           parent: null,
         },
         {
           path: '/deep_path/normalize_c',
           grant: Page.GRANT_USER_GROUP,
-          grantedGroup: testUser1GroupId,
+          grantedGroups: [{ item: testUser1GroupId, type: 'UserGroup' }],
           parent: null,
         },
 
@@ -451,19 +465,19 @@ describe('V5 page migration', () => {
         {
           path: '/normalize_d',
           grant: Page.GRANT_USER_GROUP,
-          grantedGroup: testUser1GroupId,
+          grantedGroups: [{ item: testUser1GroupId, type: 'UserGroup' }],
           parent: null,
         },
         {
           path: '/normalize_d/normalize_e',
           grant: Page.GRANT_USER_GROUP,
-          grantedGroup: testUser1GroupId,
+          grantedGroups: [{ item: testUser1GroupId, type: 'UserGroup' }],
           parent: id2,
         },
         {
           path: '/normalize_f',
           grant: Page.GRANT_USER_GROUP,
-          grantedGroup: testUser1GroupId,
+          grantedGroups: [{ item: testUser1GroupId, type: 'UserGroup' }],
           parent: null,
         },
 
@@ -694,7 +708,7 @@ describe('V5 page migration', () => {
         {
           path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned/normalize_16_group',
           grant: Page.GRANT_USER_GROUP,
-          grantedGroup: testUser1GroupId,
+          grantedGroups: [{ item: testUser1GroupId, type: 'UserGroup' }],
         },
 
         // 2
@@ -729,7 +743,7 @@ describe('V5 page migration', () => {
         {
           path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned/normalize_20_group',
           grant: Page.GRANT_USER_GROUP,
-          grantedGroup: rootUserGroupId,
+          grantedGroups: [{ item: rootUserGroupId, type: 'UserGroup' }],
         },
 
         // 3
@@ -773,7 +787,7 @@ describe('V5 page migration', () => {
         {
           path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned/normalize_24_rootGroup',
           grant: Page.GRANT_USER_GROUP,
-          grantedGroup: rootUserGroupId,
+          grantedGroups: [{ item: rootUserGroupId, type: 'UserGroup' }],
         },
       ]);
     });
@@ -1047,7 +1061,7 @@ describe('V5 page migration', () => {
       expect(page3AM.parent).toStrictEqual(rootPage._id);
     });
 
-    test('should throw error if a page with isolated group becomes the parent of other page with different gourp after normalizing', async() => {
+    test('should throw error if a page with isolated group becomes the parent of other page with different group after normalizing', async() => {
       const page4 = await Page.findOne({ _id: pageId4, path: '/normalize_4', isEmpty: true });
       const page5 = await Page.findOne({ _id: pageId5, path: '/normalize_4/normalize_5', parent: page4._id });
       const page6 = await Page.findOne({ _id: pageId6, path: '/normalize_4' }); // NOT v5

+ 74 - 54
apps/app/test/integration/service/v5.non-public-page.test.ts

@@ -343,7 +343,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdRename2,
         path: '/np_rename2',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdB,
+        grantedGroups: [{ item: groupIdB }],
         creator: npDummyUser2._id,
         lastUpdateUser: npDummyUser2._id,
         parent: rootPage._id,
@@ -352,7 +352,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdRename3,
         path: '/np_rename2/np_rename3',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdC,
+        grantedGroups: [{ item: groupIdC }],
         creator: npDummyUser3._id,
         lastUpdateUser: npDummyUser3._id,
         parent: pageIdRename2._id,
@@ -361,7 +361,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdRename4,
         path: '/np_rename4_destination',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdIsolate,
+        grantedGroups: [{ item: groupIdIsolate }],
         creator: npDummyUser3._id,
         lastUpdateUser: npDummyUser3._id,
         parent: rootPage._id,
@@ -370,7 +370,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdRename5,
         path: '/np_rename5',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdB,
+        grantedGroups: [{ item: groupIdB }],
         creator: npDummyUser2._id,
         lastUpdateUser: npDummyUser2._id,
         parent: rootPage._id,
@@ -379,7 +379,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdRename6,
         path: '/np_rename5/np_rename6',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdB,
+        grantedGroups: [{ item: groupIdB }],
         creator: npDummyUser2._id,
         lastUpdateUser: npDummyUser2._id,
         parent: pageIdRename5,
@@ -388,7 +388,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdRename7,
         path: '/np_rename7_destination',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdIsolate,
+        grantedGroups: { item: groupIdIsolate },
         creator: npDummyUser2._id,
         lastUpdateUser: npDummyUser2._id,
         parent: pageIdRename5,
@@ -424,7 +424,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdDuplicate2,
         path: '/np_duplicate2',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdA,
+        grantedGroups: { item: groupIdA },
         creator: npDummyUser1._id,
         lastUpdateUser: npDummyUser1._id,
         revision: revisionIdDuplicate2,
@@ -434,7 +434,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdDuplicate3,
         path: '/np_duplicate2/np_duplicate3',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdB,
+        grantedGroups: { item: groupIdB },
         creator: npDummyUser2._id,
         lastUpdateUser: npDummyUser2._id,
         revision: revisionIdDuplicate3,
@@ -531,7 +531,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdDelete2,
         path: '/npdel2_ug',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdA,
+        grantedGroups: { item: groupIdA },
         status: Page.STATUS_PUBLISHED,
         isEmpty: false,
         parent: rootPage._id,
@@ -541,7 +541,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdDelete3,
         path: '/npdel3_top',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdA,
+        grantedGroups: { item: groupIdA },
         status: Page.STATUS_PUBLISHED,
         isEmpty: false,
         parent: rootPage._id,
@@ -551,7 +551,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdDelete4,
         path: '/npdel3_top/npdel4_ug',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdB,
+        grantedGroups: { item: groupIdB },
         status: Page.STATUS_PUBLISHED,
         isEmpty: false,
         parent: pageIdDelete3._id,
@@ -566,7 +566,7 @@ describe('PageService page operations with non-public pages', () => {
       {
         path: '/npdel3_top/npdel4_ug/npdel5_ug',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdC,
+        grantedGroups: { item: groupIdC },
         status: Page.STATUS_PUBLISHED,
         isEmpty: false,
         parent: pageIdDelete4._id,
@@ -589,7 +589,7 @@ describe('PageService page operations with non-public pages', () => {
       {
         path: '/npdc2_ug',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdA,
+        grantedGroups: { item: groupIdA },
         status: Page.STATUS_PUBLISHED,
         isEmpty: false,
         parent: rootPage._id,
@@ -598,7 +598,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdDeleteComp1,
         path: '/npdc3_ug',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdA,
+        grantedGroups: { item: groupIdA },
         status: Page.STATUS_PUBLISHED,
         isEmpty: false,
         parent: rootPage._id,
@@ -607,7 +607,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdDeleteComp2,
         path: '/npdc3_ug/npdc4_ug',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdB,
+        grantedGroups: { item: groupIdB },
         status: Page.STATUS_PUBLISHED,
         isEmpty: false,
         parent: pageIdDeleteComp1,
@@ -615,7 +615,7 @@ describe('PageService page operations with non-public pages', () => {
       {
         path: '/npdc3_ug/npdc4_ug/npdc5_ug',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdC,
+        grantedGroups: { item: groupIdC },
         status: Page.STATUS_PUBLISHED,
         isEmpty: false,
         parent: pageIdDeleteComp2,
@@ -643,7 +643,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdRevert2,
         path: '/trash/np_revert2',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdA,
+        grantedGroups: { item: groupIdA },
         revision: revisionIdRevert2,
         status: Page.STATUS_DELETED,
       },
@@ -665,7 +665,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdRevert5,
         path: '/trash/np_revert5',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdA,
+        grantedGroups: { item: groupIdA },
         revision: revisionIdRevert5,
         status: Page.STATUS_DELETED,
       },
@@ -673,7 +673,7 @@ describe('PageService page operations with non-public pages', () => {
         _id: pageIdRevert6,
         path: '/trash/np_revert5/middle/np_revert6',
         grant: Page.GRANT_USER_GROUP,
-        grantedGroup: groupIdB,
+        grantedGroups: { item: groupIdB },
         revision: revisionIdRevert6,
         status: Page.STATUS_DELETED,
       },
@@ -897,8 +897,8 @@ describe('PageService page operations with non-public pages', () => {
       const _path2 = '/np_rename2';
       const _path3 = '/np_rename2/np_rename3';
       const _propertiesD = { grant: Page.GRANT_PUBLIC };
-      const _properties2 = { grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdB };
-      const _properties3 = { grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdC };
+      const _properties2 = { grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdB } } };
+      const _properties3 = { grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdC } } };
       const _pageD = await Page.findOne({ path: _pathD, ..._propertiesD });
       const _page2 = await Page.findOne({ path: _path2, ..._properties2 });
       const _page3 = await Page.findOne({ path: _path3, ..._properties3, parent: _page2._id });
@@ -934,9 +934,9 @@ describe('PageService page operations with non-public pages', () => {
       const _pathD = '/np_rename4_destination';
       const _path2 = '/np_rename5';
       const _path3 = '/np_rename5/np_rename6';
-      const _propertiesD = { grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdIsolate };
-      const _properties2 = { grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdB };
-      const _properties3 = { grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdB };
+      const _propertiesD = { grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdIsolate } } };
+      const _properties2 = { grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdB } } };
+      const _properties3 = { grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdB } } };
       const _pageD = await Page.findOne({ path: _pathD, ..._propertiesD });// isolate
       const _page2 = await Page.findOne({ path: _path2, ..._properties2 });// groupIdB
       const _page3 = await Page.findOne({ path: _path3, ..._properties3, parent: _page2 });// groupIdB
@@ -971,7 +971,7 @@ describe('PageService page operations with non-public pages', () => {
       const _pathD = '/np_rename7_destination';
       const _path2 = '/np_rename8';
       const _path3 = '/np_rename8/np_rename9';
-      const _pageD = await Page.findOne({ path: _pathD, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdIsolate });
+      const _pageD = await Page.findOne({ path: _pathD, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdIsolate } } });
       const _page2 = await Page.findOne({ path: _path2, grant: Page.GRANT_RESTRICTED });
       const _page3 = await Page.findOne({ path: _path3, grant: Page.GRANT_RESTRICTED });
       expect(_pageD).toBeTruthy();
@@ -1042,9 +1042,9 @@ describe('PageService page operations with non-public pages', () => {
     test('Should duplicate multiple pages with GRANT_USER_GROUP', async() => {
       const _path1 = '/np_duplicate2';
       const _path2 = '/np_duplicate2/np_duplicate3';
-      const _page1 = await Page.findOne({ path: _path1, parent: rootPage._id, grantedGroup: groupIdA })
+      const _page1 = await Page.findOne({ path: _path1, parent: rootPage._id, grantedGroups: { $elemMatch: { item: groupIdA } } })
         .populate({ path: 'revision', model: 'Revision', grantedPage: groupIdA._id });
-      const _page2 = await Page.findOne({ path: _path2, parent: _page1._id, grantedGroup: groupIdB })
+      const _page2 = await Page.findOne({ path: _path2, parent: _page1._id, grantedGroups: { $elemMatch: { item: groupIdB } } })
         .populate({ path: 'revision', model: 'Revision', grantedPage: groupIdB._id });
       const _revision1 = _page1.revision;
       const _revision2 = _page2.revision;
@@ -1065,8 +1065,16 @@ describe('PageService page operations with non-public pages', () => {
       expect(duplicatedPage2).toBeTruthy();
       expect(duplicatedRevision1).toBeTruthy();
       expect(duplicatedRevision2).toBeTruthy();
-      expect(duplicatedPage1.grantedGroup).toStrictEqual(groupIdA._id);
-      expect(duplicatedPage2.grantedGroup).toStrictEqual(groupIdB._id);
+      const grantedGroups1 = JSON.parse(JSON.stringify(duplicatedPage1.grantedGroups)).map((group) => {
+        delete group._id;
+        return group;
+      });
+      const grantedGroups2 = JSON.parse(JSON.stringify(duplicatedPage2.grantedGroups)).map((group) => {
+        delete group._id;
+        return group;
+      });
+      expect(grantedGroups1).toStrictEqual([{ item: groupIdA._id.toString(), type: 'UserGroup' }]);
+      expect(grantedGroups2).toStrictEqual([{ item: groupIdB._id.toString(), type: 'UserGroup' }]);
       expect(duplicatedPage1.parent).toStrictEqual(_page1.parent);
       expect(duplicatedPage2.parent).toStrictEqual(duplicatedPage1._id);
       expect(duplicatedRevision1.body).toBe(_revision1.body);
@@ -1156,7 +1164,7 @@ describe('PageService page operations with non-public pages', () => {
     describe('Delete single page with grant USER_GROUP', () => {
       test('should be able to delete', async() => {
         const _path = '/npdel2_ug';
-        const _page1 = await Page.findOne({ path: _path, grantedGroup: groupIdA });
+        const _page1 = await Page.findOne({ path: _path, grantedGroups: { $elemMatch: { item: groupIdA } } });
         expect(_page1).toBeTruthy();
 
         const isRecursively = false;
@@ -1165,8 +1173,8 @@ describe('PageService page operations with non-public pages', () => {
           endpoint: '/_api/v3/pages/rename',
         });
 
-        const pageN = await Page.findOne({ path: _path, grantedGroup: groupIdA });
-        const page1 = await Page.findOne({ path: `/trash${_path}`, grantedGroup: groupIdA });
+        const pageN = await Page.findOne({ path: _path, grantedGroups: { $elemMatch: { item: groupIdA } } });
+        const page1 = await Page.findOne({ path: `/trash${_path}`, grantedGroups: { $elemMatch: { item: groupIdA } } });
         expect(pageN).toBeNull();
         expect(page1).toBeTruthy();
         expect(page1.status).toBe(Page.STATUS_DELETED);
@@ -1179,9 +1187,9 @@ describe('PageService page operations with non-public pages', () => {
         const _pathT = '/npdel3_top';
         const _path1 = '/npdel3_top/npdel4_ug';
         const _path2 = '/npdel3_top/npdel4_ug/npdel5_ug';
-        const _pageT = await Page.findOne({ path: _pathT, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdA }); // A
-        const _page1 = await Page.findOne({ path: _path1, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdB }); // B
-        const _page2 = await Page.findOne({ path: _path2, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdC }); // C
+        const _pageT = await Page.findOne({ path: _pathT, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdA } } }); // A
+        const _page1 = await Page.findOne({ path: _path1, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdB } } }); // B
+        const _page2 = await Page.findOne({ path: _path2, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdC } } }); // C
         const _pageR = await Page.findOne({ path: _path1, grant: Page.GRANT_RESTRICTED }); // Restricted
         expect(_pageT).toBeTruthy();
         expect(_page1).toBeTruthy();
@@ -1194,12 +1202,12 @@ describe('PageService page operations with non-public pages', () => {
           endpoint: '/_api/v3/pages/rename',
         });
 
-        const pageTNotExist = await Page.findOne({ path: _pathT, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdA }); // A should not exist
-        const page1NotExist = await Page.findOne({ path: _path1, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdB }); // B should not exist
-        const page2NotExist = await Page.findOne({ path: _path2, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdC }); // C should not exist
-        const pageT = await Page.findOne({ path: `/trash${_pathT}`, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdA }); // A
-        const page1 = await Page.findOne({ path: `/trash${_path1}`, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdB }); // B
-        const page2 = await Page.findOne({ path: `/trash${_path2}`, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdC }); // C
+        const pageTNotExist = await Page.findOne({ path: _pathT, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdA } } }); // A should not exist
+        const page1NotExist = await Page.findOne({ path: _path1, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdB } } }); // B should not exist
+        const page2NotExist = await Page.findOne({ path: _path2, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdC } } }); // C should not exist
+        const pageT = await Page.findOne({ path: `/trash${_pathT}`, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdA } } }); // A
+        const page1 = await Page.findOne({ path: `/trash${_path1}`, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdB } } }); // B
+        const page2 = await Page.findOne({ path: `/trash${_path2}`, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdC } } }); // C
         const pageR = await Page.findOne({ path: _path1, grant: Page.GRANT_RESTRICTED }); // Restricted
         expect(page1NotExist).toBeNull();
         expect(pageTNotExist).toBeNull();
@@ -1256,7 +1264,7 @@ describe('PageService page operations with non-public pages', () => {
     describe('Delete single page with grant USER_GROUP', () => {
       test('should be able to delete completely', async() => {
         const _path = '/npdc2_ug';
-        const _page = await Page.findOne({ path: _path, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdA });
+        const _page = await Page.findOne({ path: _path, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdA } } });
         expect(_page).toBeTruthy();
 
         await deleteCompletely(_page, npDummyUser1, {}, false, false, {
@@ -1264,7 +1272,7 @@ describe('PageService page operations with non-public pages', () => {
           endpoint: '/_api/v3/pages/rename',
         });
 
-        const page = await Page.findOne({ path: _path, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdA });
+        const page = await Page.findOne({ path: _path, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdA } } });
         expect(page).toBeNull();
       });
     });
@@ -1273,9 +1281,9 @@ describe('PageService page operations with non-public pages', () => {
         const _path1 = '/npdc3_ug';
         const _path2 = '/npdc3_ug/npdc4_ug';
         const _path3 = '/npdc3_ug/npdc4_ug/npdc5_ug';
-        const _page1 = await Page.findOne({ path: _path1, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdA });
-        const _page2 = await Page.findOne({ path: _path2, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdB });
-        const _page3 = await Page.findOne({ path: _path3, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdC });
+        const _page1 = await Page.findOne({ path: _path1, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdA } } });
+        const _page2 = await Page.findOne({ path: _path2, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdB } } });
+        const _page3 = await Page.findOne({ path: _path3, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdC } } });
         const _page4 = await Page.findOne({ path: _path2, grant: Page.GRANT_RESTRICTED });
         expect(_page1).toBeTruthy();
         expect(_page2).toBeTruthy();
@@ -1287,9 +1295,9 @@ describe('PageService page operations with non-public pages', () => {
           endpoint: '/_api/v3/pages/rename',
         });
 
-        const page1 = await Page.findOne({ path: _path1, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdA });
-        const page2 = await Page.findOne({ path: _path2, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdB });
-        const page3 = await Page.findOne({ path: _path3, grant: Page.GRANT_USER_GROUP, grantedGroup: groupIdC });
+        const page1 = await Page.findOne({ path: _path1, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdA } } });
+        const page2 = await Page.findOne({ path: _path2, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdB } } });
+        const page3 = await Page.findOne({ path: _path3, grant: Page.GRANT_USER_GROUP, grantedGroups: { $elemMatch: { item: groupIdC } } });
         const page4 = await Page.findOne({ path: _path2, grant: Page.GRANT_RESTRICTED });
 
         expect(page1).toBeNull();
@@ -1371,7 +1379,11 @@ describe('PageService page operations with non-public pages', () => {
       expect(revertedPage.parent).toStrictEqual(rootPage._id);
       expect(revertedPage.status).toBe(Page.STATUS_PUBLISHED);
       expect(revertedPage.grant).toBe(Page.GRANT_USER_GROUP);
-      expect(revertedPage.grantedGroup).toStrictEqual(groupIdA);
+      const grantedGroups = JSON.parse(JSON.stringify(revertedPage.grantedGroups)).map((group) => {
+        delete group._id;
+        return group;
+      });
+      expect(grantedGroups).toStrictEqual([{ item: groupIdA.toString(), type: 'UserGroup' }]);
       expect(pageTagRelation.isPageTrashed).toBe(false);
     });
     test(`revert multiple pages: only target page should be reverted.
@@ -1417,8 +1429,8 @@ describe('PageService page operations with non-public pages', () => {
       const beforeRevertPath1 = '/trash/np_revert5';
       const beforeRevertPath2 = '/trash/np_revert5/middle/np_revert6';
       const beforeRevertPath3 = '/trash/np_revert5/middle';
-      const trashedPage1 = await Page.findOne({ path: beforeRevertPath1, status: Page.STATUS_DELETED, grantedGroup: groupIdA });
-      const trashedPage2 = await Page.findOne({ path: beforeRevertPath2, status: Page.STATUS_DELETED, grantedGroup: groupIdB });
+      const trashedPage1 = await Page.findOne({ path: beforeRevertPath1, status: Page.STATUS_DELETED, grantedGroups: { $elemMatch: { item: groupIdA } } });
+      const trashedPage2 = await Page.findOne({ path: beforeRevertPath2, status: Page.STATUS_DELETED, grantedGroups: { $elemMatch: { item: groupIdB } } });
       const nonExistantPage3 = await Page.findOne({ path: beforeRevertPath3 }); // not exist
       const revision1 = await Revision.findOne({ pageId: trashedPage1._id });
       const revision2 = await Revision.findOne({ pageId: trashedPage2._id });
@@ -1453,8 +1465,16 @@ describe('PageService page operations with non-public pages', () => {
       expect(revertedPage1.status).toBe(Page.STATUS_PUBLISHED);
       expect(revertedPage2.status).toBe(Page.STATUS_PUBLISHED);
       expect(newlyCreatedPage.status).toBe(Page.STATUS_PUBLISHED);
-      expect(revertedPage1.grantedGroup).toStrictEqual(groupIdA);
-      expect(revertedPage2.grantedGroup).toStrictEqual(groupIdB);
+      const grantedGroups1 = JSON.parse(JSON.stringify(revertedPage1.grantedGroups)).map((group) => {
+        delete group._id;
+        return group;
+      });
+      const grantedGroups2 = JSON.parse(JSON.stringify(revertedPage2.grantedGroups)).map((group) => {
+        delete group._id;
+        return group;
+      });
+      expect(grantedGroups1).toStrictEqual([{ item: groupIdA.toString(), type: 'UserGroup' }]);
+      expect(grantedGroups2).toStrictEqual([{ item: groupIdB.toString(), type: 'UserGroup' }]);
       expect(newlyCreatedPage.grant).toBe(Page.GRANT_PUBLIC);
 
     });

+ 11 - 6
packages/core/src/interfaces/page.ts

@@ -3,9 +3,17 @@ import type { HasObjectId } from './has-object-id';
 import type { IRevision, HasRevisionShortbody, IRevisionHasId } from './revision';
 import type { SubscriptionStatusType } from './subscription';
 import type { ITag } from './tag';
-import type { IUser, IUserGroupHasId, IUserHasId } from './user';
+import type {
+  IUser, IUserGroup, IUserGroupHasId, IUserHasId,
+} from './user';
 
-export type GroupType = 'UserGroup' | 'ExternalUserGroup'
+export const GroupType = { userGroup: 'UserGroup', externalUserGroup: 'ExternalUserGroup' } as const;
+export type GroupType = typeof GroupType[keyof typeof GroupType];
+
+export type GrantedGroup = {
+  type: GroupType,
+  item: Ref<IUserGroup>,
+}
 
 export type IPage = {
   path: string,
@@ -21,10 +29,7 @@ export type IPage = {
   isEmpty: boolean,
   grant: PageGrant,
   grantedUsers: Ref<IUser>[],
-  grantedGroups: {
-    type: GroupType,
-    item: Ref<any>,
-  }[],
+  grantedGroups: GrantedGroup[],
   lastUpdateUser: Ref<IUser>,
   liker: Ref<IUser>[],
   commentCount: number