Skip to content

Allow `group_approvers` to reference inaccessible groups

What does this MR do and why?

Scan result policies afford listing group members as eligible MR approvers by listing groups by their name or ID with the group_approvers attribute.

Currently, groups listed as group_approvers are filtered by the policy last editor's permissions. This is a problem, because membership changes of the last editor impact the groups eligible for approval.

This MR:

  • removes the visibility scope when looking up approval groups, allowing policy editors to list any (potentially private) groups as an eligible approver.
  • introduces a new GraphQL type that isolates all accessible group attributes.

The MR approval widget does not leak group membership (#368487).

Refer to #414994 (comment 1481635564) for details on this.

How to set up and validate locally

  1. Impersonate User 1 and create a group approver-group
  2. Impersonate User 2 and create a project example-project
  3. Add the User 1 to the project
  4. Navigate to Secure > Policies and create the following scan result policy for the project:
type: scan_result_policy
name: Container Scanning
description: ''
enabled: true
rules:
  - type: scan_finding
    scanners:
      - container_scanning
    vulnerabilities_allowed: 0
    severity_levels: []
    vulnerability_states: []
    branch_type: protected
actions:
  - type: require_approval
    approvals_required: 1
    group_approvers: [approver-group]
  1. Open a MR that adds the following .gitlab-ci.yml:
include:
  - template: Security/Container-Scanning.gitlab-ci.yml

container_scanning:
  variables:
    CS_IMAGE: "nginx:1"
  1. Impersonate User 1, navigate to to the MR and verify it can be approved

Database queries

SELECT
    "namespaces".*
FROM ((
        SELECT
            "namespaces".*
        FROM
            "namespaces"
        WHERE
            "namespaces"."type" = 'Group'
            AND "namespaces"."id" = 278964)
    UNION (
        SELECT
            "namespaces".*
        FROM
            "namespaces"
        WHERE
            "namespaces"."type" = 'Group'
            AND "namespaces"."id" IN (
                SELECT
                    "routes"."namespace_id"
                FROM
                    "routes"
                WHERE
                    "routes"."source_type" = 'Namespace'
                    AND (LOWER(routes.path) IN ('fdroid', 'graphviz'))))
        UNION (
            SELECT
                "namespaces".*
            FROM
                "namespaces"
            WHERE
                "namespaces"."type" = 'Group'
                AND (LOWER(path) IN ('fdroid', 'graphviz')))) namespaces
WHERE
    "namespaces"."type" = 'Group';

postgres.ai


SELECT
	"namespaces".*
FROM ((
		SELECT
			"namespaces".*
		FROM
			"namespaces"
		WHERE
			"namespaces"."type" = 'Group'
			AND(traversal_ids @> ('{9970}'))
			AND "namespaces"."id" = 58054119)
	UNION (
		SELECT
			"namespaces".*
		FROM
			"namespaces"
		WHERE
			"namespaces"."type" = 'Group'
			AND(traversal_ids @> ('{9970}'))
			AND "namespaces"."id" IN(
				SELECT
					"routes"."namespace_id" FROM "routes"
				WHERE
					"routes"."source_type" = 'Namespace'
					AND(LOWER(routes.path)
						IN('dev-subdepartment', 'gitaly-planning'))))
	UNION (
		SELECT
			"namespaces".*
		FROM
			"namespaces"
		WHERE
			"namespaces"."type" = 'Group'
			AND(traversal_ids @> ('{9970}'))
			AND(LOWER(path)
				IN('dev-subdepartment', 'gitaly-planning')))) namespaces
WHERE
	"namespaces"."type" = 'Group';

postgres.ai

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 #414994 (closed)

Edited by Dominic Bauer

Merge request reports