Skip to content

Support 2FA requirement per-group

username-removed-423116 requested to merge siemens/gitlab-ce:feature/enforce-2fa-per-group into master

What does this MR do?

Adds a 2FA requirement checkbox and grace-period on groups, as an alternative to the current 2FA requirement in the application settings which is per instance.

This development is sponsored by @siemens (/cc @bufferoverflow)

Are there points in the code the reviewer needs to double check?

  • Because the 2FA requirement is checked on each request, I decided to cache this on the User model, which also makes it easy to re-use the existing logic and should also help with extending this to a per-project setting in the future.
  • Multiple 2FA requirements don't really conflict with each other, so this MR just checks if any of the user's groups or the instance has one set, and picks the smallest grace-period among the groups and instance.
  • The cached columns on the user are updated when the columns on the group change (in Groups::UpdateService) and when a membership is created/destroyed (in GroupMember after_create/after_destroy hooks). I think this should catch all cases, but I might be missing something.
  • The group 2FA requirement can be edited by administrators or group owners.

Why was this MR needed?

On large instances like gitlab.com it's not feasible to enforce 2FA for all users, so this provides a more granular setting on the group level.

Screenshots (if relevant)

Group settings (before)

Selection_001

Group settings (after)

Selection_002

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Merge request reports