Commit 4a568321 authored by Etienne Baqué's avatar Etienne Baqué

Made distinction between OAuth and Ldap block auto users

Also added some tests to check user activation when user cap not
reached, upon OAuth user save.
parent 0938f9f1
...@@ -409,13 +409,13 @@ module EE ...@@ -409,13 +409,13 @@ module EE
end end
def activate_based_on_user_cap? def activate_based_on_user_cap?
!blocked_auto_created_omniauth_user? && !blocked_auto_created_oauth_ldap_user? &&
blocked_pending_approval? && blocked_pending_approval? &&
self.class.user_cap_max.present? self.class.user_cap_max.present?
end end
def blocked_auto_created_omniauth_user? def blocked_auto_created_oauth_ldap_user?
::Gitlab.config.omniauth.block_auto_created_users && identities.any? identities.any? && block_auto_created_users?
end end
def has_valid_credit_card? def has_valid_credit_card?
...@@ -439,6 +439,17 @@ module EE ...@@ -439,6 +439,17 @@ module EE
private private
def block_auto_created_users?
if ldap_user?
provider = ldap_identity.provider
return false unless provider
::Gitlab::Auth::Ldap::Config.new(provider).block_auto_created_users
else
::Gitlab.config.omniauth.block_auto_created_users
end
end
def created_after_credit_card_release_day?(project) def created_after_credit_card_release_day?(project)
created_at >= ::Users::CreditCardValidation::RELEASE_DAY || created_at >= ::Users::CreditCardValidation::RELEASE_DAY ||
::Feature.enabled?(:ci_require_credit_card_for_old_users, project, default_enabled: :yaml) ::Feature.enabled?(:ci_require_credit_card_for_old_users, project, default_enabled: :yaml)
......
...@@ -5,6 +5,8 @@ module EE ...@@ -5,6 +5,8 @@ module EE
module Auth module Auth
module OAuth module OAuth
module User module User
protected
def activate_user_if_user_cap_not_reached def activate_user_if_user_cap_not_reached
if activate_user_based_on_user_cap?(gl_user) if activate_user_based_on_user_cap?(gl_user)
gl_user.activate gl_user.activate
...@@ -12,8 +14,6 @@ module EE ...@@ -12,8 +14,6 @@ module EE
end end
end end
protected
def find_ldap_person(auth_hash, adapter) def find_ldap_person(auth_hash, adapter)
if auth_hash.provider == 'kerberos' if auth_hash.provider == 'kerberos'
::Gitlab::Auth::Ldap::Person.find_by_kerberos_principal(auth_hash.uid, adapter) ::Gitlab::Auth::Ldap::Person.find_by_kerberos_principal(auth_hash.uid, adapter)
......
...@@ -44,5 +44,66 @@ RSpec.describe Gitlab::Auth::OAuth::User do ...@@ -44,5 +44,66 @@ RSpec.describe Gitlab::Auth::OAuth::User do
expect(gl_user.email).to eq(real_email) expect(gl_user.email).to eq(real_email)
end end
describe '#save' do
let(:user) { build(:omniauth_user, :blocked_pending_approval) }
before do
allow(oauth_user).to receive(:gl_user).and_return(user)
end
subject(:save_user) { oauth_user.save } # rubocop: disable Rails/SaveBang
describe '#activate_user_if_user_cap_not_reached' do
context 'when a user can be activated based on user cap' do
before do
allow(user).to receive(:activate_based_on_user_cap?).and_return(true)
end
context 'when the user cap has not been reached yet' do
it 'activates the user' do
allow(::User).to receive(:user_cap_reached?).and_return(false)
expect(oauth_user).to receive(:log_user_changes).with(
user, 'OAuth', 'user cap not reached yet, unblocking'
)
expect do
save_user
user.reload
end.to change { user.state }.from('blocked_pending_approval').to('active')
end
end
context 'when the user cap has been reached' do
it 'leaves the user as blocked' do
allow(::User).to receive(:user_cap_reached?).and_return(true)
expect(oauth_user).not_to receive(:log_user_changes)
expect do
save_user
user.reload
end.not_to change { user.state }
expect(user.state).to eq('blocked_pending_approval')
end
end
end
context 'when a user cannot be activated based on user cap' do
before do
allow(user).to receive(:activate_based_on_user_cap?).and_return(false)
end
it 'leaves the user as blocked' do
expect(oauth_user).not_to receive(:log_user_changes)
expect do
save_user
user.reload
end.not_to change { user.state }
expect(user.state).to eq('blocked_pending_approval')
end
end
end
end
end end
end end
...@@ -1923,7 +1923,7 @@ RSpec.describe User do ...@@ -1923,7 +1923,7 @@ RSpec.describe User do
end end
end end
describe '#blocked_auto_created_omniauth_user?' do describe '#blocked_auto_created_oauth_ldap_user?' do
context 'when the auto-creation of an omniauth user is blocked' do context 'when the auto-creation of an omniauth user is blocked' do
before do before do
stub_omniauth_setting(block_auto_created_users: true) stub_omniauth_setting(block_auto_created_users: true)
...@@ -1933,7 +1933,7 @@ RSpec.describe User do ...@@ -1933,7 +1933,7 @@ RSpec.describe User do
it 'is true' do it 'is true' do
omniauth_user = create(:omniauth_user) omniauth_user = create(:omniauth_user)
expect(omniauth_user.blocked_auto_created_omniauth_user?).to be_truthy expect(omniauth_user.blocked_auto_created_oauth_ldap_user?).to be_truthy
end end
end end
...@@ -1941,7 +1941,37 @@ RSpec.describe User do ...@@ -1941,7 +1941,37 @@ RSpec.describe User do
it 'is false' do it 'is false' do
user = build(:user) user = build(:user)
expect(user.blocked_auto_created_omniauth_user?).to be_falsey expect(user.blocked_auto_created_oauth_ldap_user?).to be_falsey
end
end
context 'when the config for auto-creation of LDAP user is set' do
let(:ldap_user) { create(:omniauth_user, :ldap) }
before do
allow_next_instance_of(::Gitlab::Auth::Ldap::Config) do |config|
allow(config).to receive_messages(block_auto_created_users: ldap_auto_create_blocked)
end
end
subject(:blocked_user?) { ldap_user.blocked_auto_created_oauth_ldap_user? }
context 'when it blocks the creation of a LDAP user' do
let(:ldap_auto_create_blocked) { true }
it { is_expected.to be_truthy }
context 'when no provider is linked to the user' do
let(:ldap_user) { create(:user) }
it { is_expected.to be_falsey }
end
end
context 'when it does not block the creation of a LDAP user' do
let(:ldap_auto_create_blocked) { false }
it { is_expected.to be_falsey }
end end
end end
end end
...@@ -2041,7 +2071,7 @@ RSpec.describe User do ...@@ -2041,7 +2071,7 @@ RSpec.describe User do
with_them do with_them do
before do before do
allow(user).to receive(:blocked_auto_created_omniauth_user?).and_return(blocked_auto_created_omniauth) allow(user).to receive(:blocked_auto_created_oauth_ldap_user?).and_return(blocked_auto_created_omniauth)
allow(user).to receive(:blocked_pending_approval?).and_return(blocked_pending_approval) allow(user).to receive(:blocked_pending_approval?).and_return(blocked_pending_approval)
allow(described_class.user_cap_max).to receive(:present?).and_return(user_cap_max_present) allow(described_class.user_cap_max).to receive(:present?).and_return(user_cap_max_present)
end end
......
...@@ -22,13 +22,11 @@ RSpec.shared_examples 'finding user when user cap is set' do ...@@ -22,13 +22,11 @@ RSpec.shared_examples 'finding user when user cap is set' do
context 'when the user cap has not been reached' do context 'when the user cap has not been reached' do
let(:new_user_signups_cap) { 100 } let(:new_user_signups_cap) { 100 }
context 'when the user can be activated based on user cap' do
before do before do
stub_omniauth_setting(block_auto_created_users: block) stub_omniauth_setting(block_auto_created_users: false)
end end
context 'when the user can be activated based on user cap' do
let(:block) { false }
it 'activates the user' do it 'activates the user' do
o_auth_user.save # rubocop:disable Rails/SaveBang o_auth_user.save # rubocop:disable Rails/SaveBang
...@@ -51,7 +49,11 @@ RSpec.shared_examples 'finding user when user cap is set' do ...@@ -51,7 +49,11 @@ RSpec.shared_examples 'finding user when user cap is set' do
end end
context 'when the user cannot be activated based on user cap' do context 'when the user cannot be activated based on user cap' do
let(:block) { true } before do
allow_next_instance_of(::Gitlab::Auth::Ldap::Config) do |config|
allow(config).to receive_messages(block_auto_created_users: true)
end
end
it 'does not activate the user' do it 'does not activate the user' do
o_auth_user.save # rubocop:disable Rails/SaveBang o_auth_user.save # rubocop:disable Rails/SaveBang
......
...@@ -101,12 +101,12 @@ module Gitlab ...@@ -101,12 +101,12 @@ module Gitlab
'OAuth' 'OAuth'
end end
protected
def activate_user_if_user_cap_not_reached def activate_user_if_user_cap_not_reached
nil nil
end end
protected
def should_save? def should_save?
true true
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