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/:idwithrequire_two_factor_authentication: trueanswers200and the subgroup's own setting becomestrue, 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:
- 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: addedtrailer and describes the whole change, so it is the right message if the MR is squashed. - Keep
require_two_factor_authenticationas the group's own setting: request specs that pin the subgroup's own value, with and without subgroups allowed to set their own rules. - Document the
require_two_factor_authentication_from_ancestorattribute: the Groups API page and the 2FA in subgroups section. - Leave the attribute details to the Groups API page: the technical writer's suggestion on the 2FA page.
- Aggregate failures in the subgroup 2FA request examples:
:aggregate_failureson 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 withNoMethodError, the newAPI::Entities::GroupDetailexamples failed because the key was missing, and the new request examples gotnilinstead oftrue. - 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 wholeGET /groups/:idblock ofspec/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.shon 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
- 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.
- Create a subgroup in it.
- Call
curl --header "PRIVATE-TOKEN: <token>" "http://gdk.test:3000/api/v4/groups/<subgroup id>". - The response has
"require_two_factor_authentication": falseand"require_two_factor_authentication_from_ancestor": true. For the top-level group,require_two_factor_authentication_from_ancestorisfalse.
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.