Merge branch '215697-allow-groups-to-disable-2fa-requirement-for-subgroups-ui' into 'master'

Resolve "Allow groups to disable 2FA requirement for subgroups" -UI

See merge request gitlab-org/gitlab!44712

app/controllers/groups_controller.rb

@@ -242,7 +242,8 @@ class GroupsController < Groups::ApplicationController
       :project_creation_level,
       :subgroup_creation_level,
       :default_branch_protection,
-      :default_branch_name
+      :default_branch_name,
+      :allow_mfa_for_subgroups
     ]
   end
 

app/models/group.rb

@@ -564,6 +564,13 @@ class Group < Namespace
     access_level_roles.values
   end
 
+  def parent_allows_two_factor_authentication?
+    return true unless has_parent?
+
+    ancestor_settings = ancestors.find_by(parent_id: nil).namespace_settings
+    ancestor_settings.allow_mfa_for_subgroups
+  end
+
   private
 
   def update_two_factor_requirement
@@ -598,8 +605,7 @@ class Group < Namespace
     return unless has_parent?
     return unless require_two_factor_authentication
 
-    ancestor_settings = ancestors.find_by(parent_id: nil).namespace_settings
-    return if ancestor_settings.allow_mfa_for_subgroups
+    return if parent_allows_two_factor_authentication?
 
     errors.add(:require_two_factor_authentication, _('is forbidden by a top-level group'))
   end

app/services/groups/create_service.rb

@@ -49,6 +49,7 @@ module Groups
 
     def remove_unallowed_params
       params.delete(:default_branch_protection) unless can?(current_user, :create_group_with_default_branch_protection)
+      params.delete(:allow_mfa_for_subgroups)
     end
 
     def create_chat_team?

app/services/groups/update_service.rb

@@ -99,7 +99,6 @@ module Groups
 
     def update_two_factor_requirement_for_subgroups
       settings = group.namespace_settings
-
       return if settings.allow_mfa_for_subgroups
 
       if settings.previous_changes.include?(:allow_mfa_for_subgroups)

app/views/groups/settings/_permissions.html.haml

@@ -38,7 +38,7 @@
     = render 'groups/settings/project_creation_level', f: f, group: @group
     = render 'groups/settings/subgroup_creation_level', f: f, group: @group
     = render_if_exists 'groups/settings/prevent_forking', f: f, group: @group
-    = render 'groups/settings/two_factor_auth', f: f
+    = render 'groups/settings/two_factor_auth', f: f, group: @group
     = render_if_exists 'groups/personal_access_token_expiration_policy', f: f, group: @group
     = render_if_exists 'groups/member_lock_setting', f: f, group: @group
   = f.submit _('Save changes'), class: 'btn btn-success gl-mt-3 js-dirty-submit', data: { qa_selector: 'save_permissions_changes_button' }

app/views/groups/settings/_two_factor_auth.html.haml

+- return unless group.parent_allows_two_factor_authentication?
 - docs_link_url = help_page_path('security/two_factor_authentication', anchor: 'enforcing-2fa-for-all-users-in-a-group')
 - docs_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: docs_link_url }
 
@@ -9,8 +10,14 @@
   .form-check
     = f.check_box :require_two_factor_authentication, class: 'form-check-input', data: { qa_selector: 'require_2fa_checkbox' }
     = f.label :require_two_factor_authentication, class: 'form-check-label' do
-      %span= _('Require all users in this group to setup Two-factor authentication')
+      %span= _('Require all users in this group to setup two-factor authentication')
 .form-group
   = f.label :two_factor_grace_period, _('Time before enforced'), class: 'label-bold'
   = f.text_field :two_factor_grace_period, class: 'form-control form-control-sm w-auto'
   .form-text.text-muted= _('Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication')
+- unless group.has_parent?
+  .form-group
+    .form-check
+      = f.check_box :allow_mfa_for_subgroups, class: 'form-check-input', checked: group.namespace_settings.allow_mfa_for_subgroups
+      = f.label :allow_mfa_for_subgroups, class: 'form-check-label' do
+        = _('Allow subgroups to set up their own two-factor authentication rules')

changelogs/unreleased/215697-allow-groups-to-disable-2fa-requirement-for-subgroups-ui.yml

+---
+title: Allow groups to disable 2FA requirement for subgroups
+merge_request: 44712
+author:
+type: added

doc/security/two_factor_authentication.md

@@ -65,6 +65,8 @@ The following are important notes about 2FA:
   2FA enabled, 2FA is **not** required for those individually added members.
 - If there are multiple 2FA requirements (for example, group + all users, or multiple
   groups) the shortest grace period will be used.
+- It is possible to disallow subgroups from setting up their own 2FA requirements.
+  Navigate to the top-level group's **Settings > General > Permissions, LFS, 2FA > Two-factor authentication** and uncheck the **Allow subgroups to set up their own two-factor authentication rule** field. This action will cause all subgroups with 2FA requirements to stop requiring that from their members.
 
 ## Disabling 2FA for everyone
 

locale/gitlab.pot

@@ -2697,6 +2697,9 @@ msgstr ""
 msgid "Allow requests to the local network from web hooks and services"
 msgstr ""
 
+msgid "Allow subgroups to set up their own two-factor authentication rules"
+msgstr ""
+
 msgid "Allow this key to push to repository as well? (Default only allows pull access.)"
 msgstr ""
 
@@ -22276,7 +22279,7 @@ msgstr ""
 msgid "Require admin approval for new sign-ups"
 msgstr ""
 
-msgid "Require all users in this group to setup Two-factor authentication"
+msgid "Require all users in this group to setup two-factor authentication"
 msgstr ""
 
 msgid "Require all users to accept Terms of Service and Privacy Policy when they access GitLab."

spec/models/group_spec.rb

@@ -1571,4 +1571,24 @@ RSpec.describe Group do
       end
     end
   end
+
+  describe '#parent_allows_two_factor_authentication?' do
+    it 'returns true for top-level group' do
+      expect(group.parent_allows_two_factor_authentication?).to eq(true)
+    end
+
+    context 'for subgroup' do
+      let(:subgroup) { create(:group, parent: group) }
+
+      it 'returns true if parent group allows two factor authentication for its descendants' do
+        expect(subgroup.parent_allows_two_factor_authentication?).to eq(true)
+      end
+
+      it 'returns true if parent group allows two factor authentication for its descendants' do
+        group.namespace_settings.update!(allow_mfa_for_subgroups: false)
+
+        expect(subgroup.parent_allows_two_factor_authentication?).to eq(false)
+      end
+    end
+  end
 end

spec/services/groups/create_service_spec.rb

@@ -45,6 +45,15 @@ RSpec.describe Groups::CreateService, '#execute' do
     end
   end
 
+  context 'creating a group with `allow_mfa_for_subgroups` attribute' do
+    let(:params) { group_params.merge(allow_mfa_for_subgroups: false) }
+    let(:service) { described_class.new(user, params) }
+
+    it 'creates group without error' do
+      expect(service.execute).to be_persisted
+    end
+  end
+
   describe 'creating a top level group' do
     let(:service) { described_class.new(user, group_params) }