Modify container_image policies to check project_feature

*_container_image policies should now check
project_feature.container_registry_access_level instead of
projects.container_registry_enabled.

app/models/project_feature.rb

@@ -24,7 +24,11 @@ class ProjectFeature < ApplicationRecord
 
   set_available_features(FEATURES)
 
-  PRIVATE_FEATURES_MIN_ACCESS_LEVEL = { merge_requests: Gitlab::Access::REPORTER, metrics_dashboard: Gitlab::Access::REPORTER }.freeze
+  PRIVATE_FEATURES_MIN_ACCESS_LEVEL = {
+    merge_requests: Gitlab::Access::REPORTER,
+    metrics_dashboard: Gitlab::Access::REPORTER,
+    container_registry: Gitlab::Access::REPORTER
+  }.freeze
   PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT = { repository: Gitlab::Access::REPORTER }.freeze
 
   class << self

app/policies/project_policy.rb

@@ -51,7 +51,7 @@ class ProjectPolicy < BasePolicy
 
   desc "Container registry is disabled"
   condition(:container_registry_disabled, scope: :subject) do
-    !project.container_registry_enabled
+    !access_allowed_to?(:container_registry)
   end
 
   desc "Project has an external wiki"

ee/spec/policies/project_policy_spec.rb

@@ -9,6 +9,8 @@ RSpec.describe ProjectPolicy do
 
   let(:project) { public_project }
 
+  let_it_be(:auditor) { create(:user, :auditor) }
+
   subject { described_class.new(current_user, project) }
 
   before do
@@ -58,7 +60,7 @@ RSpec.describe ProjectPolicy do
     it_behaves_like 'project policies as admin without admin mode'
 
     context 'auditor' do
-      let(:current_user) { create(:user, :auditor) }
+      let(:current_user) { auditor }
 
       before do
         stub_licensed_features(security_dashboard: true, license_scanning: true, threat_monitoring: true)
@@ -91,6 +93,45 @@ RSpec.describe ProjectPolicy do
       it_behaves_like 'project private features with read_all_resources ability' do
         let(:user) { current_user }
       end
+
+      context 'with project feature related policies' do
+        using RSpec::Parameterized::TableSyntax
+
+        project_features = {
+          container_registry_access_level: [:read_container_image],
+          merge_requests_access_level: [:read_merge_request]
+        }
+
+        where(:project_visibility, :access_level, :allowed) do
+          :public   | ProjectFeature::ENABLED  | true
+          :public   | ProjectFeature::PRIVATE  | true
+          :public   | ProjectFeature::DISABLED | false
+          :internal | ProjectFeature::ENABLED  | true
+          :internal | ProjectFeature::PRIVATE  | true
+          :internal | ProjectFeature::DISABLED | false
+          :private  | ProjectFeature::ENABLED  | true
+          :private  | ProjectFeature::PRIVATE  | true
+          :private  | ProjectFeature::DISABLED | false
+        end
+
+        # For each project feature, check that an auditor is always allowed read
+        # permissions unless the feature is disabled.
+        project_features.each do |feature, permissions|
+          with_them do
+            let(:project) { send("#{project_visibility}_project") }
+
+            it 'always allows permissions except when feature disabled' do
+              project.project_feature.update!("#{feature}": access_level)
+
+              if allowed
+                expect_allowed(*permissions)
+              else
+                expect_disallowed(*permissions)
+              end
+            end
+          end
+        end
+      end
     end
   end
 

spec/policies/project_policy_spec.rb

@@ -1434,4 +1434,98 @@ RSpec.describe ProjectPolicy do
       end
     end
   end
