From 1ec2b514bb574937bc378f46fa922768a58d5dca Mon Sep 17 00:00:00 2001 From: Thorsten Eckel Date: Fri, 12 Jan 2018 13:38:43 +0100 Subject: [PATCH] Fixed issue #1746 - Multiple LDAP users with same samaccountname cause repeatedly update of local user. --- .../import/ldap/user/lookup/attributes.rb | 19 +++++- .../unit/import/ldap/users/sub_sequence.rb | 8 ++- .../ldap/user/lookup/attributes_spec.rb | 64 +++++++++++++++++++ 3 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 spec/lib/sequencer/unit/import/ldap/user/lookup/attributes_spec.rb diff --git a/lib/sequencer/unit/import/ldap/user/lookup/attributes.rb b/lib/sequencer/unit/import/ldap/user/lookup/attributes.rb index a04e49722..c97b93e9e 100644 --- a/lib/sequencer/unit/import/ldap/user/lookup/attributes.rb +++ b/lib/sequencer/unit/import/ldap/user/lookup/attributes.rb @@ -4,11 +4,24 @@ class Sequencer module Ldap module User module Lookup - class Attributes < Sequencer::Unit::Import::Common::Model::Lookup::Attributes + class Attributes < Sequencer::Unit::Import::Common::Model::FindBy::UserAttributes + + uses :found_ids, :external_sync_source + private - def attributes - %i[login email] + def lookup(attribute:, value:) + entries = model_class.where(attribute => value).to_a + return if entries.blank? + not_synced(entries) + end + + def not_synced(entries) + entries.find(&method(:not_synced?)) + end + + def not_synced?(entry) + found_ids.exclude?(entry.id) end end end diff --git a/lib/sequencer/unit/import/ldap/users/sub_sequence.rb b/lib/sequencer/unit/import/ldap/users/sub_sequence.rb index 05066c669..b890ad508 100644 --- a/lib/sequencer/unit/import/ldap/users/sub_sequence.rb +++ b/lib/sequencer/unit/import/ldap/users/sub_sequence.rb @@ -12,7 +12,6 @@ class Sequencer provides :found_ids def process - found_ids = [] ldap_connection.search(ldap_config[:user_filter], attributes: relevant_attributes) do |entry| result = sequence_resource(entry) @@ -26,13 +25,18 @@ class Sequencer private + def found_ids + @found_ids ||= [] + end + def default_params super.merge( dn_roles: dn_roles, ldap_config: ldap_config, model_class: model_class, external_sync_source: external_sync_source, - signup_role_ids: signup_role_ids + signup_role_ids: signup_role_ids, + found_ids: found_ids, ) end diff --git a/spec/lib/sequencer/unit/import/ldap/user/lookup/attributes_spec.rb b/spec/lib/sequencer/unit/import/ldap/user/lookup/attributes_spec.rb new file mode 100644 index 000000000..a3ba06c4e --- /dev/null +++ b/spec/lib/sequencer/unit/import/ldap/user/lookup/attributes_spec.rb @@ -0,0 +1,64 @@ +require 'rails_helper' + +RSpec.describe Sequencer::Unit::Import::Ldap::User::Lookup::Attributes, sequencer: :unit do + + let(:model_class) { ::User } + let(:external_sync_source) { 'test' } + + it 'finds entries via lookup attributes' do + + current_user = create(:user) + + # ExternalSync.create( + # source: external_sync_source, + # source_id: remote_id, + # o_id: user.id, + # object: user.class, + # ) + + provided = process( + found_ids: [], + model_class: model_class, + external_sync_source: external_sync_source, + mapped: { + login: current_user.login, + email: current_user.email, + } + ) + + expect(provided[:instance]).to eq(current_user) + end + + it "doesn't find already synced/found entries with same lookup attributes" do + + other_user = create(:user) + + provided = process( + found_ids: [other_user.id], + model_class: model_class, + external_sync_source: external_sync_source, + mapped: { + login: other_user.login, + email: other_user.email, + } + ) + + expect(provided[:instance]).to be_nil + end + + it "doesn't not synced users" do + + provided = process( + found_ids: [], + model_class: model_class, + external_sync_source: external_sync_source, + mapped: { + login: 'example.login', + email: 'test@example.com', + } + ) + + expect(provided[:instance]).to be_nil + end + +end