Merge branch 'disable_access_to_security_and_compliance_controllers' into 'master'

[RUN AS-IF-FOSS] Disable access to security and compliance controllers

See merge request gitlab-org/gitlab!53919

app/controllers/projects/security/configuration_controller.rb

@@ -3,6 +3,8 @@
 module Projects
   module Security
     class ConfigurationController < Projects::ApplicationController
+      include SecurityAndCompliancePermissions
+
       feature_category :static_application_security_testing
 
       def show

app/controllers/projects_controller.rb

@@ -395,6 +395,7 @@ class ProjectsController < Projects::ApplicationController
       metrics_dashboard_access_level
       analytics_access_level
       operations_access_level
+      security_and_compliance_access_level
     ]
   end
 

app/helpers/projects_helper.rb

@@ -379,10 +379,15 @@ module ProjectsHelper
   private
 
   def can_read_security_configuration?(project, current_user)
-    ::Feature.enabled?(:secure_security_and_compliance_configuration_page_on_ce, @subject, default_enabled: :yaml) &&
+    show_security_and_compliance_config? &&
+      can?(current_user, :access_security_and_compliance, project) &&
       can?(current_user, :read_security_configuration, project)
   end
 
+  def show_security_and_compliance_config?
+    ::Feature.enabled?(:secure_security_and_compliance_configuration_page_on_ce, @subject, default_enabled: :yaml)
+  end
+
   def get_project_security_nav_tabs(project, current_user)
     if can_read_security_configuration?(project, current_user)
       [:security_and_compliance, :security_configuration]
@@ -646,7 +651,8 @@ module ProjectsHelper
       metricsDashboardAccessLevel: feature.metrics_dashboard_access_level,
       operationsAccessLevel: feature.operations_access_level,
       showDefaultAwardEmojis: project.show_default_award_emojis?,
-      allowEditingCommitMessages: project.allow_editing_commit_messages?
+      allowEditingCommitMessages: project.allow_editing_commit_messages?,
+      securityAndComplianceAccessLevel: project.security_and_compliance_access_level
     }
   end
 
@@ -668,10 +674,13 @@ module ProjectsHelper
       pagesAvailable: Gitlab.config.pages.enabled,
       pagesAccessControlEnabled: Gitlab.config.pages.access_control,
       pagesAccessControlForced: ::Gitlab::Pages.access_control_is_forced?,
-      pagesHelpPath: help_page_path('user/project/pages/introduction', anchor: 'gitlab-pages-access-control')
+      pagesHelpPath: help_page_path('user/project/pages/introduction', anchor: 'gitlab-pages-access-control'),
+      securityAndComplianceAvailable: show_security_and_compliance_toggle?
     }
   end
 
+  alias_method :show_security_and_compliance_toggle?, :show_security_and_compliance_config?
+
   def project_permissions_panel_data_json(project)
     project_permissions_panel_data(project).to_json.html_safe
   end

app/models/concerns/project_features_compatibility.rb

@@ -34,6 +34,10 @@ module ProjectFeaturesCompatibility
     write_feature_attribute_boolean(:snippets_access_level, value)
   end
 
+  def security_and_compliance_enabled=(value)
+    write_feature_attribute_boolean(:security_and_compliance_access_level, value)
+  end
+
   def repository_access_level=(value)
     write_feature_attribute_string(:repository_access_level, value)
   end
@@ -78,6 +82,10 @@ module ProjectFeaturesCompatibility
     write_feature_attribute_string(:operations_access_level, value)
   end
 
+  def security_and_compliance_access_level=(value)
+    write_feature_attribute_string(:security_and_compliance_access_level, value)
+  end
+
   private
 
   def write_feature_attribute_boolean(field, value)

app/models/project.rb

@@ -392,7 +392,8 @@ class Project < ApplicationRecord
     :merge_requests_access_level, :forking_access_level, :issues_access_level,
     :wiki_access_level, :snippets_access_level, :builds_access_level,
     :repository_access_level, :pages_access_level, :metrics_dashboard_access_level, :analytics_access_level,
-    :operations_enabled?, :operations_access_level, to: :project_feature, allow_nil: true
+    :operations_enabled?, :operations_access_level, :security_and_compliance_access_level,
+    to: :project_feature, allow_nil: true
   delegate :show_default_award_emojis, :show_default_award_emojis=,
     :show_default_award_emojis?,
     to: :project_setting, allow_nil: true

app/models/project_feature.rb

@@ -3,7 +3,8 @@
 class ProjectFeature < ApplicationRecord
   include Featurable
 
