Check authorization in merge services

Move authorization checks to merge services
instead relying solely on external checks.

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