Commit de19d0ec authored by Bob Van Landuyt's avatar Bob Van Landuyt
Browse files

Fixes rejected pushes from maintainers

Before the push git would make a call to
`/:namespace/:project/git-receive-pack`. This would perform an access
check without a ref. So the `Project#branch_allows_maintainer_push?`
would return false.

This adjusts `Project#branch_allows_maintainer_push?` to return true
when passing no branch name if there are merge requests open that
would allow the user to push.

The actual check then happens when a call to
`/api/v4/internal/allowed` is made from a git hook.
parent ae68e5c56306
...@@ -2139,10 +2139,14 @@ def fetch_branch_allows_collaboration?(user, branch_name) ...@@ -2139,10 +2139,14 @@ def fetch_branch_allows_collaboration?(user, branch_name)
check_access = -> do check_access = -> do
next false if empty_repo? next false if empty_repo?
merge_request = source_of_merge_requests.opened merge_requests = source_of_merge_requests.opened
.where(allow_collaboration: true) .where(allow_collaboration: true)
.find_by(source_branch: branch_name)
merge_request&.can_be_merged_by?(user) if branch_name
merge_requests.find_by(source_branch: branch_name)&.can_be_merged_by?(user)
merge_requests.any? { |merge_request| merge_request.can_be_merged_by?(user) }
end end
if if
title: Fix bug where maintainer would not be allowed to push to forks with merge requests
that have `Allow maintainer edits` enabled.
merge_request: 18968
type: fixed
...@@ -3658,6 +3658,11 @@ def domain_variable ...@@ -3658,6 +3658,11 @@ def domain_variable
.to be_truthy .to be_truthy
end end
it 'allows access when there are merge requests open but no branch name is given' do
expect(project.branch_allows_collaboration?(user, nil))
.to be_truthy
it 'does not allow guest users access' do it 'does not allow guest users access' do
guest = create(:user) guest = create(:user)
target_project.add_guest(guest) target_project.add_guest(guest)
require "spec_helper" require "spec_helper"
describe 'Git HTTP requests' do describe 'Git HTTP requests' do
include ProjectForksHelper
include TermsHelper include TermsHelper
include GitHttpHelpers include GitHttpHelpers
include WorkhorseHelpers include WorkhorseHelpers
...@@ -305,6 +306,22 @@ ...@@ -305,6 +306,22 @@
expect(response.body).to eq(change_access_error(:push_code)) expect(response.body).to eq(change_access_error(:push_code))
end end
end end
context 'when merge requests are open that allow maintainer access' do
let(:canonical_project) { create(:project, :public, :repository) }
let(:project) { fork_project(canonical_project, nil, repository: true) }
before do
source_project: project,
target_project: canonical_project,
source_branch: 'fixes',
allow_collaboration: true)
it_behaves_like 'pushes are allowed'
end end
end end
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