Merge pull request #8266 from weseek/fix/135572-135573-group-not-related-to-user-gets-granted

fix: Remove groups not related to the user from the user groups that are specified automatically when creating child pages

apps/app/src/components/PageAlert/FixPageGrantAlert.tsx

@@ -9,7 +9,7 @@ import {
 import { apiv3Put } from '~/client/util/apiv3-client';
 import { toastError, toastSuccess } from '~/client/util/toastr';
 import { IPageGrantData } from '~/interfaces/page';
-import { ApplicableGroup, IRecordApplicableGrant, IResIsGrantNormalizedGrantData } from '~/interfaces/page-grant';
+import { PopulatedGrantedGroup, IRecordApplicableGrant, IResIsGrantNormalizedGrantData } from '~/interfaces/page-grant';
 import { useCurrentUser } from '~/stores/context';
 import { useSWRxApplicableGrant, useSWRxIsGrantNormalized, useSWRxCurrentPage } from '~/stores/page';
 
@@ -31,7 +31,7 @@ const FixPageGrantModal = (props: ModalProps): JSX.Element => {
   const [selectedGrant, setSelectedGrant] = useState<PageGrant>(PageGrant.GRANT_RESTRICTED);
 
   const [isGroupSelectModalShown, setIsGroupSelectModalShown] = useState(false);
-  const [selectedGroups, setSelectedGroups] = useState<ApplicableGroup[]>([]);
+  const [selectedGroups, setSelectedGroups] = useState<PopulatedGrantedGroup[]>([]);
 
   // Alert message state
   const [shouldShowModalAlert, setShowModalAlert] = useState<boolean>(false);
@@ -47,7 +47,7 @@ const FixPageGrantModal = (props: ModalProps): JSX.Element => {
     }
   }, [isOpen]);
 
