ENH Add field for toggling between everyone and groups

src/Extension/SiteConfigExtension.php

@@ -8,8 +8,10 @@
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\ListboxField;
 use SilverStripe\Forms\OptionsetField;
+use SilverStripe\MFA\Service\EnforcementManager;
 use SilverStripe\ORM\DataExtension;
 use SilverStripe\ORM\FieldType\DBField;
+use SilverStripe\ORM\ValidationResult;
 use SilverStripe\Security\Group;
 use SilverStripe\View\Requirements;
 
@@ -34,6 +36,9 @@ class SiteConfigExtension extends DataExtension
     private static $db = [
         'MFARequired' => 'Boolean',
         'MFAGracePeriodExpires' => 'Date',
+        'MFAAppliesTo' => 'Enum(["'
+            . EnforcementManager::APPLIES_TO_EVERYONE . '","'
+            . EnforcementManager::APPLIES_TO_GROUPS . '"])',
     ];
 
     private static $many_many = [
@@ -53,8 +58,8 @@ public function updateCMSFields(FieldList $fields)
             'MFARequired',
             '',
             [
-                false => _t(__CLASS__ . '.MFA_OPTIONAL', 'MFA is optional for everyone'),
-                true => _t(__CLASS__ . '.MFA_REQUIRED', 'MFA is required for everyone'),
+                false => _t(__CLASS__ . '.MFA_OPTIONAL2', 'MFA is optional'),
+                true => _t(__CLASS__ . '.MFA_REQUIRED2', 'MFA is required'),
             ]
         );
         $mfaOptions->addExtraClass('mfa-settings__required');
@@ -76,6 +81,15 @@ public function updateCMSFields(FieldList $fields)
         }
         asort($groupsMap);
 
+        $mfaAppliesToWho = OptionsetField::create(
+            'MFAAppliesTo',
+            _t(__CLASS__ . '.MFA_APPLIES_TO_TITLE', 'Who do these MFA settings apply to?'),
+            [
+                EnforcementManager::APPLIES_TO_EVERYONE => _t(__CLASS__ . '.EVERYONE', 'Everyone'),
+                EnforcementManager::APPLIES_TO_GROUPS => _t(__CLASS__ . '.ONLY_GROUPS', 'Only these groups (choose from list)'),
+            ]
+        );
+
         $mfaGroupRestrict = ListboxField::create(
             "MFAGroupRestrictions",
             _t(__CLASS__ . '.MFA_GROUP_RESTRICTIONS', "MFA Groups")
@@ -87,11 +101,10 @@ public function updateCMSFields(FieldList $fields)
             )
             ->setDescription(_t(
                 __CLASS__ . '.MFA_GROUP_RESTRICTIONS_DESCRIPTION',
-                'MFA will only be enabled for members of these selected groups. ' .
-                'If no groups are selected, MFA will be enabled for all users'
+                'MFA will only be enabled for members of these selected groups.'
             ));
 
-        $mfaOptions = CompositeField::create($mfaOptions, $mfaGraceEnd, $mfaGroupRestrict)
+        $mfaOptions = CompositeField::create($mfaOptions, $mfaGraceEnd, $mfaAppliesToWho, $mfaGroupRestrict)
             ->setTitle(DBField::create_field(
                 'HTMLFragment',
                 _t(__CLASS__ . '.MULTI_FACTOR_AUTHENTICATION', 'Multi-factor authentication (MFA)')
@@ -101,6 +114,19 @@ public function updateCMSFields(FieldList $fields)
         $fields->addFieldToTab('Root.Access', $mfaOptions);
     }
 
+    public function validate(ValidationResult $validationResult)
+    {
+        if ($this->owner->MFAAppliesTo == EnforcementManager::APPLIES_TO_GROUPS && !$this->owner->MFAGroupRestrictions()->exists()) {
+            $validationResult->addFieldError(
+                'MFAGroupRestrictions',
+                _t(
+                    __CLASS__ . '.MFA_GROUP_RESTRICTIONS_VALIDATION',
+                    'At least one group must be selected, or the MFA settings should apply to everyone.'
+                )
+            );
+        }
+    }
+
     /**
      * Gets an anchor tag for CMS users to click to find out more about MFA in the SilverStripe CMS
      *

src/Service/EnforcementManager.php

@@ -23,6 +23,9 @@ class EnforcementManager
     use Configurable;
     use Injectable;
 
+    public const APPLIES_TO_EVERYONE = 'everyone';
+    public const APPLIES_TO_GROUPS = 'groups';
+
     /**
      * Indicate how many MFA methods the user must authenticate with before they are considered logged in
      *
@@ -72,7 +75,7 @@ public function canSkipMFA(Member $member): bool
             return true;
         }
 
-        if ($this->isMFARequired()) {
+        if ($this->isMFARequired() && $this->isUserInMFAEnabledGroup($member)) {
             return false;
         }
 
@@ -108,14 +111,14 @@ public function shouldRedirectToMFA(Member $member): bool
             return false;
         }
 
-        if (!$this->isUserInMFAEnabledGroup($member) && !$this->hasCompletedRegistration($member)) {
-            return false;
-        }
-
         if ($member->RegisteredMFAMethods()->exists()) {
             return true;
         }
 
+        if (!$this->isUserInMFAEnabledGroup($member) && !$this->hasCompletedRegistration($member)) {
+            return false;
+        }
+
         if ($this->isGracePeriodInEffect()) {
             return true;
         }
@@ -156,7 +159,9 @@ public function hasCompletedRegistration(Member $member): bool
      * Whether MFA is required for eligible users. This takes into account whether a grace period is set and whether
      * we're currently inside the window for it.
      *
-     * Note that in determining this, we ignore whether or not MFA is enabled for the site in general.
+     * Note that in determining this, we ignore whether or not MFA is enabled for the site in general. We also ignore
+     * whether any given member is in the list of groups for which MFA has been enabled - for that, call
+     * {@see isUserInMFAEnabledGroup()}
      *
      * @return bool
      */
@@ -276,9 +281,16 @@ protected function isUserInMFAEnabledGroup(Member $member): bool
         /** @var SiteConfig&SiteConfigExtension $siteConfig */
         $siteConfig = SiteConfig::current_site_config();
 
+        // If we aren't restricting by groups, then we pass this check and move on
+        // to any other applicable checks.
+        if ($siteConfig->MFAAppliesTo !== self::APPLIES_TO_GROUPS) {
+            return true;
+        }
+
         $groups = $siteConfig->MFAGroupRestrictions();
 
-        // If no groups are set in the Site Config MFAGroupRestrictions field, MFA is enabled for all users
+        // If no groups are set in the Site Config MFAGroupRestrictions field, MFA is enabled for all users.
+        // This should generally not be possible, but can happen if a group is deleted after being set in that field.
         if ($groups->count() === 0) {
             return true;
         }