Merge branch 'security-lastest-commit-exposed-private-group-guest-user-14-9' into '14-9-stable-ee'

Latest commit exposed through fork of a private project

See merge request gitlab-org/security/gitlab!2294

app/controllers/projects/merge_requests/creations_controller.rb

@@ -81,7 +81,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
   def branch_to
     @target_project = selected_target_project
-    if @target_project && params[:ref].present?
+    if @target_project && params[:ref].present? && Ability.allowed?(current_user, :create_merge_request_in, @target_project)
       @ref = params[:ref]
       @commit = @target_project.commit(Gitlab::Git::BRANCH_REF_PREFIX + @ref)
     end

app/policies/project_policy.rb

@@ -240,7 +240,6 @@ class ProjectPolicy < BasePolicy
   rule { can?(:guest_access) }.policy do
     enable :read_project
-    enable :create_merge_request_in
     enable :read_issue_board
     enable :read_issue_board_list
     enable :read_wiki
@@ -664,6 +663,10 @@ class ProjectPolicy < BasePolicy
     enable :read_security_configuration
   end
+  rule { can?(:guest_access) & can?(:read_commit_status) }.policy do
+    enable :create_merge_request_in
+  end
+
   # Design abilities could also be prevented in the issue policy.
   rule { design_management_disabled }.policy do
     prevent :read_design

spec/controllers/projects/merge_requests/creations_controller_spec.rb

@@ -186,6 +186,7 @@ RSpec.describe Projects::MergeRequests::CreationsController do
     it 'fetches the commit if a user has access' do
       expect(Ability).to receive(:allowed?).with(user, :read_project, project) { true }
+      expect(Ability).to receive(:allowed?).with(user, :create_merge_request_in, project) { true }.at_least(:once)
       get :branch_to,
           params: {
@@ -199,8 +200,25 @@ RSpec.describe Projects::MergeRequests::CreationsController do
       expect(response).to have_gitlab_http_status(:ok)
     end
+    it 'does not load the commit when the user cannot create_merge_request_in' do
+      expect(Ability).to receive(:allowed?).with(user, :read_project, project) { true }
+      expect(Ability).to receive(:allowed?).with(user, :create_merge_request_in, project) { false }.at_least(:once)
+
+      get :branch_to,
+          params: {
+            namespace_id: fork_project.namespace,
+            project_id: fork_project,
+            target_project_id: project.id,
+            ref: 'master'
+          }
+
+      expect(assigns(:commit)).to be_nil
+      expect(response).to have_gitlab_http_status(:ok)
+    end
+
     it 'does not load the commit when the user cannot read the project' do
       expect(Ability).to receive(:allowed?).with(user, :read_project, project) { false }
+      expect(Ability).to receive(:allowed?).with(user, :create_merge_request_in, project) { true }.at_least(:once)
       get :branch_to,
           params: {

spec/policies/project_policy_spec.rb

@@ -103,6 +103,44 @@ RSpec.describe ProjectPolicy do
     end
   end
+  context 'creating_merge_request_in' do
+    context 'when project is public' do
+      let(:project) { public_project }
+
+      context 'when the current_user is guest' do
+        let(:current_user) { guest }
+
+        it { is_expected.to be_allowed(:create_merge_request_in) }
+      end
+    end
+
+    context 'when project is internal' do
+      let(:project) { internal_project }
+
+      context 'when the current_user is guest' do
+        let(:current_user) { guest }
+
+        it { is_expected.to be_allowed(:create_merge_request_in) }
+      end
+    end
+
+    context 'when project is private' do
+      let(:project) { private_project }
+
+      context 'when the current_user is guest' do
+        let(:current_user) { guest }
+
+        it { is_expected.not_to be_allowed(:create_merge_request_in) }
+      end
+
+      context 'when the current_user is reporter or above' do
+        let(:current_user) { reporter }
+
+        it { is_expected.to be_allowed(:create_merge_request_in) }
+      end
+    end
+  end
+
   context 'pipeline feature' do
     let(:project)      { private_project }
     let(:current_user) { developer }

spec/support/shared_contexts/policies/project_policy_shared_context.rb

@@ -15,7 +15,7 @@ RSpec.shared_context 'ProjectPolicy context' do
   let(:base_guest_permissions) do
     %i[
-      award_emoji create_issue create_merge_request_in create_note
+      award_emoji create_issue create_note
       create_project read_issue_board read_issue read_issue_iid read_issue_link
       read_label read_planning_hierarchy read_issue_board_list read_milestone read_note read_project
       read_project_for_iids read_project_member read_release read_snippet
@@ -31,7 +31,7 @@ RSpec.shared_context 'ProjectPolicy context' do
       read_commit_status read_confidential_issues read_container_image
       read_deployment read_environment read_merge_request
       read_metrics_dashboard_annotation read_pipeline read_prometheus
-      read_sentry_issue update_issue
+      read_sentry_issue update_issue create_merge_request_in
     ]
   end