From 7818399700fada62ba88463d9a4fcbd766d6fa51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sun, 25 Aug 2024 17:13:53 -0300 Subject: [PATCH] Fix 2FA not getting checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- psalm-baseline.xml | 5 -- src/Plugins/TwoFactor/Simple.php | 2 +- src/TwoFactor.php | 6 +- .../unit/Plugins/AuthenticationPluginTest.php | 89 ++++++++++++++++++- tests/unit/TwoFactorTest.php | 37 +++++++- 5 files changed, 124 insertions(+), 15 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index a99dc9cea4..d498627d1a 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -10002,11 +10002,6 @@ - - - - - diff --git a/src/Plugins/TwoFactor/Simple.php b/src/Plugins/TwoFactor/Simple.php index af81b5700e..dae11210e2 100644 --- a/src/Plugins/TwoFactor/Simple.php +++ b/src/Plugins/TwoFactor/Simple.php @@ -26,7 +26,7 @@ class Simple extends TwoFactorPlugin */ public function check(ServerRequest $request): bool { - return isset($_POST['2fa_confirm']); + return $request->hasBodyParam('2fa_confirm'); } /** diff --git a/src/TwoFactor.php b/src/TwoFactor.php index 88cdfcbdb3..03912b2a5d 100644 --- a/src/TwoFactor.php +++ b/src/TwoFactor.php @@ -204,7 +204,11 @@ class TwoFactor return $this->backend->check($request); } - if (! isset($_SESSION['two_factor_check']) || ! is_bool($_SESSION['two_factor_check'])) { + if ( + ! isset($_SESSION['two_factor_check']) + || ! is_bool($_SESSION['two_factor_check']) + || ! $_SESSION['two_factor_check'] + ) { $_SESSION['two_factor_check'] = $this->backend->check($request); } diff --git a/tests/unit/Plugins/AuthenticationPluginTest.php b/tests/unit/Plugins/AuthenticationPluginTest.php index 6d52611872..a76b5f2bd0 100644 --- a/tests/unit/Plugins/AuthenticationPluginTest.php +++ b/tests/unit/Plugins/AuthenticationPluginTest.php @@ -4,6 +4,9 @@ declare(strict_types=1); namespace PhpMyAdmin\Tests\Plugins; +use PhpMyAdmin\Config; +use PhpMyAdmin\ConfigStorage\Relation; +use PhpMyAdmin\ConfigStorage\RelationParameters; use PhpMyAdmin\DatabaseInterface; use PhpMyAdmin\Exceptions\AuthenticationFailure; use PhpMyAdmin\Exceptions\ExitException; @@ -20,12 +23,21 @@ final class AuthenticationPluginTest extends AbstractTestCase { public function testCheckTwoFactor(): void { + /** @psalm-suppress DeprecatedMethod */ + $config = Config::getInstance(); + /** @psalm-suppress InaccessibleProperty */ + $config->config->debug->simple2fa = true; + $GLOBALS['lang'] = 'en'; $dbiDummy = $this->createDbiDummy(); - $dbiDummy->addResult('SHOW TABLES FROM `phpmyadmin`;', [['pma__userconfig'], ['Tables_in_phpmyadmin']]); - $dbiDummy->addSelectDb('phpmyadmin'); - $dbi = $this->createDatabaseInterface($dbiDummy); - DatabaseInterface::$instance = $dbi; + $dbiDummy->addResult( + // phpcs:ignore Generic.Files.LineLength.TooLong + "SELECT `config_data`, UNIX_TIMESTAMP(`timevalue`) ts FROM `db_pma`.`pma__userconfig` WHERE `username` = 'test_user'", + [['{"2fa":{"backend":"simple","settings":[]}}', '1724620722']], + ['config_data', 'ts'], + ); + $dbiDummy->addResult('SELECT CURRENT_USER();', [['test_user@localhost']]); + DatabaseInterface::$instance = $this->createDatabaseInterface($dbiDummy); $object = new class extends AuthenticationPlugin { public function showLoginForm(): Response|null @@ -46,6 +58,13 @@ final class AuthenticationPluginTest extends AbstractTestCase $_SESSION['two_factor_check'] = false; + $relationParameters = RelationParameters::fromArray([ + 'user' => 'test_user', + 'db' => 'db_pma', + 'userconfigwork' => true, + 'userconfig' => 'pma__userconfig', + ]); + (new ReflectionProperty(Relation::class, 'cache'))->setValue(null, $relationParameters); (new ReflectionProperty(ResponseRenderer::class, 'instance'))->setValue(null, null); $responseRenderer = ResponseRenderer::getInstance(); $responseRenderer->setAjax(false); @@ -60,5 +79,67 @@ final class AuthenticationPluginTest extends AbstractTestCase 'You have enabled two factor authentication, please confirm your login.', (string) $response->getBody(), ); + + $dbiDummy->assertAllQueriesConsumed(); + $dbiDummy->assertAllSelectsConsumed(); + } + + public function testCheckTwoFactorConfirmation(): void + { + /** @psalm-suppress DeprecatedMethod */ + $config = Config::getInstance(); + /** @psalm-suppress InaccessibleProperty */ + $config->config->debug->simple2fa = true; + + $GLOBALS['lang'] = 'en'; + $dbiDummy = $this->createDbiDummy(); + $dbiDummy->addResult( + // phpcs:ignore Generic.Files.LineLength.TooLong + "SELECT `config_data`, UNIX_TIMESTAMP(`timevalue`) ts FROM `db_pma`.`pma__userconfig` WHERE `username` = 'test_user'", + [['{"2fa":{"backend":"simple","settings":[]}}', '1724620722']], + ['config_data', 'ts'], + ); + DatabaseInterface::$instance = $this->createDatabaseInterface($dbiDummy); + + $object = new class extends AuthenticationPlugin { + public function showLoginForm(): Response|null + { + return null; + } + + public function readCredentials(): bool + { + return false; + } + + public function showFailure(AuthenticationFailure $failure): Response + { + throw new ExitException(); + } + }; + + $_SESSION['two_factor_check'] = false; + + $relationParameters = RelationParameters::fromArray([ + 'user' => 'test_user', + 'db' => 'db_pma', + 'userconfigwork' => true, + 'userconfig' => 'pma__userconfig', + ]); + (new ReflectionProperty(Relation::class, 'cache'))->setValue(null, $relationParameters); + (new ReflectionProperty(ResponseRenderer::class, 'instance'))->setValue(null, null); + $responseRenderer = ResponseRenderer::getInstance(); + $responseRenderer->setAjax(false); + + $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/') + ->withParsedBody(['2fa_confirm' => '1']); + + $object->user = 'test_user'; + $response = $object->checkTwoFactor($request); + + self::assertNull($response); + + $dbiDummy->assertAllQueriesConsumed(); + $dbiDummy->assertAllSelectsConsumed(); } } diff --git a/tests/unit/TwoFactorTest.php b/tests/unit/TwoFactorTest.php index 9bcafc463c..543b74231e 100644 --- a/tests/unit/TwoFactorTest.php +++ b/tests/unit/TwoFactorTest.php @@ -11,8 +11,12 @@ use PhpMyAdmin\ConfigStorage\Relation; use PhpMyAdmin\ConfigStorage\RelationParameters; use PhpMyAdmin\Current; use PhpMyAdmin\DatabaseInterface; +use PhpMyAdmin\Http\Factory\ServerRequestFactory; use PhpMyAdmin\Http\ServerRequest; use PhpMyAdmin\Plugins\TwoFactor\Application; +use PhpMyAdmin\Plugins\TwoFactor\Invalid; +use PhpMyAdmin\Plugins\TwoFactor\Key; +use PhpMyAdmin\Plugins\TwoFactor\Simple; use PhpMyAdmin\Tests\Stubs\DbiDummy; use PhpMyAdmin\TwoFactor; use PHPUnit\Framework\Attributes\CoversClass; @@ -29,6 +33,10 @@ use function str_replace; use const JSON_UNESCAPED_SLASHES; #[CoversClass(TwoFactor::class)] +#[CoversClass(Application::class)] +#[CoversClass(Invalid::class)] +#[CoversClass(Key::class)] +#[CoversClass(Simple::class)] class TwoFactorTest extends AbstractTestCase { protected DatabaseInterface $dbi; @@ -161,6 +169,9 @@ class TwoFactorTest extends AbstractTestCase self::assertTrue($object->check($request)); self::assertSame('', $object->render($request)); + $_SESSION['two_factor_check'] = false; + self::assertTrue($object->check($request)); + $this->dummyDbi->assertAllQueriesConsumed(); $this->loadResultForConfig(['type' => 'db']); @@ -170,9 +181,24 @@ class TwoFactorTest extends AbstractTestCase self::assertSame('', $object->setup($request)); } + public function testInvalidBackend(): void + { + $object = $this->getTwoFactorAndLoadConfig('user', ['type' => 'db', 'backend' => 'unknown-backend']); + $backend = $object->getBackend(); + self::assertSame('invalid', $backend::$id); + self::assertSame('Invalid two-factor authentication', $backend::getName()); + self::assertSame('Error fallback only!', $backend::getDescription()); + $request = ServerRequestFactory::create()->createServerRequest('POST', 'https://example.com/'); + self::assertFalse($object->check($request, true)); + self::assertStringContainsString( + 'The configured two factor authentication is not available, please install missing dependencies.', + $object->render($request), + ); + self::assertSame('', $object->setup($request)); + } + public function testSimple(): void { - $request = new ServerRequest(self::createStub(ServerRequestInterface::class)); $config = Config::getInstance(); $config->config->debug->simple2fa = true; $object = $this->getTwoFactorAndLoadConfig('user', ['type' => 'db', 'backend' => 'simple']); @@ -180,12 +206,15 @@ class TwoFactorTest extends AbstractTestCase self::assertSame('simple', $backend::$id); $config->config->debug->simple2fa = false; - unset($_POST['2fa_confirm']); + $serverRequestFactory = ServerRequestFactory::create(); + $request = $serverRequestFactory->createServerRequest('POST', 'https://example.com/'); self::assertFalse($object->check($request, true)); - $_POST['2fa_confirm'] = 1; + $request = $serverRequestFactory->createServerRequest('POST', 'https://example.com/') + ->withParsedBody(['2fa_confirm' => '1']); self::assertTrue($object->check($request, true)); - unset($_POST['2fa_confirm']); + + $request = $serverRequestFactory->createServerRequest('POST', 'https://example.com/'); /* Test rendering */ self::assertNotEquals('', $object->render($request));