From c63e6e5be3e1ca0657a2ac5b6d4c57a6fae447a0 Mon Sep 17 00:00:00 2001
From: Thorsten Eckel <te@znuny.com>
Date: Fri, 2 Feb 2018 16:19:40 +0100
Subject: [PATCH] Fixed issue #1795 - Forcing samaccountname as login attribute
 is not suitable and causes issues in divergent setups.

---
 .../app/controllers/_integration/ldap.coffee  | 26 +++++-----
 ...80202000002_custom_ldap_login_attribute.rb | 48 +++++++++++++++++++
 .../unit/import/ldap/user/mapping.rb          |  8 +---
 3 files changed, 62 insertions(+), 20 deletions(-)
 create mode 100644 db/migrate/20180202000002_custom_ldap_login_attribute.rb

diff --git a/app/assets/javascripts/app/controllers/_integration/ldap.coffee b/app/assets/javascripts/app/controllers/_integration/ldap.coffee
index 433b80b57..f3267e3f2 100644
--- a/app/assets/javascripts/app/controllers/_integration/ldap.coffee
+++ b/app/assets/javascripts/app/controllers/_integration/ldap.coffee
@@ -358,7 +358,6 @@ class ConnectionWizard extends App.WizardModal
         # remember payload
         user_attributes = {}
         for key, value of App.User.attributesGet()
-          continue if key == 'login'
           if (value.tag is 'input' || value.tag is 'richtext' || value.tag is 'textarea')  && value.type isnt 'password'
             user_attributes[key] = value.display || key
         roles = {}
@@ -394,6 +393,7 @@ class ConnectionWizard extends App.WizardModal
         givenname: 'firstname'
         sn: 'lastname'
         mail: 'email'
+        samaccountname: 'login'
         telephonenumber: 'phone'
 
     @userMappingForm.find('tbody tr.js-entry').remove()
@@ -417,14 +417,23 @@ class ConnectionWizard extends App.WizardModal
     for key in ['source', 'dest']
       if !_.isArray(user_attributes[key])
         user_attributes[key] = [user_attributes[key]]
-    user_attributes_local =
-      'samaccountname': 'login'
+    user_attributes_local = {}
     length = user_attributes.source.length-1
     for count in [0..length]
       if user_attributes.source[count] && user_attributes.dest[count]
         user_attributes_local[user_attributes.source[count]] = user_attributes.dest[count]
+
+    requiredAttribute = Object.keys(user_attributes_local).some( (local_attribute) ->
+      return user_attributes_local[local_attribute] == 'login'
+    )
+
     @wizardConfig.user_attributes = user_attributes_local
 
+    if !requiredAttribute
+      @showSlide('js-mapping')
+      @showAlert('js-mapping', App.i18n.translatePlain("Attribute '%s' is required in the mapping", 'login'))
+      return
+
     # group role map
     group_role_map = @formParam(@groupRoleForm)
     for key in ['source', 'dest']
@@ -448,17 +457,8 @@ class ConnectionWizard extends App.WizardModal
 
   buildRowsUserMap: (user_attribute_map) =>
 
-    # show static login row
-    userUidDisplayValue = @wizardConfig.wizardData.backend_user_attributes['samaccountname']
-
-    el = [
-      $(App.view('integration/ldap_user_attribute_row_read_only')(
-        key:   userUidDisplayValue,
-        value: 'Login'
-      ))
-    ]
+    el = []
     for source, dest of user_attribute_map
-      continue if source == 'samaccountname'
       continue if !(source of @wizardConfig.wizardData.backend_user_attributes)
       el.push @buildRowUserAttribute(source, dest)
     el
diff --git a/db/migrate/20180202000002_custom_ldap_login_attribute.rb b/db/migrate/20180202000002_custom_ldap_login_attribute.rb
new file mode 100644
index 000000000..60297d258
--- /dev/null
+++ b/db/migrate/20180202000002_custom_ldap_login_attribute.rb
@@ -0,0 +1,48 @@
+class CustomLdapLoginAttribute < ActiveRecord::Migration[5.1]
+  def up
+
+    # return if it's a new setup
+    return if !Setting.find_by(name: 'system_init_done')
+    return if no_change_needed?
+
+    perform_changes
+  end
+
+  private
+
+  def perform_changes
+    delete_spared
+    update_config
+  end
+
+  def delete_spared
+    # remove samaccountname which is always wrong if there is more than
+    # one other login attribute since it's automatically added
+    ldap_config[:user_attributes].delete('samaccountname')
+
+    # this should not happen but remove any other duplicate that
+    # maps to login and keep the "first" in the list
+    # - which is more or less random
+    login_attributes.reject { |e| e == 'samaccountname' }.drop(1).each do |spared|
+      ldap_config[:user_attributes].delete(spared)
+    end
+  end
+
+  def update_config
+    Import::Ldap.config = ldap_config
+  end
+
+  def login_attributes
+    @login_attributes ||= ldap_config[:user_attributes].select { |_local, remote| remote == 'login' }.keys
+  end
+
+  def no_change_needed?
+    return true if ldap_config.blank?
+    return true if ldap_config[:user_attributes].blank?
+    ldap_config[:user_attributes].values.count('login') < 2
+  end
+
+  def ldap_config
+    @ldap_config ||= Import::Ldap.config
+  end
+end
diff --git a/lib/sequencer/unit/import/ldap/user/mapping.rb b/lib/sequencer/unit/import/ldap/user/mapping.rb
index 53fe7498c..09ecfa477 100644
--- a/lib/sequencer/unit/import/ldap/user/mapping.rb
+++ b/lib/sequencer/unit/import/ldap/user/mapping.rb
@@ -9,13 +9,7 @@ class Sequencer
             private
 
             def mapping
-              ldap_config[:user_attributes].dup.tap do |config|
-                # fallback to samaccountname as login
-                # if no login is given via mapping
-                if !config.values.include?('login')
-                  config['samaccountname'] = 'login'
-                end
-              end
+              ldap_config[:user_attributes]
             end
           end
         end