Commit 8c3977f9 authored by Stan Hu's avatar Stan Hu

Merge branch 'notify_about_force_push_before_rebase' into 'master'

Don't rebase when the branch protected from force push

See merge request gitlab-org/gitlab!80329
parents 23c6affe 14db567b
...@@ -501,6 +501,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -501,6 +501,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
.can_push_to_branch?(@merge_request.source_branch) .can_push_to_branch?(@merge_request.source_branch)
access_denied! unless access_check access_denied! unless access_check
access_denied! unless merge_request.permits_force_push?
end end
def merge_access_check def merge_access_check
......
...@@ -471,6 +471,12 @@ class MergeRequest < ApplicationRecord ...@@ -471,6 +471,12 @@ class MergeRequest < ApplicationRecord
rebase_jid.present? && Gitlab::SidekiqStatus.running?(rebase_jid) rebase_jid.present? && Gitlab::SidekiqStatus.running?(rebase_jid)
end end
def permits_force_push?
return true unless ProtectedBranch.protected?(source_project, source_branch)
ProtectedBranch.allow_force_push?(source_project, source_branch)
end
# Use this method whenever you need to make sure the head_pipeline is synced with the # Use this method whenever you need to make sure the head_pipeline is synced with the
# branch head commit, for example checking if a merge request can be merged. # branch head commit, for example checking if a merge request can be merged.
# For more information check: https://gitlab.com/gitlab-org/gitlab-foss/issues/40004 # For more information check: https://gitlab.com/gitlab-org/gitlab-foss/issues/40004
......
...@@ -117,6 +117,9 @@ module API ...@@ -117,6 +117,9 @@ module API
forbidden!('Cannot push to source branch') unless forbidden!('Cannot push to source branch') unless
user_access.can_push_to_branch?(merge_request.source_branch) user_access.can_push_to_branch?(merge_request.source_branch)
forbidden!('Source branch is protected from force push') unless
merge_request.permits_force_push?
end end
params :merge_requests_params do params :merge_requests_params do
......
...@@ -57,6 +57,11 @@ module Gitlab ...@@ -57,6 +57,11 @@ module Gitlab
access_check.can_push_to_branch?(merge_request.source_branch) access_check.can_push_to_branch?(merge_request.source_branch)
end end
command :rebase do command :rebase do
unless quick_action_target.permits_force_push?
@execution_message[:rebase] = _('This merge request branch is protected from force push.')
next
end
if quick_action_target.cannot_be_merged? if quick_action_target.cannot_be_merged?
@execution_message[:rebase] = _('This merge request cannot be rebased while there are conflicts.') @execution_message[:rebase] = _('This merge request cannot be rebased while there are conflicts.')
next next
......
...@@ -37164,6 +37164,9 @@ msgstr "" ...@@ -37164,6 +37164,9 @@ msgstr ""
msgid "This means you can not push code until you create an empty repository or import existing one." msgid "This means you can not push code until you create an empty repository or import existing one."
msgstr "" msgstr ""
msgid "This merge request branch is protected from force push."
msgstr ""
msgid "This merge request cannot be rebased while there are conflicts." msgid "This merge request cannot be rebased while there are conflicts."
msgstr "" msgstr ""
......
...@@ -2078,6 +2078,20 @@ RSpec.describe Projects::MergeRequestsController do ...@@ -2078,6 +2078,20 @@ RSpec.describe Projects::MergeRequestsController do
end end
end end
context 'when source branch is protected from force push' do
before do
create(:protected_branch, project: project, name: merge_request.source_branch, allow_force_push: false)
end
it 'returns 404' do
expect_rebase_worker_for(user).never
post_rebase
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'with a forked project' do context 'with a forked project' do
let(:forked_project) { fork_project(project, fork_owner, repository: true) } let(:forked_project) { fork_project(project, fork_owner, repository: true) }
let(:fork_owner) { create(:user) } let(:fork_owner) { create(:user) }
......
...@@ -1538,6 +1538,42 @@ RSpec.describe MergeRequest, factory_default: :keep do ...@@ -1538,6 +1538,42 @@ RSpec.describe MergeRequest, factory_default: :keep do
end end
end end
describe '#permits_force_push?' do
let_it_be(:merge_request) { build_stubbed(:merge_request) }
subject { merge_request.permits_force_push? }
context 'when source branch is not protected' do
before do
allow(ProtectedBranch).to receive(:protected?).and_return(false)
end
it { is_expected.to be_truthy }
end
context 'when source branch is protected' do
before do
allow(ProtectedBranch).to receive(:protected?).and_return(true)
end
context 'when force push is not allowed' do
before do
allow(ProtectedBranch).to receive(:allow_force_push?) { false }
end
it { is_expected.to be_falsey }
end
context 'when force push is allowed' do
before do
allow(ProtectedBranch).to receive(:allow_force_push?) { true }
end
it { is_expected.to be_truthy }
end
end
end
describe '#can_remove_source_branch?' do describe '#can_remove_source_branch?' do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:merge_request, reload: true) { create(:merge_request, :simple) } let_it_be(:merge_request, reload: true) { create(:merge_request, :simple) }
......
...@@ -3344,6 +3344,18 @@ RSpec.describe API::MergeRequests do ...@@ -3344,6 +3344,18 @@ RSpec.describe API::MergeRequests do
end end
end end
context 'when merge request branch does not allow force push' do
before do
create(:protected_branch, project: project, name: merge_request.source_branch, allow_force_push: false)
end
it 'returns 403' do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/rebase", user)
expect(response).to have_gitlab_http_status(:forbidden)
end
end
it 'returns 403 if the user cannot push to the branch' do it 'returns 403 if the user cannot push to the branch' do
guest = create(:user) guest = create(:user)
project.add_guest(guest) project.add_guest(guest)
......
...@@ -73,6 +73,16 @@ RSpec.shared_examples 'rebase quick action' do ...@@ -73,6 +73,16 @@ RSpec.shared_examples 'rebase quick action' do
expect(page).to have_content 'This merge request cannot be rebased while there are conflicts.' expect(page).to have_content 'This merge request cannot be rebased while there are conflicts.'
end end
end end
context 'when the merge request branch is protected from force push' do
let!(:protected_branch) { create(:protected_branch, project: project, name: merge_request.source_branch, allow_force_push: false) }
it 'does not rebase the MR' do
add_note("/rebase")
expect(page).to have_content 'This merge request branch is protected from force push.'
end
end
end end
context 'when the current user cannot rebase the MR' do context 'when the current user cannot rebase the MR' 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