Merge branch 'dm-ldap-group-sync-filter-eep' into 'master'

Only enable LDAP group sync filter method on EEP

Closes #3698

See merge request gitlab-org/gitlab-ee!3106

app/assets/javascripts/groups/ldap_group_links.js

@@ -2,6 +2,8 @@ document.addEventListener('DOMContentLoaded', () => {
   const showGroupLink = () => {
     const $cnLink = $('.cn-link');
     const $filterLink = $('.filter-link');
+    if (!$cnLink.length || !$filterLink.length) return;
+
     const $checkedSync = $('input[name="sync_method"]:checked').val() === 'group';
 
     $cnLink.toggle($checkedSync);

app/controllers/groups/ldap_group_links_controller.rb

@@ -43,6 +43,9 @@ class Groups::LdapGroupLinksController < Groups::ApplicationController
   end
 
   def ldap_group_link_params
-    params.require(:ldap_group_link).permit(:cn, :filter, :group_access, :provider)
+    attrs = %i[cn group_access provider]
+    attrs << :filter if ::License.feature_available?(:ldap_group_sync_filter)
+
+    params.require(:ldap_group_link).permit(attrs)
   end
 end

app/controllers/groups/ldaps_controller.rb

@@ -17,6 +17,6 @@ class Groups::LdapsController < Groups::ApplicationController
   private
 
   def check_enabled_extras!
-    render_404 unless Gitlab::LDAP::Config.enabled_extras?
+    render_404 unless Gitlab::LDAP::Config.group_sync_enabled?
   end
 end

app/models/group.rb

@@ -230,20 +230,12 @@ class Group < Namespace
     ldap_group_links.first.try(:cn)
   end
 
-  def ldap_filter
-    ldap_group_links.first.try(:filter)
-  end
-
   def ldap_access
     ldap_group_links.first.try(:group_access)
   end
 
-  def ldap_cn_or_filter_present?
-    ldap_cn.present? || ldap_filter.present?
-  end
-
   def ldap_synced?
-    Gitlab.config.ldap.enabled && ldap_cn_or_filter_present?
+    Gitlab.config.ldap.enabled && ldap_group_links.any?(&:active?)
   end
 
   def post_create_hook

app/models/ldap_group_link.rb

@@ -43,6 +43,14 @@ class LdapGroupLink < ActiveRecord::Base
     config.label
   end
 
+  def active?
+    if filter.present?
+      ::License.feature_available?(:ldap_group_sync_filter)
+    elsif cn.present?
+      ::License.feature_available?(:ldap_group_sync)
+    end
+  end
+
   private
 
   def nullify_blank_attributes

app/models/license.rb

@@ -18,10 +18,11 @@ class License < ActiveRecord::Base
     issue_board_milestone
     issue_weights
     jenkins_integration
-    ldap_extras
+    ldap_group_sync
     merge_request_approvers
     merge_request_rebase
     merge_request_squash
+    multiple_ldap_servers
     multiple_issue_assignees
     multiple_issue_boards
     push_rules
@@ -42,6 +43,7 @@ class License < ActiveRecord::Base
     geo
     group_issue_boards
     jira_dev_panel_integration
+    ldap_group_sync_filter
     object_storage
     service_desk
     variable_environment_scope
@@ -108,7 +110,9 @@ class License < ActiveRecord::Base
     elastic_search
     extended_audit_events
     geo
-    ldap_extras
+    ldap_group_sync
+    ldap_group_sync_filter
+    multiple_ldap_servers
     object_storage
     repository_size_limit
   ].freeze

app/views/admin/groups/_form.html.haml

@@ -36,7 +36,7 @@
       = f.submit 'Save changes', class: "btn btn-save"
       = link_to  'Cancel', admin_group_path(@group), class: "btn btn-cancel"
 
-- if ldap_enabled? && @group.persisted?
+- if @group.persisted? && Gitlab::LDAP::Config.group_sync_enabled?
   %h3.page-title LDAP synchronizations
   = render 'ldap_group_links/form', group: @group
   = render 'ldap_group_links/ldap_group_links', group: @group

app/views/admin/groups/show.html.haml

@@ -62,11 +62,11 @@
 
         = render partial: "namespaces/shared_runner_status", locals: { namespace: @group }
 
-    .panel.panel-default
-      .panel-heading Active synchronizations
-      %ul.well-list
-        - if @group.ldap_group_links.any?
-          - @group.ldap_group_links.each do |ldap_group_link|
+    - if Gitlab::LDAP::Config.group_sync_enabled? && @group.ldap_synced?
+      .panel.panel-default
+        .panel-heading Active synchronizations
+        %ul.well-list
+          - @group.ldap_group_links.select(&:active?).each do |ldap_group_link|
             %li
               %strong= ldap_group_link.cn ? "Group: #{ldap_group_link.cn}" : "Filter: #{truncate(ldap_group_link.filter, length: 40)}"
               as

app/views/groups/group_members/_ldap_sync.html.haml

@@ -3,7 +3,7 @@
     The members of this group are managed using LDAP and cannot be added or removed here.
     Because LDAP permissions in GitLab get updated one user at a time and because GitLab caches LDAP check results, changes on your LDAP server or in this group's LDAP sync settings may take up to #{Gitlab.config.ldap['sync_time']}s to show in the list below.
     %ul
-      - @group.ldap_group_links.each do |ldap_group_link|
+      - @group.ldap_group_links.select(&:active?).each do |ldap_group_link|
         %li
           People in
           %code= ldap_group_link.cn ? "cn: #{ldap_group_link.cn}" : "filter: #{truncate(ldap_group_link.filter, length: 70)}"

app/views/groups/ldap_group_links/index.html.haml

-- page_title 'LDAP Syncrhonizations'
+- page_title 'LDAP Synchronization'
 %h3.page-title LDAP synchronizations
 = render 'ldap_group_links/form', group: @group
 = render 'ldap_group_links/ldap_group_links', group: @group

app/views/ldap_group_links/_form.html.haml

@@ -9,18 +9,19 @@
         .col-sm-10
           = f.select :provider, ldap_server_select_options, {}, class: 'form-control'
 
-      .form-group.row
-        = f.label :cn, class: 'control-label col-sm-2' do
-          Sync method
-        .col-sm-10
-          .radio
-            = label_tag :sync_method_group do
-              = radio_button_tag :sync_method, :group, true
-              LDAP Group cn
-          .radio
-            = label_tag :sync_method_filter do
-              = radio_button_tag :sync_method, :filter
-              LDAP user filter
+      - if ::License.feature_available?(:ldap_group_sync_filter)
+        .form-group.row
+          = f.label :cn, class: 'control-label col-sm-2' do
+            Sync method
+          .col-sm-10
+            .radio
+              = label_tag :sync_method_group do
+                = radio_button_tag :sync_method, :group, true
+                LDAP Group cn
+            .radio
+              = label_tag :sync_method_filter do
+                = radio_button_tag :sync_method, :filter
+                LDAP user filter
 
       .form-group.row.cn-link
         = f.label :cn, class: 'control-label col-sm-2' do
@@ -32,16 +33,17 @@
             %br
             If you select an LDAP group you do not belong to you will lose ownership of #{group.name}.
 
-      .form-group.row.filter-link
-        = f.label :filter, class: 'control-label col-sm-2' do
-          LDAP User filter
-        .col-sm-10
-          = f.text_field :filter, placeholder: 'Ex. (&(objectCategory=person)(objectClass=developer))', class: 'form-control xxlarge input-mn-300'
-          .help-block
-            - ldap_link = link_to 'LDAP Search Filter Syntax', 'https://msdn.microsoft.com/en-us/library/aa746475(v=vs.85).aspx'
-            This query must use valid #{ldap_link}. Synchronize #{group.name}'s members with this LDAP user filter.
-            %br
-            If you do not belong to this LDAP user filter you will lose ownership of #{group.name}.
+      - if ::License.feature_available?(:ldap_group_sync_filter)
+        .form-group.row.filter-link
+          = f.label :filter, class: 'control-label col-sm-2' do
+            LDAP User filter
+          .col-sm-10
+            = f.text_field :filter, placeholder: 'Ex. (&(objectCategory=person)(objectClass=developer))', class: 'form-control xxlarge input-mn-300'
+            .help-block
+              - ldap_link = link_to 'LDAP Search Filter Syntax', 'https://msdn.microsoft.com/en-us/library/aa746475(v=vs.85).aspx'
+              This query must use valid #{ldap_link}. Synchronize #{group.name}'s members with this LDAP user filter.
+              %br
+              If you do not belong to this LDAP user filter you will lose ownership of #{group.name}.
 
       .form-group.row
         = f.label :group_access, class: 'control-label col-sm-2' do

app/views/ldap_group_links/_ldap_group_link.html.haml

@@ -13,3 +13,6 @@
       Config for LDAP server
       %code= ldap_group_link.provider
       is not present in GitLab
+
+  - unless ldap_group_link.active?
+    (Inactive because syncing with an LDAP user filter is not included in the current license)

app/workers/ldap_all_groups_sync_worker.rb

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

app/workers/ldap_group_sync_worker.rb

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

app/workers/ldap_sync_worker.rb

@@ -3,7 +3,7 @@ class LdapSyncWorker
   include CronjobQueue
 
   def perform
-    return unless Gitlab::LDAP::Config.enabled_extras?
+    return unless Gitlab::LDAP::Config.group_sync_enabled?
 
     Rails.logger.info "Performing daily LDAP sync task."
     User.ldap.find_each(batch_size: 100).each do |ldap_user|

doc/administration/auth/ldap-ee.md

@@ -20,7 +20,7 @@ membership syncing.
 ## Use-cases
 
 - User Sync: Once a day, GitLab will update users against LDAP
-- Group Sync: Once an hour, GitLab will update group membership 
+- Group Sync: Once an hour, GitLab will update group membership
 based on LDAP group members
 
 ## User Sync
@@ -54,8 +54,8 @@ new groups they might be added to when the user logs in. That way they don't nee
 to wait for the hourly sync to be granted access to the groups that they are in
 in LDAP.
 
-We can also add a GitLab group to sync with one or multiple LDAP groups or we can
-also add a filter. The filter must comply with the syntax defined in [RFC 2254](https://tools.ietf.org/search/rfc2254). 
+In GitLab Enterprise Edition Premium, we can also add a GitLab group to sync with one or multiple LDAP groups or we can
+also add a filter. The filter must comply with the syntax defined in [RFC 2254](https://tools.ietf.org/search/rfc2254).
 
 A group sync process will run every hour on the hour, and `group_base` must be set
 in LDAP configuration for LDAP synchronizations based on group CN to work. This allows

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

-- if Gitlab::LDAP::Config.enabled_extras? && can?(current_user, :admin_ldap_group_links, @group)
+- if Gitlab::LDAP::Config.group_sync_enabled? && can?(current_user, :admin_ldap_group_links, @group)
   = nav_link(path: 'ldap_group_links#index') do
     = link_to group_ldap_group_links_path(@group), title: 'LDAP Group' do
       %span
-        LDAP Group
+        LDAP Synchronization
 
 - if @group.feature_available?(:group_webhooks) || show_promotions?
   = nav_link(path: 'hooks#index') do

ee/lib/ee/gitlab/ldap/config.rb

@@ -5,8 +5,8 @@ module EE
         extend ActiveSupport::Concern
 
         class_methods do
-          def enabled_extras?
-            enabled? && ::License.feature_available?(:ldap_extras)
+          def group_sync_enabled?
+            enabled? && ::License.feature_available?(:ldap_group_sync)
           end
         end
       end

ee/lib/ee/gitlab/ldap/sync/group.rb

@@ -74,6 +74,8 @@ module EE
             access_levels = AccessLevels.new
             # Only iterate over group links for the current provider
             group.ldap_group_links.with_provider(provider).each do |group_link|
+              next unless group_link.active?
+
               update_access_levels(access_levels, group_link)
             end
 

lib/api/groups.rb

@@ -213,7 +213,7 @@ module API
 
       desc 'Sync a group with LDAP.'
       post ":id/ldap_sync" do
-        not_found! unless Gitlab::LDAP::Config.enabled_extras?
+        not_found! unless Gitlab::LDAP::Config.group_sync_enabled?
 
         group = find_group!(params[:id])
         authorize! :admin_group, group

lib/gitlab/ldap/config.rb

@@ -27,7 +27,7 @@ module Gitlab
       def self.available_servers
         return [] unless enabled?
 
-        enabled_extras? ? servers : Array.wrap(servers.first)
+        ::License.feature_available?(:multiple_ldap_servers) ? servers : Array.wrap(servers.first)
       end
 
       def self.providers

spec/ee/spec/lib/ee/gitlab/ldap/sync/group_spec.rb

@@ -381,6 +381,10 @@ describe EE::Gitlab::LDAP::Sync::Group do
   end
 
   context 'filter' do
+    before do
+      stub_licensed_features(ldap_group_sync_filter: true)
+    end
+
     describe '#update_permissions' do
       before do
         # Safe-check because some permissions are removed when `Group#ldap_synced?`

spec/features/groups/group_settings_spec.rb

@@ -19,7 +19,7 @@ feature 'Edit group settings' do
       scenario 'is able to navigate to LDAP group section' do
         visit edit_group_path(group)
 
-        expect(find('.nav-sidebar')).to have_content('LDAP Group')
+        expect(find('.nav-sidebar')).to have_content('LDAP Synchronization')
       end
 
       context 'with owners not being able to manage LDAP' do
@@ -28,7 +28,7 @@ feature 'Edit group settings' do
 
           visit edit_group_path(group)
 
-          expect(find('.nav-sidebar')).not_to have_content('LDAP Group')
+          expect(find('.nav-sidebar')).not_to have_content('LDAP Synchronization')
         end
       end
     end

spec/features/groups/ldap_group_links_spec.rb

@@ -12,23 +12,49 @@ feature 'Edit group settings', :js do
   context 'LDAP sync method' do
     before do
       allow(Gitlab.config.ldap).to receive(:enabled).and_return(true)
-
-      visit group_ldap_group_links_path(group)
     end
 
-    scenario 'shows the LDAP filter section' do
-      choose('sync_method_filter')
+    context 'when the LDAP group sync filter feature is available' do
+      before do
+        stub_licensed_features(ldap_group_sync_filter: true)
+
+        visit group_ldap_group_links_path(group)
+      end
+
+      scenario 'shows the LDAP filter section' do
+        choose('sync_method_filter')
+
+        expect(page).to have_content('This query must use valid LDAP Search Filter Syntax')
+        expect(page).not_to have_content("Synchronize #{group.name}'s members with this LDAP group")
+      end
 
-      expect(page).to have_content('This query must use valid LDAP Search Filter Syntax')
-      expect(page).not_to have_content("Synchronize #{group.name}'s members with this LDAP group")
+      scenario 'shows the LDAP group section' do
+        choose('sync_method_filter') # choose filter first, as group's the default
+        choose('sync_method_group')
+
+        expect(page).to have_content("Synchronize #{group.name}'s members with this LDAP group")
+        expect(page).not_to have_content('This query must use valid LDAP Search Filter Syntax')
+      end
     end
 
-    scenario 'shows the LDAP group section' do
-      choose('sync_method_filter') # choose filter first, as group's the default
-      choose('sync_method_group')
+    context 'when the LDAP group sync filter feature is available' do
+      before do
+        stub_licensed_features(ldap_group_sync_filter: false)
+
+        visit group_ldap_group_links_path(group)
+      end
+
+      scenario 'does not show the LDAP search method switcher' do
+        expect(page).not_to have_field('sync_method_filter')
+      end
+
+      scenario 'shows the LDAP group section' do
+        expect(page).to have_content("Synchronize #{group.name}'s members with this LDAP group")
+      end
 
-      expect(page).to have_content("Synchronize #{group.name}'s members with this LDAP group")
-      expect(page).not_to have_content('This query must use valid LDAP Search Filter Syntax')
+      scenario 'does not shows the LDAP filter section' do
+        expect(page).not_to have_content('This query must use valid LDAP Search Filter Syntax')
+      end
     end
   end
 end

spec/requests/api/groups_spec.rb

@@ -700,9 +700,13 @@ describe API::Groups do
   end
 
   describe 'POST /groups/:id/ldap_sync' do
-    context 'when LDAP config enabled_extras is true' do
+    before do
+      allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true)
+    end
+
+    context 'when the ldap_group_sync feature is available' do
       before do
-        allow(Gitlab::LDAP::Config).to receive(:enabled_extras?).and_return(true)
+        stub_licensed_features(ldap_group_sync: true)
       end
 
       context 'when authenticated as the group owner' do
@@ -772,9 +776,9 @@ describe API::Groups do
       end
     end
 
-    context 'when LDAP config enabled_extras is false' do
+    context 'when the ldap_group_sync feature is not available' do
       before do
-        allow(Gitlab::LDAP::Config).to receive(:enabled_extras?).and_return(false)
+        stub_licensed_features(ldap_group_sync: false)
       end
 
       it 'returns 404 (same as CE would)' do