Просмотр исходного кода

Improved type & Added one test case

Taichi Masuyama 4 лет назад
Родитель
Сommit
150b164674

+ 4 - 2
packages/app/src/server/models/page.ts

@@ -57,6 +57,7 @@ export interface PageModel extends Model<PageDocument> {
   STATUS_DELETED
 }
 
+type IObjectId = mongoose.Types.ObjectId;
 const ObjectId = mongoose.Schema.Types.ObjectId;
 
 const schema = new Schema<PageDocument, PageModel>({
@@ -377,7 +378,7 @@ export default (crowi: Crowi): any => {
     const Page = this;
     const Revision = crowi.model('Revision');
     const {
-      format = 'markdown', redirectTo, grantedUserIds = [user._id], grantUserGroupId,
+      format = 'markdown', redirectTo, grantUserGroupId,
     } = options;
 
     // sanitize path
@@ -405,8 +406,9 @@ export default (crowi: Crowi): any => {
       try {
         // It must check descendants as well if emptyTarget is not null
         const shouldCheckDescendants = emptyPage != null;
+        const newGrantedUserIds = grant === GRANT_OWNER ? [user._id] as IObjectId[] : undefined;
 
-        isGrantNormalized = await crowi.pageGrantService.isGrantNormalized(path, grant, grantedUserIds, grantUserGroupId, shouldCheckDescendants);
+        isGrantNormalized = await crowi.pageGrantService.isGrantNormalized(path, grant, newGrantedUserIds, grantUserGroupId, shouldCheckDescendants);
       }
       catch (err) {
         logger.error(`Failed to validate grant of page at "${path}" of grant ${grant}:`, err);

+ 17 - 3
packages/app/src/server/service/page-grant.ts

@@ -12,7 +12,7 @@ type ObjectId = mongoose.Types.ObjectId;
 
 type ComparableTarget = {
   grant: number,
-  grantedUserIds: ObjectId[],
+  grantedUserIds?: ObjectId[],
   grantedGroupId: ObjectId,
   applicableUserIds?: ObjectId[],
   applicableGroupIds?: ObjectId[],
@@ -70,6 +70,10 @@ class PageGrantService {
     }
     // GRANT_OWNER
     else if (ancestor.grant === Page.GRANT_OWNER) {
+      if (target.grantedUserIds?.length !== 1) {
+        throw Error('grantedUserIds must have one user');
+      }
+
       if (target.grant !== Page.GRANT_OWNER) { // only GRANT_OWNER page can exist under GRANT_OWNER page
         return false;
       }
@@ -89,6 +93,10 @@ class PageGrantService {
       }
 
       if (target.grant === Page.GRANT_OWNER) {
+        if (target.grantedUserIds?.length !== 1) {
+          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
           return false;
         }
@@ -114,6 +122,10 @@ class PageGrantService {
     }
     // GRANT_OWNER
     else if (target.grant === Page.GRANT_OWNER) {
+      if (target.grantedUserIds?.length !== 1) {
+        throw Error('grantedUserIds must have one user');
+      }
+
       if (descendants.isPublicExist) { // public page must not exist under GRANT_OWNER page
         return false;
       }
@@ -151,7 +163,7 @@ class PageGrantService {
    * @returns Promise<ComparableAncestor>
    */
   private async generateComparableTarget(
-      grant, grantedUserIds: ObjectId[], grantedGroupId: ObjectId, includeApplicable: boolean,
+      grant, grantedUserIds: ObjectId[] | undefined, grantedGroupId: ObjectId, includeApplicable: boolean,
   ): Promise<ComparableTarget> {
     if (includeApplicable) {
       const Page = mongoose.model('Page') as PageModel;
@@ -276,7 +288,9 @@ class PageGrantService {
    * About the rule of validation, see: https://dev.growi.org/61b2cdabaa330ce7d8152844
    * @returns Promise<boolean>
    */
-  async isGrantNormalized(targetPath: string, grant, grantedUserIds: ObjectId[], grantedGroupId: ObjectId, shouldCheckDescendants = false): Promise<boolean> {
+  async isGrantNormalized(
+      targetPath: string, grant, grantedUserIds: ObjectId[] | undefined, grantedGroupId: ObjectId, shouldCheckDescendants = false,
+  ): Promise<boolean> {
     if (isTopPage(targetPath)) {
       return true;
     }

+ 0 - 1
packages/app/src/server/util/compare-objectId.ts

@@ -2,7 +2,6 @@ import mongoose from 'mongoose';
 
 type IObjectId = mongoose.Types.ObjectId;
 const ObjectId = mongoose.Types.ObjectId;
-type ObjectIdLike = IObjectId | string;
 
 export const isIncludesObjectId = (arr: (IObjectId | string)[], id: IObjectId | string): boolean => {
   const _arr = arr.map(i => i.toString());

+ 12 - 0
packages/app/src/test/integration/service/page-grant.test.js

@@ -243,6 +243,18 @@ describe('PageGrantService', () => {
 
       expect(result).toBe(false);
     });
+
+    test('Should return false when Ancestor: owned by GroupChild, Target: GroupParent', async() => {
+      const targetPath = `${pageE3GroupChildPath}/NEW`;
+      const grant = Page.GRANT_USER_GROUP;
+      const grantedUserIds = null;
+      const grantedGroupId = groupParent._id;
+      const shouldCheckDescendants = false;
+
+      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+
+      expect(result).toBe(false);
+    });
   });
 
   describe('Test isGrantNormalized method with shouldCheckDescendants true', () => {