Merge pull request #10636 from nextcloud/fix/2fa-enforced-backup-codes

Fix 2FA being enforced if only backup codes provider is active

2 files changed, 35 insertions, 4 deletions

diff --git a/lib/private/Authentication/TwoFactorAuth/Manager.php b/lib/private/Authentication/TwoFactorAuth/Manager.php
index 0ee10ac0eff..6fa41897e1e 100644
--- a/lib/private/Authentication/TwoFactorAuth/Manager.php
+++ b/lib/private/Authentication/TwoFactorAuth/Manager.php
@@ -27,6 +27,8 @@ declare(strict_types = 1);
 
 namespace OC\Authentication\TwoFactorAuth;
 
+use function array_diff;
+use function array_filter;
 use BadMethodCallException;
 use Exception;
 use OC\Authentication\Exceptions\InvalidTokenException;
@@ -47,6 +49,7 @@ class Manager {
 	const SESSION_UID_KEY = 'two_factor_auth_uid';
 	const SESSION_UID_DONE = 'two_factor_auth_passed';
 	const REMEMBER_LOGIN = 'two_factor_remember_login';
+	const BACKUP_CODES_PROVIDER_ID = 'backup_codes';
 
 	/** @var ProviderLoader */
 	private $providerLoader;
@@ -76,9 +79,9 @@ class Manager {
 	private $dispatcher;
 
 	public function __construct(ProviderLoader $providerLoader,
-		IRegistry $providerRegistry, ISession $session, IConfig $config,
-		IManager $activityManager, ILogger $logger, TokenProvider $tokenProvider,
-		ITimeFactory $timeFactory, EventDispatcherInterface $eventDispatcher) {
+								IRegistry $providerRegistry, ISession $session, IConfig $config,
+								IManager $activityManager, ILogger $logger, TokenProvider $tokenProvider,
+								ITimeFactory $timeFactory, EventDispatcherInterface $eventDispatcher) {
 		$this->providerLoader = $providerLoader;
 		$this->session = $session;
 		$this->config = $config;
@@ -107,8 +110,10 @@ class Manager {
 		$providers = $this->providerLoader->getProviders($user);
 		$fixedStates = $this->fixMissingProviderStates($providerStates, $providers, $user);
 		$enabled = array_filter($fixedStates);
+		$providerIds = array_keys($enabled);
+		$providerIdsWithoutBackupCodes = array_diff($providerIds, [self::BACKUP_CODES_PROVIDER_ID]);
 
-		return $twoFactorEnabled && !empty($enabled);
+		return $twoFactorEnabled && !empty($providerIdsWithoutBackupCodes);
 	}
 
 	/**
diff --git a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
index 34ce340049a..1d7c147d9ce 100644
--- a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
+++ b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
@@ -160,6 +160,32 @@ class ManagerTest extends TestCase {
 		$this->assertFalse($this->manager->isTwoFactorAuthenticated($this->user));
 	}
 
+	public function testIsTwoFactorAuthenticatedOnlyBackupCodes() {
+		$this->user->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('user123'));
+		$this->config->expects($this->once())
+			->method('getUserValue')
+			->with('user123', 'core', 'two_factor_auth_disabled', 0)
+			->willReturn(0);
+		$this->providerRegistry->expects($this->once())
+			->method('getProviderStates')
+			->willReturn([
+				'backup_codes' => true,
+			]);
+		$backupCodesProvider = $this->createMock(IProvider::class);
+		$backupCodesProvider
+			->method('getId')
+			->willReturn('backup_codes');
+		$this->providerLoader->expects($this->once())
+			->method('getProviders')
+			->willReturn([
+				$backupCodesProvider,
+			]);
+
+		$this->assertFalse($this->manager->isTwoFactorAuthenticated($this->user));
+	}
+
 	public function testIsTwoFactorAuthenticatedFailingProviders() {
 		$this->user->expects($this->once())
 			->method('getUID')