diff options
| -rw-r--r-- | changelogs/unreleased/33493-attempt-to-link-saml-users-to-ldap-by-email.yml | 5 | ||||
| -rw-r--r-- | lib/gitlab/ldap/person.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/o_auth/user.rb | 7 | ||||
| -rw-r--r-- | lib/gitlab/saml/user.rb | 16 | ||||
| -rw-r--r-- | spec/lib/gitlab/saml/user_spec.rb | 77 |
5 files changed, 101 insertions, 8 deletions
diff --git a/changelogs/unreleased/33493-attempt-to-link-saml-users-to-ldap-by-email.yml b/changelogs/unreleased/33493-attempt-to-link-saml-users-to-ldap-by-email.yml new file mode 100644 index 00000000000..727f3cecd52 --- /dev/null +++ b/changelogs/unreleased/33493-attempt-to-link-saml-users-to-ldap-by-email.yml @@ -0,0 +1,5 @@ +--- +title: Link SAML users to LDAP by email. +merge_request: 14216 +author: +type: changed diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb index 4d6f8ac79de..be64cc3991b 100644 --- a/lib/gitlab/ldap/person.rb +++ b/lib/gitlab/ldap/person.rb @@ -17,6 +17,10 @@ module Gitlab adapter.user('dn', dn) end + def self.find_by_email(email, adapter) + Array(adapter.config.attributes['email']).find { |attr| adapter.user(attr, email) } + end + def self.disabled_via_active_directory?(dn, adapter) adapter.dn_matches_filter?(dn, AD_USER_DISABLED) end diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 7704bf715e4..dd318b4242c 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -108,9 +108,12 @@ module Gitlab end def find_ldap_person(auth_hash, adapter) - by_uid = Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter) + person = Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter) # The `uid` might actually be a DN. Try it next. - by_uid || Gitlab::LDAP::Person.find_by_dn(auth_hash.uid, adapter) + person ||= Gitlab::LDAP::Person.find_by_dn(auth_hash.uid, adapter) + + # The `uid` might actually be a Email. Try it next. + person || Gitlab::LDAP::Person.find_by_email(auth_hash.uid, adapter) end def ldap_config diff --git a/lib/gitlab/saml/user.rb b/lib/gitlab/saml/user.rb index 0f323a9e8b2..2af54f8bb25 100644 --- a/lib/gitlab/saml/user.rb +++ b/lib/gitlab/saml/user.rb @@ -11,16 +11,16 @@ module Gitlab end def gl_user - if auto_link_ldap_user? + if auto_link_saml_user? + @user ||= find_by_email + end + + if auto_link_ldap_user? && !@user&.ldap_user? @user ||= find_or_create_ldap_user end @user ||= find_by_uid_and_provider - if auto_link_saml_user? - @user ||= find_by_email - end - if signup_enabled? @user ||= build_new_user end @@ -42,7 +42,11 @@ module Gitlab def find_by_email if auth_hash.has_attribute?(:email) user = ::User.find_by(email: auth_hash.email.downcase) - user.identities.new(extern_uid: auth_hash.uid, provider: auth_hash.provider) if user + + if user&.identities&.empty? + user.identities.new(extern_uid: auth_hash.uid, provider: auth_hash.provider) + end + user end end diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb index 19710029224..c6cade99daa 100644 --- a/spec/lib/gitlab/saml/user_spec.rb +++ b/spec/lib/gitlab/saml/user_spec.rb @@ -170,6 +170,7 @@ describe Gitlab::Saml::User do allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' } allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(ldap_user) + allow(Gitlab::LDAP::Person).to receive(:find_by_email).and_return(ldap_user) end context 'and no account for the LDAP user' do @@ -195,6 +196,82 @@ describe Gitlab::Saml::User do username: 'john') end + shared_examples 'find ldap person' do |uid_type, uid| + before do + allow(Gitlab::LDAP::Person).to receive(:"find_by_#{uid_type}").and_return(ldap_user) + end + + it 'adds the omniauth identity to the LDAP account' do + identities = [ + { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' }, + { provider: 'saml', extern_uid: extern_uid } + ] + + identities_as_hash = gl_user.identities.map do |id| + { provider: id.provider, extern_uid: id.extern_uid } + end + + saml_user.save + + expect(gl_user).to be_valid + expect(gl_user.username).to eql 'john' + expect(gl_user.email).to eql 'john@mail.com' + expect(gl_user.identities.length).to be 2 + expect(identities_as_hash).to match_array(identities) + end + end + + context 'when uid is an uid' do + it_behaves_like 'find ldap person', 'uid' do + let(:extern_uid) { uid } + let(:auth_hash) do + OmniAuth::AuthHash.new( + uid: uid, + provider: provider, + info: info_hash, + extra: { + raw_info: OneLogin::RubySaml::Attributes.new( + { 'groups' => %w(Developers Freelancers Designers) } + ) + }) + end + end + end + + context 'when uid is a dn' do + it_behaves_like 'find ldap person', 'email' do + let(:extern_uid) { 'uid=user1,ou=People,dc=example' } + let(:auth_hash) do + OmniAuth::AuthHash.new( + uid: extern_uid, + provider: provider, + info: info_hash, + extra: { + raw_info: OneLogin::RubySaml::Attributes.new( + { 'groups' => %w(Developers Freelancers Designers) } + ) + }) + end + end + end + + context 'when uid is an email' do + it_behaves_like 'find ldap person', 'email' do + let(:extern_uid) { 'john@mail.com' } + let(:auth_hash) do + OmniAuth::AuthHash.new( + uid: extern_uid, + provider: provider, + info: info_hash, + extra: { + raw_info: OneLogin::RubySaml::Attributes.new( + { 'groups' => %w(Developers Freelancers Designers) } + ) + }) + end + end + end + it 'adds the omniauth identity to the LDAP account' do saml_user.save |