Merge branch 'update-compliance-framework-policy' into 'master'

Update compliance framework policy

See merge request gitlab-org/gitlab!54088

ee/app/helpers/compliance_management/compliance_framework/group_settings_helper.rb

@@ -4,7 +4,7 @@ module ComplianceManagement
   module ComplianceFramework
     module GroupSettingsHelper
       def show_compliance_frameworks?
-        License.feature_available?(:custom_compliance_frameworks) && Feature.enabled?(:ff_custom_compliance_frameworks)
+        current_user.can?(:admin_compliance_framework, @group)
       end
 
       def compliance_frameworks_list_data

ee/app/policies/compliance_management/framework_policy.rb

@@ -5,7 +5,8 @@ module ComplianceManagement
     delegate { @subject.namespace }
 
     condition(:custom_compliance_frameworks_enabled) do
-      License.feature_available?(:custom_compliance_frameworks) && Feature.enabled?(:ff_custom_compliance_frameworks)
+      @subject.namespace.feature_available?(:custom_compliance_frameworks) &&
+        Feature.enabled?(:ff_custom_compliance_frameworks, @subject.namespace)
     end
 
     rule { can?(:owner_access) & custom_compliance_frameworks_enabled }.policy do

ee/app/policies/ee/group_policy.rb

@@ -123,6 +123,11 @@ module EE
 
       condition(:eligible_for_trial, scope: :subject) { @subject.eligible_for_trial? }
 
+      condition(:compliance_framework_available) do
+        @subject.feature_available?(:custom_compliance_frameworks) &&
+          ::Feature.enabled?(:ff_custom_compliance_frameworks, @subject)
+      end
+
       rule { public_group | logged_in_viewable }.policy do
         enable :read_wiki
         enable :download_wiki_code
@@ -342,6 +347,8 @@ module EE
         prevent :create_deploy_token
         prevent :create_subgroup
       end
+
+      rule { can?(:owner_access) & compliance_framework_available }.enable :admin_compliance_framework
     end
 
     override :lookup_access_level!

ee/app/services/ee/projects/update_service.rb

@@ -81,7 +81,7 @@ module EE
           framework_identifier = settings.delete(:framework)
           if framework_identifier.blank?
             settings.merge!(_destroy: true)
-          elsif ::Feature.enabled?(:ff_custom_compliance_frameworks)
+          elsif ::Feature.enabled?(:ff_custom_compliance_frameworks, project.namespace)
             settings[:compliance_management_framework] = project.namespace.root_ancestor.compliance_management_frameworks.find(framework_identifier)
           else
             settings[:compliance_management_framework] = ComplianceManagement::Framework.find_or_create_legacy_default_framework(project, framework_identifier)

ee/app/views/compliance_management/compliance_framework/_project_settings.html.haml

 - user_has_edit_permissions = current_user.can?(:admin_compliance_framework, @project)
 .row
   .form-group.col-md-9.gl-mb-6
-    - if Feature.enabled?(:ff_custom_compliance_frameworks)
+    - if Feature.enabled?(:ff_custom_compliance_frameworks, @project.namespace)
       - frameworks = @project.namespace.root_ancestor.compliance_management_frameworks
       - if user_has_edit_permissions
         = f.fields_for :compliance_framework_setting, ComplianceManagement::ComplianceFramework::ProjectSettings.new do |cf|

ee/spec/helpers/compliance_management/compliance_framework/group_settings_helper_spec.rb

@@ -3,19 +3,21 @@
 require 'spec_helper'
 
 RSpec.describe ComplianceManagement::ComplianceFramework::GroupSettingsHelper do
-  let_it_be(:group) { build(:group) }
+  let_it_be_with_refind(:group) { create(:group) }
+  let_it_be(:current_user) { build(:admin) }
 
   before do
     assign(:group, group)
+    allow(helper).to receive(:current_user) { current_user }
   end
 
   describe '#show_compliance_frameworks?' do
     using RSpec::Parameterized::TableSyntax
 
     where(:feature_flag_enabled, :license_feature_enabled, :result) do
-      true | true | true
-      false | true | false
-      true | false | false
+      true  | true  | true
+      false | true  | false
+      true  | false | false
       false | false | false
     end
 

ee/spec/policies/compliance_management/framework_policy_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe ComplianceManagement::FrameworkPolicy do
-  let_it_be(:framework) { create(:compliance_framework) }
+  let_it_be_with_refind(:framework) { create(:compliance_framework) }
   let(:user) { framework.namespace.owner }
 
   subject { described_class.new(user, framework) }

ee/spec/policies/group_policy_spec.rb

@@ -1447,5 +1447,34 @@ RSpec.describe GroupPolicy do
         it { is_expected.to(allowed ? be_allowed(policy) : be_disallowed(policy)) }
       end
     end
+
+    describe ':admin_compliance_framework' do
+      using RSpec::Parameterized::TableSyntax
+
+      let(:policy) { :admin_compliance_framework }
+
+      where(:role, :licensed, :feature_flag, :allowed) do
+        :owner      | true  | true  | true
+        :owner      | true  | false | false
+        :owner      | false | true  | false
+        :owner      | false | false | false
+        :admin      | true  | true  | true
+        :maintainer | true  | true  | false
+        :developer  | true  | true  | false
+        :reporter   | true  | true  | false
+        :guest      | true  | true  | false
+      end
+
+      with_them do
+        let(:current_user) { public_send(role) }
+
+        before do
+          stub_licensed_features(custom_compliance_frameworks: licensed)
+          stub_feature_flags(ff_custom_compliance_frameworks: feature_flag)
+        end
+
+        it { is_expected.to(allowed ? be_allowed(policy) : be_disallowed(policy)) }
+      end
+    end
   end
 end

ee/spec/services/compliance_management/frameworks/create_service_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe ComplianceManagement::Frameworks::CreateService do
-  let_it_be(:namespace) { create(:namespace) }
+  let_it_be_with_refind(:namespace) { create(:namespace) }
   let(:params) do
     {
       name: 'GDPR',
@@ -35,7 +35,7 @@ RSpec.describe ComplianceManagement::Frameworks::CreateService do
     end
 
     context 'namespace has a parent' do
-      let_it_be(:namespace) { create(:namespace, :with_hierarchy) }
+      let_it_be_with_reload(:namespace) { create(:namespace, :with_hierarchy) }
       let(:descendant) { namespace.descendants.first }
 
       subject { described_class.new(namespace: descendant, params: params, current_user: namespace.owner) }

ee/spec/services/compliance_management/frameworks/destroy_service_spec.rb

@@ -3,8 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe ComplianceManagement::Frameworks::DestroyService do
-  let_it_be(:namespace) { create(:namespace) }
-  let_it_be(:framework) { create(:compliance_framework, namespace: namespace) }
+  let_it_be_with_refind(:namespace) { create(:namespace) }
+  let_it_be_with_refind(:framework) { create(:compliance_framework, namespace: namespace) }
 
   context 'when feature is disabled' do
     before do

ee/spec/services/compliance_management/frameworks/update_service_spec.rb

@@ -3,8 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe ComplianceManagement::Frameworks::UpdateService do
-  let_it_be(:namespace) { create(:namespace) }
-  let_it_be(:framework) { create(:compliance_framework, namespace: namespace) }
+  let_it_be_with_refind(:namespace) { create(:namespace) }
+  let_it_be_with_refind(:framework) { create(:compliance_framework, namespace: namespace) }
   let(:current_user) { namespace.owner }
   let(:params) { { color: '#000001', description: 'New Description', name: 'New Name' } }