-  const groupListItemClickHandler = (group: ApplicableGroup) => {
+  const groupListItemClickHandler = (group: PopulatedGrantedGroup) => {
     if (selectedGroups.find(g => g.item._id === group.item._id) != null) {
       setSelectedGroups(selectedGroups.filter(g => g.item._id !== group.item._id));
     }

apps/app/src/interfaces/page-grant.ts

@@ -8,9 +8,9 @@ import { IPageGrantData } from './page';
 
 type UserGroupType = typeof GroupType.userGroup;
 type ExternalUserGroupType = typeof GroupType.externalUserGroup;
-export type ApplicableGroup = {type: UserGroupType, item: UserGroupDocument } | {type: ExternalUserGroupType, item: ExternalUserGroupDocument }
+export type PopulatedGrantedGroup = {type: UserGroupType, item: UserGroupDocument } | {type: ExternalUserGroupType, item: ExternalUserGroupDocument }
 export type IDataApplicableGroup = {
-  applicableGroups?: ApplicableGroup[]
+  applicableGroups?: PopulatedGrantedGroup[]
 }
 
 export type IDataApplicableGrant = null | IDataApplicableGroup;

apps/app/src/pages/[[...path]].page.tsx

@@ -5,7 +5,7 @@ import EventEmitter from 'events';
 
 import { isIPageInfoForEntity } from '@growi/core';
 import type {
-  IDataWithMeta, IPageInfoForEntity, IPagePopulatedToShowRevision, IUserHasId,
+  IDataWithMeta, IPageInfoForEntity, IPagePopulatedToShowRevision,
 } from '@growi/core';
 import {
   isClient, pagePathUtils, pathUtils,
@@ -22,7 +22,8 @@ import superjson from 'superjson';
 
 import { useCurrentGrowiLayoutFluidClassName, useEditorModeClassName } from '~/client/services/layout';
 import { PageView } from '~/components/Page/PageView';
-import { DrawioViewerScript } from '~/components/Script/DrawioViewerScript'; import type { CrowiRequest } from '~/interfaces/crowi-request';
+import { DrawioViewerScript } from '~/components/Script/DrawioViewerScript';
+import type { CrowiRequest } from '~/interfaces/crowi-request';
 import type { EditorConfig } from '~/interfaces/editor-settings';
 import type { IPageGrantData } from '~/interfaces/page';
 import type { RendererConfig } from '~/interfaces/services/renderer';
@@ -417,24 +418,6 @@ class MultiplePagesHitsError extends ExtensibleCustomError {
 
 }
 
-// apply parent page grant fot creating page
-async function applyGrantToPage(props: Props, ancestor: any) {
-  await ancestor.populate('grantedGroups.item');
-  const grant = {
-    grant: ancestor.grant,
-  };
-  const grantedGroups = ancestor.grantedGroups ? {
-    grantedGroups: ancestor.grantedGroups.map((group) => {
-      return {
-        id: group.item._id,
-        name: group.item.name,
-        type: group.type,
-      };
-    }),
-  } : {};
-  props.grantData = Object.assign(grant, grantedGroups);
-}
-
 async function injectPageData(context: GetServerSidePropsContext, props: Props): Promise<void> {
   const { model: mongooseModel } = await import('mongoose');
 
@@ -495,10 +478,20 @@ async function injectPageData(context: GetServerSidePropsContext, props: Props):
       props.templateBodyData = templateData.templateBody as string;
     }
 
-    // apply parent page grant
+    // apply parent page grant, without groups that user isn't related to
     const ancestor = await Page.findAncestorByPathAndViewer(currentPathname, user);
     if (ancestor != null) {
-      await applyGrantToPage(props, ancestor);
+      const userRelatedGrantedGroups = await pageService.getUserRelatedGrantedGroups(ancestor, user);
+      props.grantData = {
+        grant: ancestor.grant,
+        grantedGroups: userRelatedGrantedGroups.map((group) => {
+          return {
+            id: group.item._id,
+            name: group.item.name,
+            type: group.type,
+          };
+        }),
+      };
     }
   }
 

apps/app/src/server/models/page.ts

@@ -18,13 +18,16 @@ import mongoose, {
 import mongoosePaginate from 'mongoose-paginate-v2';
 import uniqueValidator from 'mongoose-unique-validator';
 
+import { ExternalUserGroupDocument } from '~/features/external-user-group/server/models/external-user-group';
 import ExternalUserGroupRelation from '~/features/external-user-group/server/models/external-user-group-relation';
+import { PopulatedGrantedGroup } from '~/interfaces/page-grant';
 import type { ObjectIdLike } from '~/server/interfaces/mongoose-utils';
 
 import loggerFactory from '../../utils/logger';
 import { getOrCreateModel } from '../util/mongoose-utils';
 
 import { getPageSchema, extractToAncestorsPaths, populateDataToShowRevision } from './obsolete-page';
+import { UserGroupDocument } from './user-group';
 import UserGroupRelation from './user-group-relation';
 
 const logger = loggerFactory('growi:models:page');

apps/app/src/server/service/page-grant.ts

@@ -493,14 +493,14 @@ class PageGrantService {
       [PageGrant.GRANT_RESTRICTED]: null, // any page can be restricted
     };
 
-    const userPossessedGroups = await this.getUserPossessedGroups(user);
+    const userRelatedGroups = await this.getUserRelatedGroups(user);
 
     // -- Any grant is allowed if parent is null
     const isAnyGrantApplicable = page.parent == null;
     if (isAnyGrantApplicable) {
       data[PageGrant.GRANT_PUBLIC] = null;
       data[PageGrant.GRANT_OWNER] = null;
-      data[PageGrant.GRANT_USER_GROUP] = { applicableGroups: userPossessedGroups };
+      data[PageGrant.GRANT_USER_GROUP] = { applicableGroups: userRelatedGroups };
       return data;
     }
 
@@ -516,7 +516,7 @@ class PageGrantService {
     if (grant === PageGrant.GRANT_PUBLIC) {
       data[PageGrant.GRANT_PUBLIC] = null;
       data[PageGrant.GRANT_OWNER] = null;
-      data[PageGrant.GRANT_USER_GROUP] = { applicableGroups: userPossessedGroups };
+      data[PageGrant.GRANT_USER_GROUP] = { applicableGroups: userRelatedGroups };
     }
     else if (grant === PageGrant.GRANT_OWNER) {
       const grantedUser = grantedUsers[0];
@@ -568,14 +568,14 @@ class PageGrantService {
     return data;
   }
 
-  async getUserPossessedGroups(user) {
-    const userPossessedUserGroups = await UserGroupRelation.findAllGroupsForUser(user);
-    const userPossessedExternalUserGroups = await ExternalUserGroupRelation.findAllGroupsForUser(user);
+  async getUserRelatedGroups(user) {
+    const userRelatedUserGroups = await UserGroupRelation.findAllGroupsForUser(user);
+    const userRelatedExternalUserGroups = await ExternalUserGroupRelation.findAllGroupsForUser(user);
     return [
-      ...userPossessedUserGroups.map((group) => {
+      ...userRelatedUserGroups.map((group) => {
         return { type: GroupType.userGroup, item: group };
       }),
-      ...userPossessedExternalUserGroups.map((group) => {
+      ...userRelatedExternalUserGroups.map((group) => {
         return { type: GroupType.externalUserGroup, item: group };
       }),
     ];

apps/app/src/server/service/page.ts

@@ -20,6 +20,7 @@ import { V5ConversionErrCode } from '~/interfaces/errors/v5-conversion-error';
 import {
   PageDeleteConfigValue, IPageDeleteConfigValueToProcessValidation,
 } from '~/interfaces/page-delete-config';
+import { PopulatedGrantedGroup } from '~/interfaces/page-grant';
 import {
   IPageOperationProcessInfo, IPageOperationProcessData, PageActionStage, PageActionType,
 } from '~/interfaces/page-operation';
@@ -2251,6 +2252,18 @@ class PageService {
     await PageOperation.findByIdAndDelete(pageOpId);
   }
 
+  /*
+ * get all groups of Page that user is related to
+ */
+  async getUserRelatedGrantedGroups(page: PageDocument, user): Promise<PopulatedGrantedGroup[]> {
+    const populatedPage = await page.populate<{grantedGroups: PopulatedGrantedGroup[] | null}>('grantedGroups.item');
+    const userRelatedGroupIds = [
+      ...(await UserGroupRelation.findAllGroupsForUser(user)).map(ugr => ugr._id.toString()),
+      ...(await ExternalUserGroupRelation.findAllGroupsForUser(user)).map(eugr => eugr._id.toString()),
+    ];
+    return populatedPage.grantedGroups?.filter(group => userRelatedGroupIds.includes(group.item._id.toString())) || [];
+  }
+
   private async revertDeletedPageV4(page, user, options = {}, isRecursively = false) {
     const Page = this.crowi.model('Page');
     const PageTagRelation = this.crowi.model('PageTagRelation');

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

@@ -532,13 +532,13 @@ describe('PageGrantService', () => {
 
     // parent property of all private pages is null
     test('Any grant is allowed if parent is null', async() => {
-      const userPossessedUserGroups = await UserGroupRelation.findAllGroupsForUser(user1);
-      const userPossessedExternalUserGroups = await ExternalUserGroupRelation.findAllGroupsForUser(user1);
-      const userPossessedGroups = [
-        ...userPossessedUserGroups.map((group) => {
+      const userRelatedUserGroups = await UserGroupRelation.findAllGroupsForUser(user1);
+      const userRelatedExternalUserGroups = await ExternalUserGroupRelation.findAllGroupsForUser(user1);
+      const userRelatedGroups = [
+        ...userRelatedUserGroups.map((group) => {
           return { type: GroupType.userGroup, item: group };
         }),
-        ...userPossessedExternalUserGroups.map((group) => {
+        ...userRelatedExternalUserGroups.map((group) => {
           return { type: GroupType.externalUserGroup, item: group };
         }),
       ];
@@ -551,7 +551,7 @@ describe('PageGrantService', () => {
           [PageGrant.GRANT_PUBLIC]: null,
           [PageGrant.GRANT_RESTRICTED]: null,
           [PageGrant.GRANT_OWNER]: null,
-          [PageGrant.GRANT_USER_GROUP]: { applicableGroups: userPossessedGroups },
+          [PageGrant.GRANT_USER_GROUP]: { applicableGroups: userRelatedGroups },
         },
       );
 
@@ -563,7 +563,7 @@ describe('PageGrantService', () => {
           [PageGrant.GRANT_PUBLIC]: null,
           [PageGrant.GRANT_RESTRICTED]: null,
           [PageGrant.GRANT_OWNER]: null,
-          [PageGrant.GRANT_USER_GROUP]: { applicableGroups: userPossessedGroups },
+          [PageGrant.GRANT_USER_GROUP]: { applicableGroups: userRelatedGroups },
         },
       );
 
@@ -575,20 +575,20 @@ describe('PageGrantService', () => {
           [PageGrant.GRANT_PUBLIC]: null,
           [PageGrant.GRANT_RESTRICTED]: null,
           [PageGrant.GRANT_OWNER]: null,
-          [PageGrant.GRANT_USER_GROUP]: { applicableGroups: userPossessedGroups },
+          [PageGrant.GRANT_USER_GROUP]: { applicableGroups: userRelatedGroups },
         },
       );
     });
 
 
     test('Any grant is allowed if parent is public', async() => {
-      const userPossessedUserGroups = await UserGroupRelation.findAllGroupsForUser(user1);
-      const userPossessedExternalUserGroups = await ExternalUserGroupRelation.findAllGroupsForUser(user1);
-      const userPossessedGroups = [
-        ...userPossessedUserGroups.map((group) => {
+      const userRelatedUserGroups = await UserGroupRelation.findAllGroupsForUser(user1);
+      const userRelatedExternalUserGroups = await ExternalUserGroupRelation.findAllGroupsForUser(user1);
+      const userRelatedGroups = [
+        ...userRelatedUserGroups.map((group) => {
           return { type: GroupType.userGroup, item: group };
         }),
-        ...userPossessedExternalUserGroups.map((group) => {
+        ...userRelatedExternalUserGroups.map((group) => {
           return { type: GroupType.externalUserGroup, item: group };
         }),
       ];
@@ -601,7 +601,7 @@ describe('PageGrantService', () => {
           [PageGrant.GRANT_PUBLIC]: null,
           [PageGrant.GRANT_RESTRICTED]: null,
           [PageGrant.GRANT_OWNER]: null,
-          [PageGrant.GRANT_USER_GROUP]: { applicableGroups: userPossessedGroups },
+          [PageGrant.GRANT_USER_GROUP]: { applicableGroups: userRelatedGroups },
         },
       );
 
@@ -613,7 +613,7 @@ describe('PageGrantService', () => {
           [PageGrant.GRANT_PUBLIC]: null,
           [PageGrant.GRANT_RESTRICTED]: null,
           [PageGrant.GRANT_OWNER]: null,
-          [PageGrant.GRANT_USER_GROUP]: { applicableGroups: userPossessedGroups },
+          [PageGrant.GRANT_USER_GROUP]: { applicableGroups: userRelatedGroups },
         },
       );
 
@@ -625,7 +625,7 @@ describe('PageGrantService', () => {
           [PageGrant.GRANT_PUBLIC]: null,
           [PageGrant.GRANT_RESTRICTED]: null,
           [PageGrant.GRANT_OWNER]: null,
-          [PageGrant.GRANT_USER_GROUP]: { applicableGroups: userPossessedGroups },
+          [PageGrant.GRANT_USER_GROUP]: { applicableGroups: userRelatedGroups },
         },
       );
     });