Merge branch...

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

Allow groups to disable 2FA requirement for subgroups - validations

See merge request gitlab-org/gitlab!44038

app/models/group.rb

@@ -76,6 +76,7 @@ class Group < Namespace
   validate :visibility_level_allowed_by_projects
   validate :visibility_level_allowed_by_sub_groups
   validate :visibility_level_allowed_by_parent
+  validate :two_factor_authentication_allowed
   validates :variables, variable_duplicates: true
 
   validates :two_factor_grace_period, presence: true, numericality: { greater_than_or_equal_to: 0 }
@@ -589,6 +590,16 @@ class Group < Namespace
     errors.add(:visibility_level, "#{visibility} is not allowed since there are sub-groups with higher visibility.")
   end
 
+  def two_factor_authentication_allowed
+    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
+
+    errors.add(:require_two_factor_authentication, _('is forbidden by a top-level group'))
+  end
+
   def members_from_self_and_ancestor_group_shares
     group_group_link_table = GroupGroupLink.arel_table
     group_member_table = GroupMember.arel_table

app/models/namespace_setting.rb

@@ -4,6 +4,7 @@ class NamespaceSetting < ApplicationRecord
   belongs_to :namespace, inverse_of: :namespace_settings
 
   validate :default_branch_name_content
+  validate :allow_mfa_for_group
 
   NAMESPACE_SETTINGS_PARAMS = [:default_branch_name].freeze
 
@@ -16,6 +17,12 @@ class NamespaceSetting < ApplicationRecord
       errors.add(:default_branch_name, "can not be an empty string")
     end
   end
+
+  def allow_mfa_for_group
+    if namespace&.subgroup? && allow_mfa_for_subgroups == false
+      errors.add(:allow_mfa_for_subgroups, _('is not allowed since the group is not top-level group.'))
+    end
+  end
 end
 
 NamespaceSetting.prepend_if_ee('EE::NamespaceSetting')

locale/gitlab.pot

@@ -30861,6 +30861,9 @@ msgstr ""
 msgid "is blocked by"
 msgstr ""
 
+msgid "is forbidden by a top-level group"
+msgstr ""
+
 msgid "is invalid because there is downstream lock"
 msgstr ""
 
@@ -30876,6 +30879,9 @@ msgstr ""
 msgid "is not a valid X509 certificate."
 msgstr ""
 
+msgid "is not allowed since the group is not top-level group."
+msgstr ""
+
 msgid "is not allowed. Try again with a different email address, or contact your GitLab admin."
 msgstr ""
 

spec/models/group_spec.rb

@@ -222,6 +222,36 @@ RSpec.describe Group do
         end
       end
     end
+
+    describe '#two_factor_authentication_allowed' do
+      let_it_be(:group) { create(:group) }
+
+      context 'for a parent group' do
+        it 'is valid' do
+          group.require_two_factor_authentication = true
+
+          expect(group).to be_valid
+        end
+      end
+
+      context 'for a child group' do
+        let(:sub_group) { create(:group, parent: group) }
+
+        it 'is valid when parent group allows' do
+          sub_group.require_two_factor_authentication = true
+
+          expect(sub_group).to be_valid
+        end
+
+        it 'is invalid when parent group blocks' do
+          group.namespace_settings.update!(allow_mfa_for_subgroups: false)
+          sub_group.require_two_factor_authentication = true
+
+          expect(sub_group).to be_invalid
+          expect(sub_group.errors[:require_two_factor_authentication]).to include('is forbidden by a top-level group')
+        end
+      end
+    end
   end
 
   describe '.without_integration' do

spec/models/namespace_setting_spec.rb

@@ -5,7 +5,9 @@ require 'spec_helper'
 RSpec.describe NamespaceSetting, type: :model do
   # Relationships
   #
-  it { is_expected.to belong_to(:namespace) }
+  describe "Associations" do
+    it { is_expected.to belong_to(:namespace) }
+  end
 
   describe "validations" do
     describe "#default_branch_name_content" do
@@ -43,5 +45,29 @@ RSpec.describe NamespaceSetting, type: :model do
         end
       end
     end
+
+    describe '#allow_mfa_for_group' do
+      let(:settings) {  group.namespace_settings }
+
+      context 'group is top-level group' do
+        let(:group) { create(:group) }
+
+        it 'is valid' do
+          settings.allow_mfa_for_subgroups = false
+
+          expect(settings).to be_valid
+        end
+      end
+
+      context 'group is a subgroup' do
+        let(:group) { create(:group, parent: create(:group)) }
+
+        it 'is invalid' do
+          settings.allow_mfa_for_subgroups = false
+
+          expect(settings).to be_invalid
+        end
+      end
+    end
   end
 end