If a user has already registered for MFA, enforce use of it even if t…

…hey are not in an MFA group Minor PR changes Update src/Extension/SiteConfigExtension.php Co-authored-by: Guy Marriott <guy.the.person@gmail.com>
silverstripe · Nov 4, 2020 · 0f64f6c · 0f64f6c
1 parent 60d0a54
commit 0f64f6c
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 14 deletions.
diff --git a/src/Extension/SiteConfigExtension.php b/src/Extension/SiteConfigExtension.php
@@ -69,16 +69,12 @@ public function updateCMSFields(FieldList $fields)
         ));
         $mfaGraceEnd->addExtraClass('mfa-settings__grace-period');
 
-        $mapFn = function ($groups = []) {
-            $map = [];
-            foreach ($groups as $group) {
-                // Listboxfield values are escaped, use ASCII char instead of &raquo;
-                $map[$group->ID] = $group->getBreadcrumbs(' > ');
-            }
-            asort($map);
-            return $map;
-        };
-        $groupsMap = $mapFn(Group::get());
+        $groupsMap = [];
+        foreach (Group::get() as $group) {
+            // Listboxfield values are escaped, use ASCII char instead of &raquo;
+            $groupsMap[$group->ID] = $group->getBreadcrumbs(' > ');
+        }
+        asort($groupsMap);
 
         $mfaGroupRestrict = ListboxField::create(
             "MFAGroupRestrictions",
@@ -88,7 +84,8 @@ public function updateCMSFields(FieldList $fields)
             ->setAttribute(
                 'data-placeholder',
                 _t(__CLASS__ . '.MFA_GROUP_RESTRICTIONS_PLACEHOLDER', 'Click to select group')
-            )->setDescription(_t(
+            )
+            ->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'

diff --git a/src/Service/EnforcementManager.php b/src/Service/EnforcementManager.php
@@ -108,7 +108,7 @@ public function shouldRedirectToMFA(Member $member): bool
             return false;
         }
 
-        if (!$this->isUserInMFAEnabledGroup($member)) {
+        if (!$this->isUserInMFAEnabledGroup($member) && !$this->hasCompletedRegistration($member)) {
             return false;
         }
 
@@ -271,7 +271,7 @@ protected function isEnabled(): bool
         return true;
     }
 
-    protected function isUserInMFAEnabledGroup(Member $member)
+    protected function isUserInMFAEnabledGroup(Member $member): bool
     {
         /** @var SiteConfig&SiteConfigExtension $siteConfig */
         $siteConfig = SiteConfig::current_site_config();

diff --git a/tests/Behat/features/mfa-enabled.feature b/tests/Behat/features/mfa-enabled.feature
@@ -14,4 +14,4 @@ Feature: MFA is enabled for the site
     Then I should see "Multi-factor authentication (MFA)"
     When I select "MFA is required for everyone" from the MFA settings
     And I press "Save"
-    Then I should see "Saved"
+    Then I should see a "Saved" success toast