-  FEATURES = %i(issues forking merge_requests wiki snippets builds repository pages metrics_dashboard analytics operations).freeze
+  FEATURES = %i(issues forking merge_requests wiki snippets builds repository pages metrics_dashboard analytics operations security_and_compliance).freeze
+  EXPORTABLE_FEATURES = (FEATURES - [:security_and_compliance]).freeze
 
   set_available_features(FEATURES)
 
@@ -37,16 +38,17 @@ class ProjectFeature < ApplicationRecord
   validate :repository_children_level
   validate :allowed_access_levels
 
-  default_value_for :builds_access_level,            value: ENABLED, allows_nil: false
-  default_value_for :issues_access_level,            value: ENABLED, allows_nil: false
-  default_value_for :forking_access_level,           value: ENABLED, allows_nil: false
-  default_value_for :merge_requests_access_level,    value: ENABLED, allows_nil: false
-  default_value_for :snippets_access_level,          value: ENABLED, allows_nil: false
-  default_value_for :wiki_access_level,              value: ENABLED, allows_nil: false
-  default_value_for :repository_access_level,        value: ENABLED, allows_nil: false
-  default_value_for :analytics_access_level,         value: ENABLED, allows_nil: false
+  default_value_for :builds_access_level, value: ENABLED, allows_nil: false
+  default_value_for :issues_access_level, value: ENABLED, allows_nil: false
+  default_value_for :forking_access_level, value: ENABLED, allows_nil: false
+  default_value_for :merge_requests_access_level, value: ENABLED, allows_nil: false
+  default_value_for :snippets_access_level, value: ENABLED, allows_nil: false
+  default_value_for :wiki_access_level, value: ENABLED, allows_nil: false
+  default_value_for :repository_access_level, value: ENABLED, allows_nil: false
+  default_value_for :analytics_access_level, value: ENABLED, allows_nil: false
   default_value_for :metrics_dashboard_access_level, value: PRIVATE, allows_nil: false
-  default_value_for :operations_access_level,        value: ENABLED, allows_nil: false
+  default_value_for :operations_access_level, value: ENABLED, allows_nil: false
+  default_value_for :security_and_compliance_access_level, value: PRIVATE, allows_nil: false
 
   default_value_for(:pages_access_level, allows_nil: false) do |feature|
     if ::Gitlab::Pages.access_control_is_forced?

app/policies/project_policy.rb

@@ -156,6 +156,7 @@ class ProjectPolicy < BasePolicy
     metrics_dashboard
     analytics
     operations
+    security_and_compliance
   ]
 
   features.each do |f|
@@ -640,6 +641,10 @@ class ProjectPolicy < BasePolicy
     enable :set_pipeline_variables
   end
 
+  rule { ~security_and_compliance_disabled & can?(:developer_access) }.policy do
+    enable :access_security_and_compliance
+  end
+
   private
 
   def user_is_user?

ee/app/controllers/ee/projects/security/configuration_controller.rb

@@ -7,8 +7,6 @@ module EE
         extend ::Gitlab::Utils::Override
 
         prepended do
-          include SecurityAndCompliancePermissions
-
           alias_method :vulnerable, :project
 
           before_action :ensure_security_dashboard_feature_enabled!, except: [:show]

ee/app/controllers/ee/projects_controller.rb

@@ -49,7 +49,7 @@ module EE
 
     override :project_feature_attributes
     def project_feature_attributes
-      super + [:requirements_access_level, :security_and_compliance_access_level]
+      super + [:requirements_access_level]
     end
 
     override :project_params_attributes

ee/app/finders/custom_project_templates_finder.rb

@@ -14,7 +14,7 @@ class CustomProjectTemplatesFinder < ::ProjectsFinder
   def execute
     scope = super
 
-    ::ProjectFeature::FEATURES.reduce(scope) do |scope, feature|
+    ::ProjectFeature::EXPORTABLE_FEATURES.reduce(scope) do |scope, feature|
       scope.with_feature_access_level(feature, ::ProjectFeature::DISABLED)
         .or(scope.with_feature_available_for_user(feature, current_user))
     end

ee/app/helpers/ee/projects_helper.rb

@@ -57,19 +57,22 @@ module EE
     override :project_permissions_settings
     def project_permissions_settings(project)
       super.merge(
-        requirementsAccessLevel: project.requirements_access_level,
-        securityAndComplianceAccessLevel: project.security_and_compliance_access_level
+        requirementsAccessLevel: project.requirements_access_level
       )
     end
 
     override :project_permissions_panel_data
     def project_permissions_panel_data(project)
       super.merge(
-        requirementsAvailable: project.feature_available?(:requirements),
-        securityAndComplianceAvailable: project.feature_available?(:security_and_compliance)
+        requirementsAvailable: project.feature_available?(:requirements)
       )
     end
 
