Bladeren bron

Merge pull request #5482 from weseek/fix/operate-anyone-with-link

fix: Operate anyone-with-link pages
Yuki Takei 4 jaren geleden
bovenliggende
commit
8b61240f82

+ 0 - 10
packages/app/src/server/models/obsolete-page.js

@@ -366,16 +366,6 @@ export const getPageSchema = (crowi) => {
     return queryBuilder.query.exec();
   };
 
-  pageSchema.statics.findByIdAndViewerToEdit = async function(id, user, includeEmpty = false) {
-    const baseQuery = this.findOne({ _id: id });
-    const queryBuilder = new this.PageQueryBuilder(baseQuery, includeEmpty);
-
-    // add grant conditions
-    await addConditionToFilteringByViewerToEdit(queryBuilder, user);
-
-    return queryBuilder.query.exec();
-  };
-
   // find page by path
   pageSchema.statics.findByPath = function(path, includeEmpty = false) {
     if (path == null) {

+ 1 - 11
packages/app/src/server/models/page.ts

@@ -46,7 +46,7 @@ export interface PageModel extends Model<PageDocument> {
   [x: string]: any; // for obsolete methods
   createEmptyPagesByPaths(paths: string[], onlyMigratedAsExistingPages?: boolean, publicOnly?: boolean): Promise<void>
   getParentAndFillAncestors(path: string, user): Promise<PageDocument & { _id: any }>
-  findByIdsAndViewer(pageIds: string[], user, userGroups?, includeEmpty?: boolean): Promise<PageDocument[]>
+  findByIdsAndViewer(pageIds: ObjectIdLike[], user, userGroups?, includeEmpty?: boolean): Promise<PageDocument[]>
   findByPathAndViewer(path: string | null, user, userGroups?, useFindOne?: boolean, includeEmpty?: boolean): Promise<PageDocument[]>
   findTargetAndAncestorsByPathOrId(pathOrId: string): Promise<TargetAndAncestorsResult>
   findChildrenByParentPathOrIdAndViewer(parentPathOrId: string, user, userGroups?): Promise<PageDocument[]>
@@ -840,16 +840,6 @@ schema.statics.removeLeafEmptyPagesRecursively = async function(pageId: ObjectId
   await this.deleteMany({ _id: { $in: pageIdsToRemove } });
 };
 
-schema.statics.findByPageIdsToEdit = async function(ids, user, shouldIncludeEmpty = false) {
-  const builder = new PageQueryBuilder(this.find({ _id: { $in: ids } }), shouldIncludeEmpty);
-
-  await this.addConditionToFilteringByViewerToEdit(builder, user);
-
-  const pages = await builder.query.exec();
-
-  return pages;
-};
-
 schema.statics.normalizeDescendantCountById = async function(pageId) {
   const children = await this.find({ parent: pageId });
 

+ 3 - 3
packages/app/src/server/routes/apiv3/pages.js

@@ -497,7 +497,7 @@ module.exports = (crowi) => {
     let renamedPage;
 
     try {
-      page = await Page.findByIdAndViewerToEdit(pageId, req.user, true);
+      page = await Page.findByIdAndViewer(pageId, req.user, null, true);
 
       if (page == null) {
         return res.apiv3Err(new ErrorV3(`Page '${pageId}' is not found or forbidden`, 'notfound_or_forbidden'), 401);
@@ -653,7 +653,7 @@ module.exports = (crowi) => {
       return res.apiv3Err(new ErrorV3(`Page exists '${newPagePath})'`, 'already_exists'), 409);
     }
 
-    const page = await Page.findByIdAndViewerToEdit(pageId, req.user, true);
+    const page = await Page.findByIdAndViewer(pageId, req.user, null, true);
 
     const isEmptyAndNotRecursively = page?.isEmpty && !isRecursively;
     if (page == null || isEmptyAndNotRecursively) {
@@ -748,7 +748,7 @@ module.exports = (crowi) => {
 
     let pagesToDelete;
     try {
-      pagesToDelete = await Page.findByPageIdsToEdit(pageIds, req.user, true);
+      pagesToDelete = await Page.findByIdsAndViewer(pageIds, req.user, null, true);
     }
     catch (err) {
       logger.error('Failed to find pages to delete.', err);

+ 1 - 1
packages/app/src/server/routes/page.js

@@ -1177,7 +1177,7 @@ module.exports = function(crowi, app) {
 
     const options = {};
 
-    const page = await Page.findByIdAndViewerToEdit(pageId, req.user, true);
+    const page = await Page.findByIdAndViewer(pageId, req.user, null, true);
 
     if (page == null) {
       return res.json(ApiResponse.error(`Page '${pageId}' is not found or forbidden`, 'notfound_or_forbidden'));

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

@@ -1,5 +1,5 @@
 import mongoose from 'mongoose';
-import { pagePathUtils, pathUtils } from '@growi/core';
+import { pagePathUtils, pathUtils, pageUtils } from '@growi/core';
 import escapeStringRegexp from 'escape-string-regexp';
 
 import UserGroup from '~/server/models/user-group';
@@ -357,6 +357,8 @@ class PageGrantService {
       throw Error(`The maximum number of pageIds allowed is ${LIMIT_FOR_MULTIPLE_PAGE_OP}.`);
     }
 
+    const Page = mongoose.model('Page') as unknown as PageModel;
+
     const shouldCheckDescendants = true;
     const shouldIncludeNotMigratedPages = true;
 
@@ -368,8 +370,12 @@ class PageGrantService {
         path, grant, grantedUsers: grantedUserIds, grantedGroup: grantedGroupId,
       } = page;
 
-      const isNormalized = await this.isGrantNormalized(path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants, shouldIncludeNotMigratedPages);
-      if (isNormalized) {
+      if (!pageUtils.isPageNormalized(page)) {
+        nonNormalizable.push(page);
+        continue;
+      }
+
+      if (await this.isGrantNormalized(path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants, shouldIncludeNotMigratedPages)) {
         normalizable.push(page);
       }
       else {

+ 1 - 2
packages/app/src/server/service/page.ts

@@ -107,7 +107,6 @@ class PageCursorsForDescendantsFactory {
 
     const builder = new PageQueryBuilder(this.Page.find(), this.shouldIncludeEmpty);
     builder.addConditionToFilteringByParentId(page._id);
-    // await this.Page.addConditionToFilteringByViewerToEdit(builder, this.user);
 
     const cursor = builder.query.lean().cursor({ batchSize: BULK_REINDEX_SIZE }) as QueryCursor<any>;
 
@@ -2192,7 +2191,7 @@ class PageService {
     const Page = mongoose.model('Page') as unknown as PageModel;
 
     if (isRecursively) {
-      const pages = await Page.findByPageIdsToEdit(pageIds, user, false);
+      const pages = await Page.findByIdsAndViewer(pageIds, user, null);
 
       // DO NOT await !!
       this.normalizeParentRecursivelyByPages(pages, user);

+ 2 - 0
packages/core/src/index.js

@@ -1,6 +1,7 @@
 import * as _pathUtils from './utils/path-utils';
 import * as _envUtils from './utils/env-utils';
 import * as _pagePathUtils from './utils/page-path-utils';
+import * as _pageUtils from './utils/page-utils';
 import * as _templateChecker from './utils/template-checker';
 import * as _customTagUtils from './plugin/util/custom-tag-utils';
 
@@ -8,6 +9,7 @@ import * as _customTagUtils from './plugin/util/custom-tag-utils';
 export const pathUtils = _pathUtils;
 export const envUtils = _envUtils;
 export const pagePathUtils = _pagePathUtils;
+export const pageUtils = _pageUtils;
 export const templateChecker = _templateChecker;
 export const customTagUtils = _customTagUtils;
 

+ 58 - 0
packages/core/src/utils/page-utils.ts

@@ -0,0 +1,58 @@
+import { isTopPage } from './page-path-utils';
+
+const GRANT_PUBLIC = 1;
+const GRANT_RESTRICTED = 2;
+const GRANT_SPECIFIED = 3; // DEPRECATED
+const GRANT_OWNER = 4;
+const GRANT_USER_GROUP = 5;
+const PAGE_GRANT_ERROR = 1;
+const STATUS_PUBLISHED = 'published';
+const STATUS_DELETED = 'deleted';
+
+/**
+ * Returns true if the page is on tree including the top page.
+ * @param page Page
+ * @returns boolean
+ */
+// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
+export const isOnTree = (page): boolean => {
+  const { path, parent } = page;
+
+  if (isTopPage(path)) {
+    return true;
+  }
+
+  if (parent != null) {
+    return true;
+  }
+
+  return false;
+};
+
+/**
+ * Returns true if the page meet the condition below.
+ *   - The page is on tree (has parent or the top page)
+ *   - The page's grant is GRANT_RESTRICTED or GRANT_SPECIFIED
+ *   - The page's status is STATUS_DELETED
+ * This does not check grantedUser or grantedGroup.
+ * @param page PageDocument
+ * @returns boolean
+ */
+// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
+export const isPageNormalized = (page): boolean => {
+  const { grant, status } = page;
+
+  if (grant === GRANT_RESTRICTED || grant === GRANT_SPECIFIED) {
+    return true;
+  }
+
+  if (status === STATUS_DELETED) {
+    return true;
+  }
+
+  if (isOnTree(page)) {
+    return true;
+  }
+
+  return true;
+};