Allow `group_approvers` to reference inaccessible groups
requested to merge 414994-security-policy-group-approvers-not-properly-added-to-eligible-approvers-2 into master
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
- Impersonate
User 1
and create a groupapprover-group
- Impersonate
User 2
and create a projectexample-project
- Add the
User 1
to the project - 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]
- 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"
- 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';
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';
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #414994 (closed)
Edited by Dominic Bauer