Merge branch 'feature/ldap-group-filter' into 'master'

Enhance LDAP group mappings with filters

Closes #3188

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

app/assets/javascripts/dispatcher.js

@@ -602,6 +602,7 @@ import initGroupAnalytics from './init_group_analytics';
         case 'groups:analytics:show':
           initGroupAnalytics();
           break;
+
       }
       switch (path[0]) {
         case 'sessions':

app/assets/javascripts/groups/ldap_group_links.js

+document.addEventListener('DOMContentLoaded', () => {
+  const showGroupLink = () => {
+    const $cnLink = $('.cn-link');
+    const $filterLink = $('.filter-link');
+    const $checkedSync = $('input[name="sync_method"]:checked').val() === 'group';
+
+    $cnLink.toggle($checkedSync);
+    $filterLink.toggle(!$checkedSync);
+  };
+
+  $('input[name="sync_method"]').on('change', showGroupLink);
+  showGroupLink();
+});

app/controllers/groups/ldap_group_links_controller.rb

@@ -43,6 +43,6 @@ class Groups::LdapGroupLinksController < Groups::ApplicationController
   end
 
   def ldap_group_link_params
-    params.require(:ldap_group_link).permit(:cn, :group_access, :provider)
+    params.require(:ldap_group_link).permit(:cn, :filter, :group_access, :provider)
   end
 end

app/models/group.rb

@@ -230,12 +230,20 @@ 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.present?
+    Gitlab.config.ldap.enabled && ldap_cn_or_filter_present?
   end
 
   def post_create_hook

app/models/ldap_group_link.rb

@@ -2,13 +2,28 @@ class LdapGroupLink < ActiveRecord::Base
   include Gitlab::Access
   belongs_to :group
 
-  validates :cn, :group_access, :group_id, presence: true
-  validates :cn, uniqueness: { scope: [:group_id, :provider] }
+  BLANK_ATTRIBUTES = %w[cn filter].freeze
+
+  with_options if: :cn do |link|
+    link.validates :cn, uniqueness: { scope: [:group_id, :provider] }
+    link.validates :cn, presence: true
+    link.validates :filter, absence: true
+  end
+
+  with_options if: :filter do |link|
+    link.validates :filter, uniqueness: { scope: [:group_id, :provider] }
+    link.validates :filter, ldap_filter: true, presence: true
+    link.validates :cn, absence: true
+  end
+
+  validates :group_access, :group_id, presence: true
   validates :group_access, inclusion: { in: Gitlab::Access.all_values }
   validates :provider, presence: true
 
   scope :with_provider, ->(provider) { where(provider: provider) }
 
+  before_validation :nullify_blank_attributes
+
   def access_field
     group_access
   end
@@ -27,4 +42,10 @@ class LdapGroupLink < ActiveRecord::Base
   def provider_label
     config.label
   end
+
+  private
+
+  def nullify_blank_attributes
+    BLANK_ATTRIBUTES.each { |attr| self[attr] = nil if self[attr].blank? }
+  end
 end

app/validators/ldap_filter_validator.rb

+# LdapFilteralidator
+#
+# Custom validator for LDAP filters
+#
+# Example:
+#
+#   class LdapGroupLink < ActiveRecord::Base
+#     validates :filter, ldap_filter: true
+#   end
+#
+class LdapFilterValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    Net::LDAP::Filter::FilterParser.parse(value)
+  rescue Net::LDAP::FilterSyntaxInvalidError
+    record.errors.add(attribute, 'must be a valid filter')
+  end
+end

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

@@ -37,6 +37,6 @@
       = link_to  'Cancel', admin_group_path(@group), class: "btn btn-cancel"
 
 - if ldap_enabled? && @group.persisted?
-  %h3.page-title Linked LDAP groups
+  %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

@@ -63,13 +63,12 @@
         = render partial: "namespaces/shared_runner_status", locals: { namespace: @group }
 
     .panel.panel-default
-      .panel-heading Linked LDAP groups
+      .panel-heading Active synchronizations
       %ul.well-list
         - if @group.ldap_group_links.any?
           - @group.ldap_group_links.each do |ldap_group_link|
             %li
-              cn:
-              %strong= ldap_group_link.cn
+              %strong= ldap_group_link.cn ? "Group: #{ldap_group_link.cn}" : "Filter: #{truncate(ldap_group_link.filter, length: 40)}"
               as
               %strong= ldap_group_link.human_access
 

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

@@ -5,8 +5,8 @@
     %ul
       - @group.ldap_group_links.each do |ldap_group_link|
         %li
-          People in cn
-          %code= ldap_group_link.cn
+          People in
+          %code= ldap_group_link.cn ? "cn: #{ldap_group_link.cn}" : "filter: #{truncate(ldap_group_link.filter, length: 70)}"
           are given
           %code= ldap_group_link.human_access
           access.

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

-- page_title "LDAP Groups"
-%h3.page-title Linked LDAP groups
+- page_title 'LDAP Syncrhonizations'
+%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

+- content_for :page_specific_javascripts do
+  = webpack_bundle_tag 'ldap_group_links'
 %section.ldap-group-links
   = form_for [group, LdapGroupLink.new], html: { class: 'form-horizontal' } do |f|
     .form-holder
-      .form-group.clearfix
-        = f.label :cn, class: 'control-label' do
+      .form-group.row
+        = f.label :cn, class: 'control-label col-sm-2' do
           LDAP Server
         .col-sm-10
           = f.select :provider, ldap_server_select_options, {}, class: 'form-control'
-      .form-group.clearfix
-        = f.label :cn, class: 'control-label' do
+
+      .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
           LDAP Group cn
         .col-sm-10
-          = f.hidden_field :cn, placeholder: "Ex. QA group", class: "xxlarge ajax-ldap-groups-select input-mn-300"
+          = f.hidden_field :cn, placeholder: 'Ex. QA group', class: 'xxlarge ajax-ldap-groups-select input-mn-300'
           .help-block
             Synchronize #{group.name}'s members with this LDAP group.
             %br
             If you select an LDAP group you do not belong to you will lose ownership of #{group.name}.
 
-      .form-group.clearfix
-        = f.label :group_access, class: 'control-label' do
+      .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
           LDAP Access
         .col-sm-10
           = f.select :group_access, options_for_select(GroupMember.access_level_roles), {}, class: 'form-control'
@@ -27,4 +54,4 @@
             You can manage permission levels for individual group members in the Members tab.
 
     .form-actions
-      = f.submit 'Add synchronization', class: "btn btn-create"
+      = f.submit 'Add synchronization', class: 'btn btn-create'

app/views/ldap_group_links/_ldap_group_link.html.haml

 %li
   .pull-right
     = link_to group_ldap_group_link_path(group, ldap_group_link), method: :delete, class: 'btn btn-danger btn-sm' do
-      = fa_icon('unlink', text: 'unlink')
-  %strong= ldap_group_link.cn
+      = fa_icon('unlink', text: 'Remove')
+  %strong= ldap_group_link.cn ? "Group: #{ldap_group_link.cn}" : "Filter: #{truncate(ldap_group_link.filter, length: 70)}"
 
   - if ldap_group_link.config
     .light

app/views/ldap_group_links/_ldap_group_links.html.haml

 .panel.panel-default
   .panel-heading
     %h4.panel-title
-      Linked LDAP groups (#{group.ldap_group_links.count})
+      Active synchronizations (#{group.ldap_group_links.count})
 
   - if group.ldap_group_links.any?
     %ul.well-list
       = render collection: group.ldap_group_links, partial: 'ldap_group_links/ldap_group_link', locals: { group: group }
   - else
     .panel-body
-      No linked LDAP groups
+      No LDAP synchronizations

changelogs/unreleased-ee/feature-ldap-group-filter.yml

+---
+title: Add LDAP synchronization based on filter for GitLab groups
+merge_request:
+author:
+type: added

config/webpack.config.js

@@ -53,6 +53,7 @@ var config = {
     issue_show:           './issue_show/index.js',
     integrations:         './integrations',
     job_details:          './jobs/job_details_bundle.js',
+    ldap_group_links:     './groups/ldap_group_links.js',
     locale:               './locale/index.js',
     main:                 './main.js',
     merge_conflicts:      './merge_conflicts/merge_conflicts_bundle.js',

db/migrate/20170911133813_drop_cn_constraint_to_ldap_group_links.rb

+class DropCnConstraintToLdapGroupLinks < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def change
+    change_column_null :ldap_group_links, :cn, true
+  end
+end

db/migrate/20170911134018_add_filter_to_ldap_group_links.rb

+class AddFilterToLdapGroupLinks < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def change
+    add_column(:ldap_group_links, :filter, :string)
+  end
+end

db/schema.rb

@@ -941,12 +941,13 @@ ActiveRecord::Schema.define(version: 20170921115009) do
   add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree
 
   create_table "ldap_group_links", force: :cascade do |t|
-    t.string "cn", null: false
+    t.string "cn"
     t.integer "group_access", null: false
     t.integer "group_id", null: false
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string "provider"
+    t.string "filter"
   end
 
   create_table "lfs_objects", force: :cascade do |t|

doc/administration/auth/ldap-ee.md

@@ -54,9 +54,12 @@ 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.
 
-If `group_base` is set in LDAP configuration, a group sync process will run
-every hour, on the hour. This allows GitLab group membership to be automatically
-updated based on LDAP group members.
+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
+GitLab group membership to be automatically updated based on LDAP group members.
 
 The `group_base` configuration should be a base LDAP 'container', such as an
 'organization' or 'organizational unit', that contains LDAP groups that should
@@ -97,8 +100,9 @@ production:
 
 To take advantage of group sync, group owners or masters will need to create an
 LDAP group link in their group **Settings -> LDAP Groups** page. Multiple LDAP
-groups can be linked with a single GitLab group. When the link is created, an
-access level/role is specified (Guest, Reporter, Developer, Master, or Owner).
+groups and/or filters can be linked with a single GitLab group. When the link is
+created, an access level/role is specified (Guest, Reporter, Developer, Master,
+or Owner).
 
 ## Administrator Sync
 

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

@@ -74,12 +74,7 @@ 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|
-              if member_dns = dns_for_group_cn(group_link.cn)
-                access_levels.set(member_dns, to: group_link.group_access)
-                logger.debug do
-                  "Resolved '#{group.name}' group member access: #{access_levels.to_hash}"
-                end
-              end
+              update_access_levels(access_levels, group_link)
             end
 
             update_existing_group_membership(group, access_levels)
@@ -88,7 +83,27 @@ module EE
 
           private
 
+          def update_access_levels(access_levels, group_link)
+            if member_dns = get_member_dns(group_link)
+              access_levels.set(member_dns, to: group_link.group_access)
+
+              logger.debug do
+                "Resolved '#{group.name}' group member access: #{access_levels.to_hash}"
+              end
+            end
+          end
+
+          def get_member_dns(group_link)
+            group_link.cn ? dns_for_group_cn(group_link.cn) : UserFilter.filter(@proxy, group_link.filter)
+          end
+
           def dns_for_group_cn(group_cn)
+            if config.group_base.blank?
+              logger.debug { "No `group_base` configured for '#{provider}' provider and group link CN #{group_cn}. Skipping" }
+
+              return nil
+            end
+
             proxy.dns_for_group_cn(group_cn)
           end
 
@@ -209,6 +224,10 @@ module EE
           def logger
             Rails.logger
           end
+
+          def config
+            @proxy.adapter.config
+          end
         end
       end
     end

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

@@ -26,11 +26,6 @@ module EE
           end
 
           def update_permissions
-            unless config.group_base.present?
-              logger.debug { "No `group_base` configured for '#{provider}' provider. Skipping" }
-              return nil
-            end
-
             logger.debug { "Performing LDAP group sync for '#{provider}' provider" }
             sync_groups
             logger.debug { "Finished LDAP group sync for '#{provider}' provider" }

ee/lib/ee/gitlab/ldap/user_filter.rb

+module EE
+  module Gitlab
+    module LDAP
+      class UserFilter
+        def self.filter(*args)
+          new(*args).filter
+        end
+
+        def initialize(proxy, filter)
+          @proxy = proxy
+          @filter = filter
+        end
+
+        def filter
+          logger.debug "Running filter #{@filter} against #{@proxy.provider}"
+
+          @proxy.adapter.ldap_search(options).map(&:dn).tap do |dns|
+            logger.debug "Found #{dns.count} mathing users for filter #{@filter}"
+          end
+        end
+
+        private
+
+        def options
+          { base: config.base, filter: construct_filter }
+        end
+
+        def construct_filter
+          Net::LDAP::Filter.construct(@filter)
+        end
+
+        def config
+          @proxy.adapter.config
+        end
+
+        def logger
+          Rails.logger
+        end
+      end
+    end
+  end
+end

features/steps/groups.rb

@@ -157,7 +157,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
   end
 
   step 'I see a new LDAP synchronization listed' do
-    expect(page).not_to have_content('No synchronizations yet')
+    expect(page).not_to have_content('No LDAP synchronizations')
     expect(page).to have_content('As Developer on ldap server')
   end
 

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

@@ -299,6 +299,14 @@ describe EE::Gitlab::LDAP::Sync::Group do
           expect(group.members.pluck(:access_level))
             .to match_array([::Gitlab::Access::MASTER, ::Gitlab::Access::OWNER])
         end
+
+        it 'does not update permissions when group base is missing' do
+          stub_ldap_config(group_base: nil)
+
+          expect_any_instance_of(EE::Gitlab::LDAP::Sync::Proxy).not_to receive(:dns_for_group_cn)
+
+          sync_group.update_permissions
+        end
       end
     end
 
@@ -371,4 +379,59 @@ describe EE::Gitlab::LDAP::Sync::Group do
       end
     end
   end
+
+  context 'filter' do
+    describe '#update_permissions' do
+      before do
+        # Safe-check because some permissions are removed when `Group#ldap_synced?`
+        # is true (e.g. in `GroupPolicy`).
+        expect(group).to be_ldap_synced
+        allow(EE::Gitlab::LDAP::UserFilter).to receive(:filter).and_return([user_dn(user.username)])
+
+        group.start_ldap_sync
+      end
+
+      after do
+        group.finish_ldap_sync
+      end
+
+      let(:group) do
+        create(:group_with_ldap_group_filter_link,
+               :access_requestable,
+               group_access: ::Gitlab::Access::DEVELOPER)
+      end
+
+      let(:sync_group) { described_class.new(group, proxy(adapter)) }
+
+      context 'with all functionality against one LDAP group type' do
+        context 'with basic add/update actions' do
+          let(:ldap_group1) { ldap_group_entry(user_dn(user.username)) }
+
+          it 'does not update permissions unless ldap sync status is started' do
+            group.finish_ldap_sync
+
+            expect(Rails.logger)
+                .to receive(:warn).with(/status must be 'started' before updating permissions/)
+
+            sync_group.update_permissions
+          end
+
+          it 'adds new members and sets ldap attribute to true' do
+            sync_group.update_permissions
+
+            expect(group.users).to include(user)
+            expect(group.members.find_by(user_id: user.id).ldap?).to be_truthy
+          end
+
+          it 'updates permissions when group base is missing' do
+            stub_ldap_config(group_base: nil)
+
+            sync_group.update_permissions
+
+            expect(group.users).to include(user)
+          end
+        end
+      end
+    end
+  end
 end

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

@@ -24,10 +24,6 @@ describe EE::Gitlab::LDAP::Sync::Groups do
         stub_ldap_config(group_base: nil)
       end
 
-      it 'does not call EE::Gitlab::LDAP::Sync::Group#execute' do
-        expect(EE::Gitlab::LDAP::Sync::Group).not_to receive(:execute)
-      end
-
       it 'does not call EE::Gitlab::LDAP::Sync::AdminUsers#execute' do
         expect(EE::Gitlab::LDAP::Sync::AdminUsers).not_to receive(:execute)
       end

spec/ee/spec/lib/ee/gitlab/ldap/user_filter_spec.rb

+require 'spec_helper'
+
+describe EE::Gitlab::LDAP::UserFilter do
+  include LdapHelpers
+
+  let(:auth_hash) do
+    OmniAuth::AuthHash.new(uid: 'uid=john,ou=people,dc=example,dc=com', provider: 'ldapmain')
+  end
+  let!(:fake_proxy) { fake_ldap_sync_proxy(auth_hash.provider) }
+  let(:fake_entry) { ldap_user_entry('user1') }
+
+  before do
+    stub_ldap_config(
+      base: 'dc=example,dc=com',
+      active_directory: false
+    )
+
+    allow(fake_proxy).to receive(:provider)
+  end
+
+  describe '#filter' do
+    it 'returns dns from an LDAP search' do
+      filter = '(ou=people)'
+
+      allow(fake_proxy.adapter).to receive(:ldap_search).and_return([fake_entry])
+
+      expect(described_class.filter(fake_proxy, filter)).to eq(['uid=user1,ou=users,dc=example,dc=com'])
+    end
+
+    it 'errors out with an invalid filter' do
+      filter = ')('
+
+      expect { described_class.filter(fake_proxy, filter) }.to raise_error(Net::LDAP::FilterSyntaxInvalidError, 'Invalid filter syntax.')
+    end
+  end
+end

spec/factories/groups.rb

@@ -27,20 +27,34 @@ FactoryGirl.define do
       end
     end
 
-    factory :group_with_ldap_group_link do
+    factory :group_with_ldap do
       transient do
         cn 'group1'
         group_access Gitlab::Access::GUEST
         provider 'ldapmain'
       end
 
-      after(:create) do |group, evaluator|
-        group.ldap_group_links << create(
-          :ldap_group_link,
-          cn: evaluator.cn,
-          group_access: evaluator.group_access,
-          provider: evaluator.provider
-        )
+      factory :group_with_ldap_group_link do
+        after(:create) do |group, evaluator|
+          group.ldap_group_links << create(
+            :ldap_group_link,
+              cn: evaluator.cn,
+              group_access: evaluator.group_access,
+              provider: evaluator.provider
+          )
+        end
+      end
+
+      factory :group_with_ldap_group_filter_link do
+        after(:create) do |group, evaluator|
+          group.ldap_group_links << create(
+            :ldap_group_link,
+              filter: '(a=b)',
+              cn: nil,
+              group_access: evaluator.group_access,
+              provider: evaluator.provider
+          )
+        end
       end
     end
 

spec/features/groups/ldap_group_links_spec.rb

+require 'spec_helper'
+
+feature 'Edit group settings', :js do
+  given(:user) { create(:user) }
+  given(:group) { create(:group, path: 'foo') }
+
+  background do
+    group.add_owner(user)
+    sign_in(user)
+  end
+
+  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')
+
+      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
+
+    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
+end

spec/models/ldap_group_link_spec.rb

@@ -4,9 +4,9 @@ describe LdapGroupLink do
   let(:klass) { described_class }
   let(:ldap_group_link) { build :ldap_group_link }
 
-  describe "validation" do
-    describe "cn" do
-      it "validates uniquiness based on group_id and provider" do
+  describe 'validation' do
+    describe 'cn' do
+      it 'validates uniqueness based on group_id and provider' do
         create(:ldap_group_link, cn: 'group1', group_id: 1, provider: 'ldapmain')
 
         group_link = build(:ldap_group_link,
@@ -20,15 +20,44 @@ describe LdapGroupLink do
         group_link.provider = 'ldapalt'
         expect(group_link).to be_valid
       end
+
+      it 'is invalid when a filter is also present' do
+        link = build(:ldap_group_link, filter: '(a=b)', group_id: 1, provider: 'ldapmain', cn: 'group1')
+
+        expect(link).not_to be_valid
+      end
+    end
+
+    describe 'filter' do
+      it 'validates uniqueness based on group_id and provider' do
+        create(:ldap_group_link, filter: '(a=b)', group_id: 1, provider: 'ldapmain', cn: nil)
+
+        group_link = build(:ldap_group_link,
+                           filter: '(a=b)', group_id: 1, provider: 'ldapmain', cn: nil)
+        expect(group_link).not_to be_valid
+
+        group_link.group_id = 2
+        expect(group_link).to be_valid
+
+        group_link.group_id = 1
+        group_link.provider = 'ldapalt'
+        expect(group_link).to be_valid
+      end
+
+      it 'validates the LDAP filter' do
+        link = build(:ldap_group_link, filter: 'invalid', group_id: 1, provider: 'ldapmain', cn: nil)
+
+        expect(link).not_to be_valid
+      end
     end
 
     describe 'provider' do
-      it "shows the set value" do
+      it 'shows the set value' do
         ldap_group_link.provider = '1235'
         expect( ldap_group_link.provider ).to eql '1235'
       end
 
-      it "defaults to the first ldap server if empty" do
+      it 'defaults to the first ldap server if empty' do
         expect( klass.new.provider ).to eql Gitlab::LDAP::Config.providers.first
       end
     end

spec/validators/ldap_filter_validator_spec.rb

+require 'spec_helper'
+
+describe LdapFilterValidator do
+  let(:validator) { described_class.new(attributes: [:filter]) }
+
+  describe '#validates_each' do
+    it 'adds a message when the filter is not valid' do
+      link = build(:ldap_group_link, cn: nil)
+
+      validator.validate_each(link, :filter, 'wrong filter')
+
+      expect(link.errors[:filter]).to match_array(['must be a valid filter'])
+    end
+
+    it 'has no errors when is valid' do
+      link = build(:ldap_group_link, cn: nil)
+
+      validator.validate_each(link, :filter, '(cn=Babs Jensen)')
+
+      expect(link.errors[:filter]).to eq([])
+    end
+  end
+end