Allow subgroups in Duo admin group membership access rules

Pratyaksh Golashrequested to merge
628450-allow-subgroups-duo-admin-access-rules into master

What does this MR do and why?

Admin > GitLab Duo > Configuration lets an admin restrict access based on group membership. The group picker sent topLevelOnly: true, so only top-level groups could be selected. Enforcement in accessible_for_user already accepted a rule on any group and joined direct memberships only. The admin page loaded rules through a root-groups-only scope, so a subgroup rule saved through the REST API, or a top-level group later transferred under another group, was enforced but invisible in the list. The next save from the UI then deleted that invisible rule, because the UI resubmits only what it shows.

  1. Behind feature flag duo_admin_access_rules_subgroups (type beta, default off, instance actor, pushed from Admin::GitlabDuo::ConfigurationController): the picker sends topLevelOnly: null so groups at every level are searchable. This is frontend only. No backend change is gated, because the API already accepted subgroups.
  2. Ungated: Admin::AiConfigurationPresenter now uses the all-rules scope duo_namespace_access_rules. The root-only scope duo_root_namespace_access_rules is removed. Rules whose namespace row is already gone but whose loose foreign key cleanup has not run yet are filtered out, so they do not render as the default "All eligible users" row.
  3. Ungated: new validation through_namespace_is_group rejects a rule whose namespace is a user or project namespace, or does not exist. The API returns 400 with "Through namespace must be a group".
  4. Ungated: a namespace and feature listed twice in one save is rejected before insert with "Accessible entity is listed more than once for the same namespace". Previously this hit the unique index and returned 500.
  5. Ungated: the save path deletes all rules then bulk inserts. It is now wrapped in a transaction, so a rejected save keeps the existing rules. Previously a validation error left the table empty and the feature failed open.
  6. Ungated, cosmetic: the selected-group toggle in the shared picker shows "name (full/path)", the format the dropdown rows already used. The picker is used by the admin page and the GitLab.com group settings page, so both get it. Not tied to the feature flag on purpose: it is a consistency fix, and a flag whose off state hides a format the rows already show would guard nothing.
  7. Ungated: the namespaces a save refers to are loaded once for the whole batch before validation, so the group check runs no query per row.

Deviation from the issue

The issue's Changes section said the rules list already shows the full path. It does not. Rows show the group name and link to the full path. This MR leaves the list as is. The acceptance criterion "show in the admin rules list with full path" is not met as written and needs a follow-up or a wording change on the issue.

The issue listed one spec case: subgroup id accepted, non-group namespace id rejected. This MR also adds the duplicate-namespace rejection, because live testing found the 500 described in change 4 above.

Review notes

Approvals

  • Backend maintainer and frontend maintainer, since Ruby and Vue both change. CODEOWNERS matches only the general Maintainers section for every touched file, so no database, docs, or other specialised approval is pulled in.
  • The toggle text change is user-visible. A UX look is at the reviewer's discretion.

Database

  • No migrations, no schema change. Every statement this MR touches, with its postgres.ai plan, is in the "Database review" section below.
  • The only query change is removing a parent_id IS NULL join filter from the admin listing scope on a table_size: small table.

Danger

  • Will comment on the new validate line and the new reject_duplicate! method in ee/app/models.
  • Both fire only on the delete-and-reinsert write path, which is the only writer of this model, so existing rows are never re-validated.

Deliberate non-changes

  • Membership semantics stay direct members only. Descendant coverage is a separate follow-up decided in the planning issue thread on 2026-09-11.
  • The "Create group" link still creates a top-level group.
  • The GitLab.com group-level setting is out of scope. Its service EE::Groups::UpdateService likely has the same duplicate 500, and there is no issue for it yet.
  • The docs page doc/administration/gitlab_duo/configure/access_control.md still says only top-level groups can be selected. The docs child issue under the epic owns that update while the feature flag defaults off.
  • Members with a pending access request or an expired membership pass the gate. This is a known pre-existing gap, tracked separately, and is not addressed here.

Follow-ups

  • Switch the feature flag YAML introduced_by_url from the planning issue to this MR's URL after creation.
  • Check EE::Groups::UpdateService for the same duplicate-namespace 500. No issue yet.

Specs run locally

  • ee/spec/models/ai/feature_access_rule_spec.rb
  • ee/spec/models/concerns/ai/user_authorizable_spec.rb, subgroup contexts for duo_classic and duo_agent_platform
  • ee/spec/presenters/admin/ai_configuration_presenter_spec.rb
  • ee/spec/requests/admin/gitlab_duo/configuration_controller_spec.rb, needs ENABLE_RSPACK=true locally
  • ee/spec/requests/api/settings_spec.rb, context duo_namespace_access_rules
  • ee/spec/services/application_settings/update_service_spec.rb, context when updating duo namespace access rules
  • ee/spec/frontend/ai/settings/components/group_selector_spec.js
  • Also verified live in GDK through the REST API and a headless-browser run of the admin page for both feature flag states.

