Allow abilities on forks while MR is open

When an MR is created using `allow_maintainer_to_push`, we enable some
abilities while the MR is open.

This should allow every user with developer abilities on the target
project, to push to the source project.

6 files changed, 138 insertions, 3 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index 5cd1da43645..0128967e038 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -150,6 +150,7 @@ class Project < ActiveRecord::Base
 
   # Merge Requests for target project should be removed with it
   has_many :merge_requests, foreign_key: 'target_project_id'
+  has_many :source_of_merge_requests, foreign_key: 'source_project_id', class_name: 'MergeRequest'
   has_many :issues
   has_many :labels, class_name: 'ProjectLabel'
   has_many :services
@@ -1799,6 +1800,14 @@ class Project < ActiveRecord::Base
     Badge.where("id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
   end
 
+  def branches_allowing_maintainer_access_to_user(user)
+    @branches_allowing_maintainer_access_to_user ||= Hash.new do |result, user|
+      result[user] = fetch_branches_allowing_maintainer_access_to_user(user)
+    end
+
+    @branches_allowing_maintainer_access_to_user[user]
+  end
+
   private
 
   def storage
@@ -1921,4 +1930,24 @@ class Project < ActiveRecord::Base
 
     raise ex
   end
+
+  def fetch_branches_allowing_maintainer_access_to_user(user)
+    return [] unless user
+
+    projects_with_developer_access = user.project_authorizations
+                                       .where('access_level >= ? ', Gitlab::Access::DEVELOPER)
+                                       .select(:project_id)
+    merge_requests_allowing_push = source_of_merge_requests.opened
+                                     .where(allow_maintainer_to_push: true)
+                                     .where(target_project_id: projects_with_developer_access)
+                                     .select(:source_branch).uniq
+
+    if RequestStore.active?
+      RequestStore.fetch("project-#{id}:user-#{user.id}:branches_allowing_maintainer_access") do
+        merge_requests_allowing_push.pluck(:source_branch)
+      end
+    else
+      merge_requests_allowing_push.pluck(:source_branch)
+    end
+  end
 end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 3b0550b4dd6..ce94a25a20b 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -61,6 +61,11 @@ class ProjectPolicy < BasePolicy
   desc "Project has request access enabled"
   condition(:request_access_enabled, scope: :subject) { project.request_access_enabled }
 
+  desc "The project has merge requests open that allow external users to push"
+  condition(:merge_request_allows_push, scope: :subject) do
+    project.branches_allowing_maintainer_access_to_user(@user).any?
+  end
+
   features = %w[
     merge_requests
     issues
@@ -240,6 +245,7 @@ class ProjectPolicy < BasePolicy
 
   rule { repository_disabled }.policy do
     prevent :push_code
+    prevent :push_single_branch
     prevent :download_code
     prevent :fork_project
     prevent :read_commit_status
@@ -291,6 +297,16 @@ class ProjectPolicy < BasePolicy
     prevent :read_issue
   end
 
+  # These rules are included to allow maintainers of projects to push to certain
+  # branches of forks.
+  rule { can?(:public_access) & merge_request_allows_push }.policy do
+    enable :push_single_branch
+    enable :create_build
+    enable :update_build
+    enable :create_pipeline
+    enable :update_pipeline
+  end
+
   private
 
   def team_member?
diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb
index b49ddbfc780..2f173d2e75c 100644
--- a/spec/lib/gitlab/checks/change_access_spec.rb
+++ b/spec/lib/gitlab/checks/change_access_spec.rb
@@ -30,9 +30,9 @@ describe Gitlab::Checks::ChangeAccess do
       end
     end
 
-    context 'when the user is not allowed to push code' do
+    context 'when the user is not allowed to push to the repo' do
       it 'raises an error' do
-        expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false)
+        expect(user_access).to receive(:can_do_action?).with(:push_to_repo).and_return(false)
 
         expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.')
       end
@@ -42,7 +42,7 @@ describe Gitlab::Checks::ChangeAccess do
       let(:ref) { 'refs/tags/v1.0.0' }
 
       it 'raises an error if the user is not allowed to update tags' do
-        allow(user_access).to receive(:can_do_action?).with(:push_code).and_return(true)
+        allow(user_access).to receive(:can_do_action?).with(:push_to_repo).and_return(true)
         expect(user_access).to receive(:can_do_action?).with(:admin_project).and_return(false)
 
         expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to change existing tags on this project.')
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index b20cc34dd5c..bece82e531a 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -278,6 +278,7 @@ project:
 - custom_attributes
 - lfs_file_locks
 - project_badges
+- source_of_merge_requests
 award_emoji:
 - awardable
 - user
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index b1c9e6754b9..a95a4637234 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe Project do
+  include ProjectForksHelper
+
   describe 'associations' do
     it { is_expected.to belong_to(:group) }
     it { is_expected.to belong_to(:namespace) }
@@ -3378,4 +3380,54 @@ describe Project do
       end
     end
   end
+
+  describe '#branches_allowing_maintainer_access_to_user' do
+    let(:maintainer) { create(:user) }
+    let(:target_project) { create(:project) }
+    let(:project) { fork_project(target_project) }
+    let!(:merge_request) do
+      create(
+        :merge_request,
+        target_project: target_project,
+        source_project: project,
+        source_branch: 'awesome-feature-1',
+        allow_maintainer_to_push: true
+      )
+    end
+
+    before do
+      target_project.add_developer(maintainer)
+    end
+
+    it 'includes branch names for merge requests allowing maintainer access to a user' do
+      expect(project.branches_allowing_maintainer_access_to_user(maintainer))
+        .to include('awesome-feature-1')
+    end
+
+    it 'does not include branches for closed MRs' do
+      create(:merge_request, :closed,
+             target_project: target_project,
+             source_project: project,
+             source_branch: 'rejected-feature-1',
+             allow_maintainer_to_push: true)
+
+      expect(project.branches_allowing_maintainer_access_to_user(maintainer))
+        .not_to include('rejected-feature-1')
+    end
+
+    it 'only queries once per user' do
+      expect { 3.times { project.branches_allowing_maintainer_access_to_user(maintainer) } }
+        .not_to exceed_query_limit(1)
+    end
+
+    context 'when the requeststore is active', :request_store do
+      it 'only queries once per user accross project instances' do
+        # limiting to 3 queries:
+        # 2 times loading the project
+        # once loading the accessible branches
+        expect { 2.times { described_class.find(project.id).branches_allowing_maintainer_access_to_user(maintainer) } }
+          .not_to exceed_query_limit(3)
+      end
+    end
+  end
 end
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 129344f105f..f449fbd6b52 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -308,4 +308,41 @@ describe ProjectPolicy do
   it_behaves_like 'project policies as master'
   it_behaves_like 'project policies as owner'
   it_behaves_like 'project policies as admin'
+
+  context 'when a public project has merge requests allowing access' do
+    include ProjectForksHelper
+    let(:user) { create(:user) }
+    let(:target_project) { create(:project, :public) }
+    let(:project) { fork_project(target_project) }
+    let!(:merge_request) do
+      create(
+        :merge_request,
+        target_project: target_project,
+        source_project: project,
+        allow_maintainer_to_push: true
+      )
+    end
+    let(:maintainer_abilities) do
+      %w(push_single_branch create_build update_build create_pipeline update_pipeline)
+    end
+
+    subject { described_class.new(user, project) }
+
+    it 'does not allow pushing code' do
+      expect_disallowed(*maintainer_abilities)
+    end
+
+    it 'allows pushing if the user is a member with push access to the target project' do
+      target_project.add_developer(user)
+
+      expect_allowed(*maintainer_abilities)
+    end
+
+    it 'dissallows abilities to a maintainer if the merge request was closed' do
+      target_project.add_developer(user)
+      merge_request.close!
+
+      expect_disallowed(*maintainer_abilities)
+    end
+  end
 end