Fixed issue #1097 - Can't map LDAP group to multiple Zammad roles.

app/assets/javascripts/app/controllers/_integration/ldap.coffee

@@ -61,8 +61,15 @@ class Form extends App.Controller
   render: (top = false) =>
     @config = @currentConfig()
 
+    group_role_map = {}
+    for source, dests of @config.group_role_map
+      group_role_map[source] = dests.map((dest) ->
+        App.Role.find(dest).displayName()
+      ).join ', '
+
     @html App.view('integration/ldap')(
-      config: @config
+      config: @config,
+      group_role_map: group_role_map
     )
     if _.isEmpty(@config)
       @$('.js-notConfigured').removeClass('hide')
@@ -419,7 +426,9 @@ class ConnectionWizard extends App.WizardModal
     length = group_role_map.source.length-1
     for count in [0..length]
       if group_role_map.source[count] && group_role_map.dest[count]
-        group_role_map_local[group_role_map.source[count]] = group_role_map.dest[count]
+        if !_.isArray(group_role_map_local[group_role_map.source[count]])
+          group_role_map_local[group_role_map.source[count]] = []
+        group_role_map_local[group_role_map.source[count]].push group_role_map.dest[count]
     @wizardConfig.group_role_map = group_role_map_local
 
     expertSettings = @formParam(@expertForm)
@@ -454,8 +463,9 @@ class ConnectionWizard extends App.WizardModal
 
   buildRowsGroupRole: (group_role_map) =>
     el = []
-    for source, dest of group_role_map
-      el.push @buildRowGroupRole(source, dest)
+    for source, dests of group_role_map
+      for dest in dests
+        el.push @buildRowGroupRole(source, dest)
     el
 
   buildRowGroupRole: (source, dest) =>

app/assets/javascripts/app/views/integration/ldap.jst.eco

@@ -64,7 +64,7 @@
   <% end %>
 
   <h3><%- @T('Role') %></h3>
-  <% if _.isEmpty(@config.group_role_map): %>
+  <% if _.isEmpty(@group_role_map): %>
     <table class="settings-list settings-list--stretch settings-list--placeholder">
       <thead><tr><th><%- @T('No Entries') %>
     </table>
@@ -75,10 +75,10 @@
           <th width="40%"><%- @T('LDAP') %>
           <th width="60%"><%- @T('Zammad') %>
       <tbody>
-        <% for key, value of @config.group_role_map: %>
+        <% for source, dests of @group_role_map: %>
           <tr>
-            <td class="settings-list-row-control"><%= key %>
-            <td class="settings-list-row-control"><%= App.Role.find(value).displayName() %>
+            <td class="settings-list-row-control"><%= source %>
+            <td class="settings-list-row-control"><%= dests %>
         <% end %>
   <% end %>
     </table>

db/migrate/20170529132120_ldap_multi_group_mapping.rb

@@ -0,0 +1,24 @@
+class LdapMultiGroupMapping < ActiveRecord::Migration
+  def up
+
+    # return if it's a new setup
+    return if !Setting.find_by(name: 'system_init_done')
+
+    # load existing LDAP config
+    ldap_config = Setting.get('ldap_config')
+
+    # exit early if no config is present
+    return if ldap_config.blank?
+    return if ldap_config['group_role_map'].blank?
+
+    # loop over group role mapping and check
+    # if we need to migrate to new array structure
+    ldap_config['group_role_map'].each do |source, dest|
+      next if dest.is_a?(Array)
+      ldap_config['group_role_map'][source] = [dest]
+    end
+
+    # store updated
+    Setting.set('ldap_config', ldap_config)
+  end
+end

lib/ldap/group.rb

@@ -85,16 +85,19 @@ class Ldap
         members = entry[:member]
         next if members.blank?
 
-        role = mapping[entry.dn.downcase]
-        next if role.blank?
-        role = role.to_i
+        roles = mapping[entry.dn.downcase]
+        next if roles.blank?
 
         members.each do |user_dn|
           user_dn_key = user_dn.downcase
 
-          result[user_dn_key] ||= []
-          next if result[user_dn_key].include?(role)
-          result[user_dn_key].push(role)
+          roles.each do |role|
+            role = role.to_i
+
+            result[user_dn_key] ||= []
+            next if result[user_dn_key].include?(role)
+            result[user_dn_key].push(role)
+          end
         end
       end
 

spec/lib/import/ldap/user_factory_spec.rb

@@ -35,7 +35,7 @@ RSpec.describe Import::Ldap::UserFactory do
       # group user role mapping
       expect(mocked_ldap).to receive(:search)
       # user counting
-      expect(mocked_ldap).to receive(:count).and_return(1)
+      allow(mocked_ldap).to receive(:count).and_return(1)
       # user search
       expect(mocked_ldap).to receive(:search).and_yield(mocked_entry)
 
@@ -201,7 +201,7 @@ RSpec.describe Import::Ldap::UserFactory do
       config = {
         group_filter:   '(objectClass=group)',
         group_role_map: {
-          group_dn => '1',
+          group_dn => %w(1 2),
         }
       }
 
@@ -219,7 +219,7 @@ RSpec.describe Import::Ldap::UserFactory do
       )
 
       expected = {
-        user_dn => [1]
+        user_dn => [1, 2]
       }
 
       expect(user_roles).to be_a(Hash)

spec/lib/import/ldap/user_spec.rb

@@ -29,7 +29,8 @@ RSpec.describe Import::Ldap::User do
   let(:user_roles) do
     {
       user_entry.dn => [
-        Role.find_by(name: 'Admin').id
+        Role.find_by(name: 'Admin').id,
+        Role.find_by(name: 'Agent').id
       ]
     }
   end
@@ -90,8 +91,8 @@ RSpec.describe Import::Ldap::User do
           # gets called later it will get initialized
           # with the changed dn
           user_roles[ user_entry.dn ] = [
-            Role.find_by(name: 'Agent').id,
-            Role.find_by(name: 'Admin').id
+            Role.find_by(name: 'Admin').id,
+            Role.find_by(name: 'Agent').id
           ]
 
           # change dn so no mapping will match