+    override :show_security_and_compliance_toggle?
+    def show_security_and_compliance_toggle?
+      super || show_audit_events?(project)
+    end
+
     override :default_url_to_repo
     def default_url_to_repo(project = @project)
       case default_clone_protocol

ee/app/models/concerns/ee/project_features_compatibility.rb

@@ -12,13 +12,5 @@ module EE
     def requirements_access_level=(value)
       write_feature_attribute_string(:requirements_access_level, value)
     end
-
-    def security_and_compliance_enabled=(value)
-      write_feature_attribute_boolean(:security_and_compliance_access_level, value)
-    end
-
-    def security_and_compliance_access_level=(value)
-      write_feature_attribute_string(:security_and_compliance_access_level, value)
-    end
   end
 end

ee/app/models/ee/project.rb

@@ -201,7 +201,7 @@ module EE
       delegate :auto_rollback_enabled, :auto_rollback_enabled=, :auto_rollback_enabled?, to: :ci_cd_settings
       delegate :closest_gitlab_subscription, to: :namespace
 
-      delegate :requirements_access_level, :security_and_compliance_access_level, to: :project_feature, allow_nil: true
+      delegate :requirements_access_level, to: :project_feature, allow_nil: true
       delegate :pipeline_configuration_full_path, to: :compliance_management_framework, allow_nil: true
       alias_attribute :compliance_pipeline_configuration_full_path, :pipeline_configuration_full_path
 

ee/app/models/ee/project_feature.rb

@@ -4,7 +4,7 @@ module EE
   module ProjectFeature
     extend ActiveSupport::Concern
 
-    EE_FEATURES = %i(requirements security_and_compliance).freeze
+    EE_FEATURES = %i(requirements).freeze
 
     prepended do
       set_available_features(EE_FEATURES)
@@ -19,7 +19,6 @@ module EE
       end
 
       default_value_for :requirements_access_level, value: Featurable::ENABLED, allows_nil: false
-      default_value_for :security_and_compliance_access_level, value: Featurable::PRIVATE, allows_nil: false
 
       private
 

ee/app/models/license.rb

