Merge branch 'api-empty-return-ee' into 'master'

Return 204 on all DELETE endpoints

See merge request !1312

doc/api/projects.md

@@ -1207,7 +1207,6 @@ Parameters:
 
 ```
 POST /projects/:id/housekeeping
->>>>>>> ce/master
 ```
 
 Parameters:
@@ -1300,6 +1299,3 @@ DELETE /projects/:id/push_rule
 Parameters:
 
 - `id` (required) - The ID of a project
-
-Note the JSON response differs if the push rule is available or not. If the project push rule
-is available before it is returned in the JSON response or an empty response is returned.

lib/api/api.rb

@@ -16,6 +16,7 @@ module API
       mount ::API::V3::Groups
       mount ::API::V3::Issues
       mount ::API::V3::Labels
+      mount ::API::V3::LdapGroupLinks
       mount ::API::V3::Members
       mount ::API::V3::MergeRequestDiffs
       mount ::API::V3::MergeRequests

lib/api/members.rb

@@ -99,7 +99,7 @@ module API
         end
         delete ":id/members/:user_id" do
           source = find_source(source_type, params[:id])
-          # Ensure that memeber exists
+          # Ensure the member exists
           source.members.find_by!(user_id: params[:user_id])
 
           ::Members::DestroyService.new(source, current_user, declared_params).execute

lib/api/v3/ldap_group_links.rb

+module API
+  module V3
+    class LdapGroupLinks < Grape::API
+      before { authenticate! }
+
+      params do
+        requires :id, type: String, desc: 'The ID of a group'
+      end
+      resource :groups do
+        desc 'Remove a linked LDAP group from group'
+        params do
+          requires 'cn', type: String, desc: 'The CN of a LDAP group'
+        end
+        delete ":id/ldap_group_links/:cn" do
+          group = find_group(params[:id])
+          authorize! :admin_group, group
+
+          ldap_group_link = group.ldap_group_links.find_by(cn: params[:cn])
+          if ldap_group_link
+            status(200)
+            ldap_group_link.destroy
+          else
+            render_api_error!('Linked LDAP group not found', 404)
+          end
+        end
+
+        desc 'Remove a linked LDAP group from group'
+        params do
+          requires 'cn', type: String, desc: 'The CN of a LDAP group'
+          requires 'provider', type: String, desc: 'The LDAP provider for this LDAP group'
+        end
+        delete ":id/ldap_group_links/:provider/:cn" do
+          group = find_group(params[:id])
+          authorize! :admin_group, group
+
+          ldap_group_link = group.ldap_group_links.find_by(cn: params[:cn], provider: params[:provider])
+          if ldap_group_link
+            status(200)
+            ldap_group_link.destroy
+          else
+            render_api_error!('Linked LDAP group not found', 404)
+          end
+        end
+      end
+    end
+  end
+end

lib/api/v3/services.rb

@@ -53,7 +53,7 @@ module API
             required: true,
             name: :password,
             type: String,
-            desc: 'Passord of the user'
+            desc: 'Password of the user'
           }
         ],
         'bugzilla' => [
@@ -352,7 +352,6 @@ module API
             desc: 'The ID of a transition that moves issues to a closed state. You can find this number under the JIRA workflow administration (**Administration > Issues > Workflows**) by selecting **View** under **Operations** of the desired workflow of your project. The ID of each state can be found inside the parenthesis of each transition name under the **Transitions (id)** column ([see screenshot][trans]). By default, this ID is set to `2`'
           }
         ],
-
         'kubernetes' => [
           {
             required: true,

spec/requests/api/v3/ldap_group_links_spec.rb

+require 'spec_helper'
+
+describe API::V3::LdapGroupLinks, api: true  do
+  include ApiHelpers
+
+  let(:owner) { create(:user) }
+  let(:user) { create(:user) }
+  let(:admin) { create(:admin) }
+
+  let!(:group_with_ldap_links) do
+    group = create(:group)
+    group.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER, provider: 'ldap1'
+    group.ldap_group_links.create cn: 'ldap-group2', group_access: Gitlab::Access::MASTER, provider: 'ldap2'
+    group
+  end
+
+  before do
+    group_with_ldap_links.add_owner owner
+    group_with_ldap_links.add_user user, Gitlab::Access::DEVELOPER
+  end
+  describe 'DELETE /groups/:id/ldap_group_links/:cn' do
+    context "when unauthenticated" do
+      it "returns authentication error" do
+        delete v3_api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap-group1")
+        expect(response.status).to eq 401
+      end
+    end
+
+    context "when a less priviledged user" do
+      it "does not remove the LDAP group link" do
+        expect do
+          delete v3_api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap-group1", user)
+        end.not_to change { group_with_ldap_links.ldap_group_links.count }
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context "when owner of the group" do
+      it "removes ldap group link" do
+        expect do
+          delete v3_api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap-group1", owner)
+        end.to change { group_with_ldap_links.ldap_group_links.count }.by(-1)
+
+        expect(response.status).to eq(200)
+      end
+
+      it "returns 404 if LDAP group cn not used for a LDAP group link" do
+        expect do
+          delete v3_api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap-group1356", owner)
+        end.not_to change { group_with_ldap_links.ldap_group_links.count }
+
+        expect(response.status).to eq(404)
+      end
+    end
+  end
+
+  describe 'DELETE /groups/:id/ldap_group_links/:provider/:cn' do
+    context "when unauthenticated" do
+      it "returns authentication error" do
+        delete v3_api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap2/ldap-group2")
+        expect(response.status).to eq 401
+      end
+    end
+
+    context "when a less priviledged user" do
+      it "does not remove the LDAP group link" do
+        expect do
+          delete v3_api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap2/ldap-group2", user)
+        end.not_to change { group_with_ldap_links.ldap_group_links.count }
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context "when owner of the group" do
+      it "returns 404 if LDAP group cn not used for a LDAP group link for the specified provider" do
+        expect do
+          delete v3_api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap1/ldap-group2", owner)
+        end.not_to change { group_with_ldap_links.ldap_group_links.count }
+
+        expect(response.status).to eq(404)
+      end
+
+      it "removes ldap group link" do
+        expect do
+          delete v3_api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap2/ldap-group2", owner)
+        end.to change { group_with_ldap_links.ldap_group_links.count }.by(-1)
+
+        expect(response.status).to eq(200)
+      end
+    end
+  end
+end