Policy Creation: Show error when required approvals is larger than eligible approvers

Problem

When there is more approval than eligible approvers, we should warn the user and stop the user from creating the policy. We have a new design planned if the new epic has been implemented before. Then no need to fix this.

Problem	Expected behaviour-now
Nothing happens when there is a logic error; the user can still create a policy	We should warn the user and stop the user from creating this policy

Implementation Plan

backend

• Add validationErrors to the GraphQL ScanExecutionPolicyCommitPayload type in addition to its errors
• validationErrors contains lists of (level, message, title) triples keyed by the section they belong to in the UI. Here, only the actions section is defined

Example query

mutation ($fullPath: String, $policyYaml: String!, $name: String) {
  scanExecutionPolicyCommit(
    input: {fullPath: $fullPath, policyYaml: $policyYaml, name: $name, operationMode: APPEND}
  ) {
    errors
    validationErrors {
      level
      message
      title
      field
    }
  }
}

Response:

{
  "data": {
    "scanExecutionPolicyCommit": {
      "errors": [
        "Invalid policy",
        "Required approvals exceed eligible approvers"
      ],
      "validationErrors": [
        {
          "level": "error",
          "message": "Required approvals exceed eligible approvers",
          "title": "Logic error",
          "field": "approvers_ids"
        }
      ]
    }
  }
}

Backend Diff

diff --git a/ee/app/graphql/mutations/security_policy/commit_scan_execution_policy.rb
@@ -37,16 +37,23 @@ class CommitScanExecutionPolicy < BaseMutation
             null: true,
             description: 'Name of the branch to which the policy changes are committed.'

+      field :validation_errors,
+            [Types::SecurityPolicyValidationError],
+            null: true,
+            description: 'Errors encountered during execution of the mutation.'
+
       def resolve(args)
         project_or_group = authorized_find!(**args)

         result = commit_policy(project_or_group, args)
         error_message = result[:status] == :error ? result[:message] : nil
         error_details = result[:status] == :error ? result[:details] : nil
+        validation_errors = result[:status] == :error ? result[:validation_errors] : nil

         {
           branch: result[:branch],
-          errors: [error_message, *error_details].compact
+          errors: [error_message, *error_details].compact,
+          validation_errors: validation_errors
         }
       end

diff --git a/ee/app/graphql/types/security_policy_validation_error.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module Types
+  # rubocop: disable Graphql/AuthorizeTypes
+  class SecurityPolicyValidationError < BaseObject
+    graphql_name 'SecurityPolicyValidationError'
+    description 'Security policy validation error'
+
+    field :field, GraphQL::Types::String, null: false, description: 'Error field.'
+    field :level, GraphQL::Types::String, null: false, description: 'Error level. TODO: Enum.'
+    field :message, GraphQL::Types::String, null: false, description: 'Error message.'
+    field :title, GraphQL::Types::String, null: true, description: 'Error title.'
+  end
+  # rubocop: enable Graphql/AuthorizeTypes
+end

diff --git a/ee/app/services/security/security_orchestration_policies/validate_policy_service.rb
@@ -5,6 +5,8 @@ module SecurityOrchestrationPolicies
     class ValidatePolicyService < ::BaseContainerService
       include ::Gitlab::Utils::StrongMemoize

+      ValidationError = Struct.new(:field, :level, :message, :title)
+
       def execute
         return error_with_title(s_('SecurityOrchestration|Empty policy name')) if blank_name?

@@ -13,15 +15,21 @@ def execute
         return error_with_title(s_('SecurityOrchestration|Invalid policy type')) if invalid_policy_type?
         return error_with_title(s_('SecurityOrchestration|Policy cannot be enabled without branch information')) if blank_branch_for_rule?
         return error_with_title(s_('SecurityOrchestration|Policy cannot be enabled for non-existing branches (%{branches})') % { branches: missing_branch_names.join(', ') }) if missing_branch_for_rule?
-        return error_with_title(s_('SecurityOrchestration|Required approvals exceed eligible approvers')) if required_approvals_exceed_eligible_approvers?
+
+        return error_with_title(s_('SecurityOrchestration|Required approvals exceed eligible approvers'), title: s_('SecurityOrchestration|Logic error'), field: "approvers_ids") if required_approvals_exceed_eligible_approvers?

         success
       end

       private

-      def error_with_title(message)
-        error(s_('SecurityOrchestration|Invalid policy'), :bad_request, pass_back: { details: [message] })
+      def error_with_title(message, title: nil, field: nil, level: :error)
+        pass_back = {
+          details: [message],
+          validation_errors: [ValidationError.new(field, level, message, title)]
+        }
+
+        error(s_('SecurityOrchestration|Invalid policy'), :bad_request, pass_back: pass_back)
       end

       def policy_disabled?

• frontend update graphql request
• frontend capture error in scan_result_policy_editor.vue and enable validation in policy_action_approvers.vue#L130

Incomplete Frontend Patch

diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result_policy/scan_result_policy_editor.vue b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result_policy/scan_result_policy_editor.vue
index ad11eff463fa..c8f5d3c299e9 100644
--- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result_policy/scan_result_policy_editor.vue
+++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result_policy/scan_result_policy_editor.vue
@@ -184,6 +184,7 @@ export default {
       this.policy.rules.splice(ruleIndex, 1, values);
     },
     handleError(error) {
+      // TODO trigger validation failure in policy action builder if related to number of approvers
       if (error.message.toLowerCase().includes('graphql')) {
         this.$emit('error', GRAPHQL_ERROR_MESSAGE);
       } else {
diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/utils.js b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/utils.js
index 3742bfb8000c..6bb3073e15ee 100644
--- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/utils.js
+++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/utils.js
@@ -9,9 +9,9 @@ import { DEFAULT_MR_TITLE, RULE_MODE_SCANNERS, SECURITY_POLICY_ACTIONS } from '.
  * Checks if an error exists and throws it if it does
  * @param {Object} payload contains the errors if they exist
  */
-const checkForErrors = ({ errors }) => {
-  if (errors?.length) {
-    throw new Error(errors.join('\n'));
+const checkForErrors = ({ errors, validationErrors }) => {
+  if (errors?.length || validationErrors?.length) {
+    throw new Error([...errors, ...validationErrors].join('\n'));
   }
 };
 
diff --git a/ee/app/assets/javascripts/security_orchestration/graphql/mutations/create_scan_execution_policy.mutation.graphql b/ee/app/assets/javascripts/security_orchestration/graphql/mutations/create_scan_execution_policy.mutation.graphql
index 104a180bb784..51d78e409d04 100644
--- a/ee/app/assets/javascripts/security_orchestration/graphql/mutations/create_scan_execution_policy.mutation.graphql
+++ b/ee/app/assets/javascripts/security_orchestration/graphql/mutations/create_scan_execution_policy.mutation.graphql
@@ -9,5 +9,12 @@ mutation updatePolicy(
   ) {
     branch
     errors
+    validationErrors {
+      actions {
+        level
+        message
+        title
+      }
+    }
   }
 }