clean code

lib/models/external-account.js

@@ -24,6 +24,14 @@ schema.plugin(uniqueValidator);
  */
 class ExternalAccount {
 
+  static set crowi(crowi) {
+    this._crowi = crowi;
+  }
+
+  static get crowi() {
+    return this._crowi;
+  }
+
   /**
    * get the populated user entity
    *
@@ -43,11 +51,11 @@ class ExternalAccount {
    * @static
    * @param {string} providerType
    * @param {string} accountId
-   * @param {object} createUserFunction the function that create a user document and return Promise<User>
+   * @param {object} usernameToBeRegistered the username of User entity that will be created when accountId is not found
    * @returns {Promise<ExternalAccount>}
    * @memberof ExternalAccount
    */
-  static findOrRegister(providerType, accountId, createUserFunction) {
+  static findOrRegister(providerType, accountId, usernameToBeRegistered) {
 
     return this.findOne({ providerType, accountId })
       .then((account) => {
@@ -60,9 +68,18 @@ class ExternalAccount {
         else {
           debug(`ExternalAccount '${accountId}' is not found, it is going to be registered.`);
 
+          const User = ExternalAccount.crowi.model('User');
+
           // TODO count and throw error if username is dupricated
+          return User.count({username: usernameToBeRegistered})
+            .then((count) => {
+              if (count > 0) {
+                throw new DuplicatedUsernameException(`username '${usernameToBeRegistered}' has already been existed`);
+              }
 
-          return createUserFunction().then((user) => {
+              return User.createUser('', usernameToBeRegistered, undefined, undefined, undefined);
+            })
+            .then((user) => {
               return this.create({ providerType: 'ldap', accountId, user: user._id });
             });
         }
@@ -72,7 +89,20 @@ class ExternalAccount {
 
 }
 
+/**
+ * The Exception class thrown when User.username is duplicated when creating user
+ *
+ * @class DuplicatedUsernameException
+ */
+class DuplicatedUsernameException {
+  constructor(message) {
+    this.name = this.constructor.name;
+    this.message = message;
+  }
+}
+
 module.exports = function(crowi) {
+  ExternalAccount.crowi = crowi;
   schema.loadClass(ExternalAccount);
   return mongoose.model('ExternalAccount', schema);
 }

lib/service/passport.js

@@ -116,11 +116,18 @@ class PassportService {
       (req, ldapAccountInfo, done) => {
         debug("LDAP authentication has succeeded", ldapAccountInfo);
 
-        // it is guaranteed that username can be acquired
+        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);
+        // TODO ensure to be able to select the way to determine username
+        const usernameToBeRegistered = ldapAccountInfo['uid'];
 
-        this.findOrRegisterUserByLdapInfo(ldapAccountId, ldapAccountInfo)
+        ExternalAccount.findOrRegister('ldap', ldapAccountId, usernameToBeRegistered)
+          .then((externalAccount) => {
+            return externalAccount.getPopulatedUser();
+          })
           .then((user) => {
             done(null, user);
           })
@@ -206,28 +213,6 @@ class PassportService {
     };
   }
 
-  /**
-   * find the ExternalAccount or register if not found
-   *
-   * @param {string} ldapAccountId
-   * @param {object} ldapAccountInfo
-   * @returns
-   * @memberof PassportService
-   */
-  findOrRegisterUserByLdapInfo(ldapAccountId, ldapAccountInfo) {
-    const User = this.crowi.model('User');
-    const ExternalAccount = this.crowi.model('ExternalAccount');
-
-    return ExternalAccount.findOrRegister('ldap', ldapAccountId, () => {
-        // TODO ensure to be able to select the way to determine username
-        const username = ldapAccountInfo['uid'];
-        return User.createUser('', username, undefined, undefined, undefined);
-      })
-      .then((account) => {
-        return account.getPopulatedUser();
-      });
-  }
-
   /**
    * setup serializer and deserializer
    *