Merge branch '2844-ldap-ee-license-checks' into 'master'

Add license check before enabling extra features for LDAP

Closes #2844

See merge request !2513

app/controllers/sessions_controller.rb

@@ -24,12 +24,7 @@ class SessionsController < Devise::SessionsController
 
   def new
     set_minimum_password_length
-    @ldap_servers =
-      if Gitlab.config.ldap.enabled
-        Gitlab::LDAP::Config.servers
-      else
-        []
-      end
+    @ldap_servers = Gitlab::LDAP::Config.available_servers
 
     super
   end

app/helpers/selects_helper.rb

@@ -25,7 +25,7 @@ module SelectsHelper
 
   def ldap_server_select_options
     options_from_collection_for_select(
-      Gitlab::LDAP::Config.servers,
+      Gitlab::LDAP::Config.available_servers,
       'provider_name',
       'label'
     )

app/models/license.rb

@@ -19,6 +19,7 @@ class License < ActiveRecord::Base
   ISSUE_BOARD_MILESTONE_FEATURE = 'GitLab_IssueBoardMilestone'.freeze
   ISSUE_WEIGHTS_FEATURE = 'GitLab_IssueWeights'.freeze
   JENKINS_INTEGRATION_FEATURE = 'GitLab_JenkinsIntegration'.freeze
+  LDAP_EXTRAS_FEATURE = 'GitLab_LdapExtras'.freeze
   MERGE_REQUEST_APPROVERS_FEATURE = 'GitLab_MergeRequestApprovers'.freeze
   MERGE_REQUEST_REBASE_FEATURE = 'GitLab_MergeRequestRebase'.freeze
   MERGE_REQUEST_SQUASH_FEATURE = 'GitLab_MergeRequestSquash'.freeze
@@ -39,6 +40,7 @@ class License < ActiveRecord::Base
     db_load_balancing: DB_LOAD_BALANCING_FEATURE,
     elastic_search: ELASTIC_SEARCH_FEATURE,
     geo: GEO_FEATURE,
+    ldap_extras: LDAP_EXTRAS_FEATURE,
     object_storage: OBJECT_STORAGE_FEATURE,
     related_issues: RELATED_ISSUES_FEATURE,
     repository_size_limit: REPOSITORY_SIZE_LIMIT_FEATURE,
@@ -87,6 +89,7 @@ class License < ActiveRecord::Base
     { ISSUE_BOARD_MILESTONE_FEATURE => 1 },
     { ISSUE_WEIGHTS_FEATURE => 1 },
     { JENKINS_INTEGRATION_FEATURE => 1 },
+    { LDAP_EXTRAS_FEATURE => 1 },
     { MERGE_REQUEST_APPROVERS_FEATURE => 1 },
     { MERGE_REQUEST_REBASE_FEATURE => 1 },
     { MERGE_REQUEST_SQUASH_FEATURE => 1 },

app/views/groups/ee/_settings_nav.html.haml

-- if ldap_enabled?
+- if Gitlab::LDAP::Config.enabled_extras?
   = nav_link(path: 'ldap_group_links#index') do
     = link_to group_ldap_group_links_path(@group), title: 'LDAP Group' do
       %span

app/workers/ldap_all_groups_sync_worker.rb

@@ -3,6 +3,8 @@ class LdapAllGroupsSyncWorker
   include CronjobQueue
 
   def perform
+    return unless Gitlab::LDAP::Config.enabled_extras?
+
     logger.info 'Started LDAP group sync'
     EE::Gitlab::LDAP::Sync::Groups.execute
     logger.info 'Finished LDAP group sync'

app/workers/ldap_group_sync_worker.rb

@@ -3,6 +3,8 @@ class LdapGroupSyncWorker
   include DedicatedSidekiqQueue
 
   def perform(group_ids, provider = nil)
+    return unless Gitlab::LDAP::Config.enabled_extras?
+
     groups = Group.where(id: Array(group_ids))
 
     if provider

app/workers/ldap_sync_worker.rb

@@ -3,7 +3,8 @@ class LdapSyncWorker
   include CronjobQueue
 
   def perform
-    return unless Gitlab.config.ldap.enabled
+    return unless Gitlab::LDAP::Config.enabled_extras?
+
     Rails.logger.info "Performing daily LDAP sync task."
     User.ldap.find_each(batch_size: 100).each do |ldap_user|
       Rails.logger.debug "Syncing user #{ldap_user.username}, #{ldap_user.email}"

config/initializers/omniauth.rb

 if Gitlab::LDAP::Config.enabled?
   module OmniAuth::Strategies
-    Gitlab::LDAP::Config.servers.each do |server|
+    Gitlab::LDAP::Config.available_servers.each do |server|
       # do not redeclare LDAP
       next if server['provider_name'] == 'ldap'
       const_set(server['provider_class'], Class.new(LDAP))
@@ -8,7 +8,7 @@ if Gitlab::LDAP::Config.enabled?
   end
 
   OmniauthCallbacksController.class_eval do
