diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-11-15 10:20:11 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-11-15 10:20:11 +0000 |
commit | 3a3b06b4faccaf49279bee397af6df2916a9195e (patch) | |
tree | 4cd77e6fc3160bfd21e5911ad68c073aec6c568f | |
parent | e98e7c60c5ff115f1d892a82722e0951e049e301 (diff) | |
parent | 3cff3a2e5b87c40927eb02a8884c84260ca30c2a (diff) | |
download | gitlab-ce-3a3b06b4faccaf49279bee397af6df2916a9195e.tar.gz |
Merge branch 'fix_saml_ldap_link' into 'master'
Omniauth auto link LDAP user falls back to find by DN when user cannot be found by uid
Unfortunately, SAML IDs can be an LDAP UID, DN, or something else entirely. UID and DN are most common, though. This adds a fallback scenario so we first try to find a matching LDAP user by UID, then by DN. This will fix a problem for the customer in https://gitlab.zendesk.com/agent/tickets/43298
See merge request !7002
-rw-r--r-- | changelogs/unreleased/fix_saml_ldap_link.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/o_auth/user.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/o_auth/user_spec.rb | 23 |
3 files changed, 29 insertions, 1 deletions
diff --git a/changelogs/unreleased/fix_saml_ldap_link.yml b/changelogs/unreleased/fix_saml_ldap_link.yml new file mode 100644 index 00000000000..3b6f26d610e --- /dev/null +++ b/changelogs/unreleased/fix_saml_ldap_link.yml @@ -0,0 +1,5 @@ +--- +title: Omniauth auto link LDAP user falls back to find by DN when user cannot be found + by UID +merge_request: 7002 +author: diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 0a91d3918d5..a8b4dc2a83f 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -102,6 +102,8 @@ module Gitlab Gitlab::LDAP::Config.providers.each do |provider| adapter = Gitlab::LDAP::Adapter.new(provider) @ldap_person = Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter) + # The `uid` might actually be a DN. Try it next. + @ldap_person ||= Gitlab::LDAP::Person.find_by_dn(auth_hash.uid, adapter) break if @ldap_person end @ldap_person diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 78c669e8fa5..fc9e1cb430a 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -137,11 +137,12 @@ describe Gitlab::OAuth::User, lib: true do allow(ldap_user).to receive(:username) { uid } allow(ldap_user).to receive(:email) { ['johndoe@example.com', 'john2@example.com'] } 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) end context "and no account for the LDAP user" do it "creates a user with dual LDAP and omniauth identities" do + allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) + oauth_user.save expect(gl_user).to be_valid @@ -159,6 +160,8 @@ describe Gitlab::OAuth::User, lib: true do context "and LDAP user has an account already" do let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') } it "adds the omniauth identity to the LDAP account" do + allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) + oauth_user.save expect(gl_user).to be_valid @@ -172,6 +175,24 @@ describe Gitlab::OAuth::User, lib: true do ]) end end + + context 'when an LDAP person is not found by uid' do + it 'tries to find an LDAP person by DN and adds the omniauth identity to the user' do + allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(nil) + allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(ldap_user) + + oauth_user.save + + identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } + expect(identities_as_hash) + .to match_array( + [ + { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' }, + { provider: 'twitter', extern_uid: uid } + ] + ) + end + end end context "and no corresponding LDAP person" do |