From 004e39b47bf508b2626607693e0a8ded54fda5b3 Mon Sep 17 00:00:00 2001 From: Peter Lloyd Date: Mon, 7 Jan 2019 17:54:41 +0000 Subject: [PATCH] Expose LDAP Override in API, add set and clear methods Previously, setting access_level for LDAP users was not allowed. The web interface is able to set an 'override' attribute to stop LDAP sync from changing the access_level. This adds the same ability to the API. Also update API documentation and add tests --- doc/api/members.md | 70 +++++++++++ .../unreleased-ee/pl-override-api.yml | 5 + ee/lib/ee/api/entities.rb | 9 ++ ee/lib/ee/api/members.rb | 66 +++++++++++ ee/spec/requests/api/members_spec.rb | 110 ++++++++++++++++++ lib/api/entities.rb | 1 + lib/api/members.rb | 2 + 7 files changed, 263 insertions(+) create mode 100644 ee/changelogs/unreleased-ee/pl-override-api.yml create mode 100644 ee/lib/ee/api/members.rb diff --git a/doc/api/members.md b/doc/api/members.md index da62dc53659093..eb43c1931e92fa 100644 --- a/doc/api/members.md +++ b/doc/api/members.md @@ -224,6 +224,76 @@ Example response: } ``` +### Override LDAP permissions for a member from a group + +Allows access level to be overriden for a LDAP group member + +>**Note:** This API endpoint is only available on 11.x Starter and above. + +``` +POST /groups/:id/members/:user_id/override +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user | +| `user_id` | integer | yes | The user ID of the member | + +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/:id/members/:user_id/override +``` + +Example response: + +```json +{ + "id": 1, + "username": "raymond_smith", + "name": "Raymond Smith", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon", + "web_url": "http://192.168.1.8:3000/root", + "expires_at": "2012-10-22T14:13:35Z", + "access_level": 40, + "override": true +} +``` + +### Un-override LDAP permissions for a member from a group + +Resets access level for a LDAP group member back to be level determined by the LDAP group + +>**Note:** This API endpoint is only available on 11.x Starter and above. + +``` +DELETE /groups/:id/members/:user_id/override +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user | +| `user_id` | integer | yes | The user ID of the member | + +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/:id/members/:user_id/override +``` + +Example response: + +```json +{ + "id": 1, + "username": "raymond_smith", + "name": "Raymond Smith", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon", + "web_url": "http://192.168.1.8:3000/root", + "expires_at": "2012-10-22T14:13:35Z", + "access_level": 40, + "override": false +} +``` + ## Remove a member from a group or project Removes a user from a group or project. diff --git a/ee/changelogs/unreleased-ee/pl-override-api.yml b/ee/changelogs/unreleased-ee/pl-override-api.yml new file mode 100644 index 00000000000000..78faae6ef08331 --- /dev/null +++ b/ee/changelogs/unreleased-ee/pl-override-api.yml @@ -0,0 +1,5 @@ +--- +title: Add API methods to manipulate LDAP Override attribute +merge_request: +author: Peter Lloyd +type: added diff --git a/ee/lib/ee/api/entities.rb b/ee/lib/ee/api/entities.rb index c3da875234ee1c..31beb919d94ae5 100644 --- a/ee/lib/ee/api/entities.rb +++ b/ee/lib/ee/api/entities.rb @@ -79,6 +79,15 @@ module Identity end end + module Member + extend ActiveSupport::Concern + + prepended do + expose :override, + if: ->(member, options) { member.source_type == 'Namespace' && member.ldap? } + end + end + module ProtectedRefAccess extend ActiveSupport::Concern diff --git a/ee/lib/ee/api/members.rb b/ee/lib/ee/api/members.rb new file mode 100644 index 00000000000000..5f1028343407d1 --- /dev/null +++ b/ee/lib/ee/api/members.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module EE + module API + module Members + extend ActiveSupport::Concern + + prepended do + params do + requires :id, type: String, desc: 'The ID of a group' + end + resource :groups, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + desc 'Overrides a member of a group.' do + success Entities::Member + end + params do + requires :user_id, type: Integer, desc: 'The user ID of the member' + end + # rubocop: disable CodeReuse/ActiveRecord + post ":id/members/:user_id/override" do + source = find_source(:group, params.delete(:id)) + authorize_admin_source!(:group, source) + + member = source.members.find_by!(user_id: params[:user_id]) + updated_member = + ::Members::UpdateService + .new(current_user, { override: true }) + .execute(member, permission: :override) + + if updated_member.valid? + present updated_member, with: ::API::Entities::Member + else + render_validation_error!(updated_member) + end + end + # rubocop: enable CodeReuse/ActiveRecord + + desc 'Remove an override of a member of a group.' do + success Entities::Member + end + params do + requires :user_id, type: Integer, desc: 'The user ID of the member' + end + # rubocop: disable CodeReuse/ActiveRecord + delete ":id/members/:user_id/override" do + source = find_source(:group, params.delete(:id)) + authorize_admin_source!(:group, source) + + member = source.members.find_by!(user_id: params[:user_id]) + updated_member = + ::Members::UpdateService + .new(current_user, { override: false }) + .execute(member, permission: :override) + + if updated_member.valid? + present updated_member, with: ::API::Entities::Member + else + render_validation_error!(updated_member) + end + end + # rubocop: enable CodeReuse/ActiveRecord + end + end + end + end +end diff --git a/ee/spec/requests/api/members_spec.rb b/ee/spec/requests/api/members_spec.rb index 272c41e52259c1..716f8e45888e3c 100644 --- a/ee/spec/requests/api/members_spec.rb +++ b/ee/spec/requests/api/members_spec.rb @@ -26,3 +26,113 @@ end end end + +describe "API::Members with LDAP link" do + let(:owner) { create(:user, username: 'owner_user') } + let(:developer) { create(:user) } + let(:ldap_developer) { create(:user) } + let(:ldap_developer2) { create(:user) } + + let(:group) { create(:group_with_ldap_group_link, :public) } + + let!(:owner_member) { create(:group_member, :owner, group: group, user: owner) } + let!(:ldap_member) { create(:group_member, :developer, group: group, user: ldap_developer, ldap: true) } + let!(:overridden_member) { create(:group_member, :developer, group: group, user: ldap_developer2, ldap: true, override: true) } + let!(:regular_member) { create(:group_member, :developer, group: group, user: developer, ldap: false) } + + before do + # We need to actually activate the LDAP config otherwise `Group#ldap_synced?` will always be false! + allow(Gitlab.config.ldap).to receive_messages(enabled: true) + end + + describe 'GET /groups/:id/members/:user_id has no override attribute' do + context 'project in a group' do + it 'succeeds not getting override on a non-LDAP user' do + get api("/groups/#{group.id}/members/#{developer.id}", owner) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['id']).to eq(developer.id) + expect(json_response['access_level']).to eq(Member::DEVELOPER) + expect(json_response['override']).to eq(nil) + end + end + end + + describe 'GET /groups/:id/members/:user_id has an override attribute' do + context 'project in a group' do + it 'succeeds getting override on an LDAP user' do + get api("/groups/#{group.id}/members/#{ldap_developer.id}", owner) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['id']).to eq(ldap_developer.id) + expect(json_response['override']).to eq(false) + expect(json_response['access_level']).to eq(Member::DEVELOPER) + end + end + end + + describe 'POST /groups/:id/members/:user_id/override with LDAP links' do + context 'project in a group' do + it 'succeeds when override is set on an LDAP user' do + post api("/groups/#{group.id}/members/#{ldap_developer.id}/override", owner) + expect(response).to have_gitlab_http_status(201) + expect(json_response['id']).to eq(ldap_developer.id) + expect(json_response['override']).to eq(true) + expect(json_response['access_level']).to eq(Member::DEVELOPER) + end + + it 'succeeds when access_level is modified after override has been set' do + post api("/groups/#{group.id}/members/#{ldap_developer.id}/override", owner) + expect(response).to have_gitlab_http_status(201) + + put api("/groups/#{group.id}/members/#{ldap_developer.id}", owner), + params: { access_level: Member::MAINTAINER } + + expect(response).to have_gitlab_http_status(200) + expect(json_response['id']).to eq(ldap_developer.id) + expect(json_response['override']).to eq(true) + expect(json_response['access_level']).to eq(Member::MAINTAINER) + end + end + end + + describe 'DELETE /groups/:id/members/:user_id/override with LDAP links' do + context 'project in a group' do + it 'succeeds when override is set on an LDAP user' do + delete api("/groups/#{group.id}/members/#{ldap_developer2.id}/override", owner) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['id']).to eq(ldap_developer2.id) + expect(json_response['override']).to eq(false) + end + end + end + + shared_examples 'POST /:source_type/:id/members/:user_id/override' do |source_type| + context "with :source_type == #{source_type.pluralize}" do + it 'returns 403 when override is set for a non-ldap user' do + post api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}/override", owner) + + expect(response).to have_gitlab_http_status(403) + end + end + end + + shared_examples 'DELETE /:source_type/:id/members/:user_id/override' do |source_type| + context "with :source_type == #{source_type.pluralize}" do + it 'returns 403 when override is set for a non-ldap user' do + delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}/override", owner) + + expect(response).to have_gitlab_http_status(403) + end + end + end + + it_behaves_like 'POST /:source_type/:id/members/:user_id/override', 'group' do + let(:source) { group } + end + + it_behaves_like 'DELETE /:source_type/:id/members/:user_id/override', 'group' do + let(:source) { group } + end +end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 89951498489291..d7f727cd4f9a83 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1793,6 +1793,7 @@ class Response < Grape::Entity ::API::Entities::IssueBasic.prepend_if_ee('EE::API::Entities::IssueBasic', with_descendants: true) ::API::Entities::Issue.prepend_if_ee('EE::API::Entities::Issue') ::API::Entities::List.prepend_if_ee('EE::API::Entities::List') +::API::Entities::Member.prepend_if_ee('EE::API::Entities::Member') ::API::Entities::MergeRequestBasic.prepend_if_ee('EE::API::Entities::MergeRequestBasic', with_descendants: true) ::API::Entities::Namespace.prepend_if_ee('EE::API::Entities::Namespace') ::API::Entities::Project.prepend_if_ee('EE::API::Entities::Project', with_descendants: true) diff --git a/lib/api/members.rb b/lib/api/members.rb index 461ffe71a621b9..44e32c3ef971d5 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -144,3 +144,5 @@ class Members < Grape::API end end end + +API::Members.prepend_if_ee('EE::API::Members') -- GitLab