From c6e47e8a5139cccdd51a6c68e112e28b73adaddd Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 8 Aug 2018 15:25:59 +0200 Subject: [PATCH] Fix login redirection if only one 2FA provider is active Fixes https://github.com/nextcloud/server/issues/10500. Signed-off-by: Christoph Wurst --- core/Controller/LoginController.php | 2 +- .../TwoFactorAuth/ProviderSet.php | 11 +++++++++++ tests/Core/Controller/LoginControllerTest.php | 11 ++++++----- .../TwoFactorAuth/ProviderSetTest.php | 18 ++++++++++++++++++ 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index 5bd06ac7e665f..9d6f8aed88ee3 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -334,7 +334,7 @@ public function tryLogin($user, $password, $redirect_url, $remember_login = true if ($this->twoFactorManager->isTwoFactorAuthenticated($loginResult)) { $this->twoFactorManager->prepareTwoFactorLogin($loginResult, $remember_login); - $providers = $this->twoFactorManager->getProviderSet($loginResult)->getProviders(); + $providers = $this->twoFactorManager->getProviderSet($loginResult)->get3rdPartyProviders(); 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 bbb9467798b1f..63012d9ab55a1 100644 --- a/lib/private/Authentication/TwoFactorAuth/ProviderSet.php +++ b/lib/private/Authentication/TwoFactorAuth/ProviderSet.php @@ -25,6 +25,8 @@ namespace OC\Authentication\TwoFactorAuth; +use function array_filter; +use OCA\TwoFactorBackupCodes\Provider\BackupCodesProvider; use OCP\Authentication\TwoFactorAuth\IProvider; /** @@ -65,6 +67,15 @@ public function getProviders(): array { return $this->providers; } + /** + * @return IProvider[] + */ + public function get3rdPartyProviders(): 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 7ebd6ee83405d..f3e6c8548084f 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -27,6 +27,7 @@ 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 @@ public function testLoginWithOneTwoFactorProvider() { ->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 @@ public function testLoginWithOneTwoFactorProvider() { $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 a6f0a703d5e58..3587204aba96a 100644 --- a/tests/lib/Authentication/TwoFactorAuth/ProviderSetTest.php +++ b/tests/lib/Authentication/TwoFactorAuth/ProviderSetTest.php @@ -26,6 +26,7 @@ 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 @@ public function testIndexesProviders() { $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->get3rdPartyProviders()); + } + public function testGetProvider() { $p1 = $this->createMock(IProvider::class); $p1->method('getId')->willReturn('p1');