Merge branch 'ldap_check_bind' into 'master'

Improve ldap:check errors

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/21621. 

See merge request !6601

9 files changed, 226 insertions, 78 deletions

diff --git a/changelogs/unreleased/ldap_check_bind.yml b/changelogs/unreleased/ldap_check_bind.yml
new file mode 100644
index 00000000000..daff8103a07
--- /dev/null
+++ b/changelogs/unreleased/ldap_check_bind.yml
@@ -0,0 +1,4 @@
+---
+title: Introduce better credential and error checking to `rake gitlab:ldap:check`
+merge_request: 6601
+author: 
diff --git a/doc/administration/auth/ldap.md b/doc/administration/auth/ldap.md
index bf7814875bf..fd23047f027 100644
--- a/doc/administration/auth/ldap.md
+++ b/doc/administration/auth/ldap.md
@@ -35,6 +35,10 @@ of one hour.
 To enable LDAP integration you need to add your LDAP server settings in
 `/etc/gitlab/gitlab.rb` or `/home/git/gitlab/config/gitlab.yml`.
 
+There is a Rake task to check LDAP configuration. After configuring LDAP
+using the documentation below, see [LDAP check Rake task](../raketasks/check.md#ldap-check)
+for information on the LDAP check Rake task.
+
 >**Note**: In GitLab EE, you can configure multiple LDAP servers to connect to
 one GitLab server.
 
diff --git a/doc/raketasks/check_repos_output.png b/doc/administration/img/raketasks/check_repos_output.png
index 1f632566b00..1f632566b00 100644
--- a/doc/raketasks/check_repos_output.png
+++ b/doc/administration/img/raketasks/check_repos_output.png
diff --git a/doc/administration/raketasks/check.md b/doc/administration/raketasks/check.md
new file mode 100644
index 00000000000..d1d2fed4861
--- /dev/null
+++ b/doc/administration/raketasks/check.md
@@ -0,0 +1,97 @@
+# Check Rake Tasks
+
+## Repository Integrity
+
+Even though Git is very resilient and tries to prevent data integrity issues,
+there are times when things go wrong. The following Rake tasks intend to
+help GitLab administrators diagnose problem repositories so they can be fixed.
+
+There are 3 things that are checked to determine integrity.
+
+1. Git repository file system check ([git fsck](https://git-scm.com/docs/git-fsck)).
+   This step verifies the connectivity and validity of objects in the repository.
+1. Check for `config.lock` in the repository directory.
+1. Check for any branch/references lock files in `refs/heads`.
+
+It's important to note that the existence of `config.lock` or reference locks
+alone do not necessarily indicate a problem. Lock files are routinely created
+and removed as Git and GitLab perform operations on the repository. They serve
+to prevent data integrity issues. However, if a Git operation is interrupted these
+locks may not be cleaned up properly.
+
+The following symptoms may indicate a problem with repository integrity. If users
+experience these symptoms you may use the rake tasks described below to determine
+exactly which repositories are causing the trouble.
+
+- Receiving an error when trying to push code - `remote: error: cannot lock ref`
+- A 500 error when viewing the GitLab dashboard or when accessing a specific project.
+
+### Check all GitLab repositories
+
+This task loops through all repositories on the GitLab server and runs the
+3 integrity checks described previously.
+
+**Omnibus Installation**
+
+```
+sudo gitlab-rake gitlab:repo:check
+```
+
+**Source Installation**
+
+```bash
+sudo -u git -H bundle exec rake gitlab:repo:check RAILS_ENV=production
+```
+
+### Check repositories for a specific user
+
+This task checks all repositories that a specific user has access to. This is important
+because sometimes you know which user is experiencing trouble but you don't know
+which project might be the cause.
+
+If the rake task is executed without brackets at the end, you will be prompted
+to enter a username.
+
+**Omnibus Installation**
+
+```bash
+sudo gitlab-rake gitlab:user:check_repos
+sudo gitlab-rake gitlab:user:check_repos[<username>]
+```
+
+**Source Installation**
+
+```bash
+sudo -u git -H bundle exec rake gitlab:user:check_repos RAILS_ENV=production
+sudo -u git -H bundle exec rake gitlab:user:check_repos[<username>] RAILS_ENV=production
+```
+
+Example output:
+
+![gitlab:user:check_repos output](../img/raketasks/check_repos_output.png)
+
+## LDAP Check
+
+The LDAP check Rake task will test the bind_dn and password credentials
+(if configured) and will list a sample of LDAP users. This task is also
+executed as part of the `gitlab:check` task, but can run independently
+using the command below.
+
+**Omnibus Installation**
+
+```
+sudo gitlab-rake gitlab:ldap:check
+```
+
+**Source Installation**
+
+```bash
+sudo -u git -H bundle exec rake gitlab:ldap:check RAILS_ENV=production
+```
+
+By default, the task will return a sample of 100 LDAP users. Change this
+limit by passing a number to the check task:
+
+```bash
+rake gitlab:ldap:check[50]
+```
diff --git a/doc/raketasks/check.md b/doc/raketasks/check.md
index 3ff3fee6a40..f7f6a40cd04 100644
--- a/doc/raketasks/check.md
+++ b/doc/raketasks/check.md
@@ -1,63 +1,3 @@
 # Check Rake Tasks
 
-## Repository Integrity
-
-Even though Git is very resilient and tries to prevent data integrity issues,
-there are times when things go wrong. The following Rake tasks intend to
-help GitLab administrators diagnose problem repositories so they can be fixed.
-
-There are 3 things that are checked to determine integrity.
-
-1. Git repository file system check ([git fsck](https://git-scm.com/docs/git-fsck)).
-   This step verifies the connectivity and validity of objects in the repository.
-1. Check for `config.lock` in the repository directory.
-1. Check for any branch/references lock files in `refs/heads`.
-
-It's important to note that the existence of `config.lock` or reference locks
-alone do not necessarily indicate a problem. Lock files are routinely created
-and removed as Git and GitLab perform operations on the repository. They serve
-to prevent data integrity issues. However, if a Git operation is interrupted these
-locks may not be cleaned up properly.
-
-The following symptoms may indicate a problem with repository integrity. If users
-experience these symptoms you may use the rake tasks described below to determine
-exactly which repositories are causing the trouble.
-
-- Receiving an error when trying to push code - `remote: error: cannot lock ref`
-- A 500 error when viewing the GitLab dashboard or when accessing a specific project.
-
-### Check all GitLab repositories
-
-This task loops through all repositories on the GitLab server and runs the
-3 integrity checks described previously.
-
-```
-# omnibus-gitlab
-sudo gitlab-rake gitlab:repo:check
-
-# installation from source
-bundle exec rake gitlab:repo:check RAILS_ENV=production
-```
-
-### Check repositories for a specific user
-
-This task checks all repositories that a specific user has access to. This is important
-because sometimes you know which user is experiencing trouble but you don't know
-which project might be the cause.
-
-If the rake task is executed without brackets at the end, you will be prompted
-to enter a username.
-
-```bash
-# omnibus-gitlab
-sudo gitlab-rake gitlab:user:check_repos
-sudo gitlab-rake gitlab:user:check_repos[<username>]
-
-# installation from source
-bundle exec rake gitlab:user:check_repos RAILS_ENV=production
-bundle exec rake gitlab:user:check_repos[<username>] RAILS_ENV=production
-```
-
-Example output:
-
-![gitlab:user:check_repos output](check_repos_output.png)
+This document was moved to [administration/raketasks/check](../administration/raketasks/check.md).
diff --git a/lib/gitlab/ldap/config.rb b/lib/gitlab/ldap/config.rb
index f9bb5775323..6ea069d26df 100644
--- a/lib/gitlab/ldap/config.rb
+++ b/lib/gitlab/ldap/config.rb
@@ -92,6 +92,10 @@ module Gitlab
         options['timeout'].to_i
       end
 
+      def has_auth?
+        options['password'] || options['bind_dn']
+      end
+
       protected
 
       def base_config
@@ -122,10 +126,6 @@ module Gitlab
           }
         }
       end
-
-      def has_auth?
-        options['password'] || options['bind_dn']
-      end
     end
   end
 end
diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake
index 2ae48a970ce..35c4194e87c 100644
--- a/lib/tasks/gitlab/check.rake
+++ b/lib/tasks/gitlab/check.rake
@@ -760,7 +760,7 @@ namespace :gitlab do
   end
 
   namespace :ldap do
-    task :check, [:limit] => :environment do |t, args|
+    task :check, [:limit] => :environment do |_, args|
       # Only show up to 100 results because LDAP directories can be very big.
       # This setting only affects the `rake gitlab:check` script.
       args.with_defaults(limit: 100)
@@ -768,7 +768,7 @@ namespace :gitlab do
       start_checking "LDAP"
 
       if Gitlab::LDAP::Config.enabled?
-        print_users(args.limit)
+        check_ldap(args.limit)
       else
         puts 'LDAP is disabled in config/gitlab.yml'
       end
@@ -776,21 +776,42 @@ namespace :gitlab do
       finished_checking "LDAP"
     end
 
-    def print_users(limit)
-      puts "LDAP users with access to your GitLab server (only showing the first #{limit} results)"
-
+    def check_ldap(limit)
       servers = Gitlab::LDAP::Config.providers
 
       servers.each do |server|
         puts "Server: #{server}"
-        Gitlab::LDAP::Adapter.open(server) do |adapter|
-          users = adapter.users(adapter.config.uid, '*', limit)
-          users.each do |user|
-            puts "\tDN: #{user.dn}\t #{adapter.config.uid}: #{user.uid}"
+
+        begin
+          Gitlab::LDAP::Adapter.open(server) do |adapter|
+            check_ldap_auth(adapter)
+
+            puts "LDAP users with access to your GitLab server (only showing the first #{limit} results)"
+
+            users = adapter.users(adapter.config.uid, '*', limit)
+            users.each do |user|
+              puts "\tDN: #{user.dn}\t #{adapter.config.uid}: #{user.uid}"
+            end
           end
+        rescue Net::LDAP::ConnectionRefusedError, Errno::ECONNREFUSED => e
+          puts "Could not connect to the LDAP server: #{e.message}".color(:red)
         end
       end
     end
+
+    def check_ldap_auth(adapter)
+      auth = adapter.config.has_auth?
+
+      if auth && adapter.ldap.bind
+        message = 'Success'.color(:green)
+      elsif auth
+        message = 'Failed. Check `bind_dn` and `password` configuration values'.color(:red)
+      else
+        message = 'Anonymous. No `bind_dn` or `password` configured'.color(:yellow)
+      end
+
+      puts "LDAP authentication... #{message}"
+    end
   end
 
   namespace :repo do
diff --git a/spec/lib/gitlab/ldap/config_spec.rb b/spec/lib/gitlab/ldap/config_spec.rb
index 835853a83a4..f5ebe703083 100644
--- a/spec/lib/gitlab/ldap/config_spec.rb
+++ b/spec/lib/gitlab/ldap/config_spec.rb
@@ -1,20 +1,51 @@
 require 'spec_helper'
 
 describe Gitlab::LDAP::Config, lib: true do
-  let(:config) { Gitlab::LDAP::Config.new provider }
-  let(:provider) { 'ldapmain' }
+  include LdapHelpers
+
+  let(:config) { Gitlab::LDAP::Config.new('ldapmain') }
 
   describe '#initalize' do
     it 'requires a provider' do
       expect{ Gitlab::LDAP::Config.new }.to raise_error ArgumentError
     end
 
-    it "works" do
+    it 'works' do
       expect(config).to be_a described_class
     end
 
-    it "raises an error if a unknow provider is used" do
+    it 'raises an error if a unknown provider is used' do
       expect{ Gitlab::LDAP::Config.new 'unknown' }.to raise_error(RuntimeError)
     end
   end
+
+  describe '#has_auth?' do
+    it 'is true when password is set' do
+      stub_ldap_config(
+        options: {
+          'bind_dn'  => 'uid=admin,dc=example,dc=com',
+          'password' => 'super_secret'
+        }
+      )
+
+      expect(config.has_auth?).to be_truthy
+    end
+
+    it 'is true when bind_dn is set and password is empty' do
+      stub_ldap_config(
+        options: {
+          'bind_dn'  => 'uid=admin,dc=example,dc=com',
+          'password' => ''
+        }
+      )
+
+      expect(config.has_auth?).to be_truthy
+    end
+
+    it 'is false when password and bind_dn are not set' do
+      stub_ldap_config(options: { 'bind_dn' => nil, 'password' => nil })
+
+      expect(config.has_auth?).to be_falsey
+    end
+  end
 end
diff --git a/spec/tasks/gitlab/check_rake_spec.rb b/spec/tasks/gitlab/check_rake_spec.rb
new file mode 100644
index 00000000000..538ff952bf4
--- /dev/null
+++ b/spec/tasks/gitlab/check_rake_spec.rb
@@ -0,0 +1,51 @@
+require 'rake_helper'
+
+describe 'gitlab:ldap:check rake task' do
+  include LdapHelpers
+
+  before do
+    Rake.application.rake_require 'tasks/gitlab/check'
+
+    stub_warn_user_is_not_gitlab
+  end
+
+  context 'when LDAP is not enabled' do
+    it 'does not attempt to bind or search for users' do
+      expect(Gitlab::LDAP::Config).not_to receive(:providers)
+      expect(Gitlab::LDAP::Adapter).not_to receive(:open)
+
+      run_rake_task('gitlab:ldap:check')
+    end
+  end
+
+  context 'when LDAP is enabled' do
+    let(:ldap) { double(:ldap) }
+    let(:adapter) { ldap_adapter('ldapmain', ldap) }
+
+    before do
+      allow(Gitlab::LDAP::Config)
+        .to receive_messages(
+          enabled?: true,
+          providers: ['ldapmain']
+        )
+      allow(Gitlab::LDAP::Adapter).to receive(:open).and_yield(adapter)
+      allow(adapter).to receive(:users).and_return([])
+    end
+
+    it 'attempts to bind using credentials' do
+      stub_ldap_config(has_auth?: true)
+
+      expect(ldap).to receive(:bind)
+
+      run_rake_task('gitlab:ldap:check')
+    end
+
+    it 'searches for 100 LDAP users' do
+      stub_ldap_config(uid: 'uid')
+
+      expect(adapter).to receive(:users).with('uid', '*', 100)
+
+      run_rake_task('gitlab:ldap:check')
+    end
+  end
+end