diff --git a/src/Service/EnforcementManager.php b/src/Service/EnforcementManager.php index ec12a76b..4221aad9 100644 --- a/src/Service/EnforcementManager.php +++ b/src/Service/EnforcementManager.php @@ -54,6 +54,7 @@ class EnforcementManager * * This is determined by a combination of: * + * - Whether MFA is enabled and there are methods available for use * - Whether the user has admin access (MFA is disabled by default for users that don't) * - Whether MFA is required - @see EnforcementManager::isMFARequired() * - Whether the user has registered MFA methods already @@ -63,6 +64,10 @@ class EnforcementManager */ public function canSkipMFA(Member $member): bool { + if (!$this->isEnabled()) { + return true; + } + if ($this->config()->get('requires_admin_access') && !$this->hasAdminAccess($member)) { return true; } @@ -99,13 +104,6 @@ public function shouldRedirectToMFA(Member $member): bool return false; } - $methodRegistry = MethodRegistry::singleton(); - $methods = $methodRegistry->getMethods(); - // If there are no methods available excluding backup codes, do not redirect - if (!count($methods) || (count($methods) === 1 && $methodRegistry->getBackupMethod() !== null)) { - return false; - } - if ($this->config()->get('requires_admin_access') && !$this->hasAdminAccess($member)) { return false; } @@ -246,10 +244,26 @@ protected function hasAdminAccess(Member $member): bool } /** + * MFA is enabled if: + * + * - The EnforcementManager::enabled configuration is set to true + * - There is at least one non-backup method available to register + * * @return bool */ protected function isEnabled(): bool { - return (bool) $this->config()->get('enabled'); + if (!$this->config()->get('enabled')) { + return false; + } + + $methodRegistry = MethodRegistry::singleton(); + $methods = $methodRegistry->getMethods(); + // If there are no methods available excluding backup codes, do not redirect + if (!count($methods) || (count($methods) === 1 && $methodRegistry->getBackupMethod() !== null)) { + return false; + } + + return true; } } diff --git a/tests/php/Service/EnforcementManagerTest.php b/tests/php/Service/EnforcementManagerTest.php index ea48114e..7759a911 100644 --- a/tests/php/Service/EnforcementManagerTest.php +++ b/tests/php/Service/EnforcementManagerTest.php @@ -27,6 +27,27 @@ protected function setUp() ]); EnforcementManager::config()->set('requires_admin_access', true); + EnforcementManager::config()->set('enabled', true); + } + + public function testUserCanSkipWhenMFAIsDisabled() + { + $this->setSiteConfig(['MFARequired' => true]); + EnforcementManager::config()->set('enabled', false); + + /** @var Member $member */ + $member = $this->objFromFixture(Member::class, 'sally_smith'); + $this->assertTrue(EnforcementManager::create()->canSkipMFA($member)); + } + + public function testUserCanSkipWhenNoMethodsAreAvailable() + { + $this->setSiteConfig(['MFARequired' => true]); + MethodRegistry::config()->set('methods', null); + + /** @var Member $member */ + $member = $this->objFromFixture(Member::class, 'sally_smith'); + $this->assertTrue(EnforcementManager::create()->canSkipMFA($member)); } public function testUserWithoutCMSAccessCanSkipWhenCMSAccessIsRequired()