Make UpdateTwoFactorRequirementForMembersWorker account for minimal_access users (!248677) · Merge requests · GitLab.org / GitLab

Summary

A support request surfaced a user blocked by the 2FA setup page while no group in their hierarchy enforced 2FA.

After investigation, I found that once a group withdraws 2FA enforcement, the recalculation skips minimal_access members through non_minimal_access, several scopes below the call site.

Previously that had no effect, because minimal_access members never held the cached users.require_two_factor_authentication_from_group value.

Now that they do, nothing recalculates it for them when enforcement is withdrawn, so it keeps the true written while the group still enforced 2FA.

What does this MR do and why?

What?

Groups::UpdateTwoFactorRequirementForMembersWorker now accounts for minimal_access members when a group's 2FA settings change, so the cached users.require_two_factor_authentication_from_group value is also recalculated.
Until this ships, the only way to unblock an affected user is to remove and re-add their membership.
- Users with a stale require_two_factor_authentication_from_group will be locked out once their grace period expires.

Why?

Group level 2FA state is denormalized onto users.require_two_factor_authentication_from_group:

It is only recomputed by events: after_create, after_destroy, or a group changing its 2FA settings.
There is no after_update, so changing a member's role never recalculates it.
There is no scheduled job, so any path that skips a member leaves that member's cached value frozen, and a Group Owner has no way to clear it from the group settings UI. That is why remove and re-add is the only workaround: the destroy and create hooks both call Member#update_two_factor_requirement.

The group side path skips minimal_access members, but nothing at the call site shows it. Group#update_two_factor_requirement_for_members reads as "recalculate every member in the hierarchy":

def update_two_factor_requirement_for_members
  hierarchy_members.find_each(&:update_two_factor_requirement)
end

The exclusion flow is:

hierarchy_members
active_without_invites_and_requests
without_invites_and_requests, which already accepts a minimal_access argument
non_minimal_access, which drops every member at access_level = 5

Member.seat_assignable is the only caller that opts in today.

This only becomes visible once enforcement is later removed:

Before !225038 (merged) and !225317 (merged), minimal_access members never held the cached value at all, so there was never a wrong value left behind.
Both of those MRs cover minimal_access members correctly gaining enforcement, and those paths work.
The opposite direction, a group withdrawing enforcement, was not reproducible in specs or by hand until minimal_access members could hold the value.

How it happened

Step	Event	Cached `require_two_factor_authentication_from_group`
1	User holds minimal_access in a hierarchy enforcing 2FA	`true`, correct
2	Owner disables 2FA, or turns off Allow more restrictive 2FA enforcement for subgroups	should become `false`
3	`DisallowTwoFactorForSubgroupsWorker` queues `DisallowTwoFactorForGroupWorker` per subgroup	unchanged
4	Each subgroup save triggers `Group#update_two_factor_requirement`, which queues the members worker	unchanged
5	The worker iterates `hierarchy_members`, which excludes minimal_access	stays `true`, now stale
6	Grace period expires	user is redirected to `/-/profile/two_factor_auth` on every request

Before

Owner turns 2FA enforcement off
  |
  v
DisallowTwoFactorForSubgroupsWorker
  |
  v
DisallowTwoFactorForGroupWorker
  namespaces.require_two_factor_authentication = false
  |
  v
Group#update_two_factor_requirement            (after_commit on the group)
  |
  v
Groups::UpdateTwoFactorRequirementForMembersWorker
  |
  v
hierarchy_members
  |
  |-- access_level >= 10 -> Member#update_two_factor_requirement
  |                         users.require_two_factor_authentication_from_group = false
  |
  '-- access_level  = 5  -> dropped by non_minimal_access
                            users.require_two_factor_authentication_from_group stays true
                            member is redirected to /-/profile/two_factor_auth
                            once the grace period expires

After

Owner turns 2FA enforcement off
  |
  v
DisallowTwoFactorForSubgroupsWorker
  |
  v
DisallowTwoFactorForGroupWorker
  namespaces.require_two_factor_authentication = false
  |
  v
Group#update_two_factor_requirement            (after_commit on the group)
  |
  v
Groups::UpdateTwoFactorRequirementForMembersWorker
  |
  v
hierarchy_members(minimal_access: true)
  |
  |-- access_level >= 10 -> Member#update_two_factor_requirement
  |                         users.require_two_factor_authentication_from_group = false
  |
  '-- access_level  = 5  -> Member#update_two_factor_requirement
                            users.require_two_factor_authentication_from_group = false

Observed after the BBM: Kibana. Counting users the BBM set to true that no group enforces 2FA on today:

ids = ['gl_user_ids from logs']
residual_users = User.where(id: ids).find_each.select do |u|
  u.require_two_factor_authentication_from_group &&
    !u.two_factor_enabled? &&
    u.expanded_groups_requiring_two_factor_authentication.empty?
end

residual_users.map(&:id)
# => 29 users

Screenshots & Recordings

Before	After

How to set up and validate locally

All steps run on this branch. Open a fresh Rails console after any branch switch or GDK restart: consoles and Sidekiq load code at startup and never pick up changes.

You need an EE license that includes minimal_access_role (the standard GDK Ultimate license works). SaaS simulation is not required; if you do simulate SaaS, the group's plan must include the feature.

Set up a group enforcing 2FA with a minimal_access member, then write the cached value the way the BBM (!225317 (merged)) did. update_all bypasses callbacks and license checks just like the backfill, so this works regardless of membership state or plan:

group = Group.find_by_full_path('<group>')
user  = User.find_by_username('<user>')

group.update!(require_two_factor_authentication: true, two_factor_grace_period: 0)
group.add_member(user, Gitlab::Access::MINIMAL_ACCESS)

User.where(id: user.id).update_all(require_two_factor_authentication_from_group: true, two_factor_grace_period: 0)

user.reload.require_two_factor_authentication_from_group # => true

Withdraw enforcement and run the worker by hand. update_column skips the after_commit enqueue, so the only worker execution is the explicit one below, guaranteed to run this checkout's code:

# In production this line is the end of the RFH cascade:
# allow_mfa_for_subgroups toggle -> DisallowTwoFactorForSubgroupsWorker
#   -> DisallowTwoFactorForGroupWorker -> require_two_factor_authentication: false
group.update_column(:require_two_factor_authentication, false)
Groups::UpdateTwoFactorRequirementForMembersWorker.new.perform(group.id)

user.reload.require_two_factor_authentication_from_group   # => false
user.expanded_groups_requiring_two_factor_authentication   # => []

To see the bug this MR fixes, repeat steps 2 and 3 on master in a fresh console: the final read stays true while nothing requires 2FA. Signing in as that user redirects to /-/profile/two_factor_auth on every request, with no "configure it later" skip because the grace period is zero.

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 Aug 06, 2026 by Hakeem Abdul-Razak

Make UpdateTwoFactorRequirementForMembersWorker account for minimal_access users