Merge branch 'grapify-ldap-group-links-api' into 'master'

Grapify the LDAP group link API

See merge request !1002

lib/api/ldap_group_links.rb

 module API
-  # LDAP group links API
   class LdapGroupLinks < Grape::API
     before { authenticate! }
 
+    params do
+      requires :id, type: String, desc: 'The ID of a group'
+    end
     resource :groups do
-      
-      # Add a linked LDAP group to group
-      #
-      # Parameters:
-      #   id (required) - The ID of a group
-      #   cn (required) - The CN of a LDAP group
-      #   group_access (required) - Level of permissions for the linked LDAP group
-      #   provider (required) - the LDAP provider for this LDAP group
-      #
-      # Example Request:
-      #   POST /groups/:id/ldap_group_links
+      desc 'Add a linked LDAP group to group' do
+        success Entities::LdapGroupLink
+      end
+      params do
+        requires 'cn', type: String, desc: 'The CN of a LDAP group'
+        requires 'group_access', type: Integer, values: Gitlab::Access.all_values,
+                                 desc: 'Level of permissions for the linked LDAP group'
+        requires 'provider', type: String, desc: 'The LDAP provider for this LDAP group'
+      end
       post ":id/ldap_group_links" do
         group = find_group(params[:id])
         authorize! :admin_group, group
-        required_attributes! [:cn, :group_access, :provider]
         unless validate_access_level?(params[:group_access])
           render_api_error!("Wrong group access level", 422)
         end
 
-        attrs = attributes_for_keys [:cn, :group_access, :provider]
-        
-        ldap_group_link = group.ldap_group_links.new(attrs)
+        ldap_group_link = group.ldap_group_links.new(declared_params(include_missing: false))
         if ldap_group_link.save
           present ldap_group_link, with: Entities::LdapGroupLink
         else
@@ -33,14 +30,10 @@ module API
         end
       end
 
-      # Remove a linked LDAP group from group
-      #
-      # Parameters:
-      #   id (required) - The ID of a group
-      #   cn (required) - The CN of a LDAP group
-      #
-      # Example Request:
-      #   DELETE /groups/:id/ldap_group_links/:cn
+      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
@@ -53,15 +46,11 @@ module API
         end
       end
 
-      # Remove a linked LDAP group from group for a specific LDAP provider
-      #
-      # Parameters:
-      #   id (required) - The ID of a group
-      #   provider (required) - A LDAP provider
-      #   cn (required) - The CN of a LDAP group
-      #
-      # Example Request:
-      #   DELETE /groups/:id/ldap_group_links/:provider/:cn
+      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

spec/requests/api/ldap_group_links_spec.rb

@@ -31,7 +31,7 @@ describe API::LdapGroupLinks, api: true  do
       it "does not allow less priviledged user to add LDAP group link" do
         expect do
           post api("/groups/#{group_with_ldap_links.id}/ldap_group_links", user),
-          cn: 'ldap-group4', group_access: GroupMember::GUEST
+          cn: 'ldap-group4', group_access: GroupMember::GUEST, provider: 'ldap3'
         end.not_to change { group_with_ldap_links.ldap_group_links.count }
 
         expect(response.status).to eq(403)
@@ -81,7 +81,9 @@ describe API::LdapGroupLinks, api: true  do
 
       it "returns a 422 error when group access is not known" do
         post api("//groups/#{group_with_ldap_links.id}/ldap_group_links", owner), cn: 'ldap-group3', group_access: 11, provider: 'ldap1'
-        expect(response.status).to eq(422)
+
+        expect(response.status).to eq(400)
+        expect(json_response['error']).to eq('group_access does not have a valid value')
       end
     end
   end