From 238e4f02954e910cab14c0be3b8b2569e08b5e87 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 14 Apr 2015 17:09:05 +0200 Subject: [PATCH] Add config var to block auto-created LDAP users. --- CHANGELOG | 1 + config/gitlab.yml.example | 3 ++ config/initializers/1_settings.rb | 1 + lib/gitlab/ldap/config.rb | 4 ++ lib/gitlab/ldap/user.rb | 8 +++- spec/lib/gitlab/ldap/user_spec.rb | 67 +++++++++++++++++++++++++++---- 6 files changed, 75 insertions(+), 9 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 0ecde5ef89c..e3bd3bec04b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.10.0 (unreleased) + - Add config var to block auto-created LDAP users. - Fix broken file browsing with a submodule that contains a relative link (Stan Hu) - Fix persistent XSS vulnerability around profile website URLs. - Fix project import URL regex to prevent arbitary local repos from being imported. diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 46b9f05cc17..6a78a032c58 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -146,6 +146,9 @@ production: &base # disable this setting, because the userPrincipalName contains an '@'. allow_username_or_email_login: false + # Locks down those users until they have been cleared by the admin (default: false). + block_auto_created_users: false + # Base where we can search for users # # Ex. ou=People,dc=gitlab,dc=example diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index d5cddb8dbf0..0abd34fc3e0 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -76,6 +76,7 @@ if Settings.ldap['enabled'] || Rails.env.test? Settings.ldap['servers'].each do |key, server| server['label'] ||= 'LDAP' + server['block_auto_created_users'] = false if server['block_auto_created_users'].nil? server['allow_username_or_email_login'] = false if server['allow_username_or_email_login'].nil? server['active_directory'] = true if server['active_directory'].nil? server['provider_name'] ||= "ldap#{key}".downcase diff --git a/lib/gitlab/ldap/config.rb b/lib/gitlab/ldap/config.rb index fa5b6c1e230..d2ffa2e1fe8 100644 --- a/lib/gitlab/ldap/config.rb +++ b/lib/gitlab/ldap/config.rb @@ -80,6 +80,10 @@ module Gitlab options['active_directory'] end + def block_auto_created_users + options['block_auto_created_users'] + end + protected def base_config Gitlab.config.ldap diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb index d652e19217d..f7f3ba9ad7d 100644 --- a/lib/gitlab/ldap/user.rb +++ b/lib/gitlab/ldap/user.rb @@ -55,13 +55,17 @@ module Gitlab gl_user.changed? || gl_user.identities.any?(&:changed?) end - def needs_blocking? - false + def block_after_signup? + ldap_config.block_auto_created_users end def allowed? Gitlab::LDAP::Access.allowed?(gl_user) end + + def ldap_config + Gitlab::LDAP::Config.new(auth_hash.provider) + end end end end diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb index 4f93545feb6..42015c28c81 100644 --- a/spec/lib/gitlab/ldap/user_spec.rb +++ b/spec/lib/gitlab/ldap/user_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe Gitlab::LDAP::User do - let(:gl_user) { Gitlab::LDAP::User.new(auth_hash) } + let(:ldap_user) { Gitlab::LDAP::User.new(auth_hash) } + let(:gl_user) { ldap_user.gl_user } let(:info) do { name: 'John', @@ -16,17 +17,17 @@ describe Gitlab::LDAP::User do describe :changed? do it "marks existing ldap user as changed" do existing_user = create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain') - expect(gl_user.changed?).to be_truthy + expect(ldap_user.changed?).to be_truthy end it "marks existing non-ldap user if the email matches as changed" do existing_user = create(:user, email: 'john@example.com') - expect(gl_user.changed?).to be_truthy + expect(ldap_user.changed?).to be_truthy end it "dont marks existing ldap user as changed" do existing_user = create(:omniauth_user, email: 'john@example.com', extern_uid: 'my-uid', provider: 'ldapmain') - expect(gl_user.changed?).to be_falsey + expect(ldap_user.changed?).to be_falsey end end @@ -34,12 +35,12 @@ describe Gitlab::LDAP::User do it "finds the user if already existing" do existing_user = create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain') - expect{ gl_user.save }.to_not change{ User.count } + expect{ ldap_user.save }.to_not change{ User.count } end it "connects to existing non-ldap user if the email matches" do existing_user = create(:omniauth_user, email: 'john@example.com', provider: "twitter") - expect{ gl_user.save }.to_not change{ User.count } + expect{ ldap_user.save }.to_not change{ User.count } existing_user.reload expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid' @@ -47,7 +48,59 @@ describe Gitlab::LDAP::User do end it "creates a new user if not found" do - expect{ gl_user.save }.to change{ User.count }.by(1) + expect{ ldap_user.save }.to change{ User.count }.by(1) + end + end + + + describe 'blocking' do + context 'signup' do + context 'dont block on create' do + before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: false } + + it do + ldap_user.save + expect(gl_user).to be_valid + expect(gl_user).not_to be_blocked + end + end + + context 'block on create' do + before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: true } + + it do + ldap_user.save + expect(gl_user).to be_valid + expect(gl_user).to be_blocked + end + end + end + + context 'sign-in' do + before do + ldap_user.save + ldap_user.gl_user.activate + end + + context 'dont block on create' do + before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: false } + + it do + ldap_user.save + expect(gl_user).to be_valid + expect(gl_user).not_to be_blocked + end + end + + context 'block on create' do + before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: true } + + it do + ldap_user.save + expect(gl_user).to be_valid + expect(gl_user).not_to be_blocked + end + end end end end