Merge branch 'master' into feat/resume-rename-on-server-boot

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

@@ -3523,7 +3523,8 @@ class PageService {
     }
     else {
       const parentId = parentPathOrId;
-      queryBuilder = new PageQueryBuilder(Page.find({ parent: parentId } as any), true); // TODO: improve type
+      // Use $eq for user-controlled sources. see: https://codeql.github.com/codeql-query-help/javascript/js-sql-injection/#recommendation
+      queryBuilder = new PageQueryBuilder(Page.find({ parent: { $eq: parentId } } as any), true); // TODO: improve type
     }
     await queryBuilder.addViewerCondition(user, userGroups);
 

packages/app/test/integration/service/v5.public-page.test.ts

@@ -2,6 +2,7 @@
 import { advanceTo } from 'jest-date-mock';
 import mongoose from 'mongoose';
 
+import { PageActionType, PageActionStage } from '../../../src/server/models/page-operation';
 import Tag from '../../../src/server/models/tag';
 import { getInstance } from '../setup-crowi';
 
@@ -19,10 +20,14 @@ describe('PageService page operations with only public pages', () => {
   let Comment;
   let ShareLink;
   let PageRedirect;
+  let PageOperation;
   let xssSpy;
 
   let rootPage;
 
+  // page operation ids
+  let pageOpId1;
+
   beforeAll(async() => {
     crowi = await getInstance();
     await crowi.configManager.updateConfigsInTheSameNamespace('crowi', { 'app:isV5Compatible': true });
@@ -35,6 +40,7 @@ describe('PageService page operations with only public pages', () => {
     Comment = mongoose.model('Comment');
     ShareLink = mongoose.model('ShareLink');
     PageRedirect = mongoose.model('PageRedirect');
+    PageOperation = mongoose.model('PageOperation');
 
     /*
      * Common
@@ -128,6 +134,17 @@ describe('PageService page operations with only public pages', () => {
     const pageIdForRename21 = new mongoose.Types.ObjectId();
     const pageIdForRename22 = new mongoose.Types.ObjectId();
     const pageIdForRename23 = new mongoose.Types.ObjectId();
+    const pageIdForRename24 = new mongoose.Types.ObjectId();
+    const pageIdForRename25 = new mongoose.Types.ObjectId();
+    const pageIdForRename26 = new mongoose.Types.ObjectId();
+    const pageIdForRename27 = new mongoose.Types.ObjectId();
+    const pageIdForRename28 = new mongoose.Types.ObjectId();
+
+    const pageIdForRename29 = new mongoose.Types.ObjectId();
+    const pageIdForRename30 = new mongoose.Types.ObjectId();
+
+    pageOpId1 = new mongoose.Types.ObjectId();
+    const pageOpRevisionId1 = new mongoose.Types.ObjectId();
 
     // Create Pages
     await Page.insertMany([
@@ -319,6 +336,101 @@ describe('PageService page operations with only public pages', () => {
         lastUpdateUser: dummyUser1._id,
         parent: pageIdForRename22,
       },
+      {
+        _id: pageIdForRename24,
+        path: '/v5_pageForRename24',
+        grant: Page.GRANT_PUBLIC,
+        creator: dummyUser1,
+        lastUpdateUser: dummyUser1._id,
+        parent: rootPage._id,
+        descendantCount: 0,
+      },
+      {
+        _id: pageIdForRename25,
+        path: '/v5_pageForRename25',
+        grant: Page.GRANT_PUBLIC,
+        creator: dummyUser1,
+        lastUpdateUser: dummyUser1._id,
+        parent: rootPage._id,
+        descendantCount: 0,
+      },
+      {
+        _id: pageIdForRename26,
+        path: '/v5_pageForRename26',
+        grant: Page.GRANT_PUBLIC,
+        creator: dummyUser1,
+        lastUpdateUser: dummyUser1._id,
+        parent: rootPage._id,
+        descendantCount: 0,
+      },
+      {
+        _id: pageIdForRename27,
+        path: '/v5_pageForRename27',
+        grant: Page.GRANT_PUBLIC,
+        creator: dummyUser1,
+        lastUpdateUser: dummyUser1._id,
+        parent: rootPage._id,
+        descendantCount: 1,
+      },
+      {
+        _id: pageIdForRename28,
+        path: '/v5_pageForRename27/v5_pageForRename28',
+        grant: Page.GRANT_PUBLIC,
+        creator: dummyUser1,
+        lastUpdateUser: dummyUser1._id,
+        parent: pageIdForRename27,
+        descendantCount: 0,
+      },
+      {
+        _id: pageIdForRename29,
+        path: '/v5_pageForRename29',
+        grant: Page.GRANT_PUBLIC,
+        creator: dummyUser1,
+        lastUpdateUser: dummyUser1._id,
+        parent: rootPage._id,
+        descendantCount: 1,
+      },
+      {
+        _id: pageIdForRename30,
+        path: '/v5_pageForRename29/v5_pageForRename30',
+        grant: Page.GRANT_PUBLIC,
+        creator: dummyUser1,
+        lastUpdateUser: dummyUser1._id,
+        parent: pageIdForRename29,
+        descendantCount: 0,
+      },
+    ]);
+
+    await PageOperation.insertMany([
+      {
+        _id: pageOpId1,
+        actionType: 'Rename',
+        actionStage: 'Sub',
+        fromPath: '/v5_pageForRename30',
+        toPath: '/v5_pageForRename29/v5_pageForRename30',
+        page: {
+          _id: pageIdForRename30,
+          parent: rootPage._id,
+          descendantCount: 0,
+          isEmpty: false,
+          path: '/v5_pageForRename30',
+          revision: pageOpRevisionId1,
+          status: 'published',
+          grant: 1,
+          grantedUsers: [],
+          grantedGroup: null,
+          creator: dummyUser1._id,
+          lastUpdateUser: dummyUser1._id,
+        },
+        user: {
+          _id: dummyUser1._id,
+        },
+        options: {
+          createRedirectPage: false,
+          updateMetadata: true,
+        },
+        unprocessableExpiryDate: null,
+      },
     ]);
 
     /*
@@ -1046,6 +1158,31 @@ describe('PageService page operations with only public pages', () => {
       return renamedPage;
     };
 
+    /**
+     * This function only execute renameMainOperation. renameSubOperation is basically omitted(only return null)
+     */
+    const renameMainOperation = async(page, newPagePath, user, options) => {
+      // create page operation from target page
+      const pageOp = await PageOperation.create({
+        actionType: PageActionType.Rename,
+        actionStage: PageActionStage.Main,
+        page,
+        user,
+        fromPath: page.path,
+        toPath: newPagePath,
+        options,
+      });
+
+      // mock return value
+      const mockedRenameSubOperation = jest.spyOn(crowi.pageService, 'renameSubOperation').mockReturnValue(null);
+      const renamedPage = await crowi.pageService.renameMainOperation(page, newPagePath, user, options, pageOp._id);
+
+      // restores the original implementation
+      mockedRenameSubOperation.mockRestore();
+
+      return renamedPage;
+    };
+
     test('Should NOT rename top page', async() => {
       expect(rootPage).toBeTruthy();
       let isThrown = false;
@@ -1315,6 +1452,136 @@ describe('PageService page operations with only public pages', () => {
       expect(renamedPageChild.isEmpty).toBeTruthy();
       expect(renamedPageGrandchild.isEmpty).toBe(false);
     });
+
+    test('should add 1 descendantCount to parent page in MainOperation', async() => {
+      // paths before renaming
+      const _path0 = '/v5_pageForRename24'; // out of renaming scope
+      const _path1 = '/v5_pageForRename25'; // not renamed yet
+
+      // paths after renaming
+      const path0 = '/v5_pageForRename24';
+      const path1 = '/v5_pageForRename24/v5_pageForRename25';
+
+      // new path:  same as path1
+      const newPath = '/v5_pageForRename24/v5_pageForRename25';
+
+      // pages
+      const _page0 = await Page.findOne({ path: _path0 });
+      const _page1 = await Page.findOne({ path: _path1 });
+
+      expect(_page0).toBeTruthy();
+      expect(_page1).toBeTruthy();
+      expect(_page0.descendantCount).toBe(0);
+      expect(_page1.descendantCount).toBe(0);
+
+      await renameMainOperation(_page1, newPath, dummyUser1, {});
+
+      const page0 = await Page.findById(_page0._id); // new parent
+      const page1 = await Page.findById(_page1._id); // renamed one
+      expect(page0).toBeTruthy();
+      expect(page1).toBeTruthy();
+
+      expect(page0.path).toBe(path0);
+      expect(page1.path).toBe(path1); // renamed
+      expect(page0.descendantCount).toBe(1); // originally 0, +1 in Main.
+      expect(page1.descendantCount).toBe(0);
+
+      // cleanup
+      await PageOperation.findOneAndDelete({ fromPath: _path1 });
+    });
+
+    test('should subtract 1 descendantCount from a new parent page in renameSubOperation', async() => {
+      // paths before renaming
+      const _path0 = '/v5_pageForRename29'; // out of renaming scope
+      const _path1 = '/v5_pageForRename29/v5_pageForRename30'; // already renamed
+
+      // paths after renaming
+      const path0 = '/v5_pageForRename29';
+      const path1 = '/v5_pageForRename29/v5_pageForRename30';
+
+      // new path:  same as path1
+      const newPath = '/v5_pageForRename29/v5_pageForRename30';
+
+      // page
+      const _page0 = await Page.findOne({ path: _path0 });
+      const _page1 = await Page.findOne({ path: _path1 });
+      expect(_page0).toBeTruthy();
+      expect(_page1).toBeTruthy();
+
+      // page operation
+      const fromPath = '/v5_pageForRename30';
+      const toPath = newPath;
+      const pageOperation = await PageOperation.findOne({
+        _id: pageOpId1, fromPath, toPath, actionType: PageActionType.Rename, actionStage: PageActionStage.Sub,
+      });
+      expect(pageOperation).toBeTruthy();
+
+      // descendantCount
+      expect(_page0.descendantCount).toBe(1);
+      expect(_page1.descendantCount).toBe(0);
+
+      // renameSubOperation only
+      await crowi.pageService.renameSubOperation(_page1, newPath, dummyUser1, {}, _page1, pageOperation._id);
+
+      // page
+      const page0 = await Page.findById(_page0._id); // new parent
+      const page1 = await Page.findById(_page1._id); // renamed one
+      expect(page0).toBeTruthy();
+      expect(page1).toBeTruthy();
+      expect(page0.path).toBe(path0);
+      expect(page1.path).toBe(path1); // renamed
+
+      // descendantCount
+      expect(page0.descendantCount).toBe(0); // originally 1, -1 in Sub.
+      expect(page1.descendantCount).toBe(0);
+    });
+
+    test(`should add 1 descendantCount to the a parent page in rename(Main)Operation
+    and subtract 1 descendantCount from the the parent page in rename(Sub)Operation`, async() => {
+      // paths before renaming
+      const _path0 = '/v5_pageForRename26'; // out of renaming scope
+      const _path1 = '/v5_pageForRename27'; // not renamed yet
+      const _path2 = '/v5_pageForRename27/v5_pageForRename28'; // not renamed yet
+
+      // paths after renaming
+      const path0 = '/v5_pageForRename26';
+      const path1 = '/v5_pageForRename26/v5_pageForRename27';
+      const path2 = '/v5_pageForRename26/v5_pageForRename27/v5_pageForRename28';
+
+      // new path: same as path1
+      const newPath = '/v5_pageForRename26/v5_pageForRename27';
+
+      // page
+      const _page0 = await Page.findOne({ path: _path0 });
+      const _page1 = await Page.findOne({ path: _path1 });
+      const _page2 = await Page.findOne({ path: _path2 });
+
+      expect(_page0).toBeTruthy();
+      expect(_page1).toBeTruthy();
+      expect(_page2).toBeTruthy();
+      expect(_page0.descendantCount).toBe(0);
+      expect(_page1.descendantCount).toBe(1);
+      expect(_page2.descendantCount).toBe(0);
+
+      await renamePage(_page1, newPath, dummyUser1, {});
+
+      const page0 = await Page.findById(_page0._id); // new parent
+      const page1 = await Page.findById(_page1._id); // renamed
+      const page2 = await Page.findById(_page2._id); // renamed
+      expect(page0).toBeTruthy();
+      expect(page1).toBeTruthy();
+      expect(page2).toBeTruthy();
+
+      expect(page0.path).toBe(path0);
+      expect(page1.path).toBe(path1);
+      expect(page2.path).toBe(path2);
+      expect(page0.descendantCount).toBe(2); // originally 0, +1 in Main, -1 in Sub, +2 for descendants.
+      expect(page1.descendantCount).toBe(1);
+      expect(page2.descendantCount).toBe(0);
+
+      // cleanup
+      await PageOperation.findOneAndDelete({ fromPath: _path1 });
+    });
   });
   describe('Duplicate', () => {