refactor loginWithLdap

lib/routes/login-passport.js

@@ -5,6 +5,7 @@ module.exports = function(crowi, app) {
     , passport = require('passport')
     , config = crowi.getConfig()
     , Config = crowi.model('Config')
+    , ExternalAccount = crowi.model('ExternalAccount')
     , passportService = crowi.passportService
     ;
 
@@ -61,13 +62,13 @@ module.exports = function(crowi, app) {
       });
     }
 
-    passport.authenticate('ldapauth', (err, user, info) => {
+    passport.authenticate('ldapauth', (err, ldapAccountInfo, info) => {
       if (res.headersSent) {  // dirty hack -- 2017.09.25
         return;               // cz: somehow passport.authenticate called twice when ECONNREFUSED error occurred
       }
 
       debug('--- authenticate with LdapStrategy ---');
-      debug('user', user);
+      debug('ldapAccountInfo', ldapAccountInfo);
       debug('info', info);
 
       if (err) {  // DB Error
@@ -75,19 +76,42 @@ module.exports = function(crowi, app) {
         req.flash('warningMessage', 'LDAP Server Error occured.');
         return next(); // pass and the flash message is displayed when all of authentications are failed.
       }
-      if (info) {
-        if (info.name != null && info.name === 'DuplicatedUsernameException') {
-          req.flash('isDuplicatedUsernameExceptionOccured', true);
-          return next();
-        }
-      }
-      if (!user) { return next(); }
-      req.logIn(user, (err) => {
-        if (err) { return next(); }
-        else {
-          return loginSuccess(req, res, user);
-        }
-      });
+
+      // authentication failure
+      if (!ldapAccountInfo) { return next(); }
+
+      /*
+       * authentication success
+       */
+      // it is guaranteed that username that is input from form can be acquired
+      // because this processes after authentication
+      const ldapAccountId = passportService.getLdapAccountIdFromReq(req);
+
+      const attrMapUsername = passportService.getLdapAttrNameMappedToUsername();
+      const usernameToBeRegistered = ldapAccountInfo[attrMapUsername];
+
+      // find or register(create) user
+      ExternalAccount.findOrRegister('ldap', ldapAccountId, usernameToBeRegistered)
+        .then((externalAccount) => {
+          return externalAccount.getPopulatedUser();
+        })
+        .then((user) => {
+          // login
+          req.logIn(user, (err) => {
+            if (err) { return next(); }
+            else {
+              return loginSuccess(req, res, user);
+            }
+          });
+        })
+        .catch((err) => {
+          debug('findOrRegister error: ', err);
+          if (err.name != null && err.name === 'DuplicatedUsernameException') {
+            req.flash('isDuplicatedUsernameExceptionOccured', true);
+            return next();
+          }
+        });
+
     })(req, res, next);
   }
 

lib/service/passport.js

@@ -112,34 +112,10 @@ class PassportService {
 
     debug('LdapStrategy: setting up..');
 
-    const attrMapUsername = config.crowi['security:passport-ldap:attrMapUsername'] || 'uid';
-    debug(`LdapStrategy: attrMapUsername=${attrMapUsername}`);
-
     passport.use(new LdapStrategy(this.getLdapConfigurationFunc(config, {passReqToCallback: true}),
       (req, ldapAccountInfo, done) => {
         debug("LDAP authentication has succeeded", ldapAccountInfo);
         done(null, ldapAccountInfo);
-        /*
-        const ExternalAccount = this.crowi.model('ExternalAccount');
-
-        // it is guaranteed that username that is input from form can be acquired
-        // because this processes after authentication
-        const ldapAccountId = this.getLdapAccountIdFromReq(req);
-
-        const usernameToBeRegistered = ldapAccountInfo[attrMapUsername];
-
-        // find or register(create) user
-        ExternalAccount.findOrRegister('ldap', ldapAccountId, usernameToBeRegistered)
-          .then((externalAccount) => {
-            return externalAccount.getPopulatedUser();
-          })
-          .then((user) => {
-            done(null, user);
-          })
-          .catch((err) => {
-            done(null, false, err);
-          });
-        */
       }
     ));
 
@@ -147,6 +123,17 @@ class PassportService {
     debug('LdapStrategy: setup is done');
   }
 
+  /**
+   * return attribute name for mapping to username of Crowi DB
+   *
+   * @returns
+   * @memberof PassportService
+   */
+  getLdapAttrNameMappedToUsername() {
+    const config = this.crowi.config;
+    return config.crowi['security:passport-ldap:attrMapUsername'] || 'uid';
+  }
+
   /**
    * CAUTION: this method is capable to use only when `req.body.loginForm` is not null
    *