Commit a78c443d authored by Stan Hu's avatar Stan Hu

Merge branch '6965-backport-to-ce' into 'master'

Backport LDAP changes to CE

See merge request gitlab-org/gitlab-ce!21361
parents 087cfc6f 5894dfab
...@@ -19,8 +19,10 @@ module Gitlab ...@@ -19,8 +19,10 @@ module Gitlab
# Whether user is allowed, or not, we should update # Whether user is allowed, or not, we should update
# permissions to keep things clean # permissions to keep things clean
if access.allowed? if access.allowed?
access.update_user unless Gitlab::Database.read_only?
Users::UpdateService.new(user, user: user, last_credential_check_at: Time.now).execute access.update_user
Users::UpdateService.new(user, user: user, last_credential_check_at: Time.now).execute
end
true true
else else
...@@ -60,6 +62,12 @@ module Gitlab ...@@ -60,6 +62,12 @@ module Gitlab
false false
end end
def update_user
# no-op in CE
end
private
def adapter def adapter
@adapter ||= Gitlab::Auth::LDAP::Adapter.new(provider) @adapter ||= Gitlab::Auth::LDAP::Adapter.new(provider)
end end
...@@ -68,16 +76,16 @@ module Gitlab ...@@ -68,16 +76,16 @@ module Gitlab
Gitlab::Auth::LDAP::Config.new(provider) Gitlab::Auth::LDAP::Config.new(provider)
end end
def find_ldap_user
Gitlab::Auth::LDAP::Person.find_by_dn(ldap_identity.extern_uid, adapter)
end
def ldap_user def ldap_user
return unless provider return unless provider
@ldap_user ||= find_ldap_user @ldap_user ||= find_ldap_user
end end
def find_ldap_user
Gitlab::Auth::LDAP::Person.find_by_dn(ldap_identity.extern_uid, adapter)
end
def block_user(user, reason) def block_user(user, reason)
user.ldap_block user.ldap_block
...@@ -102,10 +110,6 @@ module Gitlab ...@@ -102,10 +110,6 @@ module Gitlab
"unblocking Gitlab user \"#{user.name}\" (#{user.email})" "unblocking Gitlab user \"#{user.name}\" (#{user.email})"
) )
end end
def update_user
# no-op in CE
end
end end
end end
end end
......
...@@ -3,51 +3,61 @@ require 'spec_helper' ...@@ -3,51 +3,61 @@ require 'spec_helper'
describe Gitlab::Auth::LDAP::Access do describe Gitlab::Auth::LDAP::Access do
include LdapHelpers include LdapHelpers
let(:access) { described_class.new user }
let(:user) { create(:omniauth_user) } let(:user) { create(:omniauth_user) }
subject(:access) { described_class.new(user) }
describe '.allowed?' do describe '.allowed?' do
it 'updates the users `last_credential_check_at' do before do
allow(access).to receive(:update_user) allow(access).to receive(:update_user)
expect(access).to receive(:allowed?) { true } allow(access).to receive(:allowed?).and_return(true)
expect(described_class).to receive(:open).and_yield(access) 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) } expect { described_class.allowed?(user) }
.to change { user.last_credential_check_at } .to change { user.last_credential_check_at }
end end
end
describe '#find_ldap_user' do it "does not update user's `last_credential_check_at` when in a read-only GitLab instance" do
it 'finds a user by dn first' do allow(Gitlab::Database).to receive(:read_only?).and_return(true)
expect(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(:ldap_user)
access.find_ldap_user expect { described_class.allowed?(user) }
.not_to change { user.last_credential_check_at }
end end
end end
describe '#allowed?' do describe '#allowed?' do
subject { access.allowed? }
context 'when the user cannot be found' do context 'when the user cannot be found' do
before do before do
allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(nil) stub_ldap_person_find_by_dn(nil)
allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_email).and_return(nil) stub_ldap_person_find_by_email(user.email, nil)
end end
it { is_expected.to be_falsey } it 'returns false' do
expect(access.allowed?).to be_falsey
end
it 'blocks user in GitLab' do 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? access.allowed?
end end
end end
context 'when the user is found' do context 'when the user is found' do
let(:ldap_user) { Gitlab::Auth::LDAP::Person.new(Net::LDAP::Entry.new, 'ldapmain') }
before do 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 end
context 'and the user is disabled via active directory' do context 'and the user is disabled via active directory' do
...@@ -55,10 +65,22 @@ describe Gitlab::Auth::LDAP::Access 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) allow(Gitlab::Auth::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(true)
end end
it { is_expected.to be_falsey } it 'returns false' do
expect(access.allowed?).to be_falsey
end
it 'blocks user in GitLab' do 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? access.allowed?
end end
...@@ -92,7 +114,17 @@ describe Gitlab::Auth::LDAP::Access do ...@@ -92,7 +114,17 @@ describe Gitlab::Auth::LDAP::Access do
end end
it 'unblocks user in GitLab' do 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? access.allowed?
end end
...@@ -105,18 +137,32 @@ describe Gitlab::Auth::LDAP::Access do ...@@ -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) allow_any_instance_of(Gitlab::Auth::LDAP::Config).to receive(:active_directory).and_return(false)
end 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 context 'when user cannot be found' do
before do before do
allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(nil) stub_ldap_person_find_by_dn(nil)
allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_email).and_return(nil) stub_ldap_person_find_by_email(user.email, nil)
end end
it { is_expected.to be_falsey } it 'returns false' do
expect(access.allowed?).to be_falsey
end
it 'blocks user in GitLab' do 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? access.allowed?
end end
...@@ -128,7 +174,17 @@ describe Gitlab::Auth::LDAP::Access do ...@@ -128,7 +174,17 @@ describe Gitlab::Auth::LDAP::Access do
end end
it 'unblocks the user if it exists' do 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? access.allowed?
end end
...@@ -152,46 +208,4 @@ describe Gitlab::Auth::LDAP::Access do ...@@ -152,46 +208,4 @@ describe Gitlab::Auth::LDAP::Access do
end end
end 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 end
...@@ -37,6 +37,23 @@ module LdapHelpers ...@@ -37,6 +37,23 @@ module LdapHelpers
.to receive(:find_by_uid).with(uid, any_args).and_return(return_value) .to receive(:find_by_uid).with(uid, any_args).and_return(return_value)
end 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. # Create a simple LDAP user entry.
def ldap_user_entry(uid) def ldap_user_entry(uid)
entry = Net::LDAP::Entry.new entry = Net::LDAP::Entry.new
......
...@@ -68,6 +68,10 @@ module StubConfiguration ...@@ -68,6 +68,10 @@ module StubConfiguration
allow(Gitlab.config.repositories).to receive(:storages).and_return(Settingslogic.new(messages)) allow(Gitlab.config.repositories).to receive(:storages).and_return(Settingslogic.new(messages))
end end
def stub_kerberos_setting(messages)
allow(Gitlab.config.kerberos).to receive_messages(to_settings(messages))
end
private private
# Modifies stubbed messages to also stub possible predicate versions # Modifies stubbed messages to also stub possible predicate versions
......
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