Fix 2FA being enforced if only backup codes provider is active
Fixes https://github.com/nextcloud/server/issues/10634. Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
This commit is contained in:
parent
103a2c30fb
commit
1124b87bc0
|
@ -27,6 +27,8 @@ declare(strict_types = 1);
|
||||||
|
|
||||||
namespace OC\Authentication\TwoFactorAuth;
|
namespace OC\Authentication\TwoFactorAuth;
|
||||||
|
|
||||||
|
use function array_diff;
|
||||||
|
use function array_filter;
|
||||||
use BadMethodCallException;
|
use BadMethodCallException;
|
||||||
use Exception;
|
use Exception;
|
||||||
use OC\Authentication\Exceptions\InvalidTokenException;
|
use OC\Authentication\Exceptions\InvalidTokenException;
|
||||||
|
@ -47,6 +49,7 @@ class Manager {
|
||||||
const SESSION_UID_KEY = 'two_factor_auth_uid';
|
const SESSION_UID_KEY = 'two_factor_auth_uid';
|
||||||
const SESSION_UID_DONE = 'two_factor_auth_passed';
|
const SESSION_UID_DONE = 'two_factor_auth_passed';
|
||||||
const REMEMBER_LOGIN = 'two_factor_remember_login';
|
const REMEMBER_LOGIN = 'two_factor_remember_login';
|
||||||
|
const BACKUP_CODES_PROVIDER_ID = 'backup_codes';
|
||||||
|
|
||||||
/** @var ProviderLoader */
|
/** @var ProviderLoader */
|
||||||
private $providerLoader;
|
private $providerLoader;
|
||||||
|
@ -76,9 +79,9 @@ class Manager {
|
||||||
private $dispatcher;
|
private $dispatcher;
|
||||||
|
|
||||||
public function __construct(ProviderLoader $providerLoader,
|
public function __construct(ProviderLoader $providerLoader,
|
||||||
IRegistry $providerRegistry, ISession $session, IConfig $config,
|
IRegistry $providerRegistry, ISession $session, IConfig $config,
|
||||||
IManager $activityManager, ILogger $logger, TokenProvider $tokenProvider,
|
IManager $activityManager, ILogger $logger, TokenProvider $tokenProvider,
|
||||||
ITimeFactory $timeFactory, EventDispatcherInterface $eventDispatcher) {
|
ITimeFactory $timeFactory, EventDispatcherInterface $eventDispatcher) {
|
||||||
$this->providerLoader = $providerLoader;
|
$this->providerLoader = $providerLoader;
|
||||||
$this->session = $session;
|
$this->session = $session;
|
||||||
$this->config = $config;
|
$this->config = $config;
|
||||||
|
@ -107,8 +110,10 @@ class Manager {
|
||||||
$providers = $this->providerLoader->getProviders($user);
|
$providers = $this->providerLoader->getProviders($user);
|
||||||
$fixedStates = $this->fixMissingProviderStates($providerStates, $providers, $user);
|
$fixedStates = $this->fixMissingProviderStates($providerStates, $providers, $user);
|
||||||
$enabled = array_filter($fixedStates);
|
$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);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -27,6 +27,7 @@ use OC;
|
||||||
use OC\Authentication\Token\IProvider as TokenProvider;
|
use OC\Authentication\Token\IProvider as TokenProvider;
|
||||||
use OC\Authentication\TwoFactorAuth\Manager;
|
use OC\Authentication\TwoFactorAuth\Manager;
|
||||||
use OC\Authentication\TwoFactorAuth\ProviderLoader;
|
use OC\Authentication\TwoFactorAuth\ProviderLoader;
|
||||||
|
use OCA\TwoFactorBackupCodes\Provider\BackupCodesProvider;
|
||||||
use OCP\Activity\IEvent;
|
use OCP\Activity\IEvent;
|
||||||
use OCP\Activity\IManager;
|
use OCP\Activity\IManager;
|
||||||
use OCP\AppFramework\Utility\ITimeFactory;
|
use OCP\AppFramework\Utility\ITimeFactory;
|
||||||
|
@ -160,6 +161,32 @@ class ManagerTest extends TestCase {
|
||||||
$this->assertFalse($this->manager->isTwoFactorAuthenticated($this->user));
|
$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() {
|
public function testIsTwoFactorAuthenticatedFailingProviders() {
|
||||||
$this->user->expects($this->once())
|
$this->user->expects($this->once())
|
||||||
->method('getUID')
|
->method('getUID')
|
||||||
|
|
Loading…
Reference in New Issue