Database review

ai_instance_accessible_entity_rules is empty on GitLab.com, since the instance-level admin page these rules come from is self-managed only. The postgres.ai plans linked under each statement were taken on a clone seeded with four rows via exec, matching the size the table stays at in production: at most two rows per group an admin has added a rule for. The inline EXPLAIN output is from a local GDK with the same shape of data.

SQL and postgres.ai plans for every statement, and the picker search for context

Clone seed used before the plans, on gitlab-production-main:

exec INSERT INTO ai_instance_accessible_entity_rules (through_namespace_id, accessible_entity, created_at, updated_at) VALUES (9970, 'duo_classic', NOW(), NOW()), (9970, 'duo_agent_platform', NOW(), NOW()), (6543, 'duo_classic', NOW(), NOW()), (NULL, 'duo_classic', NOW(), NOW())

Read: admin page, presenter, REST GET (Ai::FeatureAccessRule.duo_namespace_access_rules, replaces the root-only scope)

SELECT "ai_instance_accessible_entity_rules".*
FROM "ai_instance_accessible_entity_rules"
ORDER BY through_namespace_id NULLS FIRST, "ai_instance_accessible_entity_rules"."accessible_entity" ASC
Sort  (cost=5.73..6.00 rows=108 width=47) (actual time=0.023..0.023 rows=2 loops=1)
  Sort Key: through_namespace_id NULLS FIRST, accessible_entity
  Sort Method: quicksort  Memory: 25kB
  Buffers: shared hit=7
  ->  Seq Scan on ai_instance_accessible_entity_rules  (cost=0.00..2.08 rows=108 width=47) (actual time=0.014..0.014 rows=2 loops=1)
        Buffers: shared hit=1
Planning Time: 0.263 ms
Execution Time: 0.026 ms

Plan: https://postgres.ai/console/gitlab/gitlab-production-main/sessions/57176/commands/161705

Read: namespace preload from includes(:through_namespace) (one query, primary key lookup)

SELECT "namespaces".* FROM "namespaces" WHERE "namespaces"."id" IN (<through_namespace_ids>)
Index Scan using namespaces_pkey on namespaces  (cost=0.14..2.16 rows=1 width=372) (actual time=0.012..0.012 rows=1 loops=1)
  Index Cond: (id = 140)
  Buffers: shared hit=5
Planning Time: 1.302 ms
Execution Time: 0.019 ms

Plan: https://postgres.ai/console/gitlab/gitlab-production-main/sessions/57176/commands/161707

Write: duo_namespace_access_rules=, unchanged statements, now inside one transaction

DELETE FROM "ai_instance_accessible_entity_rules"
Delete on ai_instance_accessible_entity_rules  (cost=0.00..2.08 rows=0 width=0) (actual time=0.027..0.028 rows=0 loops=1)
  Buffers: shared hit=3 dirtied=1
  ->  Seq Scan on ai_instance_accessible_entity_rules  (cost=0.00..2.08 rows=108 width=6) (actual time=0.004..0.004 rows=2 loops=1)
        Buffers: shared hit=1
Planning Time: 0.016 ms
Execution Time: 0.125 ms

Plan: https://postgres.ai/console/gitlab/gitlab-production-main/sessions/57176/commands/161710

INSERT INTO "ai_instance_accessible_entity_rules" ("through_namespace_id","accessible_entity","created_at","updated_at")
VALUES (6543, 'duo_agent_platform', NOW(), NOW()), (15846663, 'duo_classic', NOW(), NOW())
RETURNING "id"
Insert on ai_instance_accessible_entity_rules  (cost=0.00..0.03 rows=2 width=64) (actual time=0.144..0.149 rows=2 loops=1)
  Buffers: shared hit=31
  ->  Values Scan on "*VALUES*"  (cost=0.00..0.03 rows=2 width=64) (actual time=0.037..0.039 rows=2 loops=1)
        Buffers: shared hit=15
Planning Time: 0.015 ms
Execution Time: 0.155 ms

Plan: https://postgres.ai/console/gitlab/gitlab-production-main/sessions/57176/commands/161711

Write: per-row validation queries during bulk_insert!, one pair per rule row, bounded by two rows per group

The uniqueness check on the shared concern is unchanged and already ran per row:

SELECT 1 AS one FROM "ai_instance_accessible_entity_rules"
WHERE "ai_instance_accessible_entity_rules"."accessible_entity" = 'duo_classic'
  AND "ai_instance_accessible_entity_rules"."through_namespace_id" = 9970 LIMIT 1

Plan: https://postgres.ai/console/gitlab/gitlab-production-main/sessions/57176/commands/161709 (index-only scan on the unique index)

New in this MR: the namespaces for the batch are loaded once before validation, the same WHERE "namespaces"."id" IN (...) query as the preload above, so the group check runs no query per row.

Plan: https://postgres.ai/console/gitlab/gitlab-production-main/sessions/57176/commands/161707

Removed query (old duo_root_namespace_access_rules, the "before" of the listing change, no longer executed anywhere)

SELECT "ai_instance_accessible_entity_rules"."id"
FROM "ai_instance_accessible_entity_rules"
INNER JOIN "namespaces" ON "namespaces"."id" = "ai_instance_accessible_entity_rules"."through_namespace_id"
WHERE "namespaces"."parent_id" IS NULL AND "namespaces"."type" = 'Group'

Plan: https://postgres.ai/console/gitlab/gitlab-production-main/sessions/57176/commands/161706

Context, no code change in this MR: the picker's group search. Behind the feature flag the frontend sends topLevelOnly: null, so the existing GroupsFinder search runs without its parent_id IS NULL filter. Same query the groups GraphQL field runs everywhere else; this MR only drops one filter. Plans below use the deliberately broad term gitlab on a cold clone, so the I/O read time dominates.

-- feature flag on: every level
SELECT "namespaces"."id" FROM "namespaces"
WHERE "namespaces"."type" = 'Group'
  AND "namespaces"."id" IN (SELECT "routes"."source_id" FROM "routes" WHERE "routes"."source_type" = 'Namespace' AND ("routes"."path" ILIKE '%gitlab%' OR "routes"."name" ILIKE '%gitlab%'))
ORDER BY "namespaces"."name" ASC LIMIT 20

Plan: https://postgres.ai/console/gitlab/gitlab-production-main/sessions/57176/commands/161712 (9.8 s, 9.7 s of it I/O read; 20 rows)

-- feature flag off, today's behaviour: top-level only
SELECT "namespaces"."id" FROM "namespaces"
WHERE "namespaces"."type" = 'Group' AND "namespaces"."parent_id" IS NULL
  AND "namespaces"."id" IN (SELECT "routes"."source_id" FROM "routes" WHERE "routes"."source_type" = 'Namespace' AND ("routes"."path" ILIKE '%gitlab%' OR "routes"."name" ILIKE '%gitlab%'))
ORDER BY "namespaces"."name" ASC LIMIT 20

Plan: https://postgres.ai/console/gitlab/gitlab-production-main/sessions/57176/commands/161713 (63 s, 62 s of it I/O read; 20 rows)

Dropping the filter makes the search cheaper, not dearer: the limit is met sooner when subgroups count. Both numbers are cold-clone reads and belong to the existing finder, not to this MR.

Table facts from db/docs/ai_instance_accessible_entity_rules.yml: gitlab_schema: gitlab_main_cell_setting, table_size: small. Rows are bounded by two per group an admin adds a rule for. The unique index on (through_namespace_id, accessible_entity) is the one the new duplicate check pre-empts.

References

Resolves https://gitlab.com/gitlab-org/gitlab/-/issues/628450

Epic https://gitlab.com/groups/gitlab-org/-/epics/23483 Planning issue https://gitlab.com/gitlab-org/gitlab/-/issues/628158 Rollout issue https://gitlab.com/gitlab-org/gitlab/-/issues/628453

How to set up and validate locally

  1. Enable the feature flag in a rails console.
Feature.enable(:duo_admin_access_rules_subgroups)
  1. Visit /admin/gitlab_duo/configuration, click "Add group", search for a subgroup by name, and select it. The toggle shows "name (full/path)" and "Add" saves a row.
  2. Disable the feature flag and reload. The saved subgroup row is still listed, and the picker offers top-level groups only.
  3. Check the new validation in a rails console. Expected output: "Validation failed: Through namespace must be a group".
Ai::FeatureAccessRule.duo_namespace_access_rules = [{ through_namespace: { id: User.first.namespace_id }, features: %w[duo_classic] }]
  1. Run the specs.
bin/rspec ee/spec/models/ai/feature_access_rule_spec.rb ee/spec/requests/api/settings_spec.rb -e 'duo_namespace_access_rules'

Screenshots or screen recordings

All taken on this branch in a local GDK with test data (groups acme, acme/ai-pilot, zeta).

Before (feature flag off, today's behaviour) After (feature flag on)

before-picker-top-level-only.png

after-picker-all-levels.png

toggle showed the group name only

after-toggle-shows-full-path.png

subgroup rules absent from the rules table

after-list-with-subgroup-rows.png

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Pratyaksh Golash

Merge request reports