Merge pull request #5111 from weseek/imprv/update-page-validation

imprv: Update page validation

packages/app/src/server/models/obsolete-page.js

@@ -1033,7 +1033,7 @@ export const getPageSchema = (crowi) => {
     return savedPage;
   };
 
-  pageSchema.statics.updatePage = async function(pageData, body, previousBody, user, options = {}) {
+  pageSchema.statics.updatePageV4 = async function(pageData, body, previousBody, user, options = {}) {
     validateCrowi();
 
     const Revision = crowi.model('Revision');

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

@@ -23,7 +23,7 @@ const logger = loggerFactory('growi:models:page');
  */
 const GRANT_PUBLIC = 1;
 const GRANT_RESTRICTED = 2;
-const GRANT_SPECIFIED = 3;
+const GRANT_SPECIFIED = 3; // DEPRECATED
 const GRANT_OWNER = 4;
 const GRANT_USER_GROUP = 5;
 const PAGE_GRANT_ERROR = 1;
@@ -369,8 +369,8 @@ export default (crowi: Crowi): any => {
     }
 
     const isV5Compatible = crowi.configManager.getConfig('crowi', 'app:isV5Compatible');
+    // v4 compatible process
     if (!isV5Compatible) {
-      // v4 compatible process
       return this.createV4(path, body, user, options);
     }
 
@@ -379,42 +379,42 @@ export default (crowi: Crowi): any => {
     const {
       format = 'markdown', redirectTo, grantedUserIds = [user._id], grantUserGroupId,
     } = options;
+    let grant = options.grant;
 
     // sanitize path
     path = crowi.xss.process(path); // eslint-disable-line no-param-reassign
-
-    let grant = options.grant;
-    // force public
-    if (isTopPage(path)) {
-      grant = GRANT_PUBLIC;
-    }
-
+    // throw if exists
     const isExist = (await this.count({ path, isEmpty: false })) > 0; // not validate empty page
     if (isExist) {
       throw new Error('Cannot create new page to existed path');
     }
+    // force public
+    if (isTopPage(path)) {
+      grant = GRANT_PUBLIC;
+    }
 
-    // find existing empty page at target path
+    // find an existing empty page
     const emptyPage = await Page.findOne({ path, isEmpty: true });
 
     /*
      * UserGroup & Owner validation
      */
-    let isGrantNormalized = false;
-    try {
-      // It must check descendants as well if emptyTarget is not null
-      const shouldCheckDescendants = emptyPage != null;
-
-      isGrantNormalized = await crowi.pageGrantService.isGrantNormalized(path, grant, grantedUserIds, grantUserGroupId, shouldCheckDescendants);
-    }
-    catch (err) {
-      logger.error(`Failed to validate grant of page at "${path}" of grant ${grant}:`, err);
-      throw err;
+    if (grant !== GRANT_RESTRICTED) {
+      let isGrantNormalized = false;
+      try {
+        // It must check descendants as well if emptyTarget is not null
+        const shouldCheckDescendants = emptyPage != null;
+
+        isGrantNormalized = await crowi.pageGrantService.isGrantNormalized(path, grant, grantedUserIds, grantUserGroupId, shouldCheckDescendants);
+      }
+      catch (err) {
+        logger.error(`Failed to validate grant of page at "${path}" of grant ${grant}:`, err);
+        throw err;
+      }
+      if (!isGrantNormalized) {
+        throw Error('The selected grant or grantedGroup is not assignable to this page.');
+      }
     }
-    if (!isGrantNormalized) {
-      throw Error('The selected grant or grantedGroup is not assignable to this page.');
-    }
-
 
     /*
      * update empty page if exists, if not, create a new page
@@ -440,11 +440,22 @@ export default (crowi: Crowi): any => {
     page.lastUpdateUser = user;
     page.redirectTo = redirectTo;
     page.status = STATUS_PUBLISHED;
-    page.parent = options.grant === GRANT_RESTRICTED ? null : parentId;
+
+    // set parent to null when GRANT_RESTRICTED
+    if (grant === GRANT_RESTRICTED) {
+      page.parent = null;
+    }
+    else {
+      page.parent = parentId;
+    }
 
     page.applyScope(user, grant, grantUserGroupId);
 
     let savedPage = await page.save();
+
+    /*
+     * After save
+     */
     const newRevision = Revision.prepareRevision(savedPage, body, null, user, { format });
     const revision = await pushRevision(savedPage, newRevision, user);
     savedPage = await this.findByPath(revision.path);
@@ -455,6 +466,65 @@ export default (crowi: Crowi): any => {
     return savedPage;
   };
 
+  schema.statics.updatePage = async function(pageData, body, previousBody, user, options = {}) {
+    if (crowi.configManager == null || crowi.pageGrantService == null) {
+      throw Error('Crowi is not set up');
+    }
+
+    const isV5Compatible = crowi.configManager.getConfig('crowi', 'app:isV5Compatible');
+    if (!isV5Compatible) {
+      // v4 compatible process
+      return this.updatePageV4(pageData, body, previousBody, user, options);
+    }
+
+    const Revision = mongoose.model('Revision') as any; // TODO: Typescriptize model
+    const grant = options.grant || pageData.grant; // use the previous data if absence
+    const grantUserGroupId = options.grantUserGroupId || pageData.grantUserGroupId; // use the previous data if absence
+    const isSyncRevisionToHackmd = options.isSyncRevisionToHackmd;
+    const grantedUserIds = pageData.grantedUserIds || [user._id];
+
+    const newPageData = pageData;
+
+    if (grant === GRANT_RESTRICTED) {
+      newPageData.parent = null;
+    }
+    else {
+      /*
+       * UserGroup & Owner validation
+       */
+      let isGrantNormalized = false;
+      try {
+        const shouldCheckDescendants = true;
+
+        isGrantNormalized = await crowi.pageGrantService.isGrantNormalized(pageData.path, grant, grantedUserIds, grantUserGroupId, shouldCheckDescendants);
+      }
+      catch (err) {
+        logger.error(`Failed to validate grant of page at "${pageData.path}" of grant ${grant}:`, err);
+        throw err;
+      }
+      if (!isGrantNormalized) {
+        throw Error('The selected grant or grantedGroup is not assignable to this page.');
+      }
+    }
+
+    newPageData.applyScope(user, grant, grantUserGroupId);
+
+    // update existing page
+    let savedPage = await newPageData.save();
+    const newRevision = await Revision.prepareRevision(newPageData, body, previousBody, user);
+    const revision = await pushRevision(savedPage, newRevision, user);
+    savedPage = await this.findByPath(revision.path);
+    await savedPage.populateDataToShowRevision();
+
+    if (isSyncRevisionToHackmd) {
+      savedPage = await this.syncRevisionToHackmd(savedPage);
+    }
+
+    pageEvent.emit('update', savedPage, user);
+
+    return savedPage;
+  };
+
   // add old page schema methods
   const pageSchema = getPageSchema(crowi);
   schema.methods = { ...pageSchema.methods, ...schema.methods };

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

@@ -64,7 +64,7 @@ class PageGrantService {
      */
     // GRANT_PUBLIC
     if (ancestor.grant === Page.GRANT_PUBLIC) {
-      // DO NOTHING
+      // do nothing
     }
     // GRANT_OWNER
     else if (ancestor.grant === Page.GRANT_OWNER) {
@@ -106,15 +106,9 @@ class PageGrantService {
      * descendant side
      */
 
-    if (target.applicableGroupIds == null) {
-      throw Error('applicableGroupIds must not be null');
-    }
-
     // GRANT_PUBLIC
     if (target.grant === Page.GRANT_PUBLIC) {
-      if (descendants.descendantGroupIds.length !== 0 || descendants.descendantGroupIds.length !== 0) {
-        return false;
-      }
+      // do nothing
     }
     // GRANT_OWNER
     else if (target.grant === Page.GRANT_OWNER) {
@@ -128,6 +122,10 @@ class PageGrantService {
     }
     // GRANT_USER_GROUP
     else if (target.grant === Page.GRANT_USER_GROUP) {
+      if (target.applicableGroupIds == null) {
+        throw Error('applicableGroupIds must not be null');
+      }
+
       const shouldNotExistIds = excludeTestIdsFromTargetIds(descendants.descendantGroupIds, target.applicableGroupIds);
       if (shouldNotExistIds.length !== 0) {
         return false;
@@ -145,8 +143,8 @@ class PageGrantService {
       grant, grantedUserIds: ObjectId[], grantedGroupId: ObjectId, includeApplicable: boolean,
   ): Promise<ComparableTarget> {
     if (includeApplicable) {
-      const applicableGroups = await UserGroup.findGroupsWithDescendantsById(grantedGroupId);
-      const applicableGroupIds = applicableGroups.map(g => g._id);
+      const applicableGroups = grantedGroupId != null ? await UserGroup.findGroupsWithDescendantsById(grantedGroupId) : null;
+      const applicableGroupIds = applicableGroups?.map(g => g._id) || null;
 
 
       return {