Commit 71f9c43c authored by Douwe Maan's avatar Douwe Maan

Merge branch '27148-limit-bulk-create-memberships' into 'master'

Limit non-administrators to adding 100 members at a time to groups and projects

Closes #27148

See merge request !11940
parents d1c2d36f 5c602e30
......@@ -43,12 +43,13 @@ class Admin::GroupsController < Admin::ApplicationController
end
def members_update
status = Members::CreateService.new(@group, current_user, params).execute
member_params = params.permit(:user_ids, :access_level, :expires_at)
result = Members::CreateService.new(@group, current_user, member_params.merge(limit: -1)).execute
if status
if result[:status] == :success
redirect_to [:admin, @group], notice: 'Users were successfully added.'
else
redirect_to [:admin, @group], alert: 'No users specified.'
redirect_to [:admin, @group], alert: result[:message]
end
end
......
......@@ -2,14 +2,15 @@ module MembershipActions
extend ActiveSupport::Concern
def create
status = Members::CreateService.new(membershipable, current_user, params).execute
create_params = params.permit(:user_ids, :access_level, :expires_at)
result = Members::CreateService.new(membershipable, current_user, create_params).execute
redirect_url = members_page_url
if status
if result[:status] == :success
redirect_to redirect_url, notice: 'Users were successfully added.'
else
redirect_to redirect_url, alert: 'No users specified.'
redirect_to redirect_url, alert: result[:message]
end
end
......
module Members
class CreateService < BaseService
DEFAULT_LIMIT = 100
def initialize(source, current_user, params = {})
@source = source
@current_user = current_user
@params = params
@error = nil
end
def execute
return false if params[:user_ids].blank?
return error('No users specified.') if params[:user_ids].blank?
user_ids = params[:user_ids].split(',').uniq
return error("Too many users specified (limit is #{user_limit})") if
user_limit && user_ids.size > user_limit
@source.add_users(
params[:user_ids].split(','),
user_ids,
params[:access_level],
expires_at: params[:expires_at],
current_user: current_user
)
true
success
end
private
def user_limit
limit = params.fetch(:limit, DEFAULT_LIMIT)
limit && limit < 0 ? nil : limit
end
end
end
---
title: Limit non-administrators to adding 100 members at a time to groups and projects
merge_request: 11940
author:
......@@ -36,6 +36,15 @@ describe Admin::GroupsController do
expect(group.users).to include group_user
end
it 'can add unlimited members' do
put :members_update, id: group,
user_ids: 1.upto(1000).to_a.join(','),
access_level: Gitlab::Access::GUEST
expect(response).to set_flash.to 'Users were successfully added.'
expect(response).to redirect_to(admin_group_path(group))
end
it 'adds no user to members' do
put :members_update, id: group,
user_ids: '',
......
......@@ -36,7 +36,7 @@ describe Projects::ProjectMembersController do
before { project.team << [user, :master] }
it 'adds user to members' do
expect_any_instance_of(Members::CreateService).to receive(:execute).and_return(true)
expect_any_instance_of(Members::CreateService).to receive(:execute).and_return(status: :success)
post :create, namespace_id: project.namespace,
project_id: project,
......@@ -48,14 +48,14 @@ describe Projects::ProjectMembersController do
end
it 'adds no user to members' do
expect_any_instance_of(Members::CreateService).to receive(:execute).and_return(false)
expect_any_instance_of(Members::CreateService).to receive(:execute).and_return(status: :failure, message: 'Message')
post :create, namespace_id: project.namespace,
project_id: project,
user_ids: '',
access_level: Gitlab::Access::GUEST
expect(response).to set_flash.to 'No users specified.'
expect(response).to set_flash.to 'Message'
expect(response).to redirect_to(namespace_project_settings_members_path(project.namespace, project))
end
end
......
......@@ -11,7 +11,7 @@ describe Members::CreateService, services: true do
params = { user_ids: project_user.id.to_s, access_level: Gitlab::Access::GUEST }
result = described_class.new(project, user, params).execute
expect(result).to be_truthy
expect(result[:status]).to eq(:success)
expect(project.users).to include project_user
end
......@@ -19,7 +19,19 @@ describe Members::CreateService, services: true do
params = { user_ids: '', access_level: Gitlab::Access::GUEST }
result = described_class.new(project, user, params).execute
expect(result).to be_falsey
expect(result[:status]).to eq(:error)
expect(result[:message]).to be_present
expect(project.users).not_to include project_user
end
it 'limits the number of users to 100' do
user_ids = 1.upto(101).to_a.join(',')
params = { user_ids: user_ids, access_level: Gitlab::Access::GUEST }
result = described_class.new(project, user, params).execute
expect(result[:status]).to eq(:error)
expect(result[:message]).to be_present
expect(project.users).not_to include project_user
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment