Merge pull request #1129 from weseek/master

release v3.5.4

CHANGES.md

@@ -1,6 +1,11 @@
 # CHANGES
 
-## 3.5.3-RC
+## 3.5.4-RC
+
+* Fix: List private pages wrongly
+* Fix: Consecutive page deletion requests cause unexpected complete page deletion
+
+## 3.5.3
 
 * Improvement: Calculate string width when save with Spreadsheet like GUI (Handsontable)
 * Fix: Search Result Page doesn't work

config/jest.config.js

@@ -7,11 +7,6 @@ module.exports = {
 
   rootDir: '../',
 
-  // Automatically clear mock calls and instances between every test
-  clearMocks: true,
-  // Automatically reset mock state between every test
-  resetMocks: true,
-
   projects: [
     {
       displayName: 'server',
@@ -19,6 +14,8 @@ module.exports = {
       rootDir: '.',
       setupFilesAfterEnv: ['<rootDir>/src/test/setup.js'],
       testMatch: ['<rootDir>/src/test/**/*.test.js'],
+      // Automatically clear mock calls and instances between every test
+      clearMocks: true,
       // A map from regular expressions to module names that allow to stub out resources with a single module
       moduleNameMapper: {
         '@root/(.+)': '<rootDir>/$1',

package.json

@@ -1,6 +1,6 @@
 {
   "name": "growi",
-  "version": "3.5.3-RC",
+  "version": "3.5.4-RC",
   "description": "Team collaboration software using markdown",
   "tags": [
     "wiki",
@@ -168,6 +168,7 @@
     "i18next-browser-languagedetector": "^3.0.1",
     "imports-loader": "^0.8.0",
     "jest": "^24.8.0",
+    "jest-each": "^24.8.0",
     "jquery-slimscroll": "^1.3.8",
     "jquery-ui": "^1.12.1",
     "jquery.cookie": "~1.4.1",

resource/locales/en-US/translation.json

@@ -720,7 +720,7 @@
     "no_pages": "There are no pages the group has view permission",
     "how_to_add1": "Enter a username to add",
     "how_to_add2": "Select a user from user list",
-    "remove_from_group": "Remove this group"
+    "remove_from_group": "Remove this user"
   },
 
   "importer_management": {

resource/locales/en-US/welcome.md

@@ -7,7 +7,7 @@
   <div class="panel-heading">Tips</div>
   <div class="panel-body"><ul>
     <li>Ctrl(⌘)-/ to show quick help</li>
-    <li>You can <a href="https://getbootstrap.com/docs/3.3/css/">Bootstrap 3</a> to write HTML tags.</li>
+    <li>You can write HTML with <a href="https://getbootstrap.com/docs/3.3/css/">Bootstrap 3</a>.</li>
   </ul></div>
 </div>
 

src/server/models/GlobalNotificationSetting/index.js

@@ -1,5 +1,6 @@
 const mongoose = require('mongoose');
 const nodePath = require('path');
+const { pathUtils } = require('growi-commons');
 
 /**
  * parent schema for GlobalNotificationSetting model
@@ -10,7 +11,9 @@ const globalNotificationSettingSchema = new mongoose.Schema({
   triggerEvents: { type: [String] },
 });
 
-
+/*
+* e.g. "/a/b/c" => ["/a/b/c", "/a/b", "/a", "/"]
+*/
 const generatePathsOnTree = (path, pathList) => {
   pathList.push(path);
 
@@ -23,11 +26,16 @@ const generatePathsOnTree = (path, pathList) => {
   return generatePathsOnTree(newPath, pathList);
 };
 
+/*
+* e.g. "/a/b/c" => ["/a/b/c", "/a/b", "/a", "/"]
+*/
 const generatePathsToMatch = (originalPath) => {
   const pathList = generatePathsOnTree(originalPath, []);
   return pathList.map((path) => {
+    // except for the original trigger path ("/a/b/c"), append "*" to find all matches
+    // e.g. ["/a/b/c", "/a/b", "/a", "/"] => ["/a/b/c", "/a/b/*", "/a/*", "/*"]
     if (path !== originalPath) {
-      return `${path}/*`;
+      return `${pathUtils.addTrailingSlash(path)}*`;
     }
 
     return path;

src/server/models/page.js

@@ -809,7 +809,7 @@ module.exports = function(crowi) {
 
     // determine User condition
     const hidePagesRestrictedByOwner = crowi.configManager.getConfig('crowi', 'security:list-policy:hideRestrictedByOwner');
-    const hidePagesRestrictedByGroup = crowi.configManager.getConfig('crowi', 'security:list-policy:hidePagesRestrictedByGroupInList');
+    const hidePagesRestrictedByGroup = crowi.configManager.getConfig('crowi', 'security:list-policy:hideRestrictedByGroup');
 
     // determine UserGroup condition
     let userGroups = null;
@@ -1061,11 +1061,12 @@ module.exports = function(crowi) {
     const newPath = this.getDeletedPageName(pageData.path);
     const isTrashed = checkIfTrashed(pageData.path);
 
+    if (isTrashed) {
+      throw new Error('This method does NOT support deleting trashed pages.');
+    }
+
     const socketClientId = options.socketClientId || null;
     if (this.isDeletableName(pageData.path)) {
-      if (isTrashed) {
-        return this.completelyDeletePage(pageData, user, options);
-      }
 
       pageData.status = STATUS_DELETED;
       const updatedPageData = await this.rename(pageData, newPath, user, { socketClientId, createRedirectPage: true });
@@ -1084,7 +1085,7 @@ module.exports = function(crowi) {
     const isTrashed = checkIfTrashed(targetPage.path);
 
     if (isTrashed) {
-      return this.completelyDeletePageRecursively(targetPage, user, options);
+      throw new Error('This method does NOT supports deleting trashed pages.');
     }
 
     const findOpts = { includeRedirect: true };

src/server/service/acl.js

@@ -16,11 +16,17 @@ class AclService {
     };
   }
 
+  /**
+   * @returns Whether Access Control is enabled or not
+   */
   isAclEnabled() {
     const wikiMode = this.configManager.getConfig('crowi', 'security:wikiMode');
     return wikiMode !== 'public';
   }
 
+  /**
+   * @returns Whether wiki mode is set
+   */
   isWikiModeForced() {
     const wikiMode = this.configManager.getConfig('crowi', 'security:wikiMode');
     const isPrivateOrPublic = wikiMode === 'private' || wikiMode === 'public';
@@ -28,12 +34,9 @@ class AclService {
     return isPrivateOrPublic;
   }
 
-  getGuestModeValue() {
-    return this.isGuestAllowedToRead()
-      ? this.labels.SECURITY_RESTRICT_GUEST_MODE_READONLY
-      : this.labels.SECURITY_RESTRICT_GUEST_MODE_DENY;
-  }
-
+  /**
+   * @returns Whether guest users are allowed to read public pages
+   */
   isGuestAllowedToRead() {
     const wikiMode = this.configManager.getConfig('crowi', 'security:wikiMode');
 
@@ -53,6 +56,12 @@ class AclService {
     return guestMode === this.labels.SECURITY_RESTRICT_GUEST_MODE_READONLY;
   }
 
+  getGuestModeValue() {
+    return this.isGuestAllowedToRead()
+      ? this.labels.SECURITY_RESTRICT_GUEST_MODE_READONLY
+      : this.labels.SECURITY_RESTRICT_GUEST_MODE_DENY;
+  }
+
   getRestrictGuestModeLabels() {
     const labels = {};
     labels[this.labels.SECURITY_RESTRICT_GUEST_MODE_DENY] = 'security_setting.guest_mode.deny';

src/server/util/middlewares.js

@@ -189,16 +189,18 @@ module.exports = (crowi, app) => {
    */
   middlewares.loginRequired = function(isStrictly = true) {
     return function(req, res, next) {
-      const User = crowi.model('User');
 
       // when the route is not strictly restricted
       if (!isStrictly) {
         // when allowed to read
         if (crowi.aclService.isGuestAllowedToRead()) {
+          logger.debug('Allowed to read: ', req.path);
           return next();
         }
       }
 
+      const User = crowi.model('User');
+
       // check the user logged in
       //  make sure that req.user isn't username/email string to login which is set by basic-auth-connect
       if (req.user != null && (req.user instanceof Object) && '_id' in req.user) {

src/server/util/search.js

@@ -555,7 +555,7 @@ SearchClient.prototype.appendCriteriaForQueryString = function(query, queryStrin
 
 SearchClient.prototype.filterPagesByViewer = async function(query, user, userGroups) {
   const showPagesRestrictedByOwner = !this.configManager.getConfig('crowi', 'security:list-policy:hideRestrictedByOwner');
-  const showPagesRestrictedByGroup = !this.configManager.getConfig('crowi', 'security:list-policy:hidePagesRestrictedByGroupInList');
+  const showPagesRestrictedByGroup = !this.configManager.getConfig('crowi', 'security:list-policy:hideRestrictedByGroup');
 
   query = this.initializeBoolQuery(query); // eslint-disable-line no-param-reassign
 

src/test/service/acl.test.js

@@ -0,0 +1,205 @@
+import each from 'jest-each';
+
+const { getInstance } = require('../setup-crowi');
+
+describe('AclService test', () => {
+  let crowi;
+
+  const initialEnv = process.env;
+
+  beforeEach(async(done) => {
+    crowi = await getInstance();
+    process.env = initialEnv;
+    done();
+  });
+
+
+  describe('isAclEnabled()', () => {
+
+    test('to be false when FORCE_WIKI_MODE is undefined', async() => {
+      delete process.env.FORCE_WIKI_MODE;
+
+      // reload
+      await crowi.configManager.loadConfigs();
+
+      const result = crowi.aclService.isAclEnabled();
+
+      const wikiMode = crowi.configManager.getConfig('crowi', 'security:wikiMode');
+      expect(wikiMode).toBe(undefined);
+      expect(result).toBe(true);
+    });
+
+    test('to be false when FORCE_WIKI_MODE is dummy string', async() => {
+      process.env.FORCE_WIKI_MODE = 'dummy string';
+
+      // reload
+      await crowi.configManager.loadConfigs();
+
+      const result = crowi.aclService.isAclEnabled();
+
+      const wikiMode = crowi.configManager.getConfig('crowi', 'security:wikiMode');
+      expect(wikiMode).toBe('dummy string');
+      expect(result).toBe(true);
+    });
+
+    test('to be true when FORCE_WIKI_MODE=private', async() => {
+      process.env.FORCE_WIKI_MODE = 'private';
+
+      // reload
+      await crowi.configManager.loadConfigs();
+
+      const result = crowi.aclService.isAclEnabled();
+
+      const wikiMode = crowi.configManager.getConfig('crowi', 'security:wikiMode');
+      expect(wikiMode).toBe('private');
+      expect(result).toBe(true);
+    });
+
+    test('to be false when FORCE_WIKI_MODE=public', async() => {
+      process.env.FORCE_WIKI_MODE = 'public';
+
+      // reload
+      await crowi.configManager.loadConfigs();
+
+      const result = crowi.aclService.isAclEnabled();
+
+      const wikiMode = crowi.configManager.getConfig('crowi', 'security:wikiMode');
+      expect(wikiMode).toBe('public');
+      expect(result).toBe(false);
+    });
+
+  });
+
+
+  describe('isWikiModeForced()', () => {
+
+    test('to be false when FORCE_WIKI_MODE is undefined', async() => {
+      delete process.env.FORCE_WIKI_MODE;
+
+      // reload
+      await crowi.configManager.loadConfigs();
+
+      const result = crowi.aclService.isWikiModeForced();
+
+      const wikiMode = crowi.configManager.getConfig('crowi', 'security:wikiMode');
+      expect(wikiMode).toBe(undefined);
+      expect(result).toBe(false);
+    });
+
+    test('to be false when FORCE_WIKI_MODE is dummy string', async() => {
+      process.env.FORCE_WIKI_MODE = 'dummy string';
+
+      // reload
+      await crowi.configManager.loadConfigs();
+
+      const result = crowi.aclService.isWikiModeForced();
+
+      const wikiMode = crowi.configManager.getConfig('crowi', 'security:wikiMode');
+      expect(wikiMode).toBe('dummy string');
+      expect(result).toBe(false);
+    });
+
+    test('to be true when FORCE_WIKI_MODE=private', async() => {
+      process.env.FORCE_WIKI_MODE = 'private';
+
+      // reload
+      await crowi.configManager.loadConfigs();
+
+      const result = crowi.aclService.isWikiModeForced();
+
+      const wikiMode = crowi.configManager.getConfig('crowi', 'security:wikiMode');
+      expect(wikiMode).toBe('private');
+      expect(result).toBe(true);
+    });
+
+    test('to be false when FORCE_WIKI_MODE=public', async() => {
+      process.env.FORCE_WIKI_MODE = 'public';
+
+      // reload
+      await crowi.configManager.loadConfigs();
+
+      const result = crowi.aclService.isWikiModeForced();
+
+      const wikiMode = crowi.configManager.getConfig('crowi', 'security:wikiMode');
+      expect(wikiMode).toBe('public');
+      expect(result).toBe(true);
+    });
+
+  });
+
+
+  describe('isGuestAllowedToRead()', () => {
+    let getConfigSpy;
+
+    beforeEach(async(done) => {
+      // prepare spy for ConfigManager.getConfig
+      getConfigSpy = jest.spyOn(crowi.configManager, 'getConfig');
+      done();
+    });
+
+    test('to be false when FORCE_WIKI_MODE=private', async() => {
+      process.env.FORCE_WIKI_MODE = 'private';
+
+      // reload
+      await crowi.configManager.loadConfigs();
+
+      const result = crowi.aclService.isGuestAllowedToRead();
+
+      const wikiMode = crowi.configManager.getConfig('crowi', 'security:wikiMode');
+      expect(wikiMode).toBe('private');
+      expect(getConfigSpy).not.toHaveBeenCalledWith('crowi', 'security:restrictGuestMode');
+      expect(result).toBe(false);
+    });
+
+    test('to be true when FORCE_WIKI_MODE=public', async() => {
+      process.env.FORCE_WIKI_MODE = 'public';
+
+      // reload
+      await crowi.configManager.loadConfigs();
+
+      const result = crowi.aclService.isGuestAllowedToRead();
+
+      const wikiMode = crowi.configManager.getConfig('crowi', 'security:wikiMode');
+      expect(wikiMode).toBe('public');
+      expect(getConfigSpy).not.toHaveBeenCalledWith('crowi', 'security:restrictGuestMode');
+      expect(result).toBe(true);
+    });
+
+    each`
+    restrictGuestMode   | expected
+      ${undefined}      | ${false}
+      ${'Deny'}         | ${false}
+      ${'Readonly'}     | ${true}
+      ${'Open'}         | ${false}
+      ${'Restricted'}   | ${false}
+      ${'closed'}       | ${false}
+    `
+      .test('to be $expected when FORCE_WIKI_MODE is undefined'
+          + ' and `security:restrictGuestMode` is \'$restrictGuestMode\'', async({ restrictGuestMode, expected }) => {
+
+        // reload
+        await crowi.configManager.loadConfigs();
+
+        // setup mock implementation
+        getConfigSpy.mockImplementation((ns, key) => {
+          if (ns === 'crowi' && key === 'security:restrictGuestMode') {
+            return restrictGuestMode;
+          }
+          if (ns === 'crowi' && key === 'security:wikiMode') {
+            return undefined;
+          }
+          throw new Error('Unexpected behavior.');
+        });
+
+        const result = crowi.aclService.isGuestAllowedToRead();
+
+        expect(getConfigSpy).toHaveBeenCalledTimes(2);
+        expect(getConfigSpy).toHaveBeenCalledWith('crowi', 'security:wikiMode');
+        expect(getConfigSpy).toHaveBeenCalledWith('crowi', 'security:restrictGuestMode');
+        expect(result).toBe(expected);
+      });
+
+  });
+
+
+});

src/test/util/middlewares.test.js

@@ -0,0 +1,186 @@
+/* eslint-disable arrow-body-style */
+
+import each from 'jest-each';
+
+const { getInstance } = require('../setup-crowi');
+
+describe('middlewares.loginRequired', () => {
+  let crowi;
+  let middlewares;
+
+  beforeEach(async(done) => {
+    crowi = await getInstance();
+    middlewares = require('@server/util/middlewares')(crowi, null);
+    done();
+  });
+
+  // test('returns strict middlware when args is undefined', () => {
+  //   const func = middlewares.loginRequired();
+  //   expect(func).toBe(loginRequiredStrict);
+  // });
+
+  describe('not strict mode', () => {
+    // setup req/res/next
+    const req = {
+      originalUrl: 'original url 1',
+      session: {},
+    };
+    const res = {
+      redirect: jest.fn().mockReturnValue('redirect'),
+    };
+    const next = jest.fn().mockReturnValue('next');
+
+    let loginRequired;
+
+    beforeEach(async(done) => {
+      loginRequired = middlewares.loginRequired(false);
+      done();
+    });
+
+    test('pass guest user when aclService.isGuestAllowedToRead() returns true', () => {
+      // prepare spy for AclService.isGuestAllowedToRead
+      const isGuestAllowedToReadSpy = jest.spyOn(crowi.aclService, 'isGuestAllowedToRead')
+        .mockImplementation(() => true);
+
+      const result = loginRequired(req, res, next);
+
+      expect(isGuestAllowedToReadSpy).toHaveBeenCalledTimes(1);
+      expect(next).toHaveBeenCalled();
+      expect(res.redirect).not.toHaveBeenCalled();
+      expect(result).toBe('next');
+    });
+
+    test('redirect to \'/login\' when aclService.isGuestAllowedToRead() returns false', () => {
+      // prepare spy for AclService.isGuestAllowedToRead
+      const isGuestAllowedToReadSpy = jest.spyOn(crowi.aclService, 'isGuestAllowedToRead')
+        .mockImplementation(() => false);
+
+      const result = loginRequired(req, res, next);
+
+      expect(isGuestAllowedToReadSpy).toHaveBeenCalled();
+      expect(next).not.toHaveBeenCalled();
+      expect(res.redirect).toHaveBeenCalledTimes(1);
+      expect(res.redirect).toHaveBeenCalledWith('/login');
+      expect(result).toBe('redirect');
+    });
+
+  });
+
+
+  describe('strict mode', () => {
+    // setup req/res/next
+    const req = {
+      originalUrl: 'original url 1',
+      session: null,
+    };
+    const res = {
+      redirect: jest.fn().mockReturnValue('redirect'),
+      sendStatus: jest.fn().mockReturnValue('sendStatus'),
+    };
+    const next = jest.fn().mockReturnValue('next');
+
+    let loginRequired;
+    let isGuestAllowedToReadSpy;
+
+    beforeEach(async(done) => {
+      loginRequired = middlewares.loginRequired();
+      // reset session object
+      req.session = {};
+      // spy for AclService.isGuestAllowedToRead
+      isGuestAllowedToReadSpy = jest.spyOn(crowi.aclService, 'isGuestAllowedToRead');
+      done();
+    });
+
+    test('send status 403 when \'req.path\' starts with \'_api\'', () => {
+      req.path = '/_api/someapi';
+
+      const result = loginRequired(req, res, next);
+
+      expect(isGuestAllowedToReadSpy).not.toHaveBeenCalled();
+      expect(next).not.toHaveBeenCalled();
+      expect(res.redirect).not.toHaveBeenCalled();
+      expect(res.sendStatus).toHaveBeenCalledTimes(1);
+      expect(res.sendStatus).toHaveBeenCalledWith(403);
+      expect(result).toBe('sendStatus');
+    });
+
+    test('redirect to \'/login\' when the user does not loggedin', () => {
+      req.path = '/path/that/requires/loggedin';
+
+      const result = loginRequired(req, res, next);
+
+      expect(isGuestAllowedToReadSpy).not.toHaveBeenCalled();
+      expect(next).not.toHaveBeenCalled();
+      expect(res.sendStatus).not.toHaveBeenCalled();
+      expect(res.redirect).toHaveBeenCalledTimes(1);
+      expect(res.redirect).toHaveBeenCalledWith('/login');
+      expect(result).toBe('redirect');
+      expect(req.session.jumpTo).toBe('original url 1');
+    });
+
+    test('pass user who logged in', () => {
+      const User = crowi.model('User');
+
+      req.user = {
+        _id: 'user id',
+        status: User.STATUS_ACTIVE,
+      };
+
+      const result = loginRequired(req, res, next);
+
+      expect(isGuestAllowedToReadSpy).not.toHaveBeenCalled();
+      expect(res.sendStatus).not.toHaveBeenCalled();
+      expect(res.redirect).not.toHaveBeenCalled();
+      expect(next).toHaveBeenCalledTimes(1);
+      expect(result).toBe('next');
+      expect(req.session.jumpTo).toBe(undefined);
+    });
+
+    each`
+      userStatus  | expectedPath
+      ${1}        | ${'/login/error/registered'}
+      ${3}        | ${'/login/error/suspended'}
+      ${5}        | ${'/login/invited'}
+    `
+      .test('redirect to \'$expectedPath\''
+        + ' when user.status is \'$userStatus\' ', ({ userStatus, expectedPath }) => {
+
+        req.user = {
+          _id: 'user id',
+          status: userStatus,
+        };
+
+        const result = loginRequired(req, res, next);
+
+        expect(isGuestAllowedToReadSpy).not.toHaveBeenCalled();
+        expect(next).not.toHaveBeenCalled();
+        expect(res.sendStatus).not.toHaveBeenCalled();
+        expect(res.redirect).toHaveBeenCalledTimes(1);
+        expect(res.redirect).toHaveBeenCalledWith(expectedPath);
+        expect(result).toBe('redirect');
+        expect(req.session.jumpTo).toBe(undefined);
+      });
+
+    test('redirect to \'/login\' when user.status is \'STATUS_DELETED\'', () => {
+      const User = crowi.model('User');
+
+      req.path = '/path/that/requires/loggedin';
+      req.user = {
+        _id: 'user id',
+        status: User.STATUS_DELETED,
+      };
+
+      const result = loginRequired(req, res, next);
+
+      expect(isGuestAllowedToReadSpy).not.toHaveBeenCalled();
+      expect(next).not.toHaveBeenCalled();
+      expect(res.sendStatus).not.toHaveBeenCalled();
+      expect(res.redirect).toHaveBeenCalledTimes(1);
+      expect(res.redirect).toHaveBeenCalledWith('/login');
+      expect(result).toBe('redirect');
+      expect(req.session.jumpTo).toBe('original url 1');
+    });
+
+  });
+
+});