diff options
author | Oswaldo Ferreira <oswaldo@gitlab.com> | 2019-02-11 13:14:11 -0200 |
---|---|---|
committer | Oswaldo Ferreira <oswaldo@gitlab.com> | 2019-02-25 10:41:01 -0300 |
commit | 105212ce49007ffc3489c2039e55056d8df8fa95 (patch) | |
tree | 32593345a1a0aa88e2c71fc057e8e835744084a3 /app/services/merge_requests | |
parent | 1ad699677fa4b24a9bc002c6dc20164b8832bca5 (diff) | |
download | gitlab-ce-105212ce49007ffc3489c2039e55056d8df8fa95.tar.gz |
Check authorization in merge services
Move authorization checks to merge services
instead relying solely on external checks.
Diffstat (limited to 'app/services/merge_requests')
-rw-r--r-- | app/services/merge_requests/merge_base_service.rb | 4 | ||||
-rw-r--r-- | app/services/merge_requests/merge_service.rb | 21 | ||||
-rw-r--r-- | app/services/merge_requests/merge_to_ref_service.rb | 17 |
3 files changed, 34 insertions, 8 deletions
diff --git a/app/services/merge_requests/merge_base_service.rb b/app/services/merge_requests/merge_base_service.rb index 4c5a0d96957..61049f394aa 100644 --- a/app/services/merge_requests/merge_base_service.rb +++ b/app/services/merge_requests/merge_base_service.rb @@ -27,6 +27,10 @@ module MergeRequests private + def raise_error(message) + raise MergeError, message + end + def handle_merge_error(*args) # No-op end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index b1d01955d85..f5d66589196 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -18,7 +18,7 @@ module MergeRequests @merge_request = merge_request - error_check! + validate! merge_request.in_locked_state do if commit @@ -34,6 +34,17 @@ module MergeRequests private + def validate! + authorization_check! + error_check! + end + + def authorization_check! + unless @merge_request.can_be_merged_by?(current_user) + raise_error('You are not allowed to merge this merge request') + end + end + def error_check! error = if @merge_request.should_be_rebased? @@ -44,7 +55,7 @@ module MergeRequests 'No source for merge' end - raise MergeError, error if error + raise_error(error) if error end def commit @@ -54,7 +65,7 @@ module MergeRequests if commit_id log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}") else - raise MergeError, 'Conflicts detected during merge' + raise_error('Conflicts detected during merge') end merge_request.update!(merge_commit_sha: commit_id) @@ -64,10 +75,10 @@ module MergeRequests repository.merge(current_user, source, merge_request, commit_message) rescue Gitlab::Git::PreReceiveError => e handle_merge_error(log_message: e.message) - raise MergeError, 'Something went wrong during merge pre-receive hook' + raise_error('Something went wrong during merge pre-receive hook') rescue => e handle_merge_error(log_message: e.message) - raise MergeError, 'Something went wrong during merge' + raise_error('Something went wrong during merge') ensure merge_request.update!(in_progress_merge_commit_sha: nil) end diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb index c4a40044143..6b99c4fc483 100644 --- a/app/services/merge_requests/merge_to_ref_service.rb +++ b/app/services/merge_requests/merge_to_ref_service.rb @@ -14,11 +14,11 @@ module MergeRequests def execute(merge_request) @merge_request = merge_request - error_check! + validate! commit_id = commit - raise MergeError, 'Conflicts detected during merge' unless commit_id + raise_error('Conflicts detected during merge') unless commit_id success(commit_id: commit_id) rescue MergeError => error @@ -27,6 +27,11 @@ module MergeRequests private + def validate! + authorization_check! + error_check! + end + def error_check! error = if !merge_method_supported? @@ -41,7 +46,13 @@ module MergeRequests 'No source for merge' end - raise MergeError, error if error + raise_error(error) if error + end + + def authorization_check! + unless Ability.allowed?(current_user, :admin_merge_request, project) + raise_error("You are not allowed to merge to this ref") + end end def target_ref |