Merge branch 'security-check-admin-members-for-groups-with-ldap-links-14-6' into '14-6-stable-ee'

Check permission when creating members through service

See merge request gitlab-org/security/gitlab!2211

app/services/members/create_service.rb

@@ -19,6 +19,8 @@ module Members
     end
     def execute
+      raise Gitlab::Access::AccessDeniedError unless can?(current_user, create_member_permission(source), source)
+
       validate_invite_source!
       validate_invitable!
@@ -144,6 +146,17 @@ module Members
     def formatted_errors
       errors.to_sentence
     end
+
+    def create_member_permission(source)
+      case source
+      when Group
+        :admin_group_member
+      when Project
+        :admin_project_member
+      else
+        raise "Unknown source type: #{source.class}!"
+      end
+    end
   end
 end

ee/spec/requests/api/invitations_spec.rb

@@ -77,4 +77,43 @@ RSpec.describe API::Invitations, 'EE Invitations' do
       end
     end
   end
+
+  context 'group with LDAP group link' do
+    include LdapHelpers
+
+    let(:group) { create(:group_with_ldap_group_link, :public) }
+    let(:owner) { create(:user) }
+    let(:developer) { create(:user) }
+    let(:invite) { create(:group_member, :invited, source: group, user: developer) }
+
+    before do
+      create(:group_member, :owner, group: group, user: owner)
+      stub_ldap_setting(enabled: true)
+      stub_application_setting(lock_memberships_to_ldap: true)
+    end
+
+    describe 'POST /groups/:id/invitations' do
+      it 'returns a forbidden response' do
+        post api("/groups/#{group.id}/invitations", owner), params: { email: developer.email, access_level: Member::DEVELOPER }
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    describe 'PUT /groups/:id/invitations/:email' do
+      it 'returns a forbidden response' do
+        put api("/groups/#{group.id}/invitations/#{invite.invite_email}", owner), params: { access_level: Member::MAINTAINER }
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    describe 'DELETE /groups/:id/invitations/:email' do
+      it 'returns a forbidden response' do
+        delete api("/groups/#{group.id}/invitations/#{invite.invite_email}", owner)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+  end
 end

ee/spec/requests/api/members_spec.rb

@@ -914,6 +914,17 @@ RSpec.describe API::Members do
       end
     end
+    describe 'POST /groups/:id/members' do
+      let(:stranger) { create(:user) }
+
+      it 'returns a forbidden response' do
+        post api("/groups/#{group.id}/members", owner),
+             params: { user_id: stranger.id, access_level: Member::DEVELOPER }
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
     describe 'POST /groups/:id/members/:user_id/override' do
       it 'succeeds when override is set on an LDAP user' do
         post api("/groups/#{group.id}/members/#{ldap_developer.id}/override", owner)

spec/services/members/create_service_spec.rb

@@ -11,19 +11,37 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
   let(:additional_params) { { invite_source: '_invite_source_' } }
   let(:params) { { user_ids: user_ids, access_level: access_level }.merge(additional_params) }
-  subject(:execute_service) { described_class.new(user, params.merge({ source: source })).execute }
+  let(:current_user) { user }
+  subject(:execute_service) { described_class.new(current_user, params.merge({ source: source })).execute }
   before do
-    if source.is_a?(Project)
+    case source
+    when Project
       source.add_maintainer(user)
       OnboardingProgress.onboard(source.namespace)
-    else
+    when Group
       source.add_owner(user)
       OnboardingProgress.onboard(source)
     end
   end
+  context 'when the current user does not have permission to create members' do
+    let(:current_user) { create(:user) }
+
+    it 'raises a Gitlab::Access::AccessDeniedError' do
+      expect { execute_service }.to raise_error(Gitlab::Access::AccessDeniedError)
+    end
+  end
+
+  context 'when passing an invalid source' do
+    let_it_be(:source) { Object.new }
+
+    it 'raises a RuntimeError' do
+      expect { execute_service }.to raise_error(RuntimeError, 'Unknown source type: Object!')
+    end
+  end
+
   context 'when passing valid parameters' do
     it 'adds a user to members' do
       expect(execute_service[:status]).to eq(:success)