Merge branch 'fix_compliance_framework_list_api' into 'master'

Fix Namespace list API for compliance framework

See merge request gitlab-org/gitlab!55117

ee/app/graphql/ee/types/namespace_type.rb

@@ -57,12 +57,8 @@ module EE
         field :compliance_frameworks,
               ::Types::ComplianceManagement::ComplianceFrameworkType.connection_type,
               null: true,
-              description: 'Compliance frameworks available to projects in this namespace.',
-              feature_flag: :ff_custom_compliance_frameworks do
-                argument :id, ::Types::GlobalIDType[::ComplianceManagement::Framework],
-                         description: 'Global ID of a specific compliance framework to return.',
-                         required: false
-              end
+              description: 'Compliance frameworks available to projects in this namespace. Available only when feature flag `ff_custom_compliance_frameworks` is enabled.',
+              resolver: ::Resolvers::ComplianceManagement::FrameworkResolver
 
         def additional_purchased_storage_size
           object.additional_purchased_storage_size.megabytes
@@ -71,26 +67,6 @@ module EE
         def storage_size_limit
           object.root_storage_size.limit
         end
-
-        def compliance_frameworks(id: nil)
-          id = ::Types::GlobalIDType[::ComplianceManagement::Framework].coerce_isolated_input(id) unless id.nil?
-          BatchLoader::GraphQL
-            .for([object.id, id&.model_id])
-            .batch(default_value: []) do |keys, loader|
-            namespace_ids = keys.map(&:first).uniq
-            by_namespace_id = keys.group_by(&:first).transform_values { |k| k.map(&:second) }
-            frameworks = ::ComplianceManagement::Framework.with_namespaces(namespace_ids)
-            frameworks.group_by(&:namespace_id).each do |ns_id, group|
-              by_namespace_id[ns_id].each do |fw_id|
-                group.each do |fw|
-                  next unless fw_id.nil? || fw_id.to_i == fw.id
-
-                  loader.call([ns_id, fw_id]) { |array| array << fw }
-                end
-              end
-            end
-          end
-        end
       end
     end
   end

ee/app/graphql/resolvers/compliance_management/framework_resolver.rb

+# frozen_string_literal: true
+
+module Resolvers
+  module ComplianceManagement
+    class FrameworkResolver < BaseResolver
+      include Gitlab::Graphql::Authorize::AuthorizeResource
+
+      type ::Types::ComplianceManagement::ComplianceFrameworkType.connection_type, null: true
+
+      argument :id, ::Types::GlobalIDType[::ComplianceManagement::Framework],
+               description: 'Global ID of a specific compliance framework to return.',
+               required: false
+
+      def resolve(id: nil)
+        # TODO: remove this line when the compatibility layer is removed
+        # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+        id = ::Types::GlobalIDType[::ComplianceManagement::Framework].coerce_isolated_input(id) unless id.nil?
+
+        BatchLoader::GraphQL
+          .for([object.id, id&.model_id])
+          .batch(default_value: []) do |keys, loader|
+          namespace_ids = keys.map(&:first).uniq
+          by_namespace_id = keys.group_by(&:first).transform_values { |k| k.map(&:second) }
+
+          evaluate(namespace_ids, by_namespace_id, loader)
+        end
+      end
+
+      private
+
+      def evaluate(namespace_ids, by_namespace_id, loader)
+        frameworks(namespace_ids).group_by(&:namespace_id).each do |ns_id, group|
+          by_namespace_id[ns_id].each do |fw_id|
+            authorize!
+
+            group.each do |fw|
+              next unless fw_id.nil? || fw_id.to_i == fw.id
+
+              loader.call([ns_id, fw_id]) { |array| array << fw }
+            end
+          end
+        end
+      end
+
+      def frameworks(namespace_ids)
+        ::ComplianceManagement::Framework.with_namespaces(namespace_ids)
+      end
+
+      def authorize!
+        Ability.allowed?(context[:current_user], :admin_compliance_framework, object) || raise_resource_not_available_error!
+      end
+    end
+  end
+end

ee/app/policies/ee/namespace_policy.rb

@@ -6,12 +6,17 @@ module EE
 
     prepended do
       condition(:over_storage_limit, scope: :subject) { @subject.over_storage_limit? }
+      condition(:compliance_framework_available) do
+        @subject.feature_available?(:custom_compliance_frameworks) &&
+          ::Feature.enabled?(:ff_custom_compliance_frameworks, @subject)
+      end
 
       rule { admin & is_gitlab_com }.enable :update_subscription_limit
 
       rule { over_storage_limit }.policy do
         prevent :create_projects
       end
+      rule { can?(:owner_access) & compliance_framework_available }.enable :admin_compliance_framework
     end
   end
 end

ee/spec/policies/namespace_policy_spec.rb

@@ -5,6 +5,7 @@ require 'spec_helper'
 RSpec.describe NamespacePolicy do
   let(:owner) { build_stubbed(:user) }
   let(:namespace) { build_stubbed(:namespace, owner: owner) }
+  let(:admin) { build_stubbed(:admin) }
   let(:owner_permissions) { [:create_projects, :admin_namespace, :read_namespace] }
 
   subject { described_class.new(current_user, namespace) }
@@ -43,5 +44,53 @@ RSpec.describe NamespacePolicy do
     end
   end
 
+  context ':admin_compliance_framework' do
+    shared_examples 'permitted' do
+      it { is_expected.to(be_allowed(:admin_compliance_framework)) }
+    end
+
+    shared_examples 'not permitted' do
+      it { is_expected.to(be_disallowed(:admin_compliance_framework)) }
+    end
+
+    context 'when feature is available' do
+      before do
+        stub_licensed_features(custom_compliance_frameworks: true)
+        stub_feature_flags(ff_custom_compliance_frameworks: true)
+      end
+
+      context 'when user is admin', :enable_admin_mode do
+        let(:current_user) { admin }
+
+        it_behaves_like 'permitted'
+      end
+
+      context 'when user is owner' do
+        let(:current_user) { owner }
+
+        it_behaves_like 'permitted'
+      end
+    end
+
+    context 'when feature is not available' do
+      before do
+        stub_licensed_features(custom_compliance_frameworks: false)
+        stub_feature_flags(ff_custom_compliance_frameworks: false)
+      end
+
+      context 'when user is admin', :enable_admin_mode do
+        let(:current_user) { admin }
+
+        it_behaves_like 'not permitted'
+      end
+
+      context 'when user is owner' do
+        let(:current_user) { owner }
+
+        it_behaves_like 'not permitted'
+      end
+    end
+  end
+
   it_behaves_like 'update namespace limit policy'
 end

ee/spec/requests/api/graphql/namespace/compliance_frameworks_spec.rb

@@ -17,6 +17,11 @@ RSpec.describe 'getting a list of compliance frameworks for a root namespace' do
   end
 
   context 'when authenticated as the namespace owner' do
+    before do
+      stub_licensed_features(custom_compliance_frameworks: true)
+      stub_feature_flags(ff_custom_compliance_frameworks: true)
+    end
+
     let(:current_user) { namespace.owner }
 
     it 'returns the groups compliance frameworks' do
@@ -120,7 +125,7 @@ RSpec.describe 'getting a list of compliance frameworks for a root namespace' do
       it 'responds with error when querying a compliance framework' do
         post_graphql(query, current_user: current_user)
 
-        expect(graphql_errors).to contain_exactly(include('message' => "Field 'complianceFrameworks' doesn't exist on type 'Namespace'"))
+        expect(graphql_errors).to contain_exactly(include('message' => "The resource that you are attempting to access does not exist or you don't have permission to perform this action"))
       end
     end
   end