+
+  describe 'container_image policies' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:guest_operations_permissions) { [:read_container_image] }
+
+    let(:developer_operations_permissions) do
+      guest_operations_permissions + [
+        :create_container_image, :update_container_image, :destroy_container_image
+      ]
+    end
+
+    let(:maintainer_operations_permissions) do
+      developer_operations_permissions + [
+        :admin_container_image
+      ]
+    end
+
+    where(:project_visibility, :access_level, :role, :allowed) do
+      :public   | ProjectFeature::ENABLED   | :maintainer | true
+      :public   | ProjectFeature::ENABLED   | :developer  | true
+      :public   | ProjectFeature::ENABLED   | :reporter   | true
+      :public   | ProjectFeature::ENABLED   | :guest      | true
+      :public   | ProjectFeature::ENABLED   | :anonymous  | true
+      :public   | ProjectFeature::PRIVATE   | :maintainer | true
+      :public   | ProjectFeature::PRIVATE   | :developer  | true
+      :public   | ProjectFeature::PRIVATE   | :reporter   | true
+      :public   | ProjectFeature::PRIVATE   | :guest      | false
+      :public   | ProjectFeature::PRIVATE   | :anonymous  | false
+      :public   | ProjectFeature::DISABLED  | :maintainer | false
+      :public   | ProjectFeature::DISABLED  | :developer  | false
+      :public   | ProjectFeature::DISABLED  | :reporter   | false
+      :public   | ProjectFeature::DISABLED  | :guest      | false
+      :public   | ProjectFeature::DISABLED  | :anonymous  | false
+      :internal | ProjectFeature::ENABLED   | :maintainer | true
+      :internal | ProjectFeature::ENABLED   | :developer  | true
+      :internal | ProjectFeature::ENABLED   | :reporter   | true
+      :internal | ProjectFeature::ENABLED   | :guest      | true
+      :internal | ProjectFeature::ENABLED   | :anonymous  | false
+      :internal | ProjectFeature::PRIVATE   | :maintainer | true
+      :internal | ProjectFeature::PRIVATE   | :developer  | true
+      :internal | ProjectFeature::PRIVATE   | :reporter   | true
+      :internal | ProjectFeature::PRIVATE   | :guest      | false
+      :internal | ProjectFeature::PRIVATE   | :anonymous  | false
+      :internal | ProjectFeature::DISABLED  | :maintainer | false
+      :internal | ProjectFeature::DISABLED  | :developer  | false
+      :internal | ProjectFeature::DISABLED  | :reporter   | false
+      :internal | ProjectFeature::DISABLED  | :guest      | false
+      :internal | ProjectFeature::DISABLED  | :anonymous  | false
+      :private  | ProjectFeature::ENABLED   | :maintainer | true
+      :private  | ProjectFeature::ENABLED   | :developer  | true
+      :private  | ProjectFeature::ENABLED   | :reporter   | true
+      :private  | ProjectFeature::ENABLED   | :guest      | false
+      :private  | ProjectFeature::ENABLED   | :anonymous  | false
+      :private  | ProjectFeature::PRIVATE   | :maintainer | true
+      :private  | ProjectFeature::PRIVATE   | :developer  | true
+      :private  | ProjectFeature::PRIVATE   | :reporter   | true
+      :private  | ProjectFeature::PRIVATE   | :guest      | false
+      :private  | ProjectFeature::PRIVATE   | :anonymous  | false
+      :private  | ProjectFeature::DISABLED  | :maintainer | false
+      :private  | ProjectFeature::DISABLED  | :developer  | false
+      :private  | ProjectFeature::DISABLED  | :reporter   | false
+      :private  | ProjectFeature::DISABLED  | :guest      | false
+      :private  | ProjectFeature::DISABLED  | :anonymous  | false
+    end
+
+    with_them do
+      let(:current_user) { send(role) }
+      let(:project) { send("#{project_visibility}_project") }
+
+      it 'allows/disallows the abilities based on the container_registry feature access level' do
+        project.project_feature.update!(container_registry_access_level: access_level)
+
+        if allowed
+          expect_allowed(*permissions_abilities(role))
+        else
+          expect_disallowed(*permissions_abilities(role))
+        end
+      end
+
+      def permissions_abilities(role)
+        case role
+        when :maintainer
+          maintainer_operations_permissions
+        when :developer
+          developer_operations_permissions
+        when :reporter, :guest, :anonymous
+          guest_operations_permissions
+        else
+          raise "Unknown role #{role}"
+        end
+      end
+    end
+  end
 end