diff options
author | Rémy Coutable <remy@rymai.me> | 2017-10-11 17:47:08 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2018-02-27 18:02:22 +0300 |
commit | bf41063679b25371b2e64542f2f469b38502edf6 (patch) | |
tree | bc7363df6d75c628f593b29426b59ba05b10a223 /app/services/members | |
parent | 3bf448267b117e79f08ab2f4b769d24a705a5f0f (diff) |
Remove explicit audit event log in MembershipActions
Move it to Members::ApproveAccessRequestService.
Also, note that there was a double audit event log for access request
destruction.
Signed-off-by: Rémy Coutable <remy@rymai.me>
Diffstat (limited to 'app/services/members')
-rw-r--r-- | app/services/members/approve_access_request_service.rb | 42 | ||||
-rw-r--r-- | app/services/members/authorized_destroy_service.rb | 12 | ||||
-rw-r--r-- | app/services/members/base_service.rb | 53 | ||||
-rw-r--r-- | app/services/members/create_service.rb | 13 | ||||
-rw-r--r-- | app/services/members/destroy_service.rb | 37 | ||||
-rw-r--r-- | app/services/members/request_access_service.rb | 9 | ||||
-rw-r--r-- | app/services/members/update_service.rb | 17 |
7 files changed, 96 insertions, 87 deletions
diff --git a/app/services/members/approve_access_request_service.rb b/app/services/members/approve_access_request_service.rb index 2a2bb0cae5b..19431ac76dc 100644 --- a/app/services/members/approve_access_request_service.rb +++ b/app/services/members/approve_access_request_service.rb @@ -1,51 +1,25 @@ 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 - + class ApproveAccessRequestService < Members::BaseService # 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) + # :ldap - The call is from a LDAP sync: current_user can be nil in that case + def execute(access_requester, opts = {}) + raise Gitlab::Access::AccessDeniedError unless can_update_access_requester?(access_requester, opts[:ldap]) access_requester.access_level = params[:access_level] if params[:access_level] access_requester.accept_request + after_execute(member: access_requester, **opts) + access_requester end private - def can_update_access_requester?(access_requester, opts = {}) + def can_update_access_requester?(access_requester, ldap) access_requester && ( - opts[:force] || + ldap || 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 - end end end diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb index 2e89f00dad8..793a951d63b 100644 --- a/app/services/members/authorized_destroy_service.rb +++ b/app/services/members/authorized_destroy_service.rb @@ -1,12 +1,10 @@ module Members class AuthorizedDestroyService < BaseService - attr_accessor :member, :user - - def initialize(member, user = nil) - @member, @user = member, user + def initialize(current_user = nil) + @current_user = current_user end - def execute + def execute(member) return false if member.is_a?(GroupMember) && member.source.last_owner?(member.user) Member.transaction do @@ -16,7 +14,7 @@ module Members member.destroy end - if member.request? && member.user != user + if member.request? && member.user != current_user notification_service.decline_access_request(member) end @@ -36,7 +34,7 @@ module Members .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) + MergeRequestsFinder.new(current_user, group_id: member.source_id, assignee_id: member.user_id) .execute .update_all(assignee_id: nil) else diff --git a/app/services/members/base_service.rb b/app/services/members/base_service.rb new file mode 100644 index 00000000000..5c52468aaca --- /dev/null +++ b/app/services/members/base_service.rb @@ -0,0 +1,53 @@ +module Members + class BaseService < ::BaseService + attr_accessor :source + + # source - The source object that respond to `#members` (e.g. project or group) + # current_user - The user that performs the action + # params - A hash of parameters + def initialize(source, current_user, params = {}) + @source = source + @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 diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index 26906ae7167..61802ba09ce 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -1,14 +1,7 @@ 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 return error('No users specified.') if params[:user_ids].blank? @@ -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.compact.each { |member| after_execute(member: member) } + success end diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index 05b93ac8fdb..477ab127b2d 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -1,40 +1,17 @@ module Members - class DestroyService < BaseService - include MembersHelper - - attr_accessor :source - - ALLOWED_SCOPES = %i[members requesters all].freeze - - 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) + class DestroyService < Members::BaseService + def execute(member) + raise Gitlab::Access::AccessDeniedError unless can_destroy_member?(member) - member = find_member!(scope) + AuthorizedDestroyService.new(current_user).execute(member) - 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) end @@ -45,6 +22,8 @@ module Members :destroy_group_member when ProjectMember :destroy_project_member + else + raise "Unknown member type: #{member}!" end end end diff --git a/app/services/members/request_access_service.rb b/app/services/members/request_access_service.rb index 2614153d900..e30722efc13 100644 --- a/app/services/members/request_access_service.rb +++ b/app/services/members/request_access_service.rb @@ -1,12 +1,5 @@ module Members - class RequestAccessService < BaseService - attr_accessor :source - - def initialize(source, current_user) - @source = source - @current_user = current_user - end - + class RequestAccessService < Members::BaseService def execute raise Gitlab::Access::AccessDeniedError unless can_request_access?(source) diff --git a/app/services/members/update_service.rb b/app/services/members/update_service.rb new file mode 100644 index 00000000000..ebea9f96069 --- /dev/null +++ b/app/services/members/update_service.rb @@ -0,0 +1,17 @@ +module Members + class UpdateService < Members::BaseService + # returns the updated member + def execute(member, permission: :update) + permission_target = permission == :override ? source : member + raise Gitlab::Access::AccessDeniedError unless can?(current_user, action_member_permission(permission, member), permission_target) + + 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 |