Commit 334d4b1c authored by Sean McGivern's avatar Sean McGivern

Fix access checks for rebasing an MR

This has been broken since 34457b30, when the feature was implemented: a user
who can merge the MR may not have access to push to the source branch, and vice
versa.

This makes the backend check the correct thing (which the UI already did):
whether the user can push to the source branch.
parent d16c3015
......@@ -5,11 +5,10 @@ module EE
prepended do
before_action :check_merge_request_rebase_available!, only: [:rebase]
before_action :check_user_can_push_to_source_branch!, only: [:rebase]
end
def rebase
return access_denied! unless @merge_request.can_be_merged_by?(current_user)
RebaseWorker.perform_async(@merge_request.id, current_user.id)
render nothing: true, status: 200
......@@ -64,6 +63,16 @@ module EE
attrs
end
def check_user_can_push_to_source_branch!
return access_denied! unless @merge_request.source_branch_exists?
access_check = ::Gitlab::UserAccess
.new(current_user, project: @merge_request.source_project)
.can_push_to_branch?(@merge_request.source_branch)
access_denied! unless access_check
end
end
end
end
---
title: Fix rebase button when merge request is created from a fork
merge_request:
author:
......@@ -311,13 +311,13 @@ describe Projects::MergeRequestsController do
post :rebase, namespace_id: project.namespace, project_id: project, id: merge_request
end
def expect_rebase_worker
expect(RebaseWorker).to receive(:perform_async).with(merge_request.id, viewer.id)
def expect_rebase_worker_for(user)
expect(RebaseWorker).to receive(:perform_async).with(merge_request.id, user.id)
end
context 'successfully' do
it 'enqeues a RebaseWorker' do
expect_rebase_worker
expect_rebase_worker_for(viewer)
post_rebase
......@@ -329,7 +329,7 @@ describe Projects::MergeRequestsController do
let(:project) { create(:project, approvals_before_merge: 1) }
it 'returns 200' do
expect_rebase_worker
expect_rebase_worker_for(viewer)
post_rebase
......@@ -337,26 +337,46 @@ describe Projects::MergeRequestsController do
end
end
context 'user cannot merge' do
let(:viewer) { create(:user) }
context 'with a forked project' do
let(:fork_project) { create(:project, forked_from_project: project) }
let(:fork_owner) { fork_project.owner }
before do
project.add_reporter(viewer)
merge_request.update!(source_project: fork_project)
fork_project.add_reporter(user)
end
it 'returns 404' do
expect_rebase_worker.never
context 'user cannot push to source branch' do
it 'returns 404' do
expect_rebase_worker_for(viewer).never
post_rebase
post_rebase
expect(response.status).to eq(404)
expect(response.status).to eq(404)
end
end
context 'user can push to source branch' do
before do
project.add_reporter(fork_owner)
sign_in(fork_owner)
end
it 'returns 200' do
expect_rebase_worker_for(fork_owner)
post_rebase
expect(response.status).to eq(200)
end
end
end
context 'rebase unavailable in license' do
it 'returns 404' do
stub_licensed_features(merge_request_rebase: false)
expect_rebase_worker.never
expect_rebase_worker_for(viewer).never
post_rebase
......
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