Skip to content

Validate approvals_required does not exceed eligible approvers

Dominic Bauer requested to merge 395545-policy-creation-show-error-when-required-approvals-is-larger-than-eligible-approvers into master

What does this MR do and why?

DO NOT MERGE until the indexes in !119317 (merged) were built!

Scan result policies require specifying (1) eligible approvers by group/project membership or membership role (user_approvers/group_approvers/role_approvers) and (2) a number of required approvals by eligible approvers (approvals_required).

This MR:

  • validates at policy creation/edit that the number of eligible approvers does not exceed approvals_required
  • adds a validationErrors field to the return type of ScanExecutionPolicyCommit

How to set up and validate locally

Via UI

Navigate to a group or project's Security & Compliance > Policies navigation item and attempt to create a new scan result policy where approvals_required exceeds the selected eligible approvers, e.g:

type: scan_result_policy
name: Test
description: ''
enabled: true
rules:
  - type: scan_finding
    branches: []
    scanners: []
    vulnerabilities_allowed: 0
    severity_levels: []
    vulnerability_states: []
actions:
  - type: require_approval
    approvals_required: 4
    user_approvers_ids:
      - 1
      - 2
      - 3

GraphQL

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

Variables:

{
  "fullPath": "sandbox",
  "policyYaml": "type: scan_result_policy\nname: test\nenabled: true\nrules:\n- type: scan_finding\n  branches: []\n  scanners: []\n  vulnerabilities_allowed: 0\n  severity_levels: []\n  vulnerability_states: []\nactions:\n- type: require_approval\n  approvals_required: 2\n  user_approvers: [root]"
}

Response:

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

Database indexes

!119317 (merged) schedules both indexes.

CREATE INDEX index_members_on_source_and_type_and_access_level ON members USING btree (source_id, source_type, type, access_level);
CREATE UNIQUE INDEX index_project_authorizations_on_project_user_access_level ON project_authorizations USING btree (project_id, user_id, access_level);

Follow-up MRs for removing superseded indexes:

Database queries

#user_ids_by_groups 

SELECT
    "members"."user_id"
FROM
    "members"
WHERE
    "members"."type" = 'GroupMember'
    AND "members"."source_type" = 'Namespace'
    AND "members"."source_id" IN (11805471, 11787569, 9970) -- gitlab-org/govern/demos, gitlab-org/govern, gitlab-org
    AND "members"."access_level" >= 30
LIMIT 300;

#user_ids_by_roles 

SELECT
	"project_authorizations"."user_id"
FROM
	"project_authorizations"
WHERE
	"project_authorizations"."project_id" = 278964 -- gitlab-org/gitlab
	AND "project_authorizations"."access_level" IN(30, 40)
LIMIT 300;

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #395545 (closed)

Edited by Dominic Bauer

Merge request reports