Merge branch...

Merge branch '347069-update-graphql-mutation-to-commit-security-policy-project-to-a-group' into 'master'

Update mutation to commit security policy to a group

See merge request gitlab-org/gitlab!83188

doc/api/graphql/reference/index.md

@@ -4305,7 +4305,7 @@ Input type: `SavedReplyUpdateInput`
 
 ### `Mutation.scanExecutionPolicyCommit`
 
-Commits the `policy_yaml` content to the assigned security policy project for the given project(`project_path`).
+Commits the `policy_yaml` content to the assigned security policy project for the given project (`full_path`).
 
 Input type: `ScanExecutionPolicyCommitInput`
 
@@ -4314,10 +4314,11 @@ Input type: `ScanExecutionPolicyCommitInput`
 | Name | Type | Description |
 | ---- | ---- | ----------- |
 | <a id="mutationscanexecutionpolicycommitclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationscanexecutionpolicycommitfullpath"></a>`fullPath` | [`String`](#string) | Full path of the project. |
 | <a id="mutationscanexecutionpolicycommitname"></a>`name` | [`String`](#string) | Name of the policy. If the name is null, the `name` field from `policy_yaml` is used. |
 | <a id="mutationscanexecutionpolicycommitoperationmode"></a>`operationMode` | [`MutationOperationMode!`](#mutationoperationmode) | Changes the operation mode. |
 | <a id="mutationscanexecutionpolicycommitpolicyyaml"></a>`policyYaml` | [`String!`](#string) | YAML snippet of the policy. |
-| <a id="mutationscanexecutionpolicycommitprojectpath"></a>`projectPath` | [`ID!`](#id) | Full path of the project. |
+| <a id="mutationscanexecutionpolicycommitprojectpath"></a>`projectPath` **{warning-solid}** | [`ID`](#id) | **Deprecated:** Use `fullPath`. Deprecated in 14.10. |
 
 #### Fields
 

ee/app/graphql/mutations/concerns/mutations/finds_project_or_group_for_security_policies.rb

+# frozen_string_literal: true
+
+module Mutations
+  module FindsProjectOrGroupForSecurityPolicies
+    private
+
+    def find_object(args)
+      full_path = args[:project_path].presence || args[:full_path]
+
+      if full_path.blank?
+        raise Gitlab::Graphql::Errors::ArgumentError,
+              'At least one of the arguments fullPath or projectPath is required'
+      end
+
+      project = find_project(full_path)
+      group = find_group(full_path) if project.nil?
+
+      raise_resource_not_available_error! if group.nil? && project.nil?
+
+      project || group
+    end
+
+    def find_project(full_path)
+      Project.find_by_full_path(full_path)
+    end
+
+    def find_group(full_path)
+      Group
+        .find_by_full_path(full_path)
+        .then { |group| Feature.enabled?(:group_level_security_policies, group, default_enabled: :yaml) ? group : nil }
+    end
+  end
+end

ee/app/graphql/mutations/security_policy/commit_scan_execution_policy.rb

@@ -4,14 +4,19 @@ module Mutations
   module SecurityPolicy
     class CommitScanExecutionPolicy < BaseMutation
       graphql_name 'ScanExecutionPolicyCommit'
-      description 'Commits the `policy_yaml` content to the assigned security policy project for the given project(`project_path`)'
+      description 'Commits the `policy_yaml` content to the assigned security policy project for the given project (`full_path`)'
 
-      include FindsProject
+      include FindsProjectOrGroupForSecurityPolicies
 
       authorize :security_orchestration_policies
 
+      argument :full_path, GraphQL::Types::String,
+               required: false,
+               description: 'Full path of the project.'
+
       argument :project_path, GraphQL::Types::ID,
-               required: true,
+               required: false,
+               deprecated: { reason: 'Use `fullPath`', milestone: '14.10' },
                description: 'Full path of the project.'
 
       argument :policy_yaml, GraphQL::Types::String,
@@ -33,9 +38,9 @@ module Mutations
             description: 'Name of the branch to which the policy changes are committed.'
 
       def resolve(args)
-        project = authorized_find!(args[:project_path])
+        project_or_group = authorized_find!(**args)
 
-        result = commit_policy(project, args)
+        result = commit_policy(project_or_group, args)
         error_message = result[:status] == :error ? result[:message] : nil
         error_details = result[:status] == :error ? result[:details] : nil
 
@@ -47,9 +52,9 @@ module Mutations
 
       private
 
-      def commit_policy(project, args)
+      def commit_policy(project_or_group, args)
         ::Security::SecurityOrchestrationPolicies::PolicyCommitService
-          .new(project: project, current_user: current_user, params: {
+          .new(container: project_or_group, current_user: current_user, params: {
             name: args[:name],
             policy_yaml: args[:policy_yaml],
             operation: Types::MutationOperationModeEnum.enum.key(args[:operation_mode]).to_sym

ee/app/policies/ee/group_policy.rb

@@ -140,6 +140,10 @@ module EE
         @subject.feature_available?(:evaluate_group_level_compliance_pipeline)
       end
 
+      condition(:security_orchestration_policies_enabled) do
+        @subject.feature_available?(:security_orchestration_policies)
+      end
+
       rule { public_group | logged_in_viewable }.policy do
         enable :read_wiki
         enable :download_wiki_code
@@ -397,6 +401,14 @@ module EE
       rule { can?(:owner_access) & external_audit_events_available }.policy do
         enable :admin_external_audit_events
       end
+
+      rule { security_orchestration_policies_enabled & can?(:developer_access) }.policy do
+        enable :security_orchestration_policies
+      end
+
+      rule { security_orchestration_policies_enabled & can?(:owner_access) }.policy do
+        enable :update_security_orchestration_policy_project
+      end
     end
 
     override :lookup_access_level!

ee/app/services/security/security_orchestration_policies/policy_commit_service.rb

@@ -2,9 +2,9 @@
 
 module Security
   module SecurityOrchestrationPolicies
-    class PolicyCommitService < ::BaseProjectService
+    class PolicyCommitService < ::BaseContainerService
       def execute
-        @policy_configuration = project.security_orchestration_policy_configuration
+        @policy_configuration = container.security_orchestration_policy_configuration
 
         return error('Security Policy Project does not exist') unless policy_configuration.present?
 
@@ -26,7 +26,7 @@ module Security
 
       def validate_policy_yaml
         Security::SecurityOrchestrationPolicies::ValidatePolicyService
-          .new(project: project, params: { policy: policy })
+          .new(container: container, params: { policy: policy })
           .execute
       end
 
@@ -82,7 +82,7 @@ module Security
         @policy ||= Gitlab::Config::Loader::Yaml.new(params[:policy_yaml]).load!
       end
 
-      attr_reader :project, :policy_configuration
+      attr_reader :policy_configuration
     end
   end
 end

ee/app/services/security/security_orchestration_policies/validate_policy_service.rb

@@ -2,7 +2,9 @@
 
 module Security
   module SecurityOrchestrationPolicies
-    class ValidatePolicyService < ::BaseProjectService
+    class ValidatePolicyService < ::BaseContainerService
+      include ::Gitlab::Utils::StrongMemoize
+
       def execute
         return error(s_('SecurityOrchestration|Empty policy name')) if blank_name?
 
@@ -38,7 +40,8 @@ module Security
       end
 
       def missing_branch_for_rule?
-        return false if project.blank?
+        return false if container.blank?
+        return false unless project_container?
 
         missing_branch_names.present?
       end
@@ -60,7 +63,7 @@ module Security
 
       def branches_for_project
         strong_memoize(:branches_for_project) do
-          repository.branch_names
+          container.repository.branch_names
         end
       end
 

ee/config/feature_flags/development/group_level_security_policies.yml

+---
+name: group_level_security_policies
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82754
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/356258
+milestone: '14.10'
+type: development
+group: group::container security
+default_enabled: false

ee/spec/graphql/mutations/security_policy/commit_scan_execution_policy_spec.rb

@@ -7,17 +7,19 @@ RSpec.describe Mutations::SecurityPolicy::CommitScanExecutionPolicy do
   describe '#resolve' do
     let_it_be(:user) { create(:user) }
     let_it_be(:project) { create(:project, :repository, namespace: user.namespace) }
-    let_it_be(:policy_management_project) { create(:project, :repository, namespace: user.namespace) }
-    let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, security_policy_management_project: policy_management_project, project: project) }
+    let_it_be(:namespace) { create(:group) }
+    let_it_be(:project_policy_management_project) { create(:project, :repository, namespace: user.namespace) }
+    let_it_be(:namespace_policy_management_project) { create(:project, :repository, namespace: namespace) }
     let_it_be(:operation_mode) { Types::MutationOperationModeEnum.enum[:append] }
     let_it_be(:policy_name) { 'Test Policy' }
     let_it_be(:policy_yaml) { build(:scan_execution_policy, name: policy_name).merge(type: 'scan_execution_policy').to_yaml }
 
-    subject { mutation.resolve(project_path: project.full_path, name: policy_name, policy_yaml: policy_yaml, operation_mode: operation_mode) }
+    subject { mutation.resolve(full_path: container.full_path, name: policy_name, policy_yaml: policy_yaml, operation_mode: operation_mode) }
 
+    shared_context 'commits scan execution policies' do
       context 'when permission is set for user' do
         before do
-        project.add_maintainer(user)
+          container.add_maintainer(user)
 
           stub_licensed_features(security_orchestration_policies: true)
         end
@@ -40,4 +42,50 @@ RSpec.describe Mutations::SecurityPolicy::CommitScanExecutionPolicy do
         end
       end
     end
+
+    context 'when both fullPath and projectPath are not provided' do
+      subject { mutation.resolve(name: policy_name, policy_yaml: policy_yaml, operation_mode: operation_mode) }
+
+      before do
+        stub_licensed_features(security_orchestration_policies: true)
+      end
+
+      it 'raises exception' do
+        expect { subject }.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
+      end
+    end
+
+    context 'for project' do
+      let_it_be_with_refind(:policy_configuration) { create(:security_orchestration_policy_configuration, security_policy_management_project: project_policy_management_project, project: project) }
+
+      let(:container) { project }
+
+      it_behaves_like 'commits scan execution policies'
+    end
+
+    context 'for namespace' do
+      let_it_be_with_refind(:policy_configuration) { create(:security_orchestration_policy_configuration, :namespace, security_policy_management_project: namespace_policy_management_project, namespace: namespace) }
+
+      let(:container) { namespace }
+
+      context 'when feature is enabled' do
+        before do
+          stub_feature_flags(group_level_security_policies: namespace)
+        end
+
+        it_behaves_like 'commits scan execution policies'
+      end
+
+      context 'when feature is disabled' do
+        before do
+          stub_licensed_features(security_orchestration_policies: true)
+          stub_feature_flags(group_level_security_policies: false)
+        end
+
+        it 'raises exception' do
+          expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
+        end
+      end
+    end
+  end
 end

ee/spec/requests/api/graphql/mutations/security_policy/commit_scan_execution_policy_spec.rb

@@ -2,16 +2,19 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Create scan execution policy for a project' do
+RSpec.describe 'Create scan execution policy for a project/namespace' do
   include GraphqlHelpers
 
   let_it_be(:current_user) { create(:user) }
   let_it_be(:project) { create(:project, :repository, namespace: current_user.namespace) }
+  let_it_be(:namespace) { create(:group) }
+  let_it_be(:project_security_policy_management_project) { create(:project, :repository, namespace: current_user.namespace) }
+  let_it_be(:namespace_security_policy_management_project) { create(:project, :repository, namespace: namespace) }
   let_it_be(:policy_name) { 'Test Policy' }
   let_it_be(:policy_yaml) { build(:scan_execution_policy, name: policy_name).merge(type: 'scan_execution_policy').to_yaml }
 
   def mutation
-    variables = { project_path: project.full_path, name: policy_name, policy_yaml: policy_yaml, operation_mode: 'APPEND' }
+    variables = { full_path: container.full_path, name: policy_name, policy_yaml: policy_yaml, operation_mode: 'APPEND' }
 
     graphql_mutation(:scan_execution_policy_commit, variables) do
       <<-QL.strip_heredoc
@@ -26,13 +29,10 @@ RSpec.describe 'Create scan execution policy for a project' do
     graphql_mutation_response(:scan_execution_policy_commit)
   end
 
-  context 'when security_orchestration_policies_configuration already exists for project' do
-    let_it_be(:security_policy_management_project) { create(:project, :repository, namespace: current_user.namespace) }
-    let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, project: project, security_policy_management_project: security_policy_management_project) }
-
+  shared_context 'commits scan execution policies' do
     before do
-      project.add_maintainer(current_user)
-      security_policy_management_project.add_developer(current_user)
+      container.add_maintainer(current_user)
+      container_security_policy_management_project.add_developer(current_user)
 
       stub_licensed_features(security_orchestration_policies: true)
     end
@@ -41,7 +41,7 @@ RSpec.describe 'Create scan execution policy for a project' do
       post_graphql_mutation(mutation, current_user: current_user)
 
       branch = mutation_response['branch']
-      commit = security_policy_management_project.repository.commits(branch, limit: 5).first
+      commit = container_security_policy_management_project.repository.commits(branch, limit: 5).first
       expect(response).to have_gitlab_http_status(:success)
       expect(branch).not_to be_nil
       expect(commit.message).to eq('Add a new policy to .gitlab/security-policies/policy.yml')
@@ -57,4 +57,40 @@ RSpec.describe 'Create scan execution policy for a project' do
       end
     end
   end
+
+  context 'for project' do
+    let(:container_security_policy_management_project) { project_security_policy_management_project }
+
+    let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, project: project, security_policy_management_project: project_security_policy_management_project) }
+
+    let(:container) { project }
+
+    it_behaves_like 'commits scan execution policies'
+  end
+
+  context 'for namespace' do
+    let(:container_security_policy_management_project) { namespace_security_policy_management_project }
+
+    let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, :namespace, namespace: namespace, security_policy_management_project: namespace_security_policy_management_project) }
+
+    let(:container) { namespace }
+
+    context 'when feature is enabled' do
+      before do
+        stub_feature_flags(group_level_security_policies: namespace)
+      end
+
+      it_behaves_like 'commits scan execution policies'
+    end
+
+    context 'when feature is disabled' do
+      before do
+        stub_licensed_features(security_orchestration_policies: true)
+        stub_feature_flags(group_level_security_policies: false)
+      end
+
+      it_behaves_like 'a mutation that returns top-level errors',
+                  errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
+    end
+  end
 end

ee/spec/services/security/security_orchestration_policies/policy_commit_service_spec.rb

@@ -6,10 +6,8 @@ RSpec.describe Security::SecurityOrchestrationPolicies::PolicyCommitService do
   include RepoHelpers
 
   describe '#execute' do
+    let_it_be(:group) { create(:group) }
     let_it_be(:project) { create(:project, :repository) }
-    let_it_be(:current_user) { project.first_owner }
-    let_it_be(:policy_management_project) { create(:project, :repository, creator: current_user) }
-    let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, security_policy_management_project: policy_management_project, project: project) }
 
     let(:policy_hash) { build(:scan_execution_policy, name: 'Test Policy') }
     let(:input_policy_yaml) { policy_hash.merge(type: 'scan_execution_policy').to_yaml }
@@ -20,13 +18,14 @@ RSpec.describe Security::SecurityOrchestrationPolicies::PolicyCommitService do
     let(:params) { { policy_yaml: input_policy_yaml, name: policy_name, operation: operation } }
 
     subject(:service) do
-      described_class.new(project: project, current_user: current_user, params: params)
+      described_class.new(container: container, current_user: current_user, params: params)
     end
 
     around do |example|
       Timecop.scale(60) { example.run }
     end
 
+    shared_examples 'commits policy to associated project' do
       context 'when policy_yaml is invalid' do
         let(:invalid_input_policy_yaml) do
           <<-EOS
@@ -60,8 +59,8 @@ RSpec.describe Security::SecurityOrchestrationPolicies::PolicyCommitService do
         end
       end
 
-    context 'when security_orchestration_policies_configuration does not exist for project' do
-      let_it_be(:project) { create(:project, :repository) }
+      context 'when security_orchestration_policies_configuration does not exist for container' do
+        let_it_be(:container) { create(:project, :repository) }
 
         it 'does not create new project' do
           response = service.execute
@@ -127,4 +126,29 @@ RSpec.describe Security::SecurityOrchestrationPolicies::PolicyCommitService do
         end
       end
     end
+
+    context 'when service is used for project' do
+      let_it_be(:container) { project }
+      let_it_be(:current_user) { project.first_owner }
+
+      let_it_be(:policy_management_project) { create(:project, :repository, creator: current_user) }
+      let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, security_policy_management_project: policy_management_project, project: project) }
+
+      it_behaves_like 'commits policy to associated project'
+    end
+
+    context 'when service is used for namespace' do
+      let_it_be(:container) { group }
+      let_it_be(:current_user) { create(:user) }
+
+      let_it_be(:policy_management_project) { create(:project, :repository, creator: current_user) }
+      let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, :namespace, security_policy_management_project: policy_management_project, namespace: group) }
+
+      before do
+        group.add_owner(current_user)
+      end
+
+      it_behaves_like 'commits policy to associated project'
+    end
+  end
 end

