Merge pull request #10588 from nextcloud/fix/single-2fa-provider-login-redirect

Fix login redirection if only one 2FA provider is active

4 files changed, 36 insertions, 6 deletions

diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php
index 5bd06ac7e66..5db650c4c47 100644
--- a/core/Controller/LoginController.php
+++ b/core/Controller/LoginController.php
@@ -334,7 +334,7 @@ class LoginController extends Controller {
 		if ($this->twoFactorManager->isTwoFactorAuthenticated($loginResult)) {
 			$this->twoFactorManager->prepareTwoFactorLogin($loginResult, $remember_login);
 
-			$providers = $this->twoFactorManager->getProviderSet($loginResult)->getProviders();
+			$providers = $this->twoFactorManager->getProviderSet($loginResult)->getPrimaryProviders();
 			if (count($providers) === 1) {
 				// Single provider, hence we can redirect to that provider's challenge page directly
 				/* @var $provider IProvider */
diff --git a/lib/private/Authentication/TwoFactorAuth/ProviderSet.php b/lib/private/Authentication/TwoFactorAuth/ProviderSet.php
index bbb9467798b..91a00a0bf8e 100644
--- a/lib/private/Authentication/TwoFactorAuth/ProviderSet.php
+++ b/lib/private/Authentication/TwoFactorAuth/ProviderSet.php
@@ -25,6 +25,8 @@ declare(strict_types=1);
 
 namespace OC\Authentication\TwoFactorAuth;
 
+use function array_filter;
+use OCA\TwoFactorBackupCodes\Provider\BackupCodesProvider;
 use OCP\Authentication\TwoFactorAuth\IProvider;
 
 /**
@@ -65,6 +67,15 @@ class ProviderSet {
 		return $this->providers;
 	}
 
+	/**
+	 * @return IProvider[]
+	 */
+	public function getPrimaryProviders(): array {
+		return array_filter($this->providers, function(IProvider $provider) {
+			return !($provider instanceof BackupCodesProvider);
+		});
+	}
+
 	public function isProviderMissing(): bool {
 		return $this->providerMissing;
 	}
diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php
index 7ebd6ee8340..f3e6c854808 100644
--- a/tests/Core/Controller/LoginControllerTest.php
+++ b/tests/Core/Controller/LoginControllerTest.php
@@ -27,6 +27,7 @@ use OC\Authentication\TwoFactorAuth\ProviderSet;
 use OC\Core\Controller\LoginController;
 use OC\Security\Bruteforce\Throttler;
 use OC\User\Session;
+use OCA\TwoFactorBackupCodes\Provider\BackupCodesProvider;
 use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\Authentication\TwoFactorAuth\IProvider;
@@ -594,7 +595,10 @@ class LoginControllerTest extends TestCase {
 			->will($this->returnValue('john'));
 		$password = 'secret';
 		$challengeUrl = 'challenge/url';
-		$provider = $this->createMock(IProvider::class);
+		$provider1 = $this->createMock(IProvider::class);
+		$provider1->method('getId')->willReturn('u2f');
+		$provider2 = $this->createMock(BackupCodesProvider::class);
+		$provider2->method('getId')->willReturn('backup');
 
 		$this->request
 			->expects($this->once())
@@ -616,14 +620,11 @@ class LoginControllerTest extends TestCase {
 		$this->twoFactorManager->expects($this->once())
 			->method('prepareTwoFactorLogin')
 			->with($user);
-		$providerSet = new ProviderSet([$provider], false);
+		$providerSet = new ProviderSet([$provider1, $provider2], false);
 		$this->twoFactorManager->expects($this->once())
 			->method('getProviderSet')
 			->with($user)
 			->willReturn($providerSet);
-		$provider->expects($this->once())
-			->method('getId')
-			->will($this->returnValue('u2f'));
 		$this->urlGenerator->expects($this->once())
 			->method('linkToRoute')
 			->with('core.TwoFactorChallenge.showChallenge', [
diff --git a/tests/lib/Authentication/TwoFactorAuth/ProviderSetTest.php b/tests/lib/Authentication/TwoFactorAuth/ProviderSetTest.php
index a6f0a703d5e..f294e40111d 100644
--- a/tests/lib/Authentication/TwoFactorAuth/ProviderSetTest.php
+++ b/tests/lib/Authentication/TwoFactorAuth/ProviderSetTest.php
@@ -26,6 +26,7 @@ declare(strict_types=1);
 namespace Test\Authentication\TwoFactorAuth;
 
 use OC\Authentication\TwoFactorAuth\ProviderSet;
+use OCA\TwoFactorBackupCodes\Provider\BackupCodesProvider;
 use OCP\Authentication\TwoFactorAuth\IProvider;
 use Test\TestCase;
 
@@ -49,6 +50,23 @@ class ProviderSetTest extends TestCase {
 		$this->assertEquals($expected, $set->getProviders());
 	}
 
+	public function testGet3rdPartyProviders() {
+		$p1 = $this->createMock(IProvider::class);
+		$p1->method('getId')->willReturn('p1');
+		$p2 = $this->createMock(IProvider::class);
+		$p2->method('getId')->willReturn('p2');
+		$p3 = $this->createMock(BackupCodesProvider::class);
+		$p3->method('getId')->willReturn('p3');
+		$expected = [
+			'p1' => $p1,
+			'p2' => $p2,
+		];
+
+		$set = new ProviderSet([$p2, $p1], false);
+
+		$this->assertEquals($expected, $set->getPrimaryProviders());
+	}
+
 	public function testGetProvider() {
 		$p1 = $this->createMock(IProvider::class);
 		$p1->method('getId')->willReturn('p1');