Merge branch 'feature/ldap_group_link_api' into 'master'

Add API support for adding and removing LDAP group links

See merge request !4

CHANGELOG-EE

@@ -9,6 +9,8 @@ v 7.11.4
 
 v 7.11.3
   - Fixed an issue with git annex
+v 7.12.0 (unreleased)
+  - Add API support for adding and removing LDAP group links
 
 v 7.11.2
   - Fixed license upload and verification mechanism

doc/api/groups.md

@@ -176,3 +176,43 @@ Parameters:
 
 - `id` (required) - The ID or path of a user group
 - `user_id` (required) - The ID of a group member
+
+### Add LDAP group link
+
+Adds LDAP group link
+
+```
+POST /groups/:id/ldap_group_links
+```
+
+Parameters:
+
+- `id` (required) - The ID of a group
+- `cn` (required) - The CN of a LDAP group
+- `group_access` (required) - Minimum access level for members of the LDAP group
+- `provider` (optional) - LDAP provider for the LDAP group (when using several providers) 
+
+### Delete LDAP group link
+
+Deletes a LDAP group link
+
+```
+DELETE /groups/:id/ldap_group_links/:cn
+```
+
+Parameters:
+
+- `id` (required) - The ID of a group
+- `cn` (required) - The CN of a LDAP group
+
+Deletes a LDAP group link for a specific LDAP provider
+
+```
+DELETE /groups/:id/ldap_group_links/:provider/:cn
+```
+
+Parameters:
+
+- `id` (required) - The ID of a group
+- `cn` (required) - The CN of a LDAP group
+- `provider` (required) - Name of a LDAP provider
\ No newline at end of file

lib/api/api.rb

@@ -45,6 +45,7 @@ module API
     mount ProjectHooks
     mount ProjectGitHook
     mount Ldap
+    mount LdapGroupLinks
     mount Services
     mount Files
     mount Commits

lib/api/entities.rb

@@ -71,13 +71,13 @@ module API
       end
     end
 
+    class LdapGroupLink < Grape::Entity
+      expose :cn, :group_access, :provider
+    end
+
     class Group < Grape::Entity
       expose :id, :name, :path, :ldap_cn, :ldap_access, :description
-      expose :ldap_group_links, if: ->(group, _) { group.ldap_group_links.any? } do |group, _|
-        group.ldap_group_links.map do |group_link|
-          group_link.slice(:cn, :group_access)
-        end
-      end
+      expose :ldap_group_links, using: Entities::LdapGroupLink, if: lambda{ | group, options | group.ldap_group_links.any? }
     end
 
     class GroupDetail < Group

lib/api/ldap_group_links.rb

+module API
+  # LDAP group links API
+  class LdapGroupLinks < Grape::API
+    before { authenticate! }
+
+    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 (optional) - the LDAP provider for this LDAP group
+      #
+      # Example Request:
+      #   POST /groups/:id/ldap_group_links
+      post ":id/ldap_group_links" do
+        group = find_group(params[:id])
+        authorize! :admin_group, group
+        required_attributes! [:cn, :group_access]
+        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)
+        if ldap_group_link.save
+          present ldap_group_link, with: Entities::LdapGroupLink
+        else
+          render_api_error!(ldap_group_link.errors.full_messages.first, 409)
+        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
+      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
+          ldap_group_link.destroy
+        else
+          render_api_error!('Linked LDAP group not found', 404)
+        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
+      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
+          ldap_group_link.destroy
+        else
+          render_api_error!('Linked LDAP group not found', 404)
+        end
+      end
+    end
+  end
+end

spec/requests/api/groups_spec.rb

@@ -13,7 +13,7 @@ describe API::API, api: true  do
   before do
     group1.add_owner(user1)
     group2.add_owner(user2)
-    group1.ldap_group_links.create cn: 'ldap-group', group_access: Gitlab::Access::MASTER
+    group1.ldap_group_links.create cn: 'ldap-group', group_access: Gitlab::Access::MASTER, provider: 'ldap'
   end
 
   describe "GET /groups" do
@@ -39,6 +39,7 @@ describe API::API, api: true  do
         ldap_group_link = json_response.first['ldap_group_links'].first
         expect(ldap_group_link['cn']).to eq(group1.ldap_cn)
         expect(ldap_group_link['group_access']).to eq(group1.ldap_access)
+        expect(ldap_group_link['provider']).to eq('ldap')
       end
     end
 

spec/requests/api/ldap_group_links_spec.rb

+require 'spec_helper'
+
+describe API::API, 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, group_access: Gitlab::Access::DEVELOPER
+  end
+
+  describe "POST /groups/:id/ldap_group_links" do
+    context "when unauthenticated" do
+      it "should return authentication error" do
+        post api("/groups/#{group_with_ldap_links.id}/ldap_group_links")
+        response.status.should == 401
+      end
+    end
+
+    context "when a less priviledged user" do
+      it "should not allow less priviledged user to add LDAP group link" do
+        expect {
+          post api("/groups/#{group_with_ldap_links.id}/ldap_group_links", user),
+          cn: 'ldap-group4', group_access: GroupMember::GUEST
+        }.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 "should return ok and add ldap group link" do
+        expect {
+          post api("/groups/#{group_with_ldap_links.id}/ldap_group_links", owner),
+          cn: 'ldap-group3', group_access: GroupMember::GUEST, provider: 'ldap3'
+        }.to change { group_with_ldap_links.ldap_group_links.count }.by(1)
+
+        expect(response.status).to eq(201)
+        expect(json_response['cn']).to eq('ldap-group3')
+        expect(json_response['group_access']).to eq(GroupMember::GUEST)
+        expect(json_response['provider']).to eq('ldap3')
+      end
+
+      it "should return ok and add ldap group link even if no provider specified" do
+        expect {
+          post api("/groups/#{group_with_ldap_links.id}/ldap_group_links", owner),
+          cn: 'ldap-group3', group_access: GroupMember::GUEST
+        }.to change { group_with_ldap_links.ldap_group_links.count }.by(1)
+
+        expect(response.status).to eq(201)
+        expect(json_response['cn']).to eq('ldap-group3')
+        expect(json_response['group_access']).to eq(GroupMember::GUEST)
+        expect(json_response['provider']).to eq('ldapmain')
+      end
+
+      it "should return error if LDAP group link already exists" do
+        post api("//groups/#{group_with_ldap_links.id}/ldap_group_links", owner), provider: 'ldap1', cn: 'ldap-group1', group_access: GroupMember::GUEST
+        expect(response.status).to eq(409)
+      end
+
+      it "should return a 400 error when cn is not given" do
+        post api("//groups/#{group_with_ldap_links.id}/ldap_group_links", owner), group_access: GroupMember::GUEST
+        expect(response.status).to eq(400)
+      end
+
+      it "should return a 400 error when group access is not given" do
+        post api("//groups/#{group_with_ldap_links.id}/ldap_group_links", owner), cn: 'ldap-group3'
+        expect(response.status).to eq(400)
+      end
+
+      it "should return 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
+        expect(response.status).to eq(422)
+      end
+    end
+  end
+
+  describe 'DELETE /groups/:id/ldap_group_links/:cn' do
+    context "when unauthenticated" do
+      it "should return authentication error" do
+        delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap-group1")
+        response.status.should == 401
+      end
+    end
+
+    context "when a less priviledged user" do
+      it "should not remove the LDAP group link" do
+        expect {
+          delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap-group1", user)
+        }.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 "should remove ldap group link" do
+        expect {
+          delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap-group1", owner)
+        }.to change { group_with_ldap_links.ldap_group_links.count }.by(-1)
+
+        expect(response.status).to eq(200)
+      end
+
+      it "should return 404 if LDAP group cn not used for a LDAP group link" do
+        expect {
+          delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap-group1356", owner)
+        }.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 "should return authentication error" do
+        delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap2/ldap-group2")
+        response.status.should == 401
+      end
+    end
+
+    context "when a less priviledged user" do
+      it "should not remove the LDAP group link" do
+        expect {
+          delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap2/ldap-group2", user)
+        }.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 "should return 404 if LDAP group cn not used for a LDAP group link for the specified provider" do
+        expect {
+          delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap1/ldap-group2", owner)
+        }.not_to change { group_with_ldap_links.ldap_group_links.count }
+
+        expect(response.status).to eq(404)
+      end
+      
+      it "should remove ldap group link" do
+        expect {
+          delete api("/groups/#{group_with_ldap_links.id}/ldap_group_links/ldap2/ldap-group2", owner)
+        }.to change { group_with_ldap_links.ldap_group_links.count }.by(-1)
+
+        expect(response.status).to eq(200)
+      end
+    end
+  end
+
+end