Merge branch 'security-id-fix-approval-rules-access-14-8' into '14-8-stable-ee'

Fix access for approval rules API

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

app/policies/project_policy.rb

@@ -489,7 +489,7 @@ class ProjectPolicy < BasePolicy
     prevent(*create_read_update_admin_destroy(:issue_board_list))
   end
-  rule { merge_requests_disabled | repository_disabled }.policy do
+  rule { merge_requests_disabled | repository_disabled | ~can?(:download_code) }.policy do
     prevent :create_merge_request_in
     prevent :create_merge_request_from
     prevent(*create_read_update_admin_destroy(:merge_request))

spec/policies/project_policy_spec.rb

@@ -81,25 +81,24 @@ RSpec.describe ProjectPolicy do
   context 'merge requests feature' do
     let(:current_user) { owner }
+    let(:mr_permissions) do
+      [:create_merge_request_from, :read_merge_request, :update_merge_request,
+       :admin_merge_request, :create_merge_request_in]
+    end
     it 'disallows all permissions when the feature is disabled' do
       project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
-      mr_permissions = [:create_merge_request_from, :read_merge_request,
-                        :update_merge_request, :admin_merge_request,
-                        :create_merge_request_in]
-
       expect_disallowed(*mr_permissions)
     end
-  end
-  context 'for a guest in a private project' do
-    let(:current_user) { guest }
-    let(:project) { private_project }
-    it 'disallows the guest from reading the merge request and merge request iid' do
-      expect_disallowed(:read_merge_request)
-      expect_disallowed(:read_merge_request_iid)
+    context 'for a guest in a private project' do
+      let(:current_user) { guest }
+      let(:project) { private_project }
+      it 'disallows the guest from all merge request permissions' do
+        expect_disallowed(*mr_permissions)
+      end
     end
   end

spec/support/shared_contexts/policies/project_policy_shared_context.rb

@@ -26,7 +26,7 @@ RSpec.shared_context 'ProjectPolicy context' do
   let(:base_reporter_permissions) do
     %i[
       admin_issue admin_issue_link admin_label admin_issue_board_list
-      create_snippet create_incident daily_statistics download_code
+      create_snippet create_incident daily_statistics create_merge_request_in download_code
       download_wiki_code fork_project metrics_dashboard read_build
       read_commit_status read_confidential_issues read_container_image
       read_deployment read_environment read_merge_request
@@ -66,7 +66,7 @@ RSpec.shared_context 'ProjectPolicy context' do
   let(:public_permissions) do
     %i[
-      build_download_code build_read_container_image download_code
+      build_download_code build_read_container_image create_merge_request_in download_code
       download_wiki_code fork_project read_commit_status read_container_image
       read_pipeline read_release
     ]

spec/support/shared_examples/policies/project_policy_shared_examples.rb

@@ -107,6 +107,19 @@ RSpec.shared_examples 'deploy token does not get confused with user' do
 end
 RSpec.shared_examples 'project policies as guest' do
+  context 'abilities for public projects' do
+    let(:project) { public_project }
+    let(:current_user) { guest }
+
+    it do
+      expect_allowed(*guest_permissions)
+      expect_allowed(*public_permissions)
+      expect_disallowed(*developer_permissions)
+      expect_disallowed(*maintainer_permissions)
+      expect_disallowed(*owner_permissions)
+    end
+  end
+
   context 'abilities for non-public projects' do
     let(:project) { private_project }
     let(:current_user) { guest }