Commit 460a7338 authored by Lin Jen-Shin's avatar Lin Jen-Shin

Fix user access due to having multiple identities

This is an attempt for the EE part for:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12968
parent ae10e20f
...@@ -6,7 +6,7 @@ ...@@ -6,7 +6,7 @@
module Gitlab module Gitlab
module LDAP module LDAP
class Access class Access
attr_reader :adapter, :provider, :user, :ldap_user, :ldap_identity attr_reader :provider, :user, :ldap_identity
def self.open(user, &block) def self.open(user, &block)
Gitlab::LDAP::Adapter.open(user.ldap_identity.provider) do |adapter| Gitlab::LDAP::Adapter.open(user.ldap_identity.provider) do |adapter|
...@@ -32,8 +32,8 @@ module Gitlab ...@@ -32,8 +32,8 @@ module Gitlab
def initialize(user, adapter = nil) def initialize(user, adapter = nil)
@adapter = adapter @adapter = adapter
@user = user @user = user
@provider = adapter&.provider || user.ldap_identity.provider @ldap_identity = user.ldap_identity
@ldap_identity = user.identities.find_by(provider: @provider) @provider = adapter&.provider || @ldap_identity&.provider
end end
def allowed? def allowed?
...@@ -67,10 +67,12 @@ module Gitlab ...@@ -67,10 +67,12 @@ module Gitlab
end end
def find_ldap_user def find_ldap_user
return unless provider
found_user = Gitlab::LDAP::Person.find_by_dn(ldap_identity.extern_uid, adapter) found_user = Gitlab::LDAP::Person.find_by_dn(ldap_identity.extern_uid, adapter)
return found_user if found_user return found_user if found_user
if user.external_email? && [nil, provider].include?(user.email_provider) if ldap_identity
Gitlab::LDAP::Person.find_by_email(user.email, adapter) Gitlab::LDAP::Person.find_by_email(user.email, adapter)
end end
end end
...@@ -82,10 +84,17 @@ module Gitlab ...@@ -82,10 +84,17 @@ module Gitlab
def block_user(user, reason) def block_user(user, reason)
user.ldap_block user.ldap_block
Gitlab::AppLogger.info( if provider
"LDAP account \"#{ldap_identity.extern_uid}\" #{reason}, " \ Gitlab::AppLogger.info(
"blocking Gitlab user \"#{user.name}\" (#{user.email})" "LDAP account \"#{ldap_identity.extern_uid}\" #{reason}, " \
) "blocking Gitlab user \"#{user.name}\" (#{user.email})"
)
else
Gitlab::AppLogger.info(
"Account is not provided by LDAP, " \
"blocking Gitlab user \"#{user.name}\" (#{user.email})"
)
end
end end
def unblock_user(user, reason) def unblock_user(user, reason)
......
...@@ -20,14 +20,12 @@ describe Gitlab::LDAP::Access do ...@@ -20,14 +20,12 @@ describe Gitlab::LDAP::Access do
describe '#find_ldap_user' do describe '#find_ldap_user' do
it 'finds a user by dn first' do it 'finds a user by dn first' do
expect(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(:ldap_user) expect(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(:ldap_user)
expect(user).not_to receive(:external_email?)
access.find_ldap_user access.find_ldap_user
end end
it 'finds a user by email if the email came from LDAP' do it 'finds a user by email if the email came from LDAP' do
expect(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(nil) expect(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(nil)
expect(user).to receive(:external_email?).and_return(true)
expect(Gitlab::LDAP::Person).to receive(:find_by_email) expect(Gitlab::LDAP::Person).to receive(:find_by_email)
access.find_ldap_user access.find_ldap_user
...@@ -51,7 +49,7 @@ describe Gitlab::LDAP::Access do ...@@ -51,7 +49,7 @@ describe Gitlab::LDAP::Access do
end end
context 'when looking for a user by email' do context 'when looking for a user by email' do
let(:user) { create(:omniauth_user, external_email: true) } let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'my-provider') }
it { is_expected.to be_falsey } it { is_expected.to be_falsey }
end end
......
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