@@ -12,7 +12,6 @@ class License < ApplicationRecord
   EE_ALL_PLANS = [STARTER_PLAN, PREMIUM_PLAN, ULTIMATE_PLAN].freeze
 
   EES_FEATURES = %i[
-    security_and_compliance
     audit_events
     blocked_issues
     board_iteration_lists

ee/app/policies/ee/project_policy.rb

@@ -115,11 +115,6 @@ module EE
         @subject.feature_available?(:reject_unsigned_commits)
       end
 
-      with_scope :subject
-      condition(:security_and_compliance_enabled) do
-        @subject.feature_available?(:security_and_compliance) && access_allowed_to?(:security_and_compliance)
-      end
-
       with_scope :subject
       condition(:security_dashboard_enabled) do
         @subject.feature_available?(:security_dashboard)
@@ -234,10 +229,6 @@ module EE
 
       rule { can?(:read_project) & iterations_available }.enable :read_iteration
 
-      rule { security_and_compliance_enabled & can?(:developer_access) }.policy do
-        enable :access_security_and_compliance
-      end
-
       rule { security_dashboard_enabled & can?(:developer_access) }.policy do
         enable :read_vulnerability
         enable :read_vulnerability_scanner

ee/spec/policies/project_policy_spec.rb

@@ -515,92 +515,6 @@ RSpec.describe ProjectPolicy do
     end
   end
 
-  describe 'access_security_and_compliance' do
-    context 'when the "Security & Compliance" is enabled' do
-      before do
-        project.project_feature.update!(security_and_compliance_access_level: Featurable::PRIVATE)
-      end
-
-      %w[owner maintainer developer].each do |role|
-        context "when the role is #{role}" do
-          let(:current_user) { public_send(role) }
-
-          it { is_expected.to be_allowed(:access_security_and_compliance) }
-        end
-      end
-
-      context 'with admin' do
-        let(:current_user) { admin }
-
-        context 'when admin mode enabled', :enable_admin_mode do
-          it { is_expected.to be_allowed(:access_security_and_compliance) }
-        end
-
-        context 'when admin mode disabled' do
-          it { is_expected.to be_disallowed(:access_security_and_compliance) }
-        end
-      end
-
-      %w[reporter guest].each do |role|
-        context "when the role is #{role}" do
-          let(:current_user) { public_send(role) }
-
-          it { is_expected.to be_disallowed(:access_security_and_compliance) }
-        end
-      end
-
-      context 'with non member' do
-        let(:current_user) { non_member }
-
-        it { is_expected.to be_disallowed(:access_security_and_compliance) }
-      end
-
-      context 'with anonymous' do
-        let(:current_user) { anonymous }
-
-        it { is_expected.to be_disallowed(:access_security_and_compliance) }
-      end
-    end
-
-    context 'when the "Security & Compliance" is not enabled' do
-      before do
-        project.project_feature.update!(security_and_compliance_access_level: Featurable::DISABLED)
-      end
-
-      %w[owner maintainer developer reporter guest].each do |role|
-        context "when the role is #{role}" do
-          let(:current_user) { public_send(role) }
-
-          it { is_expected.to be_disallowed(:access_security_and_compliance) }
-        end
-      end
-
-      context 'with admin' do
-        let(:current_user) { admin }
-
-        context 'when admin mode enabled', :enable_admin_mode do
-          it { is_expected.to be_disallowed(:access_security_and_compliance) }
-        end
-
-        context 'when admin mode disabled' do
-          it { is_expected.to be_disallowed(:access_security_and_compliance) }
-        end
-      end
-
-      context 'with non member' do
-        let(:current_user) { non_member }
-
-        it { is_expected.to be_disallowed(:access_security_and_compliance) }
-      end
-
-      context 'with anonymous' do
-        let(:current_user) { anonymous }
-
-        it { is_expected.to be_disallowed(:access_security_and_compliance) }
-      end
-    end
-  end
-
   shared_context 'when security dashboard feature is not available' do
     before do
       stub_licensed_features(security_dashboard: false)

spec/models/concerns/project_features_compatibility_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe ProjectFeaturesCompatibility do
   let(:project) { create(:project) }
-  let(:features_enabled) { %w(issues wiki builds merge_requests snippets) }
+  let(:features_enabled) { %w(issues wiki builds merge_requests snippets security_and_compliance) }
   let(:features) { features_enabled + %w(repository pages operations) }
 
   # We had issues_enabled, snippets_enabled, builds_enabled, merge_requests_enabled and issues_enabled fields on projects table

spec/policies/project_policy_spec.rb

@@ -1263,4 +1263,90 @@ RSpec.describe ProjectPolicy do
       end
     end
   end
+
+  describe 'access_security_and_compliance' do
+    context 'when the "Security & Compliance" is enabled' do
+      before do
+        project.project_feature.update!(security_and_compliance_access_level: Featurable::PRIVATE)
+      end
+
+      %w[owner maintainer developer].each do |role|
+        context "when the role is #{role}" do
+          let(:current_user) { public_send(role) }
+
+          it { is_expected.to be_allowed(:access_security_and_compliance) }
+        end
+      end
+
+      context 'with admin' do
+        let(:current_user) { admin }
+
+        context 'when admin mode enabled', :enable_admin_mode do
+          it { is_expected.to be_allowed(:access_security_and_compliance) }
+        end
+
+        context 'when admin mode disabled' do
+          it { is_expected.to be_disallowed(:access_security_and_compliance) }
+        end
+      end
+
+      %w[reporter guest].each do |role|
+        context "when the role is #{role}" do
+          let(:current_user) { public_send(role) }
+
+          it { is_expected.to be_disallowed(:access_security_and_compliance) }
+        end
+      end
+
+      context 'with non member' do
+        let(:current_user) { non_member }
+
+        it { is_expected.to be_disallowed(:access_security_and_compliance) }
+      end
+
+      context 'with anonymous' do
+        let(:current_user) { anonymous }
+
+        it { is_expected.to be_disallowed(:access_security_and_compliance) }
+      end
+    end
+
+    context 'when the "Security & Compliance" is not enabled' do
+      before do
+        project.project_feature.update!(security_and_compliance_access_level: Featurable::DISABLED)
+      end
+
+      %w[owner maintainer developer reporter guest].each do |role|
+        context "when the role is #{role}" do
+          let(:current_user) { public_send(role) }
+
+          it { is_expected.to be_disallowed(:access_security_and_compliance) }
+        end
+      end
+
+      context 'with admin' do
+        let(:current_user) { admin }
+
+        context 'when admin mode enabled', :enable_admin_mode do
+          it { is_expected.to be_disallowed(:access_security_and_compliance) }
+        end
+
+        context 'when admin mode disabled' do
+          it { is_expected.to be_disallowed(:access_security_and_compliance) }
+        end
+      end
+
+      context 'with non member' do
+        let(:current_user) { non_member }
+
+        it { is_expected.to be_disallowed(:access_security_and_compliance) }
+      end
+
+      context 'with anonymous' do
+        let(:current_user) { anonymous }
+
+        it { is_expected.to be_disallowed(:access_security_and_compliance) }
+      end
+    end
+  end
 end