Merge remote-tracking branch 'origin/master' into support/apply-bootstrap4

CHANGES.md

@@ -1,12 +1,16 @@
 # CHANGES
 
+## v4.0.0-RC
+
+* Support: Upgrade libs
+    * bootstrap
+
 ## v3.7.0-RC
 
 * Feature: [Draw.io](https://www.draw.io/) Integration
 * Feature: SAML Attribute-based Login Control
 * Improvement: Reactify admin pages (Security)
-* Support: Upgrade libs
-    * bootstrap
+* Improvement: Behavior of pre-editing screen of HackMD when user needs to resume
 
 ## v3.6.10
 

src/client/js/components/Admin/Security/TwitterSecuritySetting.jsx

@@ -49,7 +49,7 @@ class TwitterSecurityManagement extends React.Component {
 
   render() {
     const { t, adminGeneralSecurityContainer, adminTwitterSecurityContainer } = this.props;
-    const { isTwitterEnabled } = adminTwitterSecurityContainer.state;
+    const { isTwitterEnabled } = adminGeneralSecurityContainer.state;
 
     if (this.state.isRetrieving) {
       return null;

src/client/js/components/PageEditor/MarkdownDrawioUtil.js

@@ -83,7 +83,10 @@ class MarkdownDrawioUtil {
    * return boolean value whether the cursor position is in a drawio
    */
   isInDrawioBlock(editor) {
-    return (this.getBod(editor) !== this.getEod(editor));
+    const bod = this.getBod(editor);
+    const eod = this.getEod(editor);
+
+    return (JSON.stringify(bod) !== JSON.stringify(eod));
   }
 
   /**

src/client/js/components/PageEditorByHackmd.jsx

@@ -61,6 +61,16 @@ class PageEditorByHackmd extends React.Component {
     return envVars.HACKMD_URI;
   }
 
+  get isResume() {
+    const { pageContainer } = this.props;
+    const {
+      pageIdOnHackmd, hasDraftOnHackmd, isHackmdDraftUpdatingInRealtime,
+    } = pageContainer.state;
+
+    const isPageExistsOnHackmd = (pageIdOnHackmd != null);
+    return (isPageExistsOnHackmd && hasDraftOnHackmd) || isHackmdDraftUpdatingInRealtime;
+  }
+
   /**
    * Start integration with HackMD
    */
@@ -217,11 +227,9 @@ class PageEditorByHackmd extends React.Component {
     const hackmdUri = this.getHackmdUri();
     const { pageContainer } = this.props;
     const {
-      pageIdOnHackmd, revisionId, revisionIdHackmdSynced, remoteRevisionId, hasDraftOnHackmd,
+      revisionId, revisionIdHackmdSynced, remoteRevisionId,
     } = pageContainer.state;
 
-    const isPageExistsOnHackmd = (pageIdOnHackmd != null);
-    const isResume = isPageExistsOnHackmd && hasDraftOnHackmd;
 
     let content;
 
@@ -238,7 +246,7 @@ class PageEditorByHackmd extends React.Component {
     /*
      * Resume to edit or discard changes
      */
-    else if (isResume) {
+    else if (this.isResume) {
       const isHackmdDocumentOutdated = revisionIdHackmdSynced !== remoteRevisionId;
 
       content = (
@@ -331,11 +339,9 @@ class PageEditorByHackmd extends React.Component {
     const hackmdUri = this.getHackmdUri();
     const { pageContainer } = this.props;
     const {
-      markdown, pageIdOnHackmd, hasDraftOnHackmd,
+      markdown, pageIdOnHackmd,
     } = pageContainer.state;
 
-    const isPageExistsOnHackmd = (pageIdOnHackmd != null);
-    const isResume = isPageExistsOnHackmd && hasDraftOnHackmd;
 
     let content;
 
@@ -345,7 +351,7 @@ class PageEditorByHackmd extends React.Component {
           ref={(c) => { this.hackmdEditor = c }}
           hackmdUri={hackmdUri}
           pageIdOnHackmd={pageIdOnHackmd}
-          initializationMarkdown={isResume ? null : markdown}
+          initializationMarkdown={this.isResume ? null : markdown}
           onChange={this.hackmdEditorChangeHandler}
           onSaveWithShortcut={(document) => {
             this.onSaveWithShortcut(document);

src/server/routes/login-passport.js

@@ -13,7 +13,7 @@ module.exports = function(crowi, app) {
    * @param {*} req
    * @param {*} res
    */
-  const loginSuccess = (req, res, user) => {
+  const loginSuccessHandler = (req, res, user) => {
     // update lastLoginAt
     user.updateLastLoginAt(new Date(), (err, userData) => {
       if (err) {
@@ -33,11 +33,20 @@ module.exports = function(crowi, app) {
    * @param {*} req
    * @param {*} res
    */
-  const loginFailure = (req, res, message) => {
+  const loginFailureHandler = (req, res, message) => {
     req.flash('errorMessage', message || 'Sign in failure.');
     return res.redirect('/login');
   };
 
+  /**
+   * middleware for login failure
+   * @param {*} req
+   * @param {*} res
+   */
+  const loginFailure = (req, res) => {
+    return loginFailureHandler(req, res, 'Sign in failure.');
+  };
+
   /**
    * return true(valid) or false(invalid)
    *
@@ -117,7 +126,7 @@ module.exports = function(crowi, app) {
     // login
     await req.logIn(user, (err) => {
       if (err) { return next(err) }
-      return loginSuccess(req, res, user);
+      return loginSuccessHandler(req, res, user);
     });
   };
 
@@ -209,7 +218,7 @@ module.exports = function(crowi, app) {
       req.logIn(user, (err) => {
         if (err) { return next() }
 
-        return loginSuccess(req, res, user);
+        return loginSuccessHandler(req, res, user);
       });
     })(req, res, next);
   };
@@ -235,7 +244,7 @@ module.exports = function(crowi, app) {
       response = await promisifiedPassportAuthentication(strategyName, req, res);
     }
     catch (err) {
-      return loginFailure(req, res);
+      return loginFailureHandler(req, res);
     }
 
     const userInfo = {
@@ -255,7 +264,7 @@ module.exports = function(crowi, app) {
 
     const externalAccount = await getOrCreateUser(req, res, userInfo, providerId);
     if (!externalAccount) {
-      return loginFailure(req, res);
+      return loginFailureHandler(req, res);
     }
 
     const user = await externalAccount.getPopulatedUser();
@@ -263,7 +272,7 @@ module.exports = function(crowi, app) {
     // login
     req.logIn(user, (err) => {
       if (err) { return next(err) }
-      return loginSuccess(req, res, user);
+      return loginSuccessHandler(req, res, user);
     });
   };
 
@@ -286,7 +295,7 @@ module.exports = function(crowi, app) {
       response = await promisifiedPassportAuthentication(strategyName, req, res);
     }
     catch (err) {
-      return loginFailure(req, res);
+      return loginFailureHandler(req, res);
     }
 
     const userInfo = {
@@ -297,7 +306,7 @@ module.exports = function(crowi, app) {
 
     const externalAccount = await getOrCreateUser(req, res, userInfo, providerId);
     if (!externalAccount) {
-      return loginFailure(req, res);
+      return loginFailureHandler(req, res);
     }
 
     const user = await externalAccount.getPopulatedUser();
@@ -305,7 +314,7 @@ module.exports = function(crowi, app) {
     // login
     req.logIn(user, (err) => {
       if (err) { return next(err) }
-      return loginSuccess(req, res, user);
+      return loginSuccessHandler(req, res, user);
     });
   };
 
@@ -328,7 +337,7 @@ module.exports = function(crowi, app) {
       response = await promisifiedPassportAuthentication(strategyName, req, res);
     }
     catch (err) {
-      return loginFailure(req, res);
+      return loginFailureHandler(req, res);
     }
 
     const userInfo = {
@@ -339,7 +348,7 @@ module.exports = function(crowi, app) {
 
     const externalAccount = await getOrCreateUser(req, res, userInfo, providerId);
     if (!externalAccount) {
-      return loginFailure(req, res);
+      return loginFailureHandler(req, res);
     }
 
     const user = await externalAccount.getPopulatedUser();
@@ -347,7 +356,7 @@ module.exports = function(crowi, app) {
     // login
     req.logIn(user, (err) => {
       if (err) { return next(err) }
-      return loginSuccess(req, res, user);
+      return loginSuccessHandler(req, res, user);
     });
   };
 
@@ -375,7 +384,7 @@ module.exports = function(crowi, app) {
     }
     catch (err) {
       debug(err);
-      return loginFailure(req, res);
+      return loginFailureHandler(req, res);
     }
 
     const userInfo = {
@@ -388,14 +397,14 @@ module.exports = function(crowi, app) {
 
     const externalAccount = await getOrCreateUser(req, res, userInfo, providerId);
     if (!externalAccount) {
-      return loginFailure(req, res);
+      return loginFailureHandler(req, res);
     }
 
     // login
     const user = await externalAccount.getPopulatedUser();
     req.logIn(user, (err) => {
       if (err) { return next(err) }
-      return loginSuccess(req, res, user);
+      return loginSuccessHandler(req, res, user);
     });
   };
 
@@ -423,7 +432,7 @@ module.exports = function(crowi, app) {
       response = await promisifiedPassportAuthentication(strategyName, req, res);
     }
     catch (err) {
-      return loginFailure(req, res);
+      return loginFailureHandler(req, res);
     }
 
     const userInfo = {
@@ -441,23 +450,23 @@ module.exports = function(crowi, app) {
 
     const externalAccount = await getOrCreateUser(req, res, userInfo, providerId);
     if (!externalAccount) {
-      return loginFailure(req, res);
+      return loginFailureHandler(req, res);
     }
 
     const user = await externalAccount.getPopulatedUser();
 
     // Attribute-based Login Control
     if (!crowi.passportService.verifySAMLResponseByABLCRule(response)) {
-      return loginFailure(req, res, 'Sign in failure due to insufficient privileges.');
+      return loginFailureHandler(req, res, 'Sign in failure due to insufficient privileges.');
     }
 
     // login
     req.logIn(user, (err) => {
       if (err != null) {
         logger.error(err);
-        return loginFailure(req, res);
+        return loginFailureHandler(req, res);
       }
-      return loginSuccess(req, res, user);
+      return loginSuccessHandler(req, res, user);
     });
   };
 
@@ -482,7 +491,7 @@ module.exports = function(crowi, app) {
       userId = await promisifiedPassportAuthentication(strategyName, req, res);
     }
     catch (err) {
-      return loginFailure(req, res);
+      return loginFailureHandler(req, res);
     }
 
     const userInfo = {
@@ -493,13 +502,13 @@ module.exports = function(crowi, app) {
 
     const externalAccount = await getOrCreateUser(req, res, userInfo, providerId);
     if (!externalAccount) {
-      return loginFailure(req, res);
+      return loginFailureHandler(req, res);
     }
 
     const user = await externalAccount.getPopulatedUser();
     await req.logIn(user, (err) => {
       if (err) { return next() }
-      return loginSuccess(req, res, user);
+      return loginSuccessHandler(req, res, user);
     });
   };
 

src/server/service/passport.js

@@ -719,6 +719,11 @@ class PassportService {
     if (field === '<implicit>') {
       return attributes[term] != null;
     }
+
+    if (attributes[field] == null) {
+      return false;
+    }
+
     return attributes[field].includes(term);
   }
 

src/test/service/passport.test.js

@@ -23,28 +23,30 @@ describe('PassportService test', () => {
     });
 
     /* eslint-disable indent */
+    let i = 0;
     describe.each`
       conditionId | departments   | positions     | ruleStr                                                         | expected
-      ${1}        | ${[]}         | ${['Leader']} | ${'Position'}                                                   | ${true}
-      ${2}        | ${[]}         | ${['Leader']} | ${'Position: Leader'}                                           | ${true}
-      ${3}        | ${['A']}      | ${[]}         | ${'Department: A || Department: B && Position: Leader'}         | ${true}
-      ${4}        | ${['B']}      | ${['Leader']} | ${'Department: A || Department: B && Position: Leader'}         | ${true}
-      ${5}        | ${['A', 'C']} | ${['Leader']} | ${'Department: A || Department: B && Position: Leader'}         | ${true}
-      ${6}        | ${['B', 'C']} | ${['Leader']} | ${'Department: A || Department: B && Position: Leader'}         | ${true}
-      ${7}        | ${[]}         | ${[]}         | ${'Department: A || Department: B && Position: Leader'}         | ${false}
-      ${8}        | ${['C']}      | ${['Leader']} | ${'Department: A || Department: B && Position: Leader'}         | ${false}
-      ${9}        | ${['A']}      | ${['Leader']} | ${'(Department: A || Department: B) && Position: Leader'}       | ${true}
-      ${10}       | ${['B']}      | ${['Leader']} | ${'(Department: A || Department: B) && Position: Leader'}       | ${true}
-      ${11}       | ${['C']}      | ${['Leader']} | ${'(Department: A || Department: B) && Position: Leader'}       | ${false}
-      ${12}       | ${['A', 'B']} | ${[]}         | ${'(Department: A || Department: B) && Position: Leader'}       | ${false}
-      ${13}       | ${['A']}      | ${[]}         | ${'Department: A NOT Position: Leader'}                         | ${true}
-      ${14}       | ${['A']}      | ${['Leader']} | ${'Department: A NOT Position: Leader'}                         | ${false}
-      ${15}       | ${[]}         | ${['Leader']} | ${'Department: A OR (Position NOT Position: User)'}             | ${true}
-      ${16}       | ${[]}         | ${['User']}   | ${'Department: A OR (Position NOT Position: User)'}             | ${false}
+      ${i++}      | ${undefined}  | ${undefined}  | ${'Department: A'}                                              | ${false}
+      ${i++}      | ${[]}         | ${['Leader']} | ${'Position'}                                                   | ${true}
+      ${i++}      | ${[]}         | ${['Leader']} | ${'Position: Leader'}                                           | ${true}
+      ${i++}      | ${['A']}      | ${[]}         | ${'Department: A || Department: B && Position: Leader'}         | ${true}
+      ${i++}      | ${['B']}      | ${['Leader']} | ${'Department: A || Department: B && Position: Leader'}         | ${true}
+      ${i++}      | ${['A', 'C']} | ${['Leader']} | ${'Department: A || Department: B && Position: Leader'}         | ${true}
+      ${i++}      | ${['B', 'C']} | ${['Leader']} | ${'Department: A || Department: B && Position: Leader'}         | ${true}
+      ${i++}      | ${[]}         | ${[]}         | ${'Department: A || Department: B && Position: Leader'}         | ${false}
+      ${i++}      | ${['C']}      | ${['Leader']} | ${'Department: A || Department: B && Position: Leader'}         | ${false}
+      ${i++}      | ${['A']}      | ${['Leader']} | ${'(Department: A || Department: B) && Position: Leader'}       | ${true}
+      ${i++}      | ${['B']}      | ${['Leader']} | ${'(Department: A || Department: B) && Position: Leader'}       | ${true}
+      ${i++}      | ${['C']}      | ${['Leader']} | ${'(Department: A || Department: B) && Position: Leader'}       | ${false}
+      ${i++}      | ${['A', 'B']} | ${[]}         | ${'(Department: A || Department: B) && Position: Leader'}       | ${false}
+      ${i++}      | ${['A']}      | ${[]}         | ${'Department: A NOT Position: Leader'}                         | ${true}
+      ${i++}      | ${['A']}      | ${['Leader']} | ${'Department: A NOT Position: Leader'}                         | ${false}
+      ${i++}      | ${[]}         | ${['Leader']} | ${'Department: A OR (Position NOT Position: User)'}             | ${true}
+      ${i++}      | ${[]}         | ${['User']}   | ${'Department: A OR (Position NOT Position: User)'}             | ${false}
     `('to be $expected under rule="$ruleStr"', ({
       conditionId, departments, positions, ruleStr, expected,
     }) => {
-      test(`when condition=${conditionId}`, async() => {
+      test(`when conditionId=${conditionId}`, async() => {
         const responseMock = {};
 
         // setup mock implementation