Merge pull request #348 from weseek/support/bind-new-ldap-account-to-local

Support/bind new ldap account to local

lib/form/admin/securityGeneral.js

@@ -12,6 +12,5 @@ module.exports = form(
   field('settingForm[security:restrictGuestMode]').required(),
   field('settingForm[security:registrationMode]').required(),
   field('settingForm[security:registrationWhiteList]').custom(normalizeCRLF).custom(stringToArray),
-  field('settingForm[security:externalAsLocal]').trim().toBooleanStrict(),
 );
 

lib/form/admin/securityPassportLdap.js

@@ -16,6 +16,7 @@ module.exports = form(
   field('settingForm[security:passport-ldap:bindDNPassword]'),
   field('settingForm[security:passport-ldap:searchFilter]'),
   field('settingForm[security:passport-ldap:attrMapUsername]'),
+  field('settingForm[security:passport-ldap:isTreatUsernameMatchingAsIdentical]').trim().toBooleanStrict(),
   field('settingForm[security:passport-ldap:groupSearchBase]'),
   field('settingForm[security:passport-ldap:groupSearchFilter]'),
   field('settingForm[security:passport-ldap:groupDnProperty]')

lib/locales/en-US/translation.json

@@ -276,8 +276,8 @@
     "only_those":" Only those whose e-mail address including the company address can register.",
     "insert_single":"Please insert single e-mail address per line.",
     "Authentication mechanism settings":"Authentication mechanism settings",
-    "external_as_local": "Bind the external account with the same username as the local one to the local one automatically",
-    "external_as_local_help": "Enabling this option may be helpful if you want to authenticate users with external accounts, that are originally created as local accounts."
+    "Treat username matching as identical": "Automatically bind external accounts newly logged in to local accounts when <code>username</code> match",
+    "Treat username matching as identical_warn": "WARNING: Be aware of security because the system treats the same user as a match of <code>username</code>."
   },
 
   "markdown_setting": {

lib/locales/ja/translation.json

@@ -296,8 +296,8 @@
     "only_those":"その会社のメールアドレスを持っている人のみ登録可能になります。",
     "insert_single":"1行に1メールアドレス入力してください。",
     "Authentication mechanism settings":"認証機構設定",
-    "external_as_local": "外部アカウントをそれと同じユーザー名を持ったローカルアカウントに自動的にリンクする",
-    "external_as_local_help": "ローカルアカウントとして作成されたユーザーを外部アカウントで認証したい場合に役に立つかもしれません。"
+    "Treat username matching as identical": "新規ログイン時、<code>username</code> が一致したローカルアカウントが存在した場合は自動的に紐付ける",
+    "Treat username matching as identical_warn": "WARNING: <code>username</code> の一致を以て同一ユーザーであるとみなすので、セキュリティに注意してください"
   },
   "markdown_setting": {
     "markdown_rendering": "Markdownレンダリングの設定を変更できます。",

lib/models/config.js

@@ -53,7 +53,6 @@ module.exports = function(crowi) {
       'security:registrationWhiteList' : [],
 
       'security:isEnabledPassport' : false,
-      'security:externalAsLocal': false,
       'security:passport-ldap:isEnabled' : false,
       'security:passport-ldap:serverUrl' : undefined,
       'security:passport-ldap:isUserBind' : undefined,
@@ -64,6 +63,7 @@ module.exports = function(crowi) {
       'security:passport-ldap:groupSearchBase' : undefined,
       'security:passport-ldap:groupSearchFilter' : undefined,
       'security:passport-ldap:groupDnProperty' : undefined,
+      'security:passport-ldap:isTreatUsernameMatchingAsIdentical': false,
 
       'aws:bucket'          : 'growi',
       'aws:region'          : 'ap-northeast-1',
@@ -278,9 +278,9 @@ module.exports = function(crowi) {
     return getValueForCrowiNS(config, key);
   };
 
-  configSchema.statics.shouldTreatExternalAccountAsLocal = function(config)
+  configSchema.statics.isTreatUsernameMatchingAsIdentical = function(config)
   {
-    const key = 'security:externalAsLocal';
+    const key = 'security:passport-ldap:isTreatUsernameMatchingAsIdentical';
     return getValueForCrowiNS(config, key);
   };
 

lib/models/external-account.js

@@ -70,39 +70,42 @@ class ExternalAccount {
    */
   static findOrRegister(providerType, accountId, usernameToBeRegistered) {
 
-    return this.findOne({ providerType, accountId }).then( account => {
-      // found
-      if (account != null) {
-        debug(`ExternalAccount '${accountId}' is found `, account);
-        return account;
-      }
-
-      const User = ExternalAccount.crowi.model('User');
-      const Config = ExternalAccount.crowi.model('Config');
-
-      const treatExternalUserAsLocalUser = Config.shouldTreatExternalAccountAsLocal(ExternalAccount.crowi.getConfig());
-
-      return User.find({username: usernameToBeRegistered}).then( users => {
-        // throw Exception when count is not zero
-        const maxDuplicateUserCount = treatExternalUserAsLocalUser ? 1 : 0;
-        if (users.length > maxDuplicateUserCount) {
-          throw new DuplicatedUsernameException(`username '${usernameToBeRegistered}' has already been existed`);
+    return this.findOne({ providerType, accountId })
+      .then(account => {
+        // ExternalAccount is found
+        if (account != null) {
+          debug(`ExternalAccount '${accountId}' is found `, account);
+          return account;
         }
 
-        if (users.length === 0) {
-          debug(`ExternalAccount '${accountId}' is not found, it is going to be registered.`);
-          // create user with STATUS_ACTIVE
-          return User.createUser('', usernameToBeRegistered, undefined, undefined, undefined, User.STATUS_ACTIVE);
-        }
-
-        debug(`ExternalAccount '${accountId}' will be linked to an exisiting account`);
-        return users[0];
+        const User = ExternalAccount.crowi.model('User');
+        const Config = ExternalAccount.crowi.model('Config');
+
+        // get option
+        const isTreatUsernameMatchingAsIdentical = Config.isTreatUsernameMatchingAsIdentical(ExternalAccount.crowi.getConfig());
+
+        return User.findOne({username: usernameToBeRegistered})
+          .then(user => {
+            // when the User that have the same `username` exists
+            if (user != null) {
+              if (isTreatUsernameMatchingAsIdentical) {
+                debug(`ExternalAccount '${accountId}' will be bind to an exisiting account`);
+                return user;
+              }
+              else {
+                throw new DuplicatedUsernameException(`Userr '${usernameToBeRegistered}' has already been existed`);
+              }
+            }
+
+            // create a new User with STATUS_ACTIVE
+            debug(`ExternalAccount '${accountId}' is not found, it is going to be registered.`);
+            return User.createUser('', usernameToBeRegistered, undefined, undefined, undefined, User.STATUS_ACTIVE);
+          })
+          .then(newUser => {
+            return this.create({ providerType: 'ldap', accountId, user: newUser._id });
+          });
 
-      }).then( newUser => {
-        return this.create({ providerType: 'ldap', accountId, user: newUser._id });
-      })
-
-    });
+      });
   }
 
   /**

lib/service/passport.js

@@ -170,7 +170,7 @@ class PassportService {
     const match = serverUrl.match(/(ldaps?:\/\/[^\/]+)\/(.*)?/);
     if (match == null || match.length < 1) {
       debug('LdapStrategy: serverUrl is invalid');
-      return;
+      return (req, callback) => { callback({ message: 'serverUrl is invalid'}) };
     }
     const url = match[1];
     const searchBase = match[2] || '';

lib/views/admin/security.html

@@ -90,25 +90,6 @@
             </div>
           </div>
 
-          <div class="form-group">
-            <div class="col-xs-offset-3 col-xs-6">
-              <div class="checkbox checkbox-info">
-                <input type="checkbox" id="externalAsLocal" name="settingForm[security:externalAsLocal]" value="1"
-                                                                                                                 {% if settingForm['security:externalAsLocal'] %}
-                checked
-                {% endif %}
-                />
-                <label for="externalAsLocal">
-                  {{ t("security_setting.external_as_local") }}
-                </label>
-              </div>
-  
-              <p class="help-block">
-                {{ t("security_setting.external_as_local_help") }}
-              </p>
-            </div>
-          </div>
-
           <div class="form-group">
             <div class="col-xs-offset-3 col-xs-6">
               <input type="hidden" name="_csrf" value="{{ csrf() }}">

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

@@ -122,9 +122,18 @@
               Specification of mappings when creating new users
             </small>
           </p>
+
+          <div class="checkbox checkbox-info">
+            <input type="checkbox" id="cbTreatUsernameMatchingAsIdentical" name="settingForm[security:passport-ldap:isTreatUsernameMatchingAsIdentical]" value="1"
+                {% if settingForm['security:passport-ldap:isTreatUsernameMatchingAsIdentical'] %}checked{% endif %} />
+            <label for="cbTreatUsernameMatchingAsIdentical">
+              {{ t("security_setting.Treat username matching as identical") }}
+            </label>
+          </div>
         </div>
       </div>
 
+
       <h4>Group Search Filter (Optional)</h4>
 
       <div class="form-group">