Commit 1da842eb authored by Bob Van Landuyt's avatar Bob Van Landuyt

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.
parent 116f1c48
...@@ -22,7 +22,7 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -22,7 +22,7 @@ class Projects::IssuesController < Projects::ApplicationController
before_action :authorize_update_issuable!, only: [:edit, :update, :move] before_action :authorize_update_issuable!, only: [:edit, :update, :move]
# Allow create a new branch and empty WIP merge request from current issue # 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]
prepend ::EE::Projects::IssuesController prepend ::EE::Projects::IssuesController
......
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
#{time_ago_with_tooltip(event.created_at)} #{time_ago_with_tooltip(event.created_at)}
.flex-right - if can?(current_user, :create_merge_request_in, event.project.default_merge_request_target)
- if can?(current_user, :create_merge_request_in, @project) .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 = 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') } #{ _('Create merge request') }
...@@ -191,7 +191,7 @@ module API ...@@ -191,7 +191,7 @@ module API
post ":id/merge_requests" do post ":id/merge_requests" do
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42316') 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 = declared_params(include_missing: false)
mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch)
......
...@@ -95,7 +95,7 @@ module API ...@@ -95,7 +95,7 @@ module API
post ":id/merge_requests" do post ":id/merge_requests" do
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42126') 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 = declared_params(include_missing: false)
mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) if mr_params[:remove_source_branch].present? mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) if mr_params[:remove_source_branch].present?
......
...@@ -23,7 +23,8 @@ module Gitlab ...@@ -23,7 +23,8 @@ module Gitlab
def execute def execute
raise ProjectNotFound unless project raise ProjectNotFound unless project
validate_permission!(:create_merge_request) validate_permission!(:create_merge_request_in)
validate_permission!(:create_merge_request_from)
verify_record!( verify_record!(
record: create_merge_request, record: create_merge_request,
......
...@@ -938,7 +938,7 @@ describe Projects::IssuesController do ...@@ -938,7 +938,7 @@ describe Projects::IssuesController do
end end
describe 'POST create_merge_request' do describe 'POST create_merge_request' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository, :public) }
before do before do
project.add_developer(user) project.add_developer(user)
...@@ -955,6 +955,22 @@ describe Projects::IssuesController do ...@@ -955,6 +955,22 @@ describe Projects::IssuesController do
expect(response).to match_response_schema('merge_request') expect(response).to match_response_schema('merge_request')
end 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 def create_merge_request
post :create_merge_request, namespace_id: project.namespace.to_param, post :create_merge_request, namespace_id: project.namespace.to_param,
project_id: project.to_param, project_id: project.to_param,
......
...@@ -864,7 +864,7 @@ describe API::MergeRequests do ...@@ -864,7 +864,7 @@ describe API::MergeRequests do
expect(json_response['title']).to eq('Test merge_request') expect(json_response['title']).to eq('Test merge_request')
end 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) project.project_feature.update(merge_requests_access_level: 0)
post api("/projects/#{forked_project.id}/merge_requests", user2), post api("/projects/#{forked_project.id}/merge_requests", user2),
...@@ -874,7 +874,7 @@ describe API::MergeRequests do ...@@ -874,7 +874,7 @@ describe API::MergeRequests do
author: user2, author: user2,
target_project_id: project.id target_project_id: project.id
expect(response).to have_gitlab_http_status(422) expect(response).to have_gitlab_http_status(403)
end end
it "returns 400 when source_branch is missing" do it "returns 400 when source_branch is missing" do
......
...@@ -343,7 +343,7 @@ describe API::MergeRequests do ...@@ -343,7 +343,7 @@ describe API::MergeRequests do
expect(json_response['title']).to eq('Test merge_request') expect(json_response['title']).to eq('Test merge_request')
end 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) project.project_feature.update(merge_requests_access_level: 0)
post v3_api("/projects/#{forked_project.id}/merge_requests", user2), post v3_api("/projects/#{forked_project.id}/merge_requests", user2),
...@@ -353,7 +353,7 @@ describe API::MergeRequests do ...@@ -353,7 +353,7 @@ describe API::MergeRequests do
author: user2, author: user2,
target_project_id: project.id target_project_id: project.id
expect(response).to have_gitlab_http_status(422) expect(response).to have_gitlab_http_status(403)
end end
it "returns 400 when source_branch is missing" do it "returns 400 when source_branch is missing" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment