Commit 81098643 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'rc/reduce-delta-with-ce-in-controllers-ce' into 'master'

Remove explicit audit event log in MembershipActions

See merge request gitlab-org/gitlab-ce!14824
parents 732a301d 1c88d92b
......@@ -48,7 +48,7 @@ class Admin::GroupsController < Admin::ApplicationController
def members_update
member_params = params.permit(:user_ids, :access_level, :expires_at)
result = Members::CreateService.new(@group, current_user, member_params.merge(limit: -1)).execute
result = Members::CreateService.new(current_user, member_params.merge(limit: -1)).execute(@group)
if result[:status] == :success
redirect_to [:admin, @group], notice: 'Users were successfully added.'
......
......@@ -3,20 +3,31 @@ module MembershipActions
def create
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
result = Members::CreateService.new(current_user, create_params).execute(membershipable)
if result[:status] == :success
redirect_to redirect_url, notice: 'Users were successfully added.'
redirect_to members_page_url, notice: 'Users were successfully added.'
else
redirect_to redirect_url, alert: result[:message]
redirect_to members_page_url, alert: result[:message]
end
end
def update
update_params = params.require(root_params_key).permit(:access_level, :expires_at)
member = membershipable.members_and_requesters.find(params[:id])
member = Members::UpdateService
.new(current_user, update_params)
.execute(member)
.present(current_user: current_user)
respond_to do |format|
format.js { render 'shared/members/update', locals: { member: member } }
end
end
def destroy
Members::DestroyService.new(membershipable, current_user, params)
.execute(:all)
member = membershipable.members_and_requesters.find(params[:id])
Members::DestroyService.new(current_user).execute(member)
respond_to do |format|
format.html do
......@@ -36,14 +47,17 @@ module MembershipActions
end
def approve_access_request
Members::ApproveAccessRequestService.new(membershipable, current_user, params).execute
access_requester = membershipable.requesters.find(params[:id])
Members::ApproveAccessRequestService
.new(current_user, params)
.execute(access_requester)
redirect_to members_page_url
end
def leave
member = Members::DestroyService.new(membershipable, current_user, user_id: current_user.id)
.execute(:all)
member = membershipable.members_and_requesters.find_by!(user_id: current_user.id)
Members::DestroyService.new(current_user).execute(member)
notice =
if member.request?
......@@ -62,17 +76,43 @@ module MembershipActions
end
end
def resend_invite
member = membershipable.members.find(params[:id])
if member.invite?
member.resend_invite
redirect_to members_page_url, notice: 'The invitation was successfully resent.'
else
redirect_to members_page_url, alert: 'The invitation has already been accepted.'
end
end
protected
def membershipable
raise NotImplementedError
end
def root_params_key
case membershipable
when Namespace
:group_member
when Project
:project_member
else
raise "Unknown membershipable type: #{membershipable}!"
end
end
def members_page_url
if membershipable.is_a?(Project)
case membershipable
when Namespace
polymorphic_url([membershipable, :members])
when Project
project_project_members_path(membershipable)
else
polymorphic_url([membershipable, :members])
raise "Unknown membershipable type: #{membershipable}!"
end
end
......
......@@ -27,35 +27,6 @@ class Groups::GroupMembersController < Groups::ApplicationController
@group_member = @group.group_members.new
end
def update
@group_member = @group.members_and_requesters.find(params[:id])
.present(current_user: current_user)
return render_403 unless can?(current_user, :update_group_member, @group_member)
@group_member.update_attributes(member_params)
end
def resend_invite
redirect_path = group_group_members_path(@group)
@group_member = @group.group_members.find(params[:id])
if @group_member.invite?
@group_member.resend_invite
redirect_to redirect_path, notice: 'The invitation was successfully resent.'
else
redirect_to redirect_path, alert: 'The invitation has already been accepted.'
end
end
protected
def member_params
params.require(:group_member).permit(:access_level, :user_id, :expires_at)
end
# MembershipActions concern
alias_method :membershipable, :group
end
......@@ -26,29 +26,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController
@project_member = @project.project_members.new
end
def update
@project_member = @project.members_and_requesters.find(params[:id])
.present(current_user: current_user)
return render_403 unless can?(current_user, :update_project_member, @project_member)
@project_member.update_attributes(member_params)
end
def resend_invite
redirect_path = project_project_members_path(@project)
@project_member = @project.project_members.find(params[:id])
if @project_member.invite?
@project_member.resend_invite
redirect_to redirect_path, notice: 'The invitation was successfully resent.'
else
redirect_to redirect_path, alert: 'The invitation has already been accepted.'
end
end
def import
@projects = current_user.authorized_projects.order_id_desc
end
......@@ -67,12 +44,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController
notice: notice)
end
protected
def member_params
params.require(:project_member).permit(:user_id, :access_level, :expires_at)
end
# MembershipActions concern
alias_method :membershipable, :project
end
......@@ -8,6 +8,6 @@ module AccessRequestable
extend ActiveSupport::Concern
def request_access(user)
Members::RequestAccessService.new(self, user).execute
Members::RequestAccessService.new(user).execute(self)
end
end
......@@ -128,7 +128,7 @@ class Member < ActiveRecord::Base
find_by(invite_token: invite_token)
end
def add_user(source, user, access_level, existing_members: nil, current_user: nil, expires_at: nil)
def add_user(source, user, access_level, existing_members: nil, current_user: nil, expires_at: nil, ldap: false)
# `user` can be either a User object, User ID or an email to be invited
member = retrieve_member(source, user, existing_members)
access_level = retrieve_access_level(access_level)
......@@ -143,11 +143,13 @@ class Member < ActiveRecord::Base
if member.request?
::Members::ApproveAccessRequestService.new(
source,
current_user,
id: member.id,
access_level: access_level
).execute
).execute(
member,
skip_authorization: ldap,
skip_log_audit_event: ldap
)
else
member.save
end
......
module Members
class ApproveAccessRequestService < BaseService
include MembersHelper
attr_accessor :source
# source - The source object that respond to `#requesters` (i.g. project or group)
# current_user - The user that performs the access request approval
# params - A hash of parameters
# :user_id - User ID used to retrieve the access requester
# :id - Member ID used to retrieve the access requester
# :access_level - Optional access level set when the request is accepted
def initialize(source, current_user, params = {})
@source = source
@current_user = current_user
@params = params.slice(:user_id, :id, :access_level)
end
# opts - A hash of options
# :force - Bypass permission check: current_user can be nil in that case
def execute(opts = {})
condition = params[:user_id] ? { user_id: params[:user_id] } : { id: params[:id] }
access_requester = source.requesters.find_by!(condition)
raise Gitlab::Access::AccessDeniedError unless can_update_access_requester?(access_requester, opts)
class ApproveAccessRequestService < Members::BaseService
def execute(access_requester, skip_authorization: false, skip_log_audit_event: false)
raise Gitlab::Access::AccessDeniedError unless skip_authorization || can_update_access_requester?(access_requester)
access_requester.access_level = params[:access_level] if params[:access_level]
access_requester.accept_request
after_execute(member: access_requester, skip_log_audit_event: skip_log_audit_event)
access_requester
end
private
def can_update_access_requester?(access_requester, opts = {})
access_requester && (
opts[:force] ||
can?(current_user, update_member_permission(access_requester), access_requester)
)
end
def update_member_permission(member)
case member
when GroupMember
:update_group_member
when ProjectMember
:update_project_member
end
def can_update_access_requester?(access_requester)
can?(current_user, update_member_permission(access_requester), access_requester)
end
end
end
module Members
class AuthorizedDestroyService < BaseService
attr_accessor :member, :user
def initialize(member, user = nil)
@member, @user = member, user
end
def execute
return false if member.is_a?(GroupMember) && member.source.last_owner?(member.user)
Member.transaction do
unassign_issues_and_merge_requests(member) unless member.invite?
member.notification_setting&.destroy
member.destroy
end
if member.request? && member.user != user
notification_service.decline_access_request(member)
end
member
end
private
def unassign_issues_and_merge_requests(member)
if member.is_a?(GroupMember)
issues = Issue.unscoped.select(1)
.joins(:project)
.where('issues.id = issue_assignees.issue_id AND projects.namespace_id = ?', member.source_id)
# DELETE FROM issue_assignees WHERE user_id = X AND EXISTS (...)
IssueAssignee.unscoped
.where('user_id = :user_id AND EXISTS (:sub)', user_id: member.user_id, sub: issues)
.delete_all
MergeRequestsFinder.new(user, group_id: member.source_id, assignee_id: member.user_id)
.execute
.update_all(assignee_id: nil)
else
project = member.source
# SELECT 1 FROM issues WHERE issues.id = issue_assignees.issue_id AND issues.project_id = X
issues = Issue.unscoped.select(1)
.where('issues.id = issue_assignees.issue_id')
.where(project_id: project.id)
# DELETE FROM issue_assignees WHERE user_id = X AND EXISTS (...)
IssueAssignee.unscoped
.where('user_id = :user_id AND EXISTS (:sub)', user_id: member.user_id, sub: issues)
.delete_all
project.merge_requests.opened.assigned_to(member.user).update_all(assignee_id: nil)
end
member.user.invalidate_cache_counts
end
end
end
module Members
class BaseService < ::BaseService
# current_user - The user that performs the action
# params - A hash of parameters
def initialize(current_user = nil, params = {})
@current_user = current_user
@params = params
end
def after_execute(args)
# overriden in EE::Members modules
end
private
def update_member_permission(member)
case member
when GroupMember
:update_group_member
when ProjectMember
:update_project_member
else
raise "Unknown member type: #{member}!"
end
end
def override_member_permission(member)
case member
when GroupMember
:override_group_member
when ProjectMember
:override_project_member
else
raise "Unknown member type: #{member}!"
end
end
def action_member_permission(action, member)
case action
when :update
update_member_permission(member)
when :override
override_member_permission(member)
else
raise "Unknown action '#{action}' on #{member}!"
end
end
end
end
module Members
class CreateService < BaseService
class CreateService < Members::BaseService
DEFAULT_LIMIT = 100
def initialize(source, current_user, params = {})
@source = source
@current_user = current_user
@params = params
@error = nil
end
def execute
def execute(source)
return error('No users specified.') if params[:user_ids].blank?
user_ids = params[:user_ids].split(',').uniq
......@@ -17,13 +10,15 @@ module Members
return error("Too many users specified (limit is #{user_limit})") if
user_limit && user_ids.size > user_limit
@source.add_users(
members = source.add_users(
user_ids,
params[:access_level],
expires_at: params[:expires_at],
current_user: current_user
)
members.each { |member| after_execute(member: member) }
success
end
......
module Members
class DestroyService < BaseService
include MembersHelper
class DestroyService < Members::BaseService
def execute(member, skip_authorization: false)
raise Gitlab::Access::AccessDeniedError unless skip_authorization || can_destroy_member?(member)
attr_accessor :source
return member if member.is_a?(GroupMember) && member.source.last_owner?(member.user)
ALLOWED_SCOPES = %i[members requesters all].freeze
Member.transaction do
unassign_issues_and_merge_requests(member) unless member.invite?
member.notification_setting&.destroy
def initialize(source, current_user, params = {})
@source = source
@current_user = current_user
@params = params
end
def execute(scope = :members)
raise "scope :#{scope} is not allowed!" unless ALLOWED_SCOPES.include?(scope)
member.destroy
end
member = find_member!(scope)
if member.request? && member.user != current_user
notification_service.decline_access_request(member)
end
raise Gitlab::Access::AccessDeniedError unless can_destroy_member?(member)
after_execute(member: member)
AuthorizedDestroyService.new(member, current_user).execute
member
end
private
def find_member!(scope)
condition = params[:user_id] ? { user_id: params[:user_id] } : { id: params[:id] }
case scope
when :all
source.members.find_by(condition) ||
source.requesters.find_by!(condition)
else
source.public_send(scope).find_by!(condition) # rubocop:disable GitlabSecurity/PublicSend
end
end
def can_destroy_member?(member)
member && can?(current_user, destroy_member_permission(member), member)
can?(current_user, destroy_member_permission(member), member)
end
def destroy_member_permission(member)
......@@ -45,7 +33,42 @@ module Members
:destroy_group_member
when ProjectMember
:destroy_project_member
else
raise "Unknown member type: #{member}!"
end
end
def unassign_issues_and_merge_requests(member)
if member.is_a?(GroupMember)
issues = Issue.unscoped.select(1)
.joins(:project)
.where('issues.id = issue_assignees.issue_id AND projects.namespace_id = ?', member.source_id)
# DELETE FROM issue_assignees WHERE user_id = X AND EXISTS (...)
IssueAssignee.unscoped
.where('user_id = :user_id AND EXISTS (:sub)', user_id: member.user_id, sub: issues)
.delete_all
MergeRequestsFinder.new(current_user, group_id: member.source_id, assignee_id: member.user_id)
.execute
.update_all(assignee_id: nil)
else
project = member.source
# SELECT 1 FROM issues WHERE issues.id = issue_assignees.issue_id AND issues.project_id = X
issues = Issue.unscoped.select(1)
.where('issues.id = issue_assignees.issue_id')
.where(project_id: project.id)
# DELETE FROM issue_assignees WHERE user_id = X AND EXISTS (...)
IssueAssignee.unscoped
.where('user_id = :user_id AND EXISTS (:sub)', user_id: member.user_id, sub: issues)
.delete_all
project.merge_requests.opened.assigned_to(member.user).update_all(assignee_id: nil)
end
member.user.invalidate_cache_counts
end
end
end
module Members
class RequestAccessService < BaseService
attr_accessor :source
def initialize(source, current_user)
@source = source
@current_user = current_user
end
def execute
class RequestAccessService < Members::BaseService
def execute(source)
raise Gitlab::Access::AccessDeniedError unless can_request_access?(source)
source.members.create(
......@@ -19,7 +12,7 @@ module Members
private
def can_request_access?(source)
source && can?(current_user, :request_access, source)
can?(current_user, :request_access, source)
end
end
end
module Members
class UpdateService < Members::BaseService
# returns the updated member
def execute(member, permission: :update)
raise Gitlab::Access::AccessDeniedError unless can?(current_user, action_member_permission(permission, member), member)
old_access_level = member.human_access
if member.update_attributes(params)
after_execute(action: permission, old_access_level: old_access_level, member: member)
end
member
end
end
end
:plain
var $listItem = $('#{escape_javascript(render('shared/members/member', member: @group_member))}');
$("##{dom_id(@group_member)} .list-item-name").replaceWith($listItem.find('.list-item-name'));
gl.utils.localTimeAgo($('.js-timeago'), $("##{dom_id(@group_member)}"));
:plain
var $listItem = $('#{escape_javascript(render('shared/members/member', member: @project_member))}');
$("##{dom_id(@project_member)} .list-item-name").replaceWith($listItem.find('.list-item-name'));
gl.utils.localTimeAgo($('.js-timeago'), $("##{dom_id(@project_member)}"));
- member = local_assigns.fetch(:member)
:plain
var $listItem = $('#{escape_javascript(render('shared/members/member', member: member))}');
$("##{dom_id(member)} .list-item-name").replaceWith($listItem.find('.list-item-name'));
gl.utils.localTimeAgo($('.js-timeago'), $("##{dom_id(member)}"));
......@@ -5,7 +5,7 @@ class RemoveExpiredMembersWorker
def perform
Member.expired.find_each do |member|
begin
Members::AuthorizedDestroyService.new(member).execute
Members::DestroyService.new.execute(member, skip_authorization: true)
rescue => ex
logger.error("Expired Member ID=#{member.id} cannot be removed - #{ex}")
end
......
......@@ -53,7 +53,10 @@ module API
put ':id/access_requests/:user_id/approve' do
source = find_source(source_type, params[:id])
member = ::Members::ApproveAccessRequestService.new(source, current_user, declared_params).execute
access_requester = source.requesters.find_by!(user_id: params[:user_id])
member = ::Members::ApproveAccessRequestService
.new(current_user, declared_params)
.execute(access_requester)
status :created
present member, with: Entities::Member
......@@ -70,8 +73,7 @@ module API
member = source.requesters.find_by!(user_id: params[:user_id])
destroy_conditionally!(member) do
::Members::DestroyService.new(source, current_user, params)
.execute(:requesters)
::Members::DestroyService.new(current_user).execute(member)
end
end
end
......
......@@ -81,12 +81,16 @@ module API
source = find_source(source_type, params.delete(:id))
authorize_admin_source!(source_type, source)
member = source.members.find_by!(user_id: params.delete(:user_id))
member = source.members.find_by!(user_id: params[:user_id])
updated_member =
::Members::UpdateService
.new(current_user, declared_params(include_missing: false))
.execute(member)
if member.update_attributes(declared_params(include_missing: false))
present member, with: Entities::Member
if updated_member.valid?
present updated_member, with: Entities::Member
else
render_validation_error!(member)
render_validation_error!(updated_member)
end
end
......@@ -99,7 +103,7 @@ module API
member = source.members.find_by!(user_id: params[:user_id])
destroy_conditionally!(member) do
::Members::DestroyService.new(source, current_user, declared_params).execute
::Members::DestroyService.new(current_user).execute(member)
end
end
end
......
......@@ -124,7 +124,7 @@ module API
status(200 )
{ message: "Access revoked", id: params[:user_id].to_i }
else
::Members::DestroyService.new(source, current_user, declared_params).execute
::Members::DestroyService.new(current_user).execute(member)
present member, with: ::API::Entities::Member
end
......
require 'spec_helper'
describe Members::ApproveAccessRequestService do
let(:user) { create(:user) }
let(:access_requester) { create(:user) }
let(:project) { create(:project, :public, :access_requestable) }
let(:group) { create(:group, :public, :access_requestable) }
let(:current_user) { create(:user) }
let(:access_requester_user) { create(:user) }
let(:access_requester) { source.requesters.find_by!(user_id: access_requester_user.id) }
let(:opts) { {} }
shared_examples 'a service raising ActiveRecord::RecordNotFound' do
it 'raises ActiveRecord::RecordNotFound' do
expect { described_class.new(source, user, params).execute(opts) }.to raise_error(ActiveRecord::RecordNotFound)
expect { described_class.new(current_user).execute(access_requester, opts) }.to raise_error(ActiveRecord::RecordNotFound)
end
end
shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do
it 'raises Gitlab::Access::AccessDeniedError' do
expect { described_class.new(source, user, params).execute(opts) }.to raise_error(Gitlab::Access::AccessDeniedError)
expect { described_class.new(current_user).execute(access_requester, opts) }.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
shared_examples 'a service approving an access request' do
it 'succeeds' do
expect { described_class.new(source, user, params).execute(opts) }.to change { source.requesters.count }.by(-1)
expect { described_class.new(current_user).execute(access_requester, opts) }.to change { source.requesters.count }.by(-1)
end
it 'returns a <Source>Member' do
member = described_class.new(source, user, params).execute(opts)
member = described_class.new(current_user).execute(access_requester, opts)
expect(member).to be_a "#{source.class}Member".constantize
expect(member.requested_at).to be_nil
end
context 'with a custom access level' do
let(:params2) { params.merge(user_id: access_requester.id, access_level: Gitlab::Access::MASTER) }
it 'returns a ProjectMember with the custom access level' do
member = described_class.new(source, user, params2).execute(opts)
member = described_class.new(current_user, access_level: Gitlab::Access::MASTER).execute(access_requester, opts)
expect(member.access_level).to eq Gitlab::Access::MASTER
expect(member.access_level).to eq(Gitlab::Access::MASTER)
end
end
end
context 'when no access requester are found' do
let(:params) { { user_id: 42 } }
it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do
let(:source) { project }
end
it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do
let(:source) { group }
end
end
context 'when an access requester is found' do
before do
project.request_access(access_requester)
group.request_access(access_requester)
project.request_access(access_requester_user)
group.request_access(access_requester_user)
end
let(:params) { { user_id: access_requester.id } }
context 'when current user is nil' do
let(:user) { nil }
context 'and :force option is not given' do
context 'and :ldap option is not given' do
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:source) { project }
end
......@@ -74,8 +60,8 @@ describe Members::ApproveAccessRequestService do
end
end
context 'and :force option is false' do
let(:opts) { { force: false } }
context 'and :skip_authorization option is false' do
let(:opts) { { skip_authorization: false } }
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:source) { project }
......@@ -86,8 +72,8 @@ describe Members::ApproveAccessRequestService do
end
end
context 'and :force option is true' do
let(:opts) { { force: true } }
context 'and :skip_authorization option is true' do
let(:opts) { { skip_authorization: true } }
it_behaves_like 'a service approving an access request' do
let(:source) { project }
......@@ -97,18 +83,6 @@ describe Members::ApproveAccessRequestService do
let(:source) { group }
end
end
context 'and :force param is true' do
let(:params) { { user_id: access_requester.id, force: true } }
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:source) { project }
end
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:source) { group }
end
end
end
context 'when current user cannot approve access request to the project' do
......@@ -123,8 +97,8 @@ describe Members::ApproveAccessRequestService do
context 'when current user can approve access request to the project' do
before do
project.add_master(user)
group.add_owner(user)
project.add_master(current_user)
group.add_owner(current_user)
end
it_behaves_like 'a service approving an access request' do
......@@ -134,14 +108,6 @@ describe Members::ApproveAccessRequestService do
it_behaves_like 'a service approving an access request' do
let(:source) { group }
end
context 'when given a :id' do
let(:params) { { id: project.requesters.find_by!(user_id: access_requester.id).id } }
it_behaves_like 'a service approving an access request' do
let(:source) { project }
end
end
end
end
end
require 'spec_helper'
describe Members::AuthorizedDestroyService do
let(:member_user) { create(:user) }
let(:project) { create(:project, :public) }
let(:group) { create(:group, :public) }
let(:group_project) { create(:project, :public, group: group) }
def number_of_assigned_issuables(user)
Issue.assigned_to(user).count + MergeRequest.assigned_to(user).count
end
context 'Invited users' do
# Regression spec for issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/32504
it 'destroys invited project member' do
project.add_developer(member_user)
member = create :project_member, :invited, project: project
expect { described_class.new(member, member_user).execute }
.to change { Member.count }.from(3).to(2)
end
it "doesn't destroy invited project member notification_settings" do
project.add_developer(member_user)
member = create :project_member, :invited, project: project
expect { described_class.new(member, member_user).execute }
.not_to change { NotificationSetting.count }
end
it 'destroys invited group member' do
group.add_developer(member_user)
member = create :group_member, :invited, group: group
expect { described_class.new(member, member_user).execute }
.to change { Member.count }.from(2).to(1)
end
it "doesn't destroy invited group member notification_settings" do
group.add_developer(member_user)
member = create :group_member, :invited, group: group
expect { described_class.new(member, member_user).execute }
.not_to change { NotificationSetting.count }
end
end
context 'Requested user' do
it "doesn't destroy member notification_settings" do
member = create(:project_member, user: member_user, requested_at: Time.now)
expect { described_class.new(member, member_user).execute }
.not_to change { NotificationSetting.count }
end
end
context 'Group member' do
let(:member) { group.members.find_by(user_id: member_user.id) }
before do
group.add_developer(member_user)
end
it "unassigns issues and merge requests" do
issue = create :issue, project: group_project, assignees: [member_user]
create :issue, assignees: [member_user]
merge_request = create :merge_request, target_project: group_project, source_project: group_project, assignee: member_user
create :merge_request, target_project: project, source_project: project, assignee: member_user
expect { described_class.new(member, member_user).execute }
.to change { number_of_assigned_issuables(member_user) }.from(4).to(2)
expect(issue.reload.assignee_ids).to be_empty
expect(merge_request.reload.assignee_id).to be_nil
end
it 'destroys member notification_settings' do
group.add_developer(member_user)
member = group.members.find_by(user_id: member_user.id)
expect { described_class.new(member, member_user).execute }
.to change { member_user.notification_settings.count }.by(-1)
end
end
context 'Project member' do
let(:member) { project.members.find_by(user_id: member_user.id) }
before do
project.add_developer(member_user)
end
it "unassigns issues and merge requests" do
create :issue, project: project, assignees: [member_user]
create :merge_request, target_project: project, source_project: project, assignee: member_user
expect { described_class.new(member, member_user).execute }
.to change { number_of_assigned_issuables(member_user) }.from(2).to(0)
end
it 'destroys member notification_settings' do
expect { described_class.new(member, member_user).execute }
.to change { member_user.notification_settings.count }.by(-1)
end
end
end
......@@ -11,7 +11,7 @@ describe Members::CreateService do
it 'adds user to members' do
params = { user_ids: project_user.id.to_s, access_level: Gitlab::Access::GUEST }
result = described_class.new(project, user, params).execute
result = described_class.new(user, params).execute(project)
expect(result[:status]).to eq(:success)
expect(project.users).to include project_user
......@@ -19,7 +19,7 @@ describe Members::CreateService do
it 'adds no user to members' do
params = { user_ids: '', access_level: Gitlab::Access::GUEST }
result = described_class.new(project, user, params).execute
result = described_class.new(user, params).execute(project)
expect(result[:status]).to eq(:error)
expect(result[:message]).to be_present
......@@ -30,7 +30,7 @@ describe Members::CreateService 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
result = described_class.new(user, params).execute(project)
expect(result[:status]).to eq(:error)
expect(result[:message]).to be_present
......
require 'spec_helper'
describe Members::DestroyService do
let(:user) { create(:user) }
let(:current_user) { create(:user) }
let(:member_user) { create(:user) }
let(:project) { create(:project, :public) }
let(:group) { create(:group, :public) }
let(:group_project) { create(:project, :public, group: group) }
let(:opts) { {} }
shared_examples 'a service raising ActiveRecord::RecordNotFound' do
it 'raises ActiveRecord::RecordNotFound' do
expect { described_class.new(source, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound)
expect { described_class.new(current_user).execute(member) }.to raise_error(ActiveRecord::RecordNotFound)
end
end
shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do
it 'raises Gitlab::Access::AccessDeniedError' do
expect { described_class.new(source, user, params).execute }.to raise_error(Gitlab::Access::AccessDeniedError)
expect { described_class.new(current_user).execute(member) }.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
def number_of_assigned_issuables(user)
Issue.assigned_to(user).count + MergeRequest.assigned_to(user).count
end
shared_examples 'a service destroying a member' do
it 'destroys the member' do
expect { described_class.new(source, user, params).execute }.to change { source.members.count }.by(-1)
expect { described_class.new(current_user).execute(member, opts) }.to change { member.source.members_and_requesters.count }.by(-1)
end
context 'when the given member is an access requester' do
before do
source.members.find_by(user_id: member_user).destroy
source.update_attributes(request_access_enabled: true)
source.request_access(member_user)
it 'unassigns issues and merge requests' do
if member.invite?
expect { described_class.new(current_user).execute(member, opts) }
.not_to change { number_of_assigned_issuables(member_user) }
else
create :issue, assignees: [member_user]
issue = create :issue, project: group_project, assignees: [member_user]
merge_request = create :merge_request, target_project: group_project, source_project: group_project, assignee: member_user
expect { described_class.new(current_user).execute(member, opts) }
.to change { number_of_assigned_issuables(member_user) }.from(3).to(1)
expect(issue.reload.assignee_ids).to be_empty
expect(merge_request.reload.assignee_id).to be_nil
end
let(:access_requester) { source.requesters.find_by(user_id: member_user) }
end
it_behaves_like 'a service raising ActiveRecord::RecordNotFound'
it 'destroys member notification_settings' do
if member_user.notification_settings.any?
expect { described_class.new(current_user).execute(member, opts) }
.to change { member_user.notification_settings.count }.by(-1)
else
expect { described_class.new(current_user).execute(member, opts) }
.not_to change { member_user.notification_settings.count }
end
end
end
%i[requesters all].each do |scope|
context "and #{scope} scope is passed" do
it 'destroys the access requester' do
expect { described_class.new(source, user, params).execute(scope) }.to change { source.requesters.count }.by(-1)
end
shared_examples 'a service destroying an access requester' do
it_behaves_like 'a service destroying a member'
it 'calls Member#after_decline_request' do
expect_any_instance_of(NotificationService).to receive(:decline_access_request).with(access_requester)
it 'calls Member#after_decline_request' do
expect_any_instance_of(NotificationService).to receive(:decline_access_request).with(member)
described_class.new(source, user, params).execute(scope)
end
described_class.new(current_user).execute(member)
end
context 'when current user is the member' do
it 'does not call Member#after_decline_request' do
expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(access_requester)
context 'when current user is the member' do
it 'does not call Member#after_decline_request' do
expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member)
described_class.new(source, member_user, params).execute(scope)
end
end
end
described_class.new(member_user).execute(member)
end
end
end
context 'when no member are found' do
let(:params) { { user_id: 42 } }
context 'with a member' do
before do
group_project.add_developer(member_user)
group.add_developer(member_user)
end
context 'when current user cannot destroy the given member' do
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:member) { group_project.members.find_by(user_id: member_user.id) }
end
it_behaves_like 'a service destroying a member' do
let(:opts) { { skip_authorization: true } }
let(:member) { group_project.members.find_by(user_id: member_user.id) }
end
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:member) { group.members.find_by(user_id: member_user.id) }
end
it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do
let(:source) { project }
it_behaves_like 'a service destroying a member' do
let(:opts) { { skip_authorization: true } }
let(:member) { group.members.find_by(user_id: member_user.id) }
end
end
it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do
let(:source) { group }
context 'when current user can destroy the given member' do
before do
group_project.add_master(current_user)
group.add_owner(current_user)
end
it_behaves_like 'a service destroying a member' do
let(:member) { group_project.members.find_by(user_id: member_user.id) }
end
it_behaves_like 'a service destroying a member' do
let(:member) { group.members.find_by(user_id: member_user.id) }
end
end
end
context 'when a member is found' do
context 'with an access requester' do
before do
project.add_developer(member_user)
group.add_developer(member_user)
group_project.update_attributes(request_access_enabled: true)
group.update_attributes(request_access_enabled: true)
group_project.request_access(member_user)
group.request_access(member_user)
end
let(:params) { { user_id: member_user.id } }
context 'when current user cannot destroy the given member' do
context 'when current user cannot destroy the given access requester' do
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:source) { project }
let(:member) { group_project.requesters.find_by(user_id: member_user.id) }
end
it_behaves_like 'a service destroying a member' do
let(:opts) { { skip_authorization: true } }
let(:member) { group_project.requesters.find_by(user_id: member_user.id) }
end
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:source) { group }
let(:member) { group.requesters.find_by(user_id: member_user.id) }
end
it_behaves_like 'a service destroying a member' do
let(:opts) { { skip_authorization: true } }
let(:member) { group.requesters.find_by(user_id: member_user.id) }
end
end
context 'when current user can destroy the given member' do
context 'when current user can destroy the given access requester' do
before do
project.add_master(user)
group.add_owner(user)
group_project.add_master(current_user)
group.add_owner(current_user)
end
it_behaves_like 'a service destroying an access requester' do
let(:member) { group_project.requesters.find_by(user_id: member_user.id) }
end
it_behaves_like 'a service destroying an access requester' do
let(:member) { group.requesters.find_by(user_id: member_user.id) }
end
end
end
context 'with an invited user' do
let(:project_invited_member) { create(:project_member, :invited, project: group_project) }
let(:group_invited_member) { create(:group_member, :invited, group: group) }
context 'when current user cannot destroy the given invited user' do
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:member) { project_invited_member }
end
it_behaves_like 'a service destroying a member' do
let(:source) { project }
let(:opts) { { skip_authorization: true } }
let(:member) { project_invited_member }
end
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:member) { group_invited_member }
end
it_behaves_like 'a service destroying a member' do
let(:source) { group }
let(:opts) { { skip_authorization: true } }
let(:member) { group_invited_member }
end
end
context 'when given a :id' do
let(:params) { { id: project.members.find_by!(user_id: user.id).id } }
context 'when current user can destroy the given invited user' do
before do
group_project.add_master(current_user)
group.add_owner(current_user)
end
it 'destroys the member' do
expect { described_class.new(project, user, params).execute }
.to change { project.members.count }.by(-1)
end
# Regression spec for issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/32504
it_behaves_like 'a service destroying a member' do
let(:member) { project_invited_member }
end
it_behaves_like 'a service destroying a member' do
let(:member) { group_invited_member }
end
end
end
......
......@@ -5,17 +5,17 @@ describe Members::RequestAccessService do
shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do
it 'raises Gitlab::Access::AccessDeniedError' do
expect { described_class.new(source, user).execute }.to raise_error(Gitlab::Access::AccessDeniedError)
expect { described_class.new(user).execute(source) }.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
shared_examples 'a service creating a access request' do
it 'succeeds' do
expect { described_class.new(source, user).execute }.to change { source.requesters.count }.by(1)
expect { described_class.new(user).execute(source) }.to change { source.requesters.count }.by(1)
end
it 'returns a <Source>Member' do
member = described_class.new(source, user).execute
member = described_class.new(user).execute(source)
expect(member).to be_a "#{source.class}Member".constantize
expect(member.requested_at).to be_present
......
require 'spec_helper'
describe Members::UpdateService do
let(:project) { create(:project, :public) }
let(:group) { create(:group, :public) }
let(:current_user) { create(:user) }
let(:member_user) { create(:user) }
let(:permission) { :update }
let(:member) { source.members_and_requesters.find_by!(user_id: member_user.id) }
let(:params) do
{ access_level: Gitlab::Access::MASTER }
end
shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do
it 'raises Gitlab::Access::AccessDeniedError' do
expect { described_class.new(current_user, params).execute(member, permission: permission) }
.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
shared_examples 'a service updating a member' do
it 'updates the member' do
updated_member = described_class.new(current_user, params).execute(member, permission: permission)
expect(updated_member).to be_valid
expect(updated_member.access_level).to eq(Gitlab::Access::MASTER)
end
end
before do
project.add_developer(member_user)
group.add_developer(member_user)
end
context 'when current user cannot update the given member' do
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:source) { project }
end
it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
let(:source) { group }
end
end
context 'when current user can update the given member' do
before do
project.add_master(current_user)
group.add_owner(current_user)
end
it_behaves_like 'a service updating a member' do
let(:source) { project }
end
it_behaves_like 'a service updating a member' do
let(:source) { group }
end
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