diff options
author | Robert Speicher <robert@gitlab.com> | 2016-04-06 21:50:40 +0000 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2016-04-07 14:12:53 -0400 |
commit | 0a3f36681e72c9c7b376c0ec458f66e270be2502 (patch) | |
tree | 6e86f17646ed778d1ebddbdee0533e2c46a32630 | |
parent | 44c035427e5338e50168af5082ff03f2dd41b04b (diff) | |
download | gitlab-ce-0a3f36681e72c9c7b376c0ec458f66e270be2502.tar.gz |
Merge branch 'patch/fix-ldap-unblock-user-logic' into 'master'
Unblocks user when active_directory is disabled and it can be found
We implemented a specific block state to handle user blocking that originates from LDAP filtering rules / directory state in !2242.
That introduced a regression in LDAP authentication when Active Directory support was disabled. You could have a scenario where the user would not be temporarily found (like a filtering rule), that would mark the user as `ldap_blocked`, but will never unblock it automatically when that state changed.
Fixes #14253, #13179, #13259, #13959
See merge request !3550
-rw-r--r-- | CHANGELOG | 20 | ||||
-rw-r--r-- | lib/gitlab/ldap/access.rb | 5 | ||||
-rw-r--r-- | spec/lib/gitlab/ldap/access_spec.rb | 27 |
3 files changed, 31 insertions, 21 deletions
diff --git a/CHANGELOG b/CHANGELOG index 78635d3ab78..fe1cc494db1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,30 +1,12 @@ Please view this file on the master branch, on stable branches it's out of date. -v 8.7.0 (unreleased) - - Don't attempt to look up an avatar in repo if repo directory does not exist (Stan hu) - - All images in discussions and wikis now link to their source files !3464 (Connor Shea). - - Improved Markdown rendering performance !3389 (Yorick Peterse) - - Don't attempt to look up an avatar in repo if repo directory does not exist (Stan Hu) - - Preserve time notes/comments have been updated at when moving issue - - Make HTTP(s) label consistent on clone bar (Stan Hu) - - Expose label description in API (Mariusz Jachimowicz) - - Allow back dating on issues when created through the API - - Fix avatar stretching by providing a cropping feature - - Add links to CI setup documentation from project settings and builds pages - - Handle nil descriptions in Slack issue messages (Stan Hu) - - Add default scope to projects to exclude projects pending deletion - - Implement 'Groups View' as an option for dashboard preferences !3379 (Elias W.) - - Implement 'TODOs View' as an option for dashboard preferences !3379 (Elias W.) - - Gracefully handle notes on deleted commits in merge requests (Stan Hu) - - Fall back to `In-Reply-To` and `References` headers when sub-addressing is not available (David Padilla) - - Remove "Congratulations!" tweet button on newly-created project. (Connor Shea) - v 8.6.5 - Fix importing from GitHub Enterprise. !3529 - Perform the language detection after updating merge requests in `GitPushService`, leading to faster visual feedback for the end-user. !3533 - Check permissions when user attempts to import members from another project. !3535 - Only update repository language if it is not set to improve performance. !3556 - Return status code 303 after a branch DELETE operation to avoid project deletion (Stan Hu). !3583 + - Unblock user when active_directory is disabled and it can be found !3550 - Fix a 2FA authentication spoofing vulnerability. v 8.6.4 diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index da4435c7308..f2b649e50a2 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -33,7 +33,10 @@ module Gitlab def allowed? if ldap_user - return true unless ldap_config.active_directory + unless ldap_config.active_directory + user.activate if user.ldap_blocked? + return true + end # Block user in GitLab if he/she was blocked in AD if Gitlab::LDAP::Person.disabled_via_active_directory?(user.ldap_identity.extern_uid, adapter) diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 32a19bf344b..f5b66b8156f 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -33,7 +33,7 @@ describe Gitlab::LDAP::Access, lib: true do it { is_expected.to be_falsey } - it 'should block user in GitLab' do + it 'blocks user in GitLab' do access.allowed? expect(user).to be_blocked expect(user).to be_ldap_blocked @@ -78,6 +78,31 @@ describe Gitlab::LDAP::Access, lib: true do end it { is_expected.to be_truthy } + + context 'when user cannot be found' do + before do + allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(nil) + end + + it { is_expected.to be_falsey } + + it 'blocks user in GitLab' do + access.allowed? + expect(user).to be_blocked + expect(user).to be_ldap_blocked + end + end + + context 'when user was previously ldap_blocked' do + before do + user.ldap_block + end + + it 'unblocks the user if it exists' do + access.allowed? + expect(user).not_to be_blocked + end + end end end end |