Merge branch 'improve-ldap' into 'master'

Hide ldap group links if ldap is not enabled

This MR also includes a fix for next issue: prevent 500 error on ldap group links page if group has links to ldap servers that are disabled or removed from gitlab config.

Fixes #196

See merge request !264

app/controllers/application_controller.rb

@@ -239,4 +239,10 @@ class ApplicationController < ActionController::Base
       redirect_to profile_path, notice: 'Please complete your profile with email address' and return
     end
   end
+
+  def require_ldap_enabled
+    unless Gitlab.config.ldap.enabled
+      render_404 and return
+    end
+  end
 end

app/controllers/groups/ldap_group_links_controller.rb

 class Groups::LdapGroupLinksController < ApplicationController
   before_action :group
+  before_action :require_ldap_enabled
   before_action :authorize_admin_group!
 
   layout 'group'

app/models/ldap_group_link.rb

@@ -14,6 +14,8 @@ class LdapGroupLink < ActiveRecord::Base
 
   def config
     Gitlab::LDAP::Config.new(provider)
+  rescue Gitlab::LDAP::Config::InvalidProvider
+    nil
   end
 
   # default to the first LDAP server

app/views/groups/_settings_nav.html.haml

@@ -7,10 +7,11 @@
     = link_to projects_group_path(@group) do
       %i.fa.fa-folder
       Projects
-  = nav_link(controller: :ldap_group_links) do
-    = link_to group_ldap_group_links_path(@group) do
-      %i.icon-exchange
-      LDAP Groups
+  - if ldap_enabled?
+    = nav_link(controller: :ldap_group_links) do
+      = link_to group_ldap_group_links_path(@group) do
+        %i.fa.fa-exchange
+        LDAP Groups
   = nav_link(controller: :audit_events) do
     = link_to group_audit_events_path(@group) do
       %i.fa.fa-file-text-o

app/views/ldap_group_links/_ldap_group_link.html.haml

 %li
-  = ldap_group_link.cn
-  %small.light== as #{ldap_group_link.human_access} on #{ldap_group_link.provider_label}
+  %h4= ldap_group_link.cn
   .pull-right
     = link_to group_ldap_group_link_path(group, ldap_group_link), method: :delete, class: 'btn btn-danger btn-small' do
       = fa_icon('unlink', text: 'unlink')
+
+  - if ldap_group_link.config
+    %p.light
+      As
+      %strong #{ldap_group_link.human_access}
+      on
+      %strong #{ldap_group_link.provider_label}
+      server
+  - else
+    %p.cred
+      %i.fa.fa-warning
+      Config for
+      %code #{ldap_group_link.provider}
+      does not present in GitLab

features/groups.feature

@@ -56,9 +56,11 @@ Feature: Groups
     And I should not see the "Remove avatar" button
 
   Scenario: Add new LDAP synchronization
+    Given LDAP enabled
     When I visit Group "Owned" LDAP settings page
     And I add a new LDAP synchronization
     Then I see a new LDAP synchronization listed
+    And LDAP disabled
 
   # Leave
 

features/steps/groups.rb

@@ -313,6 +313,14 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
             milestone: milestone2_project3
   end
 
+  step 'LDAP enabled' do
+    Gitlab.config.ldap.stub(:enabled).and_return(true)
+  end
+
+  step 'LDAP disabled' do
+    Gitlab.config.ldap.stub(:enabled).and_return(false)
+  end
+
   step 'I add a new LDAP synchronization' do
     within('form#new_ldap_group_link') do
       find('#ldap_group_link_cn', visible: false).set('my-group-cn')
@@ -324,6 +332,6 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
 
   step 'I see a new LDAP synchronization listed' do
     expect(page).not_to have_content('No synchronizations yet')
-    expect(page).to have_content('my-group-cn as Developer')
+    expect(page).to have_content('As Developer on ldap server')
   end
 end

lib/gitlab/ldap/config.rb

@@ -4,12 +4,16 @@ module Gitlab
     class Config
       attr_accessor :provider, :options
 
+      class InvalidProvider < StandardError; end
+
       def self.enabled?
         Gitlab.config.ldap.enabled
       end
 
       def self.servers
         Gitlab.config.ldap.servers.values
+      rescue Settingslogic::MissingSetting
+        []
       end
 
       def self.providers
@@ -21,7 +25,7 @@ module Gitlab
       end
 
       def self.invalid_provider(provider)
-        raise "Unknown provider (#{provider}). Available providers: #{providers}"
+        raise InvalidProvider.new("Unknown provider (#{provider}). Available providers: #{providers}")
       end
 
       def initialize(provider)
@@ -32,6 +36,7 @@ module Gitlab
         else
           self.class.invalid_provider(provider)
         end
+
         @options = config_for(@provider) # Use @provider, not provider
       end
 
@@ -87,6 +92,7 @@ module Gitlab
       end
 
       protected
+
       def base_config
         Gitlab.config.ldap
       end