-    Gitlab::LDAP::Config.servers.each do |server|
+    Gitlab::LDAP::Config.available_servers.each do |server|
       alias_method server['provider_name'], :ldap
     end
   end

lib/api/ldap.rb

@@ -21,7 +21,7 @@ module API
         use :search_params
       end
       get 'groups' do
-        provider = Gitlab::LDAP::Config.servers.first['provider_name']
+        provider = Gitlab::LDAP::Config.available_servers.first['provider_name']
         groups = get_group_list(provider, params[:search])
         present groups, with: Entities::LdapGroup
       end

lib/ee/gitlab/ldap/config.rb

+module EE
+  module Gitlab
+    module LDAP
+      module Config
+        extend ActiveSupport::Concern
+
+        class_methods do
+          def enabled_extras?
+            enabled? && ::License.feature_available?(:ldap_extras)
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ldap/config.rb

@@ -2,6 +2,8 @@
 module Gitlab
   module LDAP
     class Config
+      include ::EE::Gitlab::LDAP::Config
+
       attr_accessor :provider, :options
 
       InvalidProvider = Class.new(StandardError)
@@ -16,6 +18,12 @@ module Gitlab
         []
       end
 
+      def self.available_servers
+        return [] unless enabled?
+
+        enabled_extras? ? servers : Array.wrap(servers.first)
+      end
+
       def self.providers
         servers.map { |server| server['provider_name'] }
       end

spec/requests/api/ldap_spec.rb

@@ -13,6 +13,7 @@ describe API::Ldap do
       OpenStruct.new(cn: 'students')
     ]
 
+    allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true)
     allow(Gitlab::LDAP::Adapter).to receive(:new).and_return(adapter)
     allow(adapter).to receive_messages(groups: groups)
   end

spec/workers/ldap_all_groups_sync_worker_spec.rb

@@ -5,13 +5,28 @@ describe LdapAllGroupsSyncWorker do
 
   before do
     allow(Sidekiq.logger).to receive(:info)
+    allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true)
   end
 
   describe '#perform' do
-    it 'syncs all groups when group_id is nil' do
-      expect(EE::Gitlab::LDAP::Sync::Groups).to receive(:execute)
+    context 'with the default license key' do
+      it 'syncs all groups when group_id is nil' do
+        expect(EE::Gitlab::LDAP::Sync::Groups).to receive(:execute)
 
-      subject.perform
+        subject.perform
+      end
+    end
+
+    context 'without a license key' do
+      before do
+        License.destroy_all
+      end
+
+      it 'does not sync all groups' do
+        expect(EE::Gitlab::LDAP::Sync::Groups).not_to receive(:execute)
+
+        subject.perform
+      end
     end
   end
 end

spec/workers/ldap_group_sync_worker_spec.rb

@@ -14,20 +14,35 @@ describe LdapGroupSyncWorker do
 
   before do
     allow(Sidekiq.logger).to receive(:info)
+    allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true)
   end
 
   describe '#perform' do
-    it 'syncs a single group when group_id is present' do
-      expect(subject).to receive(:sync_groups).with([group])
+    context 'with the default license key' do
+      it 'syncs a single group when group_id is present' do
+        expect(subject).to receive(:sync_groups).with([group])
 
-      subject.perform(group.id)
+        subject.perform(group.id)
+      end
+
+      it 'creates a proxy for syncing a single provider' do
+        fake_proxy = expect_fake_proxy('the-provider')
+        expect(subject).to receive(:sync_groups).with([group], proxy: fake_proxy)
+
+        subject.perform(group.id, 'the-provider')
+      end
     end
 
-    it 'creates a proxy for syncing a single provider' do
-      fake_proxy = expect_fake_proxy('the-provider')
-      expect(subject).to receive(:sync_groups).with([group], proxy: fake_proxy)
+    context 'without a license key' do
+      before do
+        License.destroy_all
+      end
+
+      it 'does not sync groups' do
+        expect(subject).not_to receive(:sync_groups)
 
-      subject.perform(group.id, 'the-provider')
+        subject.perform(group.id)
+      end
     end
   end
 

spec/workers/ldap_sync_worker_spec.rb

+require 'spec_helper'
+
+describe LdapSyncWorker do
+  let(:subject) { described_class.new }
+
+  before do
+    allow(Sidekiq.logger).to receive(:info)
+    allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true)
+
+    create(:omniauth_user, provider: 'ldapmain')
+  end
+
+  describe '#perform' do
+    context 'with the default license key' do
+      it 'syncs all LDAP users' do
+        expect(Gitlab::LDAP::Access).to receive(:allowed?)
+
+        subject.perform
+      end
+    end
+
+    context 'without a license key' do
+      before do
+        License.destroy_all
+      end
+
+      it 'does not sync LDAP users' do
+        expect(Gitlab::LDAP::Access).not_to receive(:allowed?)
+
+        subject.perform
+      end
+    end
+  end
+end