ee/spec/services/security/security_orchestration_policies/validate_policy_service_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe Security::SecurityOrchestrationPolicies::ValidatePolicyService do
   describe '#execute' do
-    let(:service) { described_class.new(project: project, params: { policy: policy }) }
+    let(:service) { described_class.new(container: container, params: { policy: policy }) }
     let(:enabled) { true }
     let(:policy_type) { 'scan_execution_policy' }
     let(:name) { 'New policy' }
@@ -194,19 +194,26 @@ RSpec.describe Security::SecurityOrchestrationPolicies::ValidatePolicyService do
       end
     end
 
-    context 'when project is not provided' do
-      let_it_be(:project) { nil }
+    context 'when project or namespace is not provided' do
+      let_it_be(:container) { nil }
 
       it_behaves_like 'checks policy type'
       it_behaves_like 'checks if branches are provided in rule'
     end
 
     context 'when project is provided' do
-      let_it_be(:project) { create(:project, :repository) }
+      let_it_be(:container) { create(:project, :repository) }
 
       it_behaves_like 'checks policy type'
       it_behaves_like 'checks if branches are provided in rule'
       it_behaves_like 'checks if branches are defined in the project'
     end
+
+    context 'when namespace is provided' do
+      let_it_be(:container) { create(:namespace) }
+
+      it_behaves_like 'checks policy type'
+      it_behaves_like 'checks if branches are provided in rule'
+    end
   end
 end