Merge branch 'ldap-block_auto_created_users' into 'master'

Add config var to block auto-created LDAP users.

Addresses private issue https://dev.gitlab.org/gitlab/gitlabhq/issues/2110.

See merge request !522

7 files changed, 84 insertions, 9 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 5f577dfc4c5..7985ded63a6 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 v 7.10.0 (unreleased)
   - Allow users to be invited by email to join a group or project.
   - Don't crash when project repository doesn't exist.
+  - 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..ba40671b162 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -146,6 +146,11 @@ production: &base
         # disable this setting, because the userPrincipalName contains an '@'.
         allow_username_or_email_login: false
 
+        # To maintain tight control over the number of active users on your GitLab installation,
+        # enable this setting to keep new users blocked 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/doc/integration/ldap.md b/doc/integration/ldap.md
index 125ce31b521..b67f793c591 100644
--- a/doc/integration/ldap.md
+++ b/doc/integration/ldap.md
@@ -51,6 +51,11 @@ main: # 'main' is the GitLab 'provider ID' of this LDAP server
   # disable this setting, because the userPrincipalName contains an '@'.
   allow_username_or_email_login: false
 
+  # To maintain tight control over the number of active users on your GitLab installation,
+  # enable this setting to keep new users blocked 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/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 d054014039a..f7f3ba9ad7d 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -39,6 +39,8 @@ module Gitlab
       end
 
       def update_user_attributes
+        return unless persisted?
+
         gl_user.skip_reconfirmation!
         gl_user.email = auth_hash.email
 
@@ -53,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