Show in the group API when an ancestor group requires 2FA

What does this MR do and why?

GET /groups/:id for a subgroup answers "require_two_factor_authentication": false when its parent group requires two-factor authentication, and nothing else in the response says that the subgroup's members must set up 2FA anyway. This MR adds require_two_factor_authentication_from_ancestor to the single-group response: it is true when any ancestor of the group requires 2FA.

Why a new attribute instead of changing require_two_factor_authentication, which is what the issue proposes: that attribute is the group's own setting, and it is also what a client writes back with PUT /groups/:id. If it reported the ancestor's requirement, a client that reads a subgroup and sends the same value back would change it. I checked both cases with a request spec:

  • When the top-level group allows subgroups to set up their own rules (the default), PUT /groups/:id with require_two_factor_authentication: true answers 200 and the subgroup's own setting becomes true, so it keeps requiring 2FA if the parent stops.
  • When the top-level group does not allow it, the same request answers 400 {"message":{"require_two_factor_authentication":["is forbidden by a top-level group"]}}.

The second commit is where that decision sits, as specs that pin the current meaning of require_two_factor_authentication. If you would rather follow the issue's proposal, that is the commit to rework, and I am happy to.

What the new attribute means. It covers every ancestor, not only the top-level group, and it does not depend on Allow more restrictive 2FA enforcement for subgroups, because that is how enforcement works today: Authn::Users::TwoFactorGroupsFinder reads every ancestor of the groups a user belongs to, and since !215125 (merged) the 2FA documentation says that when a top-level group enforces 2FA, all members of descendant subgroups must use it. With a throwaway spec I checked that a direct member of the subgroup gets require_two_factor_authentication_from_group: true with that setting on and with it off.

Where it is exposed. Only on API::Entities::GroupDetail, the entity of GET /groups/:id, the endpoint the issue calls, and of the other endpoints that answer with one group (create, update, archive, unarchive, restore and share, among others). The list endpoints (GET /groups, GET /groups/:id/subgroups, GET /groups/:id/descendant_groups) present API::Entities::Group, which is in the API/EntityExposureGrowth baseline, and the attribute costs one query per group. I left them out; if they need it, I think it belongs in a follow-up with preloading. GraphQL's Group.requireTwoFactorAuthentication is also the group's own setting and is not changed here.

Cost. Group#require_two_factor_authentication_from_ancestor? runs one query for a subgroup, over the ancestor ids from traversal_ids (WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" = <parent id> AND "namespaces"."require_two_factor_authentication" = TRUE for a subgroup with one ancestor), and none for a top-level group.

I found this while mapping what the groups API sends for gitlab-mcp-server, which reads require_two_factor_authentication and sends it when a caller sets it in a group update, so it is one of the clients a change of meaning would affect. The GitLab Terraform provider's gitlab_group resource is another: it reads the attribute into its state and sends it on update when the configured value differs from that state.

The commits:

  1. Show in the group API when an ancestor group requires 2FA: the model method, the exposure, their specs, and the OpenAPI v3 document. It carries the Changelog: added trailer and describes the whole change, so it is the right message if the MR is squashed.
  2. Keep require_two_factor_authentication as the group's own setting: request specs that pin the subgroup's own value, with and without subgroups allowed to set their own rules.
  3. Document the require_two_factor_authentication_from_ancestor attribute: the Groups API page and the 2FA in subgroups section.
  4. Leave the attribute details to the Groups API page: the technical writer's suggestion on the 2FA page.
  5. Aggregate failures in the subgroup 2FA request examples: :aggregate_failures on the shared example.

References

Closes #352728

Related to #352725, which asks the group API to expose Allow more restrictive 2FA enforcement for subgroups itself. This MR does not do that.

Screenshots or screen recordings

Not a UI change. GET /groups/:id for a subgroup whose parent requires 2FA, from the request specs:

Before After
"require_two_factor_authentication": false and no other 2FA attribute "require_two_factor_authentication": false, "require_two_factor_authentication_from_ancestor": true
Specs I ran

All on master at 70f446140a9, the base of this MR:

  • Before the change, in EE: the new Group#require_two_factor_authentication_from_ancestor? examples failed with NoMethodError, the new API::Entities::GroupDetail examples failed because the key was missing, and the new request examples got nil instead of true.
  • Each commit on its own, in EE: the model describe, the entity describe and the new request context pass (12, 14 and 14 examples).
  • On the branch, in EE and with FOSS_ONLY=1: spec/lib/api/entities/group_detail_spec.rb (16 examples), the whole GET /groups/:id block of spec/requests/api/groups_spec.rb (49 examples) and the new model describe (7 examples). In EE also the model describe with #parent_allows_two_factor_authentication? and #two_factor_authentication_allowed (13 examples).
  • bin/rake gitlab:openapi:v3:check_docs: up to date.
  • RuboCop on the five Ruby files: no offenses. Both commit message linters: no problems.
  • scripts/lint-doc.sh on the two pages in the docs lint image: passed. Vale at suggestion level reports the same alerts on the two pages before and after. lychee --offline --include-fragments: no errors.
The throwaway request spec behind the PUT and enforcement claims

Run on this branch, not part of the MR:

allow_mfa_for_subgroups=true:  direct member of the subgroup -> require_two_factor_authentication_from_group=true
allow_mfa_for_subgroups=false: direct member of the subgroup -> require_two_factor_authentication_from_group=true
PUT require_two_factor_authentication=true, subgroups allowed     -> 200, subgroup's own setting now true
PUT require_two_factor_authentication=true, subgroups not allowed -> 400 {"message":{"require_two_factor_authentication":["is forbidden by a top-level group"]}}

How to set up and validate locally

  1. Create a top-level group and, in Settings > General > Permissions and group features, select All users in this group must set up two-factor authentication.
  2. Create a subgroup in it.
  3. Call curl --header "PRIVATE-TOKEN: <token>" "http://gdk.test:3000/api/v4/groups/<subgroup id>".
  4. The response has "require_two_factor_authentication": false and "require_two_factor_authentication_from_ancestor": true. For the top-level group, require_two_factor_authentication_from_ancestor is false.

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 José M. Requena Plens

Merge request reports

Loading
Loading