Commit c8b9f7a4 authored by Douwe Maan's avatar Douwe Maan Committed by Rémy Coutable

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
parent ba5b02dc
---
title: Omniauth auto link LDAP user falls back to find by DN when user cannot be found
by UID
merge_request: 7002
author:
...@@ -102,6 +102,8 @@ module Gitlab ...@@ -102,6 +102,8 @@ module Gitlab
Gitlab::LDAP::Config.providers.each do |provider| Gitlab::LDAP::Config.providers.each do |provider|
adapter = Gitlab::LDAP::Adapter.new(provider) adapter = Gitlab::LDAP::Adapter.new(provider)
@ldap_person = Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter) @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 break if @ldap_person
end end
@ldap_person @ldap_person
......
...@@ -137,11 +137,12 @@ describe Gitlab::OAuth::User, lib: true do ...@@ -137,11 +137,12 @@ describe Gitlab::OAuth::User, lib: true do
allow(ldap_user).to receive(:username) { uid } allow(ldap_user).to receive(:username) { uid }
allow(ldap_user).to receive(:email) { ['johndoe@example.com', 'john2@example.com'] } 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(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 end
context "and no account for the LDAP user" do context "and no account for the LDAP user" do
it "creates a user with dual LDAP and omniauth identities" 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 oauth_user.save
expect(gl_user).to be_valid expect(gl_user).to be_valid
...@@ -159,6 +160,8 @@ describe Gitlab::OAuth::User, lib: true do ...@@ -159,6 +160,8 @@ describe Gitlab::OAuth::User, lib: true do
context "and LDAP user has an account already" 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') } 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 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 oauth_user.save
expect(gl_user).to be_valid expect(gl_user).to be_valid
...@@ -172,6 +175,24 @@ describe Gitlab::OAuth::User, lib: true do ...@@ -172,6 +175,24 @@ describe Gitlab::OAuth::User, lib: true do
]) ])
end end
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 end
context "and no corresponding LDAP person" do context "and no corresponding LDAP person" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment