clean code

lib/routes/login-passport.js

@@ -42,6 +42,22 @@ module.exports = function(crowi, app) {
     return res.redirect('/login');
   };
 
+  /**
+   * return true(valid) or false(invalid)
+   *
+   *  true ... group filter is not defined or the user has one or more groups
+   *  false ... group filter is defined and the user has any group
+   *
+   */
+  function isValidLdapUserByGroupFilter(user) {
+    let bool = true;
+    if (user._groups != null) {
+      if (user._groups.length == 0) {
+        bool = false;
+      }
+    }
+    return bool;
+  }
   /**
    * middleware that login with LdapStrategy
    * @param {*} req
@@ -80,10 +96,8 @@ module.exports = function(crowi, app) {
       // authentication failure
       if (!ldapAccountInfo) { return next(); }
       // check groups
-      if (ldapAccountInfo._groups != null) {
-        if (ldapAccountInfo._groups.length == 0) {
-          return loginFailure(req, res, next);
-        }
+      if (!isValidLdapUserByGroupFilter(ldapAccountInfo)) {
+        return loginFailure(req, res, next);
       }
 
       /*
@@ -160,13 +174,11 @@ module.exports = function(crowi, app) {
       }
       if (user) {
         // check groups
-        if (user._groups != null) {
-          if (user._groups.length == 0) {
-            return res.json({
-              status: 'warning',
-              message: 'An user is found, but the groups are empty.',
-            });
-          }
+        if (!isValidLdapUserByGroupFilter(user)) {
+          return res.json({
+            status: 'warning',
+            message: 'An user is found, but the groups are empty.',
+          });
         }
         return res.json({
           status: 'success',

lib/service/passport.js

@@ -202,7 +202,7 @@ class PassportService {
       let serverOpt = { url, bindDN: fixedBindDN, bindCredentials: fixedBindCredentials, searchBase, searchFilter };
 
       if (groupSearchBase && groupSearchFilter) {
-        serverOpt = Object.assign({ groupSearchBase, groupSearchFilter, groupDnProperty}, serverOpt);
+        serverOpt = Object.assign(serverOpt, { groupSearchBase, groupSearchFilter, groupDnProperty });
       }
 
       process.nextTick(() => {

lib/views/admin/widget/passport/ldap.html

@@ -151,7 +151,9 @@
               The query used to filter for groups.<br>
               Use <code>{% raw %}{{dn}}{% endraw %}</code> to have it replaced of the found user object.<br>
               <br>
-              Example: <code>(&(cn=group1)(memberUid={% raw %}{{dn}}{% endraw %}))</code> hits the groups which has <code>cn=group1</code> and <code>memberUid</code> equals to the user's <code>uid</code> (when <code>Group DN Property</code> is not changed from the default value.)
+              Example: <code>(&(cn=group1)(memberUid={% raw %}{{dn}}{% endraw %}))</code> hits the groups
+              which has <code>cn=group1</code> and <code>memberUid</code> includes the user's <code>uid</code>
+              (when <code>Group DN Property</code> is not changed from the default value.)
             </small>
           </p>
         </div>