## Summary There's an inconsistency between turning on 2FA for a group vs adding a new group member. * Turning on 2FA for a group only considers direct members while adding a new group member will consider 2FA requirement from the whole group hierarchy, both approaches are wrong. * Turning on 2FA for a group in `Group#update_two_factor_requirement` should propagate to subgroups. * Adding a new group member in `User#expanded_groups_requiring_two_factor_authentication` should only consider ancestors, not subgroups. ## Steps to reproduce (Bottom-up Propagation) 1. **Create a group hierarchy**: * Create a **top-level group (Group A)**. * Create a **subgroup (Group B)** under **Group A**. 2. **Set up 2FA for Group B**: * Go to **Group B**'s settings and enable `require_two_factor_authentication`. 3. **Add a new user to Group A** (the parent group). * Sign in with new user belonging to **top-level group (Group A)** * Observe the 2FA requirement propagation behaviour from **subgroup (Group B)**, the which forces 2FA requirement on the new user * The user should **not inherit the 2FA requirement** from **Group B** (the subgroup) ## Example Spec The following spec is currently failing: ```ruby it 'does not enable 2FA for ancestor group member' do ancestor_group = create(:group) group.update!(require_two_factor_authentication: true, parent: ancestor_group) expect { ancestor_group.add_user(user, GroupMember::OWNER) }.not_to change { user.reload.require_two_factor_authentication_from_group }.from(false) end ``` ## Current Behaviour If **subgroup (Group B)** has 2FA enabled and **top-level group (Group A)** does not, a new user added to Group A still inherits Group B's 2FA requirement due to the current [subgroup membership inheritance](https://docs.gitlab.com/ee/user/group/subgroups/#subgroup-membership) works. ## Expected Behaviour When adding a new member to the **top-level group (Group A)**, the 2FA enforcement should only consider **top-level group (Group A) and its ancestors**. The user should **not inherit** the 2FA requirement from subgroups like **Group B**. ## Relevant Context Fixing `Group#update_two_factor_requirement` is already tackled by https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24965, we should also fix `User#expanded_groups_requiring_two_factor_authentication` and create a background migration to update 2FA requirement for all users. /cc @jeremy @tkuah ## Related Support Tickets (internal) 1. https://gitlab.zendesk.com/agent/tickets/141403 2. https://gitlab.zendesk.com/agent/tickets/155201