Commit 832744e6 authored by Etienne Baqué's avatar Etienne Baqué

Merge branch 'dblessing_ldap_unblock_on_sign_in' into 'master'

Unblock LDAP blocked user on sign-in with other auth methods

See merge request gitlab-org/gitlab!73471
parents 2143d854 8190473e
...@@ -473,7 +473,11 @@ class User < ApplicationRecord ...@@ -473,7 +473,11 @@ class User < ApplicationRecord
end end
def active_for_authentication? def active_for_authentication?
super && can?(:log_in) return false unless super
check_ldap_if_ldap_blocked!
can?(:log_in)
end end
# The messages for these keys are defined in `devise.en.yml` # The messages for these keys are defined in `devise.en.yml`
...@@ -2167,6 +2171,13 @@ class User < ApplicationRecord ...@@ -2167,6 +2171,13 @@ class User < ApplicationRecord
def ci_job_token_scope_cache_key def ci_job_token_scope_cache_key
"users:#{id}:ci:job_token_scope" "users:#{id}:ci:job_token_scope"
end end
# An `ldap_blocked` user will be unblocked if LDAP indicates they are allowed.
def check_ldap_if_ldap_blocked!
return unless ::Gitlab::Auth::Ldap::Config.enabled? && ldap_blocked?
::Gitlab::Auth::Ldap::Access.allowed?(self)
end
end end
User.prepend_mod_with('User') User.prepend_mod_with('User')
...@@ -6,6 +6,7 @@ RSpec.describe User do ...@@ -6,6 +6,7 @@ RSpec.describe User do
include ProjectForksHelper include ProjectForksHelper
include TermsHelper include TermsHelper
include ExclusiveLeaseHelpers include ExclusiveLeaseHelpers
include LdapHelpers
it_behaves_like 'having unique enum values' it_behaves_like 'having unique enum values'
...@@ -5808,7 +5809,7 @@ RSpec.describe User do ...@@ -5808,7 +5809,7 @@ RSpec.describe User do
end end
describe '#active_for_authentication?' do describe '#active_for_authentication?' do
subject { user.active_for_authentication? } subject(:active_for_authentication?) { user.active_for_authentication? }
let(:user) { create(:user) } let(:user) { create(:user) }
...@@ -5818,6 +5819,14 @@ RSpec.describe User do ...@@ -5818,6 +5819,14 @@ RSpec.describe User do
end end
it { is_expected.to be false } it { is_expected.to be false }
it 'does not check if LDAP is allowed' do
stub_ldap_setting(enabled: true)
expect(Gitlab::Auth::Ldap::Access).not_to receive(:allowed?)
active_for_authentication?
end
end end
context 'when user is a ghost user' do context 'when user is a ghost user' do
...@@ -5828,6 +5837,28 @@ RSpec.describe User do ...@@ -5828,6 +5837,28 @@ RSpec.describe User do
it { is_expected.to be false } it { is_expected.to be false }
end end
context 'when user is ldap_blocked' do
before do
user.ldap_block
end
it 'rechecks if LDAP is allowed when LDAP is enabled' do
stub_ldap_setting(enabled: true)
expect(Gitlab::Auth::Ldap::Access).to receive(:allowed?)
active_for_authentication?
end
it 'does not check if LDAP is allowed when LDAP is not enabled' do
stub_ldap_setting(enabled: false)
expect(Gitlab::Auth::Ldap::Access).not_to receive(:allowed?)
active_for_authentication?
end
end
context 'based on user type' do context 'based on user type' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
......
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