Check authorization in merge services

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

2 files changed, 40 insertions, 16 deletions

diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index 04a62aa454d..ede79b87bcc 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -224,6 +224,18 @@ describe MergeRequests::MergeService do
         expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
       end
 
+      it 'logs and saves error if user is not authorized' do
+        unauthorized_user = create(:user)
+        project.add_reporter(unauthorized_user)
+
+        service = described_class.new(project, unauthorized_user)
+
+        service.execute(merge_request)
+
+        expect(merge_request.merge_error)
+          .to eq('You are not allowed to merge this merge request')
+      end
+
       it 'logs and saves error if there is an PreReceiveError exception' do
         error_message = 'error message'
 
diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb
index 435a863cbd4..696f1b83157 100644
--- a/spec/services/merge_requests/merge_to_ref_service_spec.rb
+++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb
@@ -3,6 +3,22 @@
 require 'spec_helper'
 
 describe MergeRequests::MergeToRefService do
+  shared_examples_for 'MergeService for target ref' do
+    it 'target_ref has the same state of target branch' do
+      repo = merge_request.target_project.repository
+
+      process_merge_to_ref
+      merge_service.execute(merge_request)
+
+      ref_commits = repo.commits(merge_request.merge_ref_path, limit: 3)
+      target_branch_commits = repo.commits(merge_request.target_branch, limit: 3)
+
+      ref_commits.zip(target_branch_commits).each do |ref_commit, target_branch_commit|
+        expect(ref_commit.parents).to eq(target_branch_commit.parents)
+      end
+    end
+  end
+
   set(:user) { create(:user) }
   let(:merge_request) { create(:merge_request, :simple) }
   let(:project) { merge_request.project }
@@ -76,22 +92,6 @@ describe MergeRequests::MergeToRefService do
         MergeRequests::MergeService.new(project, user, {})
       end
 
-      shared_examples_for 'MergeService for target ref' do
-        it 'target_ref has the same state of target branch' do
-          repo = merge_request.target_project.repository
-
-          process_merge_to_ref
-          merge_service.execute(merge_request)
-
-          ref_commits = repo.commits(merge_request.merge_ref_path, limit: 3)
-          target_branch_commits = repo.commits(merge_request.target_branch, limit: 3)
-
-          ref_commits.zip(target_branch_commits).each do |ref_commit, target_branch_commit|
-            expect(ref_commit.parents).to eq(target_branch_commit.parents)
-          end
-        end
-      end
-
       context 'when merge commit' do
         it_behaves_like 'MergeService for target ref'
       end
@@ -176,5 +176,17 @@ describe MergeRequests::MergeToRefService do
 
       it { expect(todo).not_to be_done }
     end
+
+    it 'returns error when user has no authorization to admin the merge request' do
+      unauthorized_user = create(:user)
+      project.add_reporter(unauthorized_user)
+
+      service = described_class.new(project, unauthorized_user)
+
+      result = service.execute(merge_request)
+
+      expect(result[:status]).to eq(:error)
+      expect(result[:message]).to eq('You are not allowed to merge to this ref')
+    end
   end
 end