From aec7e0da0e68e53c1ee0a3cb47a5dcbbb0bbf1cf Mon Sep 17 00:00:00 2001
From: Scott Sutherland <ssutherland@silverstripe.com>
Date: Thu, 5 Nov 2020 09:39:32 +1300
Subject: [PATCH] If a user has already registered for MFA, enforce use of it
 even if they are not in an MFA group

Minor PR changes
---
 src/Extension/SiteConfigExtension.php    | 10 +++++-----
 src/Service/EnforcementManager.php       |  4 ++--
 tests/Behat/features/mfa-enabled.feature |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/Extension/SiteConfigExtension.php b/src/Extension/SiteConfigExtension.php
index 71812275..082d85e7 100644
--- a/src/Extension/SiteConfigExtension.php
+++ b/src/Extension/SiteConfigExtension.php
@@ -69,16 +69,15 @@ public function updateCMSFields(FieldList $fields)
         ));
         $mfaGraceEnd->addExtraClass('mfa-settings__grace-period');
 
-        $mapFn = function ($groups = []) {
+        $groupsMap = (function () {
             $map = [];
-            foreach ($groups as $group) {
+            foreach (Group::get() 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());
+        })();
 
         $mfaGroupRestrict = ListboxField::create(
             "MFAGroupRestrictions",
@@ -88,7 +87,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
index a78ca9ea..2c710216 100644
--- 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
index 1e18da60..3db7b49d 100644
--- 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