From 0ebeb58a6cfc89459bf3af2f678f5ea619e89832 Mon Sep 17 00:00:00 2001 From: Thorsten Eckel Date: Tue, 30 May 2017 08:26:45 +0200 Subject: [PATCH] Fixed issue #1097 - Can't map LDAP group to multiple Zammad roles. --- .../app/controllers/_integration/ldap.coffee | 18 ++++++++++---- .../app/views/integration/ldap.jst.eco | 8 +++---- ...20170529132120_ldap_multi_group_mapping.rb | 24 +++++++++++++++++++ lib/ldap/group.rb | 15 +++++++----- spec/lib/import/ldap/user_factory_spec.rb | 6 ++--- spec/lib/import/ldap/user_spec.rb | 7 +++--- 6 files changed, 58 insertions(+), 20 deletions(-) create mode 100644 db/migrate/20170529132120_ldap_multi_group_mapping.rb diff --git a/app/assets/javascripts/app/controllers/_integration/ldap.coffee b/app/assets/javascripts/app/controllers/_integration/ldap.coffee index cda274205..b497b0f05 100644 --- a/app/assets/javascripts/app/controllers/_integration/ldap.coffee +++ b/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) => diff --git a/app/assets/javascripts/app/views/integration/ldap.jst.eco b/app/assets/javascripts/app/views/integration/ldap.jst.eco index 5cfed344f..b580e21fa 100644 --- a/app/assets/javascripts/app/views/integration/ldap.jst.eco +++ b/app/assets/javascripts/app/views/integration/ldap.jst.eco @@ -64,7 +64,7 @@ <% end %>

<%- @T('Role') %>

- <% if _.isEmpty(@config.group_role_map): %> + <% if _.isEmpty(@group_role_map): %>
<%- @T('No Entries') %>
@@ -75,10 +75,10 @@ <%- @T('LDAP') %> <%- @T('Zammad') %> - <% for key, value of @config.group_role_map: %> + <% for source, dests of @group_role_map: %> - <%= key %> - <%= App.Role.find(value).displayName() %> + <%= source %> + <%= dests %> <% end %> <% end %> diff --git a/db/migrate/20170529132120_ldap_multi_group_mapping.rb b/db/migrate/20170529132120_ldap_multi_group_mapping.rb new file mode 100644 index 000000000..3c277aa23 --- /dev/null +++ b/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 diff --git a/lib/ldap/group.rb b/lib/ldap/group.rb index eb3a10cc7..0e63a8dc2 100644 --- a/lib/ldap/group.rb +++ b/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 diff --git a/spec/lib/import/ldap/user_factory_spec.rb b/spec/lib/import/ldap/user_factory_spec.rb index 80f1a4c31..05c34b0c0 100644 --- a/spec/lib/import/ldap/user_factory_spec.rb +++ b/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) diff --git a/spec/lib/import/ldap/user_spec.rb b/spec/lib/import/ldap/user_spec.rb index 823218edc..f49140828 100644 --- a/spec/lib/import/ldap/user_spec.rb +++ b/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