From 61e1ea700a8250ccb9e8617d9c5025d7a27abe5a Mon Sep 17 00:00:00 2001 From: Alex Lossent Date: Tue, 26 May 2015 19:04:13 +0200 Subject: [PATCH] Add API support for adding and removing LDAP group links --- CHANGELOG-EE | 3 + doc/api/groups.md | 40 +++++ lib/api/api.rb | 1 + lib/api/entities.rb | 10 +- lib/api/ldap_group_links.rb | 78 ++++++++++ spec/requests/api/groups_spec.rb | 3 +- spec/requests/api/ldap_group_links_spec.rb | 162 +++++++++++++++++++++ 7 files changed, 291 insertions(+), 6 deletions(-) create mode 100644 lib/api/ldap_group_links.rb create mode 100644 spec/requests/api/ldap_group_links_spec.rb diff --git a/CHANGELOG-EE b/CHANGELOG-EE index 37f99c41a6..d948aa1721 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -1,3 +1,6 @@ +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 diff --git a/doc/api/groups.md b/doc/api/groups.md index c903a850fd..5ec0c00db8 100644 --- a/doc/api/groups.md +++ b/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 diff --git a/lib/api/api.rb b/lib/api/api.rb index ac5c969661..1f1d586cb6 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -45,6 +45,7 @@ module API mount ProjectHooks mount ProjectGitHook mount Ldap + mount LdapGroupLinks mount Services mount Files mount Commits diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 860ec01cfd..dd32e67139 100644 --- a/lib/api/entities.rb +++ b/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 diff --git a/lib/api/ldap_group_links.rb b/lib/api/ldap_group_links.rb new file mode 100644 index 0000000000..248d92846b --- /dev/null +++ b/lib/api/ldap_group_links.rb @@ -0,0 +1,78 @@ +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 diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 53b729fb80..e81b357b33 100644 --- a/spec/requests/api/groups_spec.rb +++ b/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 diff --git a/spec/requests/api/ldap_group_links_spec.rb b/spec/requests/api/ldap_group_links_spec.rb new file mode 100644 index 0000000000..4cdab52f3f --- /dev/null +++ b/spec/requests/api/ldap_group_links_spec.rb @@ -0,0 +1,162 @@ +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