Correct permissions for creating merge requests from issues

This could only be possible for users that can create merge requests
within a project.

So they need to be a allowed to create a branch and create a merge request.

8 files changed, 28 insertions, 11 deletions

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 9dcd13b9385..767e492f566 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -20,7 +20,7 @@ class Projects::IssuesController < Projects::ApplicationController
   before_action :authorize_update_issuable!, only: [:edit, :update, :move]
 
   # Allow create a new branch and empty WIP merge request from current issue
-  before_action :authorize_create_merge_request_in!, only: [:create_merge_request]
+  before_action :authorize_create_merge_request_from!, only: [:create_merge_request]
 
   respond_to :html
 
diff --git a/app/views/projects/_last_push.html.haml b/app/views/projects/_last_push.html.haml
index e9a8eae4375..f6d396c8127 100644
--- a/app/views/projects/_last_push.html.haml
+++ b/app/views/projects/_last_push.html.haml
@@ -13,7 +13,7 @@
 
         #{time_ago_with_tooltip(event.created_at)}
 
-      .flex-right
-        - if can?(current_user, :create_merge_request_in, @project)
+      - if can?(current_user, :create_merge_request_in, event.project.default_merge_request_target)
+        .flex-right
           = link_to new_mr_path_from_push_event(event), title: _("New merge request"), class: "btn btn-info btn-sm qa-create-merge-request" do
             #{ _('Create merge request') }
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 3264a26f7d2..d4cc18f622b 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -189,7 +189,7 @@ module API
       post ":id/merge_requests" do
         Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42316')
 
-        authorize! :create_merge_request, user_project
+        authorize! :create_merge_request_from, user_project
 
         mr_params = declared_params(include_missing: false)
         mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch)
diff --git a/lib/api/v3/merge_requests.rb b/lib/api/v3/merge_requests.rb
index ce216497996..9b0f70e2bfe 100644
--- a/lib/api/v3/merge_requests.rb
+++ b/lib/api/v3/merge_requests.rb
@@ -93,7 +93,7 @@ module API
         post ":id/merge_requests" do
           Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42126')
 
-          authorize! :create_merge_request, user_project
+          authorize! :create_merge_request_from, user_project
 
           mr_params = declared_params(include_missing: false)
           mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) if mr_params[:remove_source_branch].present?
diff --git a/lib/gitlab/email/handler/create_merge_request_handler.rb b/lib/gitlab/email/handler/create_merge_request_handler.rb
index 3436306e122..2f864f2082b 100644
--- a/lib/gitlab/email/handler/create_merge_request_handler.rb
+++ b/lib/gitlab/email/handler/create_merge_request_handler.rb
@@ -23,7 +23,8 @@ module Gitlab
         def execute
           raise ProjectNotFound unless project
 
-          validate_permission!(:create_merge_request)
+          validate_permission!(:create_merge_request_in)
+          validate_permission!(:create_merge_request_from)
 
           verify_record!(
             record: create_merge_request,
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 01b5506b64b..6c6b1e4a500 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -938,7 +938,7 @@ describe Projects::IssuesController do
   end
 
   describe 'POST create_merge_request' do
-    let(:project) { create(:project, :repository) }
+    let(:project) { create(:project, :repository, :public) }
 
     before do
       project.add_developer(user)
@@ -955,6 +955,22 @@ describe Projects::IssuesController do
       expect(response).to match_response_schema('merge_request')
     end
 
+    it 'is not available when the project is archived' do
+      project.update(archived: true)
+
+      create_merge_request
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
+    it 'is not available for users who cannot create merge requests' do
+      sign_in(create(:user))
+
+      create_merge_request
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
     def create_merge_request
       post :create_merge_request, namespace_id: project.namespace.to_param,
                                   project_id: project.to_param,
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 3764aec0c71..f64623d7018 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -861,7 +861,7 @@ describe API::MergeRequests do
         expect(json_response['title']).to eq('Test merge_request')
       end
 
-      it 'returns 422 when target project has disabled merge requests' do
+      it 'returns 403 when target project has disabled merge requests' do
         project.project_feature.update(merge_requests_access_level: 0)
 
         post api("/projects/#{forked_project.id}/merge_requests", user2),
@@ -871,7 +871,7 @@ describe API::MergeRequests do
              author: user2,
              target_project_id: project.id
 
-        expect(response).to have_gitlab_http_status(422)
+        expect(response).to have_gitlab_http_status(403)
       end
 
       it "returns 400 when source_branch is missing" do
diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb
index 6b748369f0d..be70cb24dce 100644
--- a/spec/requests/api/v3/merge_requests_spec.rb
+++ b/spec/requests/api/v3/merge_requests_spec.rb
@@ -340,7 +340,7 @@ describe API::MergeRequests do
         expect(json_response['title']).to eq('Test merge_request')
       end
 
-      it "returns 422 when target project has disabled merge requests" do
+      it "returns 403 when target project has disabled merge requests" do
         project.project_feature.update(merge_requests_access_level: 0)
 
         post v3_api("/projects/#{forked_project.id}/merge_requests", user2),
@@ -350,7 +350,7 @@ describe API::MergeRequests do
              author: user2,
              target_project_id: project.id
 
-        expect(response).to have_gitlab_http_status(422)
+        expect(response).to have_gitlab_http_status(403)
       end
 
       it "returns 400 when source_branch is missing" do