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

Fix login redirection if only one 2FA provider is active

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 */

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;
 	}

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', [

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');