Backport LDAP changes to CE
This commit is contained in:
parent
bde1bdae71
commit
5894dfabc5
4 changed files with 120 additions and 81 deletions
|
@ -19,8 +19,10 @@ module Gitlab
|
|||
# Whether user is allowed, or not, we should update
|
||||
# permissions to keep things clean
|
||||
if access.allowed?
|
||||
access.update_user
|
||||
Users::UpdateService.new(user, user: user, last_credential_check_at: Time.now).execute
|
||||
unless Gitlab::Database.read_only?
|
||||
access.update_user
|
||||
Users::UpdateService.new(user, user: user, last_credential_check_at: Time.now).execute
|
||||
end
|
||||
|
||||
true
|
||||
else
|
||||
|
@ -60,6 +62,12 @@ module Gitlab
|
|||
false
|
||||
end
|
||||
|
||||
def update_user
|
||||
# no-op in CE
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def adapter
|
||||
@adapter ||= Gitlab::Auth::LDAP::Adapter.new(provider)
|
||||
end
|
||||
|
@ -68,16 +76,16 @@ module Gitlab
|
|||
Gitlab::Auth::LDAP::Config.new(provider)
|
||||
end
|
||||
|
||||
def find_ldap_user
|
||||
Gitlab::Auth::LDAP::Person.find_by_dn(ldap_identity.extern_uid, adapter)
|
||||
end
|
||||
|
||||
def ldap_user
|
||||
return unless provider
|
||||
|
||||
@ldap_user ||= find_ldap_user
|
||||
end
|
||||
|
||||
def find_ldap_user
|
||||
Gitlab::Auth::LDAP::Person.find_by_dn(ldap_identity.extern_uid, adapter)
|
||||
end
|
||||
|
||||
def block_user(user, reason)
|
||||
user.ldap_block
|
||||
|
||||
|
@ -102,10 +110,6 @@ module Gitlab
|
|||
"unblocking Gitlab user \"#{user.name}\" (#{user.email})"
|
||||
)
|
||||
end
|
||||
|
||||
def update_user
|
||||
# no-op in CE
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -3,51 +3,61 @@ require 'spec_helper'
|
|||
describe Gitlab::Auth::LDAP::Access do
|
||||
include LdapHelpers
|
||||
|
||||
let(:access) { described_class.new user }
|
||||
let(:user) { create(:omniauth_user) }
|
||||
|
||||
describe '.allowed?' do
|
||||
it 'updates the users `last_credential_check_at' do
|
||||
allow(access).to receive(:update_user)
|
||||
expect(access).to receive(:allowed?) { true }
|
||||
expect(described_class).to receive(:open).and_yield(access)
|
||||
subject(:access) { described_class.new(user) }
|
||||
|
||||
describe '.allowed?' do
|
||||
before do
|
||||
allow(access).to receive(:update_user)
|
||||
allow(access).to receive(:allowed?).and_return(true)
|
||||
allow(described_class).to receive(:open).and_yield(access)
|
||||
end
|
||||
|
||||
it "updates the user's `last_credential_check_at`" do
|
||||
expect { described_class.allowed?(user) }
|
||||
.to change { user.last_credential_check_at }
|
||||
end
|
||||
end
|
||||
|
||||
describe '#find_ldap_user' do
|
||||
it 'finds a user by dn first' do
|
||||
expect(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(:ldap_user)
|
||||
it "does not update user's `last_credential_check_at` when in a read-only GitLab instance" do
|
||||
allow(Gitlab::Database).to receive(:read_only?).and_return(true)
|
||||
|
||||
access.find_ldap_user
|
||||
expect { described_class.allowed?(user) }
|
||||
.not_to change { user.last_credential_check_at }
|
||||
end
|
||||
end
|
||||
|
||||
describe '#allowed?' do
|
||||
subject { access.allowed? }
|
||||
|
||||
context 'when the user cannot be found' do
|
||||
before do
|
||||
allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(nil)
|
||||
allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_email).and_return(nil)
|
||||
stub_ldap_person_find_by_dn(nil)
|
||||
stub_ldap_person_find_by_email(user.email, nil)
|
||||
end
|
||||
|
||||
it { is_expected.to be_falsey }
|
||||
it 'returns false' do
|
||||
expect(access.allowed?).to be_falsey
|
||||
end
|
||||
|
||||
it 'blocks user in GitLab' do
|
||||
expect(access).to receive(:block_user).with(user, 'does not exist anymore')
|
||||
access.allowed?
|
||||
|
||||
expect(user).to be_blocked
|
||||
expect(user).to be_ldap_blocked
|
||||
end
|
||||
|
||||
it 'logs the reason' do
|
||||
expect(Gitlab::AppLogger).to receive(:info).with(
|
||||
"LDAP account \"123456\" does not exist anymore, " \
|
||||
"blocking Gitlab user \"#{user.name}\" (#{user.email})"
|
||||
)
|
||||
|
||||
access.allowed?
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the user is found' do
|
||||
let(:ldap_user) { Gitlab::Auth::LDAP::Person.new(Net::LDAP::Entry.new, 'ldapmain') }
|
||||
|
||||
before do
|
||||
allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(ldap_user)
|
||||
stub_ldap_person_find_by_dn(Net::LDAP::Entry.new)
|
||||
end
|
||||
|
||||
context 'and the user is disabled via active directory' do
|
||||
|
@ -55,10 +65,22 @@ describe Gitlab::Auth::LDAP::Access do
|
|||
allow(Gitlab::Auth::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(true)
|
||||
end
|
||||
|
||||
it { is_expected.to be_falsey }
|
||||
it 'returns false' do
|
||||
expect(access.allowed?).to be_falsey
|
||||
end
|
||||
|
||||
it 'blocks user in GitLab' do
|
||||
expect(access).to receive(:block_user).with(user, 'is disabled in Active Directory')
|
||||
access.allowed?
|
||||
|
||||
expect(user).to be_blocked
|
||||
expect(user).to be_ldap_blocked
|
||||
end
|
||||
|
||||
it 'logs the reason' do
|
||||
expect(Gitlab::AppLogger).to receive(:info).with(
|
||||
"LDAP account \"123456\" is disabled in Active Directory, " \
|
||||
"blocking Gitlab user \"#{user.name}\" (#{user.email})"
|
||||
)
|
||||
|
||||
access.allowed?
|
||||
end
|
||||
|
@ -92,7 +114,17 @@ describe Gitlab::Auth::LDAP::Access do
|
|||
end
|
||||
|
||||
it 'unblocks user in GitLab' do
|
||||
expect(access).to receive(:unblock_user).with(user, 'is not disabled anymore')
|
||||
access.allowed?
|
||||
|
||||
expect(user).not_to be_blocked
|
||||
expect(user).not_to be_ldap_blocked
|
||||
end
|
||||
|
||||
it 'logs the reason' do
|
||||
expect(Gitlab::AppLogger).to receive(:info).with(
|
||||
"LDAP account \"123456\" is not disabled anymore, " \
|
||||
"unblocking Gitlab user \"#{user.name}\" (#{user.email})"
|
||||
)
|
||||
|
||||
access.allowed?
|
||||
end
|
||||
|
@ -105,18 +137,32 @@ describe Gitlab::Auth::LDAP::Access do
|
|||
allow_any_instance_of(Gitlab::Auth::LDAP::Config).to receive(:active_directory).and_return(false)
|
||||
end
|
||||
|
||||
it { is_expected.to be_truthy }
|
||||
it 'returns true' do
|
||||
expect(access.allowed?).to be_truthy
|
||||
end
|
||||
|
||||
context 'when user cannot be found' do
|
||||
before do
|
||||
allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(nil)
|
||||
allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_email).and_return(nil)
|
||||
stub_ldap_person_find_by_dn(nil)
|
||||
stub_ldap_person_find_by_email(user.email, nil)
|
||||
end
|
||||
|
||||
it { is_expected.to be_falsey }
|
||||
it 'returns false' do
|
||||
expect(access.allowed?).to be_falsey
|
||||
end
|
||||
|
||||
it 'blocks user in GitLab' do
|
||||
expect(access).to receive(:block_user).with(user, 'does not exist anymore')
|
||||
access.allowed?
|
||||
|
||||
expect(user).to be_blocked
|
||||
expect(user).to be_ldap_blocked
|
||||
end
|
||||
|
||||
it 'logs the reason' do
|
||||
expect(Gitlab::AppLogger).to receive(:info).with(
|
||||
"LDAP account \"123456\" does not exist anymore, " \
|
||||
"blocking Gitlab user \"#{user.name}\" (#{user.email})"
|
||||
)
|
||||
|
||||
access.allowed?
|
||||
end
|
||||
|
@ -128,7 +174,17 @@ describe Gitlab::Auth::LDAP::Access do
|
|||
end
|
||||
|
||||
it 'unblocks the user if it exists' do
|
||||
expect(access).to receive(:unblock_user).with(user, 'is available again')
|
||||
access.allowed?
|
||||
|
||||
expect(user).not_to be_blocked
|
||||
expect(user).not_to be_ldap_blocked
|
||||
end
|
||||
|
||||
it 'logs the reason' do
|
||||
expect(Gitlab::AppLogger).to receive(:info).with(
|
||||
"LDAP account \"123456\" is available again, " \
|
||||
"unblocking Gitlab user \"#{user.name}\" (#{user.email})"
|
||||
)
|
||||
|
||||
access.allowed?
|
||||
end
|
||||
|
@ -152,46 +208,4 @@ describe Gitlab::Auth::LDAP::Access do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#block_user' do
|
||||
before do
|
||||
user.activate
|
||||
allow(Gitlab::AppLogger).to receive(:info)
|
||||
|
||||
access.block_user user, 'reason'
|
||||
end
|
||||
|
||||
it 'blocks the user' do
|
||||
expect(user).to be_blocked
|
||||
expect(user).to be_ldap_blocked
|
||||
end
|
||||
|
||||
it 'logs the reason' do
|
||||
expect(Gitlab::AppLogger).to have_received(:info).with(
|
||||
"LDAP account \"123456\" reason, " \
|
||||
"blocking Gitlab user \"#{user.name}\" (#{user.email})"
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#unblock_user' do
|
||||
before do
|
||||
user.ldap_block
|
||||
allow(Gitlab::AppLogger).to receive(:info)
|
||||
|
||||
access.unblock_user user, 'reason'
|
||||
end
|
||||
|
||||
it 'activates the user' do
|
||||
expect(user).not_to be_blocked
|
||||
expect(user).not_to be_ldap_blocked
|
||||
end
|
||||
|
||||
it 'logs the reason' do
|
||||
Gitlab::AppLogger.info(
|
||||
"LDAP account \"123456\" reason, " \
|
||||
"unblocking Gitlab user \"#{user.name}\" (#{user.email})"
|
||||
)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -37,6 +37,23 @@ module LdapHelpers
|
|||
.to receive(:find_by_uid).with(uid, any_args).and_return(return_value)
|
||||
end
|
||||
|
||||
def stub_ldap_person_find_by_dn(entry, provider = 'ldapmain')
|
||||
person = ::Gitlab::Auth::LDAP::Person.new(entry, provider) if entry.present?
|
||||
|
||||
allow(::Gitlab::Auth::LDAP::Person)
|
||||
.to receive(:find_by_dn)
|
||||
.and_return(person)
|
||||
end
|
||||
|
||||
def stub_ldap_person_find_by_email(email, entry, provider = 'ldapmain')
|
||||
person = ::Gitlab::Auth::LDAP::Person.new(entry, provider) if entry.present?
|
||||
|
||||
allow(::Gitlab::Auth::LDAP::Person)
|
||||
.to receive(:find_by_email)
|
||||
.with(email, anything)
|
||||
.and_return(person)
|
||||
end
|
||||
|
||||
# Create a simple LDAP user entry.
|
||||
def ldap_user_entry(uid)
|
||||
entry = Net::LDAP::Entry.new
|
||||
|
|
|
@ -68,6 +68,10 @@ module StubConfiguration
|
|||
allow(Gitlab.config.repositories).to receive(:storages).and_return(Settingslogic.new(messages))
|
||||
end
|
||||
|
||||
def stub_kerberos_setting(messages)
|
||||
allow(Gitlab.config.kerberos).to receive_messages(to_settings(messages))
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
# Modifies stubbed messages to also stub possible predicate versions
|
||||
|
|
Loading…
Reference in a new issue