Merge branch 'ldap-group-sync-fix-to-master' into 'master'

Merge branch 'cherry-pick-b01d0422' into 'master'

See merge request gitlab/gitlab-ee!608

changelogs/unreleased-ee/sh-fix-ldap-group-sync-regression-10-4.yml

+---
+title: Fix LDAP group sync no longer configurable for regular users
+merge_request:
+author:
+type: fixed

ee/lib/api/ldap.rb

 module API
   class Ldap < Grape::API
-    before { authenticated_as_admin! }
+    # Admin users by default should be able to access these API endpoints.
+    # However, non-admin users can access these endpoints if the "Allow group
+    # owners to manage LDAP-related group settings" is enabled, and they own a
+    # group.
+    before { authenticated_with_ldap_admin_access! }
 
     resource :ldap do
       helpers do

ee/lib/ee/api/helpers.rb

@@ -20,6 +20,21 @@ module EE
       def check_project_feature_available!(feature)
         not_found! unless user_project.feature_available?(feature)
       end
+
+      # Normally, only admin users should have access to see LDAP
+      # groups. However, due to the "Allow group owners to manage LDAP-related
+      # group settings" setting, any group owner can sync LDAP groups with
+      # their project.
+      #
+      # In the future, we should also check that the user has access to manage
+      # a specific group so that we can use the Ability class.
+      def authenticated_with_ldap_admin_access!
+        authenticate!
+
+        forbidden! unless current_user.admin? ||
+            ::Gitlab::CurrentSettings.current_application_settings
+              .allow_group_owners_to_manage_ldap
+      end
     end
   end
 end

ee/spec/requests/api/ldap_spec.rb

@@ -17,6 +17,7 @@ describe API::Ldap do
     allow(Gitlab::Auth::LDAP::Config).to receive(:enabled?).and_return(true)
     allow(Gitlab::Auth::LDAP::Adapter).to receive(:new).and_return(adapter)
     allow(adapter).to receive_messages(groups: groups)
+    stub_application_setting(allow_group_owners_to_manage_ldap: false)
   end
 
   describe "GET /ldap/groups" do
@@ -34,6 +35,20 @@ describe API::Ldap do
       end
     end
 
+    context 'when group owners are allowed to manage LDAP' do
+      before do
+        stub_application_setting(allow_group_owners_to_manage_ldap: true)
+      end
+
+      it "returns an array of ldap groups" do
+        get api("/ldap/groups", user)
+        expect(response.status).to eq 200
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq 2
+        expect(json_response.first['cn']).to eq 'developers'
+      end
+    end
+
     context "when authenticated as admin" do
       it "returns an array of ldap groups" do
         get api("/ldap/groups", admin)
@@ -60,6 +75,20 @@ describe API::Ldap do
       end
     end
 
+    context 'when group owners are allowed to manage LDAP' do
+      before do
+        stub_application_setting(allow_group_owners_to_manage_ldap: true)
+      end
+
+      it "returns an array of ldap groups" do
+        get api("/ldap/ldapmain/groups", admin)
+        expect(response.status).to eq 200
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq 2
+        expect(json_response.first['cn']).to eq 'developers'
+      end
+    end
+
     context "when authenticated as admin" do
       it "returns an array of ldap groups" do
         get api("/ldap/ldapmain/groups", admin)