From 56c1c9fc5ca035f289b32856a751a69f0b9f97dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Fri, 20 Oct 2017 15:50:59 +0200 Subject: [PATCH 01/12] Share base code for AuthenticationPlugin::authSetUser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michal Čihař --- .../classes/Plugins/Auth/AuthenticationConfig.php | 12 ------------ .../classes/Plugins/Auth/AuthenticationCookie.php | 3 ++- .../classes/Plugins/Auth/AuthenticationHttp.php | 4 +--- .../classes/Plugins/Auth/AuthenticationSignon.php | 2 +- libraries/classes/Plugins/AuthenticationPlugin.php | 7 ++++++- 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/libraries/classes/Plugins/Auth/AuthenticationConfig.php b/libraries/classes/Plugins/Auth/AuthenticationConfig.php index 9d53919249..1e0ca95255 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationConfig.php +++ b/libraries/classes/Plugins/Auth/AuthenticationConfig.php @@ -57,18 +57,6 @@ class AuthenticationConfig extends AuthenticationPlugin return true; } - /** - * Set the user and password after last checkings if required - * - * @return boolean always true - */ - public function authSetUser() - { - $this->setSessionAccessTime(); - - return true; - } - /** * User is not allowed to login to MySQL -> authentication failed * diff --git a/libraries/classes/Plugins/Auth/AuthenticationCookie.php b/libraries/classes/Plugins/Auth/AuthenticationCookie.php index 29cd4091ee..f0e1ac39f5 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationCookie.php +++ b/libraries/classes/Plugins/Auth/AuthenticationCookie.php @@ -485,7 +485,8 @@ class AuthenticationCookie extends AuthenticationPlugin // Avoid showing the password in phpinfo()'s output unset($GLOBALS['PHP_AUTH_PW']); unset($_SERVER['PHP_AUTH_PW']); - $this->setSessionAccessTime(); + + return parent::authSetUser(); } /** diff --git a/libraries/classes/Plugins/Auth/AuthenticationHttp.php b/libraries/classes/Plugins/Auth/AuthenticationHttp.php index d58c35c1f8..9f6065d207 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationHttp.php +++ b/libraries/classes/Plugins/Auth/AuthenticationHttp.php @@ -205,9 +205,7 @@ class AuthenticationHttp extends AuthenticationPlugin unset($GLOBALS['PHP_AUTH_PW']); unset($_SERVER['PHP_AUTH_PW']); - $this->setSessionAccessTime(); - - return true; + return parent::authSetUser(); } /** diff --git a/libraries/classes/Plugins/Auth/AuthenticationSignon.php b/libraries/classes/Plugins/Auth/AuthenticationSignon.php index faa43ab7ae..8d5370dae8 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationSignon.php +++ b/libraries/classes/Plugins/Auth/AuthenticationSignon.php @@ -221,7 +221,7 @@ class AuthenticationSignon extends AuthenticationPlugin $cfg['Server']['user'] = $PHP_AUTH_USER; $cfg['Server']['password'] = $PHP_AUTH_PW; - return true; + return parent::authSetUser(); } /** diff --git a/libraries/classes/Plugins/AuthenticationPlugin.php b/libraries/classes/Plugins/AuthenticationPlugin.php index d112c5a487..b08042c56c 100644 --- a/libraries/classes/Plugins/AuthenticationPlugin.php +++ b/libraries/classes/Plugins/AuthenticationPlugin.php @@ -38,7 +38,12 @@ abstract class AuthenticationPlugin * * @return boolean */ - abstract public function authSetUser(); + public function authSetUser(); + { + $this->setSessionAccessTime(); + + return true; + } /** * Stores user credentials after successful login. From 13d2ff1f5b98db2924e6518a90078416604ad8c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Fri, 20 Oct 2017 15:57:26 +0200 Subject: [PATCH 02/12] Consistely use no return value for AuthenticationPlugin::authFails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It really does not return in the end, so make the docs consistent with the code. Signed-off-by: Michal Čihař --- .../classes/Plugins/Auth/AuthenticationConfig.php | 4 +--- libraries/classes/Plugins/Auth/AuthenticationHttp.php | 10 +++------- libraries/classes/Plugins/AuthenticationPlugin.php | 4 ++-- test/classes/Plugins/Auth/AuthenticationConfigTest.php | 6 +----- test/classes/Plugins/Auth/AuthenticationHttpTest.php | 8 ++------ 5 files changed, 9 insertions(+), 23 deletions(-) diff --git a/libraries/classes/Plugins/Auth/AuthenticationConfig.php b/libraries/classes/Plugins/Auth/AuthenticationConfig.php index 1e0ca95255..0331e6544f 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationConfig.php +++ b/libraries/classes/Plugins/Auth/AuthenticationConfig.php @@ -60,7 +60,7 @@ class AuthenticationConfig extends AuthenticationPlugin /** * User is not allowed to login to MySQL -> authentication failed * - * @return boolean always true (no return indeed) + * @return void */ public function authFails() { @@ -160,7 +160,5 @@ class AuthenticationConfig extends AuthenticationPlugin if (!defined('TESTSUITE')) { exit; } - - return true; } } diff --git a/libraries/classes/Plugins/Auth/AuthenticationHttp.php b/libraries/classes/Plugins/Auth/AuthenticationHttp.php index 9f6065d207..65045149d0 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationHttp.php +++ b/libraries/classes/Plugins/Auth/AuthenticationHttp.php @@ -211,20 +211,16 @@ class AuthenticationHttp extends AuthenticationPlugin /** * User is not allowed to login to MySQL -> authentication failed * - * @return bool true + * @return void */ public function authFails() { $error = $GLOBALS['dbi']->getError(); if ($error && $GLOBALS['errno'] != 1045) { Core::fatalError($error); - - return true; + } else { + $this->authForm(); } - - $this->authForm(); - - return true; } /** diff --git a/libraries/classes/Plugins/AuthenticationPlugin.php b/libraries/classes/Plugins/AuthenticationPlugin.php index b08042c56c..a1f28d100f 100644 --- a/libraries/classes/Plugins/AuthenticationPlugin.php +++ b/libraries/classes/Plugins/AuthenticationPlugin.php @@ -38,7 +38,7 @@ abstract class AuthenticationPlugin * * @return boolean */ - public function authSetUser(); + public function authSetUser() { $this->setSessionAccessTime(); @@ -57,7 +57,7 @@ abstract class AuthenticationPlugin /** * User is not allowed to login to MySQL -> authentication failed * - * @return boolean + * @return void */ abstract public function authFails(); diff --git a/test/classes/Plugins/Auth/AuthenticationConfigTest.php b/test/classes/Plugins/Auth/AuthenticationConfigTest.php index 27ec8e0c06..09f7a58d5e 100644 --- a/test/classes/Plugins/Auth/AuthenticationConfigTest.php +++ b/test/classes/Plugins/Auth/AuthenticationConfigTest.php @@ -101,13 +101,9 @@ class AuthenticationConfigTest extends PmaTestCase $GLOBALS['dbi'] = $dbi; ob_start(); - $result = $this->object->authFails(); + $this->object->authFails(); $html = ob_get_clean(); - $this->assertTrue( - $result - ); - $this->assertContains( 'You probably did not create a configuration file. You might want ' . 'to use the setup script to create one.', diff --git a/test/classes/Plugins/Auth/AuthenticationHttpTest.php b/test/classes/Plugins/Auth/AuthenticationHttpTest.php index cd70af767f..9799f64ea0 100644 --- a/test/classes/Plugins/Auth/AuthenticationHttpTest.php +++ b/test/classes/Plugins/Auth/AuthenticationHttpTest.php @@ -426,14 +426,10 @@ class AuthenticationHttpTest extends PmaTestCase $GLOBALS['cfg']['Server']['host'] = 'host'; $GLOBALS['errno'] = 1045; - $this->assertTrue( - $this->object->authFails() - ); + $this->object->authFails(); // case 3 $GLOBALS['errno'] = 1043; - $this->assertTrue( - $this->object->authFails() - ); + $this->object->authFails(); } } From 026d7221781e23480aaa364389e6ec16db4ae9fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Fri, 20 Oct 2017 16:00:56 +0200 Subject: [PATCH 03/12] Rationalize AuthenticationPlugin API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the API more consistent, remove not needed auth prefix and stop talking about advanced authentication which has been there about 10 years ago. API changed: - authCheck is now readCredentials - authSetUser is now storeCredentials - auth is now showLoginForm - authFails is now showFailure - storeUserCredentials is now rememberCredentials Signed-off-by: Michal Čihař --- .../Plugins/Auth/AuthenticationConfig.php | 8 +- .../Plugins/Auth/AuthenticationCookie.php | 32 ++++--- .../Plugins/Auth/AuthenticationHttp.php | 12 +-- .../Plugins/Auth/AuthenticationSignon.php | 14 +-- .../classes/Plugins/AuthenticationPlugin.php | 12 +-- libraries/common.inc.php | 16 ++-- .../Plugins/Auth/AuthenticationConfigTest.php | 16 ++-- .../Plugins/Auth/AuthenticationCookieTest.php | 86 +++++++++---------- .../Plugins/Auth/AuthenticationHttpTest.php | 26 +++--- .../Plugins/Auth/AuthenticationSignonTest.php | 66 +++++++------- 10 files changed, 143 insertions(+), 145 deletions(-) diff --git a/libraries/classes/Plugins/Auth/AuthenticationConfig.php b/libraries/classes/Plugins/Auth/AuthenticationConfig.php index 0331e6544f..e9e7a6e144 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationConfig.php +++ b/libraries/classes/Plugins/Auth/AuthenticationConfig.php @@ -26,7 +26,7 @@ class AuthenticationConfig extends AuthenticationPlugin * * @return boolean always true */ - public function auth() + public function showLoginForm() { $response = Response::getInstance(); if ($response->isAjax()) { @@ -44,11 +44,11 @@ class AuthenticationConfig extends AuthenticationPlugin } /** - * Gets advanced authentication settings + * Gets authentication credentials * * @return boolean always true */ - public function authCheck() + public function readCredentials() { if ($GLOBALS['token_provided'] && $GLOBALS['token_mismatch']) { return false; @@ -62,7 +62,7 @@ class AuthenticationConfig extends AuthenticationPlugin * * @return void */ - public function authFails() + public function showFailure() { $conn_error = $GLOBALS['dbi']->getError(); if (!$conn_error) { diff --git a/libraries/classes/Plugins/Auth/AuthenticationCookie.php b/libraries/classes/Plugins/Auth/AuthenticationCookie.php index f0e1ac39f5..1c9eeb0864 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationCookie.php +++ b/libraries/classes/Plugins/Auth/AuthenticationCookie.php @@ -79,7 +79,7 @@ class AuthenticationCookie extends AuthenticationPlugin * * @return boolean|void */ - public function auth() + public function showLoginForm() { global $conn_error; @@ -272,22 +272,22 @@ class AuthenticationCookie extends AuthenticationPlugin } /** - * Gets advanced authentication settings + * Gets authentication credentials * * this function DOES NOT check authentication - it just checks/provides * authentication credentials required to connect to the MySQL server * usually with $GLOBALS['dbi']->connect() * * it returns false if something is missing - which usually leads to - * auth() which displays login form + * showLoginForm() which displays login form * * it returns true if all seems ok which usually leads to auth_set_user() * - * it directly switches to authFails() if user inactivity timeout is reached + * it directly switches to showFailure() if user inactivity timeout is reached * * @return boolean whether we get authentication settings or not */ - public function authCheck() + public function readCredentials() { global $conn_error; @@ -416,7 +416,7 @@ class AuthenticationCookie extends AuthenticationPlugin Util::cacheUnset('proc_priv'); $GLOBALS['no_activity'] = true; - $this->authFails(); + $this->showFailure(); if (! defined('TESTSUITE')) { exit; } else { @@ -455,7 +455,7 @@ class AuthenticationCookie extends AuthenticationPlugin * * @return boolean always true */ - public function authSetUser() + public function storeCredentials() { global $cfg; @@ -486,7 +486,7 @@ class AuthenticationCookie extends AuthenticationPlugin unset($GLOBALS['PHP_AUTH_PW']); unset($_SERVER['PHP_AUTH_PW']); - return parent::authSetUser(); + return parent::storeCredentials(); } /** @@ -494,19 +494,17 @@ class AuthenticationCookie extends AuthenticationPlugin * * @return void|bool */ - public function storeUserCredentials() + public function rememberCredentials() { - global $cfg; - // Name and password cookies need to be refreshed each time // Duration = one month for username - $this->storeUsernameCookie($cfg['Server']['user']); + $this->storeUsernameCookie($this->user); // Duration = as configured // Do not store password cookie on password change as we will // set the cookie again after password has been changed if (! isset($_POST['change_pw'])) { - $this->storePasswordCookie($cfg['Server']['password']); + $this->storePasswordCookie($this->password); } // Set server cookies if required (once per session) and, in this case, @@ -600,15 +598,15 @@ class AuthenticationCookie extends AuthenticationPlugin /** * User is not allowed to login to MySQL -> authentication failed * - * prepares error message and switches to auth() which display the error + * prepares error message and switches to showLoginForm() which display the error * and the login form * * this function MUST exit/quit the application, - * currently done by call to auth() + * currently done by call to showLoginForm() * * @return void */ - public function authFails() + public function showFailure() { global $conn_error; @@ -623,7 +621,7 @@ class AuthenticationCookie extends AuthenticationPlugin $response->header('Cache-Control: no-store, no-cache, must-revalidate'); $response->header('Pragma: no-cache'); - $this->auth(); + $this->showLoginForm(); } /** diff --git a/libraries/classes/Plugins/Auth/AuthenticationHttp.php b/libraries/classes/Plugins/Auth/AuthenticationHttp.php index 65045149d0..a11fee5ac6 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationHttp.php +++ b/libraries/classes/Plugins/Auth/AuthenticationHttp.php @@ -29,7 +29,7 @@ class AuthenticationHttp extends AuthenticationPlugin * * @return boolean always true (no return indeed) */ - public function auth() + public function showLoginForm() { $response = Response::getInstance(); if ($response->isAjax()) { @@ -100,14 +100,14 @@ class AuthenticationHttp extends AuthenticationPlugin } /** - * Gets advanced authentication settings + * Gets authentication credentials * * @global string $PHP_AUTH_USER the username * @global string $PHP_AUTH_PW the password * * @return boolean whether we get authentication settings or not */ - public function authCheck() + public function readCredentials() { global $PHP_AUTH_USER, $PHP_AUTH_PW; @@ -193,7 +193,7 @@ class AuthenticationHttp extends AuthenticationPlugin * * @return boolean always true */ - public function authSetUser() + public function storeCredentials() { global $cfg, $server; global $PHP_AUTH_USER, $PHP_AUTH_PW; @@ -205,7 +205,7 @@ class AuthenticationHttp extends AuthenticationPlugin unset($GLOBALS['PHP_AUTH_PW']); unset($_SERVER['PHP_AUTH_PW']); - return parent::authSetUser(); + return parent::storeCredentials(); } /** @@ -213,7 +213,7 @@ class AuthenticationHttp extends AuthenticationPlugin * * @return void */ - public function authFails() + public function showFailure() { $error = $GLOBALS['dbi']->getError(); if ($error && $GLOBALS['errno'] != 1045) { diff --git a/libraries/classes/Plugins/Auth/AuthenticationSignon.php b/libraries/classes/Plugins/Auth/AuthenticationSignon.php index 8d5370dae8..2d1e6bf728 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationSignon.php +++ b/libraries/classes/Plugins/Auth/AuthenticationSignon.php @@ -24,7 +24,7 @@ class AuthenticationSignon extends AuthenticationPlugin * * @return boolean always true (no return indeed) */ - public function auth() + public function showLoginForm() { unset($_SESSION['LAST_SIGNON_URL']); if (empty($GLOBALS['cfg']['Server']['SignonURL'])) { @@ -41,14 +41,14 @@ class AuthenticationSignon extends AuthenticationPlugin } /** - * Gets advanced authentication settings + * Gets authentication credentials * * @global string $PHP_AUTH_USER the username * @global string $PHP_AUTH_PW the password * * @return boolean whether we get authentication settings or not */ - public function authCheck() + public function readCredentials() { global $PHP_AUTH_USER, $PHP_AUTH_PW; @@ -213,7 +213,7 @@ class AuthenticationSignon extends AuthenticationPlugin * * @return boolean always true */ - public function authSetUser() + public function storeCredentials() { global $cfg; global $PHP_AUTH_USER, $PHP_AUTH_PW; @@ -221,7 +221,7 @@ class AuthenticationSignon extends AuthenticationPlugin $cfg['Server']['user'] = $PHP_AUTH_USER; $cfg['Server']['password'] = $PHP_AUTH_PW; - return parent::authSetUser(); + return parent::storeCredentials(); } /** @@ -229,7 +229,7 @@ class AuthenticationSignon extends AuthenticationPlugin * * @return boolean always true (no return indeed) */ - public function authFails() + public function showFailure() { /* Session name */ $session_name = $GLOBALS['cfg']['Server']['SignonSession']; @@ -249,7 +249,7 @@ class AuthenticationSignon extends AuthenticationPlugin /* Set error message */ $_SESSION['PMA_single_signon_error_message'] = $this->getErrorMessage(); } - $this->auth(); + $this->showLoginForm(); } /** diff --git a/libraries/classes/Plugins/AuthenticationPlugin.php b/libraries/classes/Plugins/AuthenticationPlugin.php index a1f28d100f..5dcd2eedb3 100644 --- a/libraries/classes/Plugins/AuthenticationPlugin.php +++ b/libraries/classes/Plugins/AuthenticationPlugin.php @@ -24,21 +24,21 @@ abstract class AuthenticationPlugin * * @return boolean */ - abstract public function auth(); + abstract public function showLoginForm(); /** - * Gets advanced authentication settings + * Gets authentication credentials * * @return boolean */ - abstract public function authCheck(); + abstract public function readCredentials(); /** * Set the user and password after last checkings if required * * @return boolean */ - public function authSetUser() + public function storeCredentials() { $this->setSessionAccessTime(); @@ -50,7 +50,7 @@ abstract class AuthenticationPlugin * * @return void */ - public function storeUserCredentials() + public function rememberCredentials() { } @@ -59,7 +59,7 @@ abstract class AuthenticationPlugin * * @return void */ - abstract public function authFails(); + abstract public function showFailure(); /** * Perform logout diff --git a/libraries/common.inc.php b/libraries/common.inc.php index 34a377715d..212a4e9c11 100644 --- a/libraries/common.inc.php +++ b/libraries/common.inc.php @@ -512,12 +512,12 @@ if (! defined('PMA_MINIMUM_COMMON')) { /** @var AuthenticationPlugin $auth_plugin */ $auth_plugin = new $auth_class($plugin_manager); - if (! $auth_plugin->authCheck()) { + if (! $auth_plugin->readCredentials()) { /* Force generating of new session on login */ Session::secure(); - $auth_plugin->auth(); + $auth_plugin->showLoginForm(); } else { - $auth_plugin->authSetUser(); + $auth_plugin->storeCredentials(); } // Check IP-based Allow/Deny rules as soon as possible to reject the @@ -552,7 +552,7 @@ if (! defined('PMA_MINIMUM_COMMON')) { // Ejects the user if banished if ($allowDeny_forbidden) { Logging::logUser($cfg['Server']['user'], 'allow-denied'); - $auth_plugin->authFails(); + $auth_plugin->showFailure(); } } // end if @@ -560,7 +560,7 @@ if (! defined('PMA_MINIMUM_COMMON')) { if (! $cfg['Server']['AllowRoot'] && $cfg['Server']['user'] == 'root') { $allowDeny_forbidden = true; Logging::logUser($cfg['Server']['user'], 'root-denied'); - $auth_plugin->authFails(); + $auth_plugin->showFailure(); } // is a login without password allowed? @@ -569,7 +569,7 @@ if (! defined('PMA_MINIMUM_COMMON')) { ) { $login_without_password_is_forbidden = true; Logging::logUser($cfg['Server']['user'], 'empty-denied'); - $auth_plugin->authFails(); + $auth_plugin->showFailure(); } // Try to connect MySQL with the control user profile (will be used to @@ -589,7 +589,7 @@ if (! defined('PMA_MINIMUM_COMMON')) { if ($userlink === false) { Logging::logUser($cfg['Server']['user'], 'mysql-denied'); - $GLOBALS['auth_plugin']->authFails(); + $GLOBALS['auth_plugin']->showFailure(); } // Set timestamp for the session, if required. @@ -633,7 +633,7 @@ if (! defined('PMA_MINIMUM_COMMON')) { $controllink = $GLOBALS['dbi']->connect(DatabaseInterface::CONNECT_USER); } - $auth_plugin->storeUserCredentials(); + $auth_plugin->rememberCredentials(); /* Log success */ Logging::logUser($cfg['Server']['user']); diff --git a/test/classes/Plugins/Auth/AuthenticationConfigTest.php b/test/classes/Plugins/Auth/AuthenticationConfigTest.php index 09f7a58d5e..9fee31be22 100644 --- a/test/classes/Plugins/Auth/AuthenticationConfigTest.php +++ b/test/classes/Plugins/Auth/AuthenticationConfigTest.php @@ -47,43 +47,43 @@ class AuthenticationConfigTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::auth + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::showLoginForm * * @return void */ public function testAuth() { $this->assertTrue( - $this->object->auth() + $this->object->showLoginForm() ); } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authCheck + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::readCredentials * * @return void */ public function testAuthCheck() { $this->assertTrue( - $this->object->authCheck() + $this->object->readCredentials() ); } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authSetUser + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::storeCredentials * * @return void */ public function testAuthSetUser() { $this->assertTrue( - $this->object->authSetUser() + $this->object->storeCredentials() ); } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authFails + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::showFailure * * @return void */ @@ -101,7 +101,7 @@ class AuthenticationConfigTest extends PmaTestCase $GLOBALS['dbi'] = $dbi; ob_start(); - $this->object->authFails(); + $this->object->showFailure(); $html = ob_get_clean(); $this->assertContains( diff --git a/test/classes/Plugins/Auth/AuthenticationCookieTest.php b/test/classes/Plugins/Auth/AuthenticationCookieTest.php index 0b37133ca0..741630f5da 100644 --- a/test/classes/Plugins/Auth/AuthenticationCookieTest.php +++ b/test/classes/Plugins/Auth/AuthenticationCookieTest.php @@ -60,7 +60,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::auth + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::showLoginForm * * @return void * @group medium @@ -87,12 +87,12 @@ class AuthenticationCookieTest extends PmaTestCase $GLOBALS['conn_error'] = true; $this->assertTrue( - $this->object->auth() + $this->object->showLoginForm() ); } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::auth + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::showLoginForm * * @return void * @group medium @@ -196,7 +196,7 @@ class AuthenticationCookieTest extends PmaTestCase $GLOBALS['error_handler'] = $mockErrorHandler; ob_start(); - $this->object->auth(); + $this->object->showLoginForm(); $result = ob_get_clean(); // assertions @@ -261,7 +261,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::auth + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::showLoginForm * * @return void * @group medium @@ -299,7 +299,7 @@ class AuthenticationCookieTest extends PmaTestCase $GLOBALS['error_handler'] = new ErrorHandler; ob_start(); - $this->object->auth(); + $this->object->showLoginForm(); $result = ob_get_clean(); // assertions @@ -344,7 +344,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::auth with headers + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::showLoginForm with headers * * @return void */ @@ -362,7 +362,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::auth with headers + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::showLoginForm with headers * * @return void */ @@ -382,7 +382,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authCheck + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::readCredentials * * @return void */ @@ -394,7 +394,7 @@ class AuthenticationCookieTest extends PmaTestCase $_REQUEST['pma_username'] = 'testPMAUser'; $this->assertFalse( - $this->object->authCheck() + $this->object->readCredentials() ); $this->assertEquals( @@ -404,7 +404,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authCheck + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::readCredentials * * @return void */ @@ -428,7 +428,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authCheck + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::readCredentials * * @return void */ @@ -454,7 +454,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authCheck + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::readCredentials * * @return void */ @@ -469,7 +469,7 @@ class AuthenticationCookieTest extends PmaTestCase $GLOBALS['cfg']['AllowArbitraryServer'] = true; $this->assertTrue( - $this->object->authCheck() + $this->object->readCredentials() ); $this->assertEquals( @@ -493,7 +493,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authCheck + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::readCredentials * * @return void */ @@ -508,12 +508,12 @@ class AuthenticationCookieTest extends PmaTestCase $_COOKIE['pma_iv-1'] = base64_encode('testiv09testiv09'); $this->assertFalse( - $this->object->authCheck() + $this->object->readCredentials() ); } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authCheck + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::readCredentials * * @return void */ @@ -529,12 +529,12 @@ class AuthenticationCookieTest extends PmaTestCase $GLOBALS['cfg']['LoginCookieValidity'] = 1440; $this->assertFalse( - $this->object->authCheck() + $this->object->readCredentials() ); } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authCheck (mock blowfish functions reqd) + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::readCredentials (mock blowfish functions reqd) * * @return void */ @@ -562,7 +562,7 @@ class AuthenticationCookieTest extends PmaTestCase ->will($this->returnValue('testBF')); $this->assertFalse( - $this->object->authCheck() + $this->object->readCredentials() ); $this->assertEquals( @@ -572,7 +572,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authCheck (mocking blowfish functions) + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::readCredentials (mocking blowfish functions) * * @return void */ @@ -602,7 +602,7 @@ class AuthenticationCookieTest extends PmaTestCase ->will($this->returnValue('{"password":""}')); $this->assertTrue( - $this->object->authCheck() + $this->object->readCredentials() ); $this->assertTrue( @@ -617,7 +617,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authCheck (mocking the object itself) + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::readCredentials (mocking the object itself) * * @return void */ @@ -638,14 +638,14 @@ class AuthenticationCookieTest extends PmaTestCase // mock for blowfish function $this->object = $this->getMockBuilder('PhpMyAdmin\Plugins\Auth\AuthenticationCookie') ->disableOriginalConstructor() - ->setMethods(array('authFails')) + ->setMethods(array('showFailure')) ->getMock(); $this->object->expects($this->once()) - ->method('authFails'); + ->method('showFailure'); $this->assertFalse( - $this->object->authCheck() + $this->object->readCredentials() ); $this->assertTrue( @@ -654,7 +654,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authSetUser + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::storeCredentials * * @return void */ @@ -679,7 +679,7 @@ class AuthenticationCookieTest extends PmaTestCase $GLOBALS['cfg']['LoginCookieStore'] = true; $GLOBALS['from_cookie'] = true; - $this->object->authSetUser(); + $this->object->storeCredentials(); $this->assertFalse( isset($GLOBALS['PHP_AUTH_PW']) @@ -689,7 +689,7 @@ class AuthenticationCookieTest extends PmaTestCase isset($_SERVER['PHP_AUTH_PW']) ); - $this->object->storeUserCredentials(); + $this->object->rememberCredentials(); $this->assertTrue( isset($_COOKIE['pmaUser-2']) @@ -710,7 +710,7 @@ class AuthenticationCookieTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authSetUser (check for headers redirect) + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::storeCredentials (check for headers redirect) * * @return void */ @@ -741,12 +741,12 @@ class AuthenticationCookieTest extends PmaTestCase $this->stringContains('&server=2&lang=en&collation_connection=utf-8') ); - $this->object->authSetUser(); - $this->object->storeUserCredentials(); + $this->object->storeCredentials(); + $this->object->rememberCredentials(); } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::authFails + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::showFailure * * @return void */ @@ -754,7 +754,7 @@ class AuthenticationCookieTest extends PmaTestCase { $this->object = $this->getMockBuilder('PhpMyAdmin\Plugins\Auth\AuthenticationCookie') ->disableOriginalConstructor() - ->setMethods(array('auth')) + ->setMethods(array('showLoginForm')) ->getMock(); $GLOBALS['server'] = 2; @@ -766,7 +766,7 @@ class AuthenticationCookieTest extends PmaTestCase array('Cache-Control: no-store, no-cache, must-revalidate'), array('Pragma: no-cache') ); - $this->object->authFails(); + $this->object->showFailure(); $this->assertEquals( $GLOBALS['conn_error'], @@ -780,7 +780,7 @@ class AuthenticationCookieTest extends PmaTestCase { $this->object = $this->getMockBuilder('PhpMyAdmin\Plugins\Auth\AuthenticationCookie') ->disableOriginalConstructor() - ->setMethods(array('auth')) + ->setMethods(array('showLoginForm')) ->getMock(); $GLOBALS['server'] = 2; @@ -793,7 +793,7 @@ class AuthenticationCookieTest extends PmaTestCase array('Cache-Control: no-store, no-cache, must-revalidate'), array('Pragma: no-cache') ); - $this->object->authFails(); + $this->object->showFailure(); $this->assertEquals( $GLOBALS['conn_error'], @@ -805,7 +805,7 @@ class AuthenticationCookieTest extends PmaTestCase { $this->object = $this->getMockBuilder('PhpMyAdmin\Plugins\Auth\AuthenticationCookie') ->disableOriginalConstructor() - ->setMethods(array('auth')) + ->setMethods(array('showLoginForm')) ->getMock(); $GLOBALS['server'] = 2; @@ -819,7 +819,7 @@ class AuthenticationCookieTest extends PmaTestCase array('Cache-Control: no-store, no-cache, must-revalidate'), array('Pragma: no-cache') ); - $this->object->authFails(); + $this->object->showFailure(); $this->assertEquals( $GLOBALS['conn_error'], @@ -831,7 +831,7 @@ class AuthenticationCookieTest extends PmaTestCase { $this->object = $this->getMockBuilder('PhpMyAdmin\Plugins\Auth\AuthenticationCookie') ->disableOriginalConstructor() - ->setMethods(array('auth')) + ->setMethods(array('showLoginForm')) ->getMock(); $GLOBALS['server'] = 2; @@ -853,7 +853,7 @@ class AuthenticationCookieTest extends PmaTestCase array('Cache-Control: no-store, no-cache, must-revalidate'), array('Pragma: no-cache') ); - $this->object->authFails(); + $this->object->showFailure(); $this->assertEquals( $GLOBALS['conn_error'], @@ -865,7 +865,7 @@ class AuthenticationCookieTest extends PmaTestCase { $this->object = $this->getMockBuilder('PhpMyAdmin\Plugins\Auth\AuthenticationCookie') ->disableOriginalConstructor() - ->setMethods(array('auth')) + ->setMethods(array('showLoginForm')) ->getMock(); $dbi = $this->getMockBuilder('PhpMyAdmin\DatabaseInterface') @@ -886,7 +886,7 @@ class AuthenticationCookieTest extends PmaTestCase array('Cache-Control: no-store, no-cache, must-revalidate'), array('Pragma: no-cache') ); - $this->object->authFails(); + $this->object->showFailure(); $this->assertEquals( $GLOBALS['conn_error'], diff --git a/test/classes/Plugins/Auth/AuthenticationHttpTest.php b/test/classes/Plugins/Auth/AuthenticationHttpTest.php index 9799f64ea0..8b5c4d1ec3 100644 --- a/test/classes/Plugins/Auth/AuthenticationHttpTest.php +++ b/test/classes/Plugins/Auth/AuthenticationHttpTest.php @@ -107,13 +107,13 @@ class AuthenticationHttpTest extends PmaTestCase $this->object->logOut(); } else { $this->assertFalse( - $this->object->auth() + $this->object->showLoginForm() ); } } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationHttp::auth + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationHttp::showLoginForm * * @return void */ @@ -169,7 +169,7 @@ class AuthenticationHttpTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationHttp::authCheck + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationHttp::readCredentials * * @param string $user test username * @param string $pass test password @@ -181,7 +181,7 @@ class AuthenticationHttpTest extends PmaTestCase * @param string $old_usr value for $_REQUEST['old_usr'] * * @return void - * @dataProvider authCheckProvider + * @dataProvider readCredentialsProvider */ public function testAuthCheck($user, $pass, $userIndex, $passIndex, $expectedReturn, $expectedUser, $expectedPass, $old_usr = '' @@ -196,7 +196,7 @@ class AuthenticationHttpTest extends PmaTestCase $this->assertEquals( $expectedReturn, - $this->object->authCheck() + $this->object->readCredentials() ); $this->assertEquals( @@ -218,7 +218,7 @@ class AuthenticationHttpTest extends PmaTestCase * * @return array Test data */ - public function authCheckProvider() + public function readCredentialsProvider() { return array( array( @@ -271,7 +271,7 @@ class AuthenticationHttpTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationHttp::authSetUser + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationHttp::storeCredentials * * @return void */ @@ -285,7 +285,7 @@ class AuthenticationHttpTest extends PmaTestCase $GLOBALS['cfg']['Server']['user'] = 'testUser'; $this->assertTrue( - $this->object->authSetUser() + $this->object->storeCredentials() ); $this->assertEquals( @@ -326,7 +326,7 @@ class AuthenticationHttpTest extends PmaTestCase ); $this->assertTrue( - $this->object->authSetUser() + $this->object->storeCredentials() ); $this->assertEquals( @@ -359,7 +359,7 @@ class AuthenticationHttpTest extends PmaTestCase ); $this->assertTrue( - $this->object->authSetUser() + $this->object->storeCredentials() ); $this->assertEquals( @@ -407,7 +407,7 @@ class AuthenticationHttpTest extends PmaTestCase $GLOBALS['errno'] = 31; ob_start(); - $this->object->authFails(); + $this->object->showFailure(); $result = ob_get_clean(); $this->assertContains( @@ -426,10 +426,10 @@ class AuthenticationHttpTest extends PmaTestCase $GLOBALS['cfg']['Server']['host'] = 'host'; $GLOBALS['errno'] = 1045; - $this->object->authFails(); + $this->object->showFailure(); // case 3 $GLOBALS['errno'] = 1043; - $this->object->authFails(); + $this->object->showFailure(); } } diff --git a/test/classes/Plugins/Auth/AuthenticationSignonTest.php b/test/classes/Plugins/Auth/AuthenticationSignonTest.php index 529aa7cd26..b3afcf1793 100644 --- a/test/classes/Plugins/Auth/AuthenticationSignonTest.php +++ b/test/classes/Plugins/Auth/AuthenticationSignonTest.php @@ -45,7 +45,7 @@ class AuthenticationSignonTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::auth + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::showLoginForm * * @return void */ @@ -54,7 +54,7 @@ class AuthenticationSignonTest extends PmaTestCase $GLOBALS['cfg']['Server']['SignonURL'] = ''; ob_start(); - $this->object->auth(); + $this->object->showLoginForm(); $result = ob_get_clean(); $this->assertContains( @@ -64,7 +64,7 @@ class AuthenticationSignonTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::auth + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::showLoginForm * * @return void */ @@ -79,7 +79,7 @@ class AuthenticationSignonTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::auth + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::showLoginForm * * @return void */ @@ -95,7 +95,7 @@ class AuthenticationSignonTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::authCheck + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::readCredentials * * @return void */ @@ -105,12 +105,12 @@ class AuthenticationSignonTest extends PmaTestCase $_SESSION['LAST_SIGNON_URL'] = 'https://example.com/SignonDiffURL'; $this->assertFalse( - $this->object->authCheck() + $this->object->readCredentials() ); } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::authCheck + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::readCredentials * * @return void */ @@ -126,7 +126,7 @@ class AuthenticationSignonTest extends PmaTestCase $GLOBALS['cfg']['Server']['user'] = 'user'; $this->assertTrue( - $this->object->authCheck() + $this->object->readCredentials() ); $this->assertEquals( @@ -146,7 +146,7 @@ class AuthenticationSignonTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::authCheck + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::readCredentials * * @return void */ @@ -202,7 +202,7 @@ class AuthenticationSignonTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::authCheck + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::readCredentials * * @return void */ @@ -225,7 +225,7 @@ class AuthenticationSignonTest extends PmaTestCase $_SESSION['PMA_single_signon_token'] = 'pmaToken'; $this->assertTrue( - $this->object->authCheck() + $this->object->readCredentials() ); $this->assertEquals( @@ -240,7 +240,7 @@ class AuthenticationSignonTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::authSetUser + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::storeCredentials * * @return void */ @@ -250,7 +250,7 @@ class AuthenticationSignonTest extends PmaTestCase $GLOBALS['PHP_AUTH_PW'] = 'testPass123'; $this->assertTrue( - $this->object->authSetUser() + $this->object->storeCredentials() ); $this->assertEquals( @@ -265,7 +265,7 @@ class AuthenticationSignonTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::authFails + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::showFailure * * @return void */ @@ -276,15 +276,15 @@ class AuthenticationSignonTest extends PmaTestCase $this->object = $this->getMockBuilder('PhpMyAdmin\Plugins\Auth\AuthenticationSignon') ->disableOriginalConstructor() - ->setMethods(array('auth')) + ->setMethods(array('showLoginForm')) ->getMock(); $this->object->expects($this->exactly(1)) - ->method('auth'); + ->method('showLoginForm'); $GLOBALS['login_without_password_is_forbidden'] = true; - $this->object->authFails(); + $this->object->showFailure(); $this->assertEquals( 'Login without a password is forbidden by configuration ' @@ -294,7 +294,7 @@ class AuthenticationSignonTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::authFails + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::showFailure * * @return void */ @@ -305,16 +305,16 @@ class AuthenticationSignonTest extends PmaTestCase $this->object = $this->getMockBuilder('PhpMyAdmin\Plugins\Auth\AuthenticationSignon') ->disableOriginalConstructor() - ->setMethods(array('auth')) + ->setMethods(array('showLoginForm')) ->getMock(); $this->object->expects($this->exactly(1)) - ->method('auth'); + ->method('showLoginForm'); $GLOBALS['login_without_password_is_forbidden'] = null; $GLOBALS['allowDeny_forbidden'] = true; - $this->object->authFails(); + $this->object->showFailure(); $this->assertEquals( 'Access denied!', @@ -323,7 +323,7 @@ class AuthenticationSignonTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::authFails + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::showFailure * * @return void */ @@ -334,17 +334,17 @@ class AuthenticationSignonTest extends PmaTestCase $this->object = $this->getMockBuilder('PhpMyAdmin\Plugins\Auth\AuthenticationSignon') ->disableOriginalConstructor() - ->setMethods(array('auth')) + ->setMethods(array('showLoginForm')) ->getMock(); $this->object->expects($this->exactly(1)) - ->method('auth'); + ->method('showLoginForm'); $GLOBALS['allowDeny_forbidden'] = null; $GLOBALS['no_activity'] = true; $GLOBALS['cfg']['LoginCookieValidity'] = '1440'; - $this->object->authFails(); + $this->object->showFailure(); $this->assertEquals( 'No activity within 1440 seconds; please log in again.', @@ -353,7 +353,7 @@ class AuthenticationSignonTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::authFails + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::showFailure * * @return void */ @@ -364,11 +364,11 @@ class AuthenticationSignonTest extends PmaTestCase $this->object = $this->getMockBuilder('PhpMyAdmin\Plugins\Auth\AuthenticationSignon') ->disableOriginalConstructor() - ->setMethods(array('auth')) + ->setMethods(array('showLoginForm')) ->getMock(); $this->object->expects($this->exactly(1)) - ->method('auth'); + ->method('showLoginForm'); $dbi = $this->getMockBuilder('PhpMyAdmin\DatabaseInterface') ->disableOriginalConstructor() @@ -381,7 +381,7 @@ class AuthenticationSignonTest extends PmaTestCase $GLOBALS['dbi'] = $dbi; $GLOBALS['no_activity'] = null; - $this->object->authFails(); + $this->object->showFailure(); $this->assertEquals( 'error<123>', @@ -390,7 +390,7 @@ class AuthenticationSignonTest extends PmaTestCase } /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::authFails + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationSignon::showFailure * * @return void */ @@ -401,11 +401,11 @@ class AuthenticationSignonTest extends PmaTestCase $this->object = $this->getMockBuilder('PhpMyAdmin\Plugins\Auth\AuthenticationSignon') ->disableOriginalConstructor() - ->setMethods(array('auth')) + ->setMethods(array('showLoginForm')) ->getMock(); $this->object->expects($this->exactly(1)) - ->method('auth'); + ->method('showLoginForm'); $dbi = $this->getMockBuilder('PhpMyAdmin\DatabaseInterface') ->disableOriginalConstructor() @@ -417,7 +417,7 @@ class AuthenticationSignonTest extends PmaTestCase $GLOBALS['dbi'] = $dbi; - $this->object->authFails(); + $this->object->showFailure(); $this->assertEquals( 'Cannot log in to the MySQL server', From 9bc5bfd74cfe4ba386331e9c9d79aec637a00104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Fri, 20 Oct 2017 16:11:49 +0200 Subject: [PATCH 04/12] Move authentication logic to AuthenticationPlugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #11731 Signed-off-by: Michal Čihař --- .../classes/Plugins/AuthenticationPlugin.php | 21 ++++++++++++++++++- libraries/common.inc.php | 8 +------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/libraries/classes/Plugins/AuthenticationPlugin.php b/libraries/classes/Plugins/AuthenticationPlugin.php index 5dcd2eedb3..58b10c9a49 100644 --- a/libraries/classes/Plugins/AuthenticationPlugin.php +++ b/libraries/classes/Plugins/AuthenticationPlugin.php @@ -9,6 +9,7 @@ namespace PhpMyAdmin\Plugins; use PhpMyAdmin\Core; use PhpMyAdmin\Sanitize; +use PhpMyAdmin\Session; use PhpMyAdmin\Url; /** @@ -192,5 +193,23 @@ abstract class AuthenticationPlugin $time = time(); } $_SESSION['browser_access_time'][$guid] = $time; - } + } + + /** + * High level authentication interface + * + * Gets the credentials or shows login form if necessary + * + * @return void + */ + public function authenticate() + { + if (! $this->readCredentials()) { + /* Force generating of new session on login */ + Session::secure(); + $this->showLoginForm(); + } else { + $this->storeCredentials(); + } + } } diff --git a/libraries/common.inc.php b/libraries/common.inc.php index 212a4e9c11..5784508ff8 100644 --- a/libraries/common.inc.php +++ b/libraries/common.inc.php @@ -512,13 +512,7 @@ if (! defined('PMA_MINIMUM_COMMON')) { /** @var AuthenticationPlugin $auth_plugin */ $auth_plugin = new $auth_class($plugin_manager); - if (! $auth_plugin->readCredentials()) { - /* Force generating of new session on login */ - Session::secure(); - $auth_plugin->showLoginForm(); - } else { - $auth_plugin->storeCredentials(); - } + $auth_plugin->authenticate(); // Check IP-based Allow/Deny rules as soon as possible to reject the // user based on mod_access in Apache From 2327dbf3c3c109b907ea478be098c77e80e1bf0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Fri, 20 Oct 2017 16:16:26 +0200 Subject: [PATCH 05/12] Move allow/deny rules check to AuthenticationPlugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #11731 Signed-off-by: Michal Čihař --- .../classes/Plugins/AuthenticationPlugin.php | 66 +++++++++++++++++++ libraries/common.inc.php | 53 --------------- 2 files changed, 66 insertions(+), 53 deletions(-) diff --git a/libraries/classes/Plugins/AuthenticationPlugin.php b/libraries/classes/Plugins/AuthenticationPlugin.php index 58b10c9a49..dce10d3b3b 100644 --- a/libraries/classes/Plugins/AuthenticationPlugin.php +++ b/libraries/classes/Plugins/AuthenticationPlugin.php @@ -8,6 +8,8 @@ namespace PhpMyAdmin\Plugins; use PhpMyAdmin\Core; +use PhpMyAdmin\IpAllowDeny; +use PhpMyAdmin\Logging; use PhpMyAdmin\Sanitize; use PhpMyAdmin\Session; use PhpMyAdmin\Url; @@ -211,5 +213,69 @@ abstract class AuthenticationPlugin } else { $this->storeCredentials(); } + + $this->checkRules(); + } + + /** + * Check configuration defined restrictions for authentication + * + * @return void + */ + public function checkRules() + { + global $cfg; + + // Check IP-based Allow/Deny rules as soon as possible to reject the + // user based on mod_access in Apache + if (isset($cfg['Server']['AllowDeny']) + && isset($cfg['Server']['AllowDeny']['order']) + ) { + $allowDeny_forbidden = false; // default + if ($cfg['Server']['AllowDeny']['order'] == 'allow,deny') { + $allowDeny_forbidden = true; + if (IpAllowDeny::allowDeny('allow')) { + $allowDeny_forbidden = false; + } + if (IpAllowDeny::allowDeny('deny')) { + $allowDeny_forbidden = true; + } + } elseif ($cfg['Server']['AllowDeny']['order'] == 'deny,allow') { + if (IpAllowDeny::allowDeny('deny')) { + $allowDeny_forbidden = true; + } + if (IpAllowDeny::allowDeny('allow')) { + $allowDeny_forbidden = false; + } + } elseif ($cfg['Server']['AllowDeny']['order'] == 'explicit') { + if (IpAllowDeny::allowDeny('allow') && ! IpAllowDeny::allowDeny('deny')) { + $allowDeny_forbidden = false; + } else { + $allowDeny_forbidden = true; + } + } // end if ... elseif ... elseif + + // Ejects the user if banished + if ($allowDeny_forbidden) { + Logging::logUser($cfg['Server']['user'], 'allow-denied'); + $this->showFailure(); + } + } // end if + + // is root allowed? + if (! $cfg['Server']['AllowRoot'] && $cfg['Server']['user'] == 'root') { + $allowDeny_forbidden = true; + Logging::logUser($cfg['Server']['user'], 'root-denied'); + $this->showFailure(); + } + + // is a login without password allowed? + if (! $cfg['Server']['AllowNoPassword'] + && $cfg['Server']['password'] === '' + ) { + $login_without_password_is_forbidden = true; + Logging::logUser($cfg['Server']['user'], 'empty-denied'); + $this->showFailure(); + } } } diff --git a/libraries/common.inc.php b/libraries/common.inc.php index 5784508ff8..d41784f8e6 100644 --- a/libraries/common.inc.php +++ b/libraries/common.inc.php @@ -36,7 +36,6 @@ use PhpMyAdmin\Core; use PhpMyAdmin\DatabaseInterface; use PhpMyAdmin\Database\DatabaseList; use PhpMyAdmin\ErrorHandler; -use PhpMyAdmin\IpAllowDeny; use PhpMyAdmin\LanguageManager; use PhpMyAdmin\Logging; use PhpMyAdmin\Message; @@ -514,58 +513,6 @@ if (! defined('PMA_MINIMUM_COMMON')) { $auth_plugin->authenticate(); - // Check IP-based Allow/Deny rules as soon as possible to reject the - // user based on mod_access in Apache - if (isset($cfg['Server']['AllowDeny']) - && isset($cfg['Server']['AllowDeny']['order']) - ) { - $allowDeny_forbidden = false; // default - if ($cfg['Server']['AllowDeny']['order'] == 'allow,deny') { - $allowDeny_forbidden = true; - if (IpAllowDeny::allowDeny('allow')) { - $allowDeny_forbidden = false; - } - if (IpAllowDeny::allowDeny('deny')) { - $allowDeny_forbidden = true; - } - } elseif ($cfg['Server']['AllowDeny']['order'] == 'deny,allow') { - if (IpAllowDeny::allowDeny('deny')) { - $allowDeny_forbidden = true; - } - if (IpAllowDeny::allowDeny('allow')) { - $allowDeny_forbidden = false; - } - } elseif ($cfg['Server']['AllowDeny']['order'] == 'explicit') { - if (IpAllowDeny::allowDeny('allow') && ! IpAllowDeny::allowDeny('deny')) { - $allowDeny_forbidden = false; - } else { - $allowDeny_forbidden = true; - } - } // end if ... elseif ... elseif - - // Ejects the user if banished - if ($allowDeny_forbidden) { - Logging::logUser($cfg['Server']['user'], 'allow-denied'); - $auth_plugin->showFailure(); - } - } // end if - - // is root allowed? - if (! $cfg['Server']['AllowRoot'] && $cfg['Server']['user'] == 'root') { - $allowDeny_forbidden = true; - Logging::logUser($cfg['Server']['user'], 'root-denied'); - $auth_plugin->showFailure(); - } - - // is a login without password allowed? - if (! $cfg['Server']['AllowNoPassword'] - && $cfg['Server']['password'] === '' - ) { - $login_without_password_is_forbidden = true; - Logging::logUser($cfg['Server']['user'], 'empty-denied'); - $auth_plugin->showFailure(); - } - // Try to connect MySQL with the control user profile (will be used to // get the privileges list for the current user but the true user link // must be open after this one so it would be default one for all the From 62535ea5b982e3175a03cdd6a51d640f5b46abf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Fri, 20 Oct 2017 16:25:00 +0200 Subject: [PATCH 06/12] Pass failure reason to showFailure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This way we can avoid relying on global variables to check it. Signed-off-by: Michal Čihař --- .../Plugins/Auth/AuthenticationConfig.php | 5 +++- .../Plugins/Auth/AuthenticationCookie.php | 11 ++++--- .../Plugins/Auth/AuthenticationHttp.php | 5 +++- .../Plugins/Auth/AuthenticationSignon.php | 10 +++++-- .../classes/Plugins/AuthenticationPlugin.php | 29 ++++++++++--------- libraries/common.inc.php | 3 +- .../Plugins/Auth/AuthenticationConfigTest.php | 2 +- .../Plugins/Auth/AuthenticationCookieTest.php | 21 ++++---------- .../Plugins/Auth/AuthenticationHttpTest.php | 6 ++-- .../Plugins/Auth/AuthenticationSignonTest.php | 18 ++++-------- 10 files changed, 53 insertions(+), 57 deletions(-) diff --git a/libraries/classes/Plugins/Auth/AuthenticationConfig.php b/libraries/classes/Plugins/Auth/AuthenticationConfig.php index e9e7a6e144..8a2169df0d 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationConfig.php +++ b/libraries/classes/Plugins/Auth/AuthenticationConfig.php @@ -60,10 +60,13 @@ class AuthenticationConfig extends AuthenticationPlugin /** * User is not allowed to login to MySQL -> authentication failed * + * @param string $failure String describing why authentication has failed + * * @return void */ - public function showFailure() + public function showFailure($failure) { + parent::showFailure($failure); $conn_error = $GLOBALS['dbi']->getError(); if (!$conn_error) { $conn_error = __('Cannot connect: invalid settings.'); diff --git a/libraries/classes/Plugins/Auth/AuthenticationCookie.php b/libraries/classes/Plugins/Auth/AuthenticationCookie.php index 1c9eeb0864..44565c981c 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationCookie.php +++ b/libraries/classes/Plugins/Auth/AuthenticationCookie.php @@ -415,8 +415,7 @@ class AuthenticationCookie extends AuthenticationPlugin Util::cacheUnset('table_priv'); Util::cacheUnset('proc_priv'); - $GLOBALS['no_activity'] = true; - $this->showFailure(); + $this->showFailure('no-activity'); if (! defined('TESTSUITE')) { exit; } else { @@ -604,16 +603,20 @@ class AuthenticationCookie extends AuthenticationPlugin * this function MUST exit/quit the application, * currently done by call to showLoginForm() * + * @param string $failure String describing why authentication has failed + * * @return void */ - public function showFailure() + public function showFailure($failure) { global $conn_error; + parent::showFailure($failure); + // Deletes password cookie and displays the login form $GLOBALS['PMA_Config']->removeCookie('pmaAuth-' . $GLOBALS['server']); - $conn_error = $this->getErrorMessage(); + $conn_error = $this->getErrorMessage($failure); $response = Response::getInstance(); diff --git a/libraries/classes/Plugins/Auth/AuthenticationHttp.php b/libraries/classes/Plugins/Auth/AuthenticationHttp.php index a11fee5ac6..6fcc170ef5 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationHttp.php +++ b/libraries/classes/Plugins/Auth/AuthenticationHttp.php @@ -211,10 +211,13 @@ class AuthenticationHttp extends AuthenticationPlugin /** * User is not allowed to login to MySQL -> authentication failed * + * @param string $failure String describing why authentication has failed + * * @return void */ - public function showFailure() + public function showFailure($failure) { + parent::showFailure($failure); $error = $GLOBALS['dbi']->getError(); if ($error && $GLOBALS['errno'] != 1045) { Core::fatalError($error); diff --git a/libraries/classes/Plugins/Auth/AuthenticationSignon.php b/libraries/classes/Plugins/Auth/AuthenticationSignon.php index 2d1e6bf728..2f666b6c23 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationSignon.php +++ b/libraries/classes/Plugins/Auth/AuthenticationSignon.php @@ -227,10 +227,14 @@ class AuthenticationSignon extends AuthenticationPlugin /** * User is not allowed to login to MySQL -> authentication failed * - * @return boolean always true (no return indeed) + * @param string $failure String describing why authentication has failed + * + * @return void */ - public function showFailure() + public function showFailure($failure) { + parent::showFailure($failure); + /* Session name */ $session_name = $GLOBALS['cfg']['Server']['SignonSession']; @@ -247,7 +251,7 @@ class AuthenticationSignon extends AuthenticationPlugin } /* Set error message */ - $_SESSION['PMA_single_signon_error_message'] = $this->getErrorMessage(); + $_SESSION['PMA_single_signon_error_message'] = $this->getErrorMessage($failure); } $this->showLoginForm(); } diff --git a/libraries/classes/Plugins/AuthenticationPlugin.php b/libraries/classes/Plugins/AuthenticationPlugin.php index dce10d3b3b..2ec78aaf70 100644 --- a/libraries/classes/Plugins/AuthenticationPlugin.php +++ b/libraries/classes/Plugins/AuthenticationPlugin.php @@ -60,9 +60,15 @@ abstract class AuthenticationPlugin /** * User is not allowed to login to MySQL -> authentication failed * + * @param string $failure String describing why authentication has failed + * * @return void */ - abstract public function showFailure(); + public function showFailure($failure) + { + global $cfg; + Logging::logUser($cfg['Server']['user'], $failure); + } /** * Perform logout @@ -129,18 +135,20 @@ abstract class AuthenticationPlugin /** * Returns error message for failed authentication. * + * @param string $failure String describing why authentication has failed + * * @return string */ - public function getErrorMessage() + public function getErrorMessage($failure) { - if (!empty($GLOBALS['login_without_password_is_forbidden'])) { + if ($failure == 'empty-denied') { return __( 'Login without a password is forbidden by configuration' . ' (see AllowNoPassword)' ); - } elseif (!empty($GLOBALS['allowDeny_forbidden'])) { + } elseif ($failure == 'root-denied' || $failure == 'allow-denied') { return __('Access denied!'); - } elseif (!empty($GLOBALS['no_activity'])) { + } elseif ($failure == 'no-activity') { return sprintf( __('No activity within %s seconds; please log in again.'), intval($GLOBALS['cfg']['LoginCookieValidity']) @@ -257,25 +265,20 @@ abstract class AuthenticationPlugin // Ejects the user if banished if ($allowDeny_forbidden) { - Logging::logUser($cfg['Server']['user'], 'allow-denied'); - $this->showFailure(); + $this->showFailure('allow-denied'); } } // end if // is root allowed? if (! $cfg['Server']['AllowRoot'] && $cfg['Server']['user'] == 'root') { - $allowDeny_forbidden = true; - Logging::logUser($cfg['Server']['user'], 'root-denied'); - $this->showFailure(); + $this->showFailure('root-denied'); } // is a login without password allowed? if (! $cfg['Server']['AllowNoPassword'] && $cfg['Server']['password'] === '' ) { - $login_without_password_is_forbidden = true; - Logging::logUser($cfg['Server']['user'], 'empty-denied'); - $this->showFailure(); + $this->showFailure('empty-denied'); } } } diff --git a/libraries/common.inc.php b/libraries/common.inc.php index d41784f8e6..90333d07c7 100644 --- a/libraries/common.inc.php +++ b/libraries/common.inc.php @@ -529,8 +529,7 @@ if (! defined('PMA_MINIMUM_COMMON')) { $userlink = $GLOBALS['dbi']->connect(DatabaseInterface::CONNECT_USER); if ($userlink === false) { - Logging::logUser($cfg['Server']['user'], 'mysql-denied'); - $GLOBALS['auth_plugin']->showFailure(); + $GLOBALS['auth_plugin']->showFailure('mysql-denied'); } // Set timestamp for the session, if required. diff --git a/test/classes/Plugins/Auth/AuthenticationConfigTest.php b/test/classes/Plugins/Auth/AuthenticationConfigTest.php index 9fee31be22..f5feb95a60 100644 --- a/test/classes/Plugins/Auth/AuthenticationConfigTest.php +++ b/test/classes/Plugins/Auth/AuthenticationConfigTest.php @@ -101,7 +101,7 @@ class AuthenticationConfigTest extends PmaTestCase $GLOBALS['dbi'] = $dbi; ob_start(); - $this->object->showFailure(); + $this->object->showFailure(''); $html = ob_get_clean(); $this->assertContains( diff --git a/test/classes/Plugins/Auth/AuthenticationCookieTest.php b/test/classes/Plugins/Auth/AuthenticationCookieTest.php index 741630f5da..b294125c1c 100644 --- a/test/classes/Plugins/Auth/AuthenticationCookieTest.php +++ b/test/classes/Plugins/Auth/AuthenticationCookieTest.php @@ -647,10 +647,6 @@ class AuthenticationCookieTest extends PmaTestCase $this->assertFalse( $this->object->readCredentials() ); - - $this->assertTrue( - $GLOBALS['no_activity'] - ); } /** @@ -760,13 +756,11 @@ class AuthenticationCookieTest extends PmaTestCase $GLOBALS['server'] = 2; $_COOKIE['pmaAuth-2'] = 'pass'; - $GLOBALS['login_without_password_is_forbidden'] = '1'; - $this->mockResponse( array('Cache-Control: no-store, no-cache, must-revalidate'), array('Pragma: no-cache') ); - $this->object->showFailure(); + $this->object->showFailure('empty-denied'); $this->assertEquals( $GLOBALS['conn_error'], @@ -786,14 +780,11 @@ class AuthenticationCookieTest extends PmaTestCase $GLOBALS['server'] = 2; $_COOKIE['pmaAuth-2'] = 'pass'; - $GLOBALS['login_without_password_is_forbidden'] = ''; - $GLOBALS['allowDeny_forbidden'] = '1'; - $this->mockResponse( array('Cache-Control: no-store, no-cache, must-revalidate'), array('Pragma: no-cache') ); - $this->object->showFailure(); + $this->object->showFailure('allow-denied'); $this->assertEquals( $GLOBALS['conn_error'], @@ -812,14 +803,13 @@ class AuthenticationCookieTest extends PmaTestCase $_COOKIE['pmaAuth-2'] = 'pass'; $GLOBALS['allowDeny_forbidden'] = ''; - $GLOBALS['no_activity'] = '1'; $GLOBALS['cfg']['LoginCookieValidity'] = 10; $this->mockResponse( array('Cache-Control: no-store, no-cache, must-revalidate'), array('Pragma: no-cache') ); - $this->object->showFailure(); + $this->object->showFailure('no-activity'); $this->assertEquals( $GLOBALS['conn_error'], @@ -846,14 +836,13 @@ class AuthenticationCookieTest extends PmaTestCase ->will($this->returnValue(false)); $GLOBALS['dbi'] = $dbi; - $GLOBALS['no_activity'] = ''; $GLOBALS['errno'] = 42; $this->mockResponse( array('Cache-Control: no-store, no-cache, must-revalidate'), array('Pragma: no-cache') ); - $this->object->showFailure(); + $this->object->showFailure(''); $this->assertEquals( $GLOBALS['conn_error'], @@ -886,7 +875,7 @@ class AuthenticationCookieTest extends PmaTestCase array('Cache-Control: no-store, no-cache, must-revalidate'), array('Pragma: no-cache') ); - $this->object->showFailure(); + $this->object->showFailure(''); $this->assertEquals( $GLOBALS['conn_error'], diff --git a/test/classes/Plugins/Auth/AuthenticationHttpTest.php b/test/classes/Plugins/Auth/AuthenticationHttpTest.php index 8b5c4d1ec3..9f1fc54ecc 100644 --- a/test/classes/Plugins/Auth/AuthenticationHttpTest.php +++ b/test/classes/Plugins/Auth/AuthenticationHttpTest.php @@ -407,7 +407,7 @@ class AuthenticationHttpTest extends PmaTestCase $GLOBALS['errno'] = 31; ob_start(); - $this->object->showFailure(); + $this->object->showFailure(''); $result = ob_get_clean(); $this->assertContains( @@ -426,10 +426,10 @@ class AuthenticationHttpTest extends PmaTestCase $GLOBALS['cfg']['Server']['host'] = 'host'; $GLOBALS['errno'] = 1045; - $this->object->showFailure(); + $this->object->showFailure(''); // case 3 $GLOBALS['errno'] = 1043; - $this->object->showFailure(); + $this->object->showFailure(''); } } diff --git a/test/classes/Plugins/Auth/AuthenticationSignonTest.php b/test/classes/Plugins/Auth/AuthenticationSignonTest.php index b3afcf1793..77042c18a2 100644 --- a/test/classes/Plugins/Auth/AuthenticationSignonTest.php +++ b/test/classes/Plugins/Auth/AuthenticationSignonTest.php @@ -282,9 +282,7 @@ class AuthenticationSignonTest extends PmaTestCase $this->object->expects($this->exactly(1)) ->method('showLoginForm'); - $GLOBALS['login_without_password_is_forbidden'] = true; - - $this->object->showFailure(); + $this->object->showFailure('empty-denied'); $this->assertEquals( 'Login without a password is forbidden by configuration ' @@ -311,10 +309,7 @@ class AuthenticationSignonTest extends PmaTestCase $this->object->expects($this->exactly(1)) ->method('showLoginForm'); - $GLOBALS['login_without_password_is_forbidden'] = null; - $GLOBALS['allowDeny_forbidden'] = true; - - $this->object->showFailure(); + $this->object->showFailure('allow-denied'); $this->assertEquals( 'Access denied!', @@ -340,11 +335,9 @@ class AuthenticationSignonTest extends PmaTestCase $this->object->expects($this->exactly(1)) ->method('showLoginForm'); - $GLOBALS['allowDeny_forbidden'] = null; - $GLOBALS['no_activity'] = true; $GLOBALS['cfg']['LoginCookieValidity'] = '1440'; - $this->object->showFailure(); + $this->object->showFailure('no-activity'); $this->assertEquals( 'No activity within 1440 seconds; please log in again.', @@ -379,9 +372,8 @@ class AuthenticationSignonTest extends PmaTestCase ->will($this->returnValue('error<123>')); $GLOBALS['dbi'] = $dbi; - $GLOBALS['no_activity'] = null; - $this->object->showFailure(); + $this->object->showFailure(''); $this->assertEquals( 'error<123>', @@ -417,7 +409,7 @@ class AuthenticationSignonTest extends PmaTestCase $GLOBALS['dbi'] = $dbi; - $this->object->showFailure(); + $this->object->showFailure(''); $this->assertEquals( 'Cannot log in to the MySQL server', From 8e8f4bd3bb25279f788d32413212bb3892b2a919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Fri, 20 Oct 2017 17:04:07 +0200 Subject: [PATCH 07/12] Avoid using PHP_AUTH_USER and PHP_AUTH_PW globals for auth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use object attributes to store the actual credentials and avoid messing up with global variables. Signed-off-by: Michal Čihař --- .../Plugins/Auth/AuthenticationConfig.php | 3 + .../Plugins/Auth/AuthenticationCookie.php | 22 ++--- .../Plugins/Auth/AuthenticationHttp.php | 84 +++++++------------ .../Plugins/Auth/AuthenticationSignon.php | 33 +------- .../classes/Plugins/AuthenticationPlugin.php | 28 +++++-- .../Plugins/Auth/AuthenticationConfigTest.php | 4 + .../Plugins/Auth/AuthenticationCookieTest.php | 26 ++---- .../Plugins/Auth/AuthenticationHttpTest.php | 23 ++--- .../Plugins/Auth/AuthenticationSignonTest.php | 12 +-- 9 files changed, 96 insertions(+), 139 deletions(-) diff --git a/libraries/classes/Plugins/Auth/AuthenticationConfig.php b/libraries/classes/Plugins/Auth/AuthenticationConfig.php index 8a2169df0d..a6727339dd 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationConfig.php +++ b/libraries/classes/Plugins/Auth/AuthenticationConfig.php @@ -54,6 +54,9 @@ class AuthenticationConfig extends AuthenticationPlugin return false; } + $this->user = $GLOBALS['cfg']['Server']['user']; + $this->password = $GLOBALS['cfg']['Server']['password']; + return true; } diff --git a/libraries/classes/Plugins/Auth/AuthenticationCookie.php b/libraries/classes/Plugins/Auth/AuthenticationCookie.php index 44565c981c..e387a7cfd8 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationCookie.php +++ b/libraries/classes/Plugins/Auth/AuthenticationCookie.php @@ -100,7 +100,7 @@ class AuthenticationCookie extends AuthenticationPlugin if ($GLOBALS['cfg']['LoginCookieRecall'] && ! empty($GLOBALS['cfg']['blowfish_secret']) ) { - $default_user = $GLOBALS['PHP_AUTH_USER']; + $default_user = $this->user; $default_server = $GLOBALS['pma_auth_server']; $autocomplete = ''; } else { @@ -298,7 +298,7 @@ class AuthenticationCookie extends AuthenticationPlugin */ $GLOBALS['pma_auth_server'] = ''; - $GLOBALS['PHP_AUTH_USER'] = $GLOBALS['PHP_AUTH_PW'] = ''; + $this->user = $this->password = ''; $GLOBALS['from_cookie'] = false; if (isset($_REQUEST['pma_username']) && strlen($_REQUEST['pma_username']) > 0) { @@ -349,8 +349,8 @@ class AuthenticationCookie extends AuthenticationPlugin } // The user just logged in - $GLOBALS['PHP_AUTH_USER'] = Core::sanitizeMySQLUser($_REQUEST['pma_username']); - $GLOBALS['PHP_AUTH_PW'] = isset($_REQUEST['pma_password']) ? $_REQUEST['pma_password'] : ''; + $this->user = Core::sanitizeMySQLUser($_REQUEST['pma_username']); + $this->password = isset($_REQUEST['pma_password']) ? $_REQUEST['pma_password'] : ''; if ($GLOBALS['cfg']['AllowArbitraryServer'] && isset($_REQUEST['pma_servername']) ) { @@ -378,15 +378,15 @@ class AuthenticationCookie extends AuthenticationPlugin return true; } - // At the end, try to set the $GLOBALS['PHP_AUTH_USER'] - // and $GLOBALS['PHP_AUTH_PW'] variables from cookies + // At the end, try to set the $this->user + // and $this->password variables from cookies // check cookies if (empty($_COOKIE['pmaUser-' . $GLOBALS['server']])) { return false; } - $GLOBALS['PHP_AUTH_USER'] = $this->cookieDecrypt( + $this->user = $this->cookieDecrypt( $_COOKIE['pmaUser-' . $GLOBALS['server']], $this->_getEncryptionSecret() ); @@ -439,7 +439,7 @@ class AuthenticationCookie extends AuthenticationPlugin if (! is_array($auth_data) || ! isset($auth_data['password'])) { return false; } - $GLOBALS['PHP_AUTH_PW'] = $auth_data['password']; + $this->password = $auth_data['password']; if ($GLOBALS['cfg']['AllowArbitraryServer'] && ! empty($auth_data['server'])) { $GLOBALS['pma_auth_server'] = $auth_data['server']; } @@ -478,12 +478,6 @@ class AuthenticationCookie extends AuthenticationPlugin } unset($tmp_host, $tmp_port, $parts); } - $cfg['Server']['user'] = $GLOBALS['PHP_AUTH_USER']; - $cfg['Server']['password'] = $GLOBALS['PHP_AUTH_PW']; - - // Avoid showing the password in phpinfo()'s output - unset($GLOBALS['PHP_AUTH_PW']); - unset($_SERVER['PHP_AUTH_PW']); return parent::storeCredentials(); } diff --git a/libraries/classes/Plugins/Auth/AuthenticationHttp.php b/libraries/classes/Plugins/Auth/AuthenticationHttp.php index 6fcc170ef5..59dcc98e72 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationHttp.php +++ b/libraries/classes/Plugins/Auth/AuthenticationHttp.php @@ -102,62 +102,67 @@ class AuthenticationHttp extends AuthenticationPlugin /** * Gets authentication credentials * - * @global string $PHP_AUTH_USER the username - * @global string $PHP_AUTH_PW the password - * * @return boolean whether we get authentication settings or not */ public function readCredentials() { - global $PHP_AUTH_USER, $PHP_AUTH_PW; - // Grabs the $PHP_AUTH_USER variable - if (empty($PHP_AUTH_USER)) { + if (isset($GLOBALS['PHP_AUTH_USER'])) { + $this->user = $GLOBALS['PHP_AUTH_USER']; + } + if (empty($this->user)) { if (Core::getenv('PHP_AUTH_USER')) { - $PHP_AUTH_USER = Core::getenv('PHP_AUTH_USER'); + $this->user = Core::getenv('PHP_AUTH_USER'); } elseif (Core::getenv('REMOTE_USER')) { // CGI, might be encoded, see below - $PHP_AUTH_USER = Core::getenv('REMOTE_USER'); + $this->user = Core::getenv('REMOTE_USER'); } elseif (Core::getenv('REDIRECT_REMOTE_USER')) { // CGI, might be encoded, see below - $PHP_AUTH_USER = Core::getenv('REDIRECT_REMOTE_USER'); + $this->user = Core::getenv('REDIRECT_REMOTE_USER'); } elseif (Core::getenv('AUTH_USER')) { // WebSite Professional - $PHP_AUTH_USER = Core::getenv('AUTH_USER'); + $this->user = Core::getenv('AUTH_USER'); } elseif (Core::getenv('HTTP_AUTHORIZATION')) { // IIS, might be encoded, see below - $PHP_AUTH_USER = Core::getenv('HTTP_AUTHORIZATION'); + $this->user = Core::getenv('HTTP_AUTHORIZATION'); } elseif (Core::getenv('Authorization')) { // FastCGI, might be encoded, see below - $PHP_AUTH_USER = Core::getenv('Authorization'); + $this->user = Core::getenv('Authorization'); } } // Grabs the $PHP_AUTH_PW variable - if (empty($PHP_AUTH_PW)) { + if (isset($GLOBALS['PHP_AUTH_PW'])) { + $this->password = $GLOBALS['PHP_AUTH_PW']; + } + if (empty($this->password)) { if (Core::getenv('PHP_AUTH_PW')) { - $PHP_AUTH_PW = Core::getenv('PHP_AUTH_PW'); + $this->password = Core::getenv('PHP_AUTH_PW'); } elseif (Core::getenv('REMOTE_PASSWORD')) { // Apache/CGI - $PHP_AUTH_PW = Core::getenv('REMOTE_PASSWORD'); + $this->password = Core::getenv('REMOTE_PASSWORD'); } elseif (Core::getenv('AUTH_PASSWORD')) { // WebSite Professional - $PHP_AUTH_PW = Core::getenv('AUTH_PASSWORD'); + $this->password = Core::getenv('AUTH_PASSWORD'); } } // Sanitize empty password login - if (is_null($PHP_AUTH_PW)) { - $PHP_AUTH_PW = ''; + if (is_null($this->password)) { + $this->password = ''; } + // Avoid showing the password in phpinfo()'s output + unset($GLOBALS['PHP_AUTH_PW']); + unset($_SERVER['PHP_AUTH_PW']); + // Decode possibly encoded information (used by IIS/CGI/FastCGI) // (do not use explode() because a user might have a colon in his password - if (strcmp(substr($PHP_AUTH_USER, 0, 6), 'Basic ') == 0) { - $usr_pass = base64_decode(substr($PHP_AUTH_USER, 6)); + if (strcmp(substr($this->user, 0, 6), 'Basic ') == 0) { + $usr_pass = base64_decode(substr($this->user, 6)); if (!empty($usr_pass)) { $colon = strpos($usr_pass, ':'); if ($colon) { - $PHP_AUTH_USER = substr($usr_pass, 0, $colon); - $PHP_AUTH_PW = substr($usr_pass, $colon + 1); + $this->user = substr($usr_pass, 0, $colon); + $this->password = substr($usr_pass, $colon + 1); } unset($colon); } @@ -165,49 +170,24 @@ class AuthenticationHttp extends AuthenticationPlugin } // sanitize username - $PHP_AUTH_USER = Core::sanitizeMySQLUser($PHP_AUTH_USER); + $this->user = Core::sanitizeMySQLUser($this->user); // User logged out -> ensure the new username is not the same $old_usr = isset($_REQUEST['old_usr']) ? $_REQUEST['old_usr'] : ''; if (! empty($old_usr) - && (isset($PHP_AUTH_USER) && hash_equals($old_usr, $PHP_AUTH_USER)) + && (isset($this->user) && hash_equals($old_usr, $this->user)) ) { - $PHP_AUTH_USER = ''; + $this->user = ''; } // Returns whether we get authentication settings or not - if (empty($PHP_AUTH_USER)) { + if (empty($this->user)) { return false; } else { return true; } } - /** - * Set the user and password after last checkings if required - * - * @global array $cfg the valid servers settings - * @global integer $server the id of the current server - * @global string $PHP_AUTH_USER the current username - * @global string $PHP_AUTH_PW the current password - * - * @return boolean always true - */ - public function storeCredentials() - { - global $cfg, $server; - global $PHP_AUTH_USER, $PHP_AUTH_PW; - - $cfg['Server']['user'] = $PHP_AUTH_USER; - $cfg['Server']['password'] = $PHP_AUTH_PW; - - // Avoid showing the password in phpinfo()'s output - unset($GLOBALS['PHP_AUTH_PW']); - unset($_SERVER['PHP_AUTH_PW']); - - return parent::storeCredentials(); - } - /** * User is not allowed to login to MySQL -> authentication failed * @@ -233,6 +213,6 @@ class AuthenticationHttp extends AuthenticationPlugin */ public function getLoginFormURL() { - return './index.php?old_usr=' . $GLOBALS['PHP_AUTH_USER']; + return './index.php?old_usr=' . $this->user; } } diff --git a/libraries/classes/Plugins/Auth/AuthenticationSignon.php b/libraries/classes/Plugins/Auth/AuthenticationSignon.php index 2f666b6c23..9391f0b92b 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationSignon.php +++ b/libraries/classes/Plugins/Auth/AuthenticationSignon.php @@ -43,15 +43,10 @@ class AuthenticationSignon extends AuthenticationPlugin /** * Gets authentication credentials * - * @global string $PHP_AUTH_USER the username - * @global string $PHP_AUTH_PW the password - * * @return boolean whether we get authentication settings or not */ public function readCredentials() { - global $PHP_AUTH_USER, $PHP_AUTH_PW; - /* Check if we're using same signon server */ $signon_url = $GLOBALS['cfg']['Server']['SignonURL']; if (isset($_SESSION['LAST_SIGNON_URL']) @@ -91,7 +86,7 @@ class AuthenticationSignon extends AuthenticationPlugin } include $script_name; - list ($PHP_AUTH_USER, $PHP_AUTH_PW) + list ($this->user, $this->password) = get_login_credentials($GLOBALS['cfg']['Server']['user']); } elseif (isset($_COOKIE[$session_name])) { /* Does session exist? */ /* End current session */ @@ -131,10 +126,10 @@ class AuthenticationSignon extends AuthenticationPlugin /* Grab credentials if they exist */ if (isset($_SESSION['PMA_single_signon_user'])) { - $PHP_AUTH_USER = $_SESSION['PMA_single_signon_user']; + $this->user = $_SESSION['PMA_single_signon_user']; } if (isset($_SESSION['PMA_single_signon_password'])) { - $PHP_AUTH_PW = $_SESSION['PMA_single_signon_password']; + $this->password = $_SESSION['PMA_single_signon_password']; } if (isset($_SESSION['PMA_single_signon_host'])) { $single_signon_host = $_SESSION['PMA_single_signon_host']; @@ -193,7 +188,7 @@ class AuthenticationSignon extends AuthenticationPlugin } // Returns whether we get authentication settings or not - if (empty($PHP_AUTH_USER)) { + if (empty($this->user)) { unset($_SESSION['LAST_SIGNON_URL']); return false; @@ -204,26 +199,6 @@ class AuthenticationSignon extends AuthenticationPlugin } } - /** - * Set the user and password after last checkings if required - * - * @global array $cfg the valid servers settings - * @global string $PHP_AUTH_USER the current username - * @global string $PHP_AUTH_PW the current password - * - * @return boolean always true - */ - public function storeCredentials() - { - global $cfg; - global $PHP_AUTH_USER, $PHP_AUTH_PW; - - $cfg['Server']['user'] = $PHP_AUTH_USER; - $cfg['Server']['password'] = $PHP_AUTH_PW; - - return parent::storeCredentials(); - } - /** * User is not allowed to login to MySQL -> authentication failed * diff --git a/libraries/classes/Plugins/AuthenticationPlugin.php b/libraries/classes/Plugins/AuthenticationPlugin.php index 2ec78aaf70..a153f8f190 100644 --- a/libraries/classes/Plugins/AuthenticationPlugin.php +++ b/libraries/classes/Plugins/AuthenticationPlugin.php @@ -22,6 +22,20 @@ use PhpMyAdmin\Url; */ abstract class AuthenticationPlugin { + /** + * Username + * + * @var string + */ + public $user = ''; + + /** + * Password + * + * @var string + */ + public $password = ''; + /** * Displays authentication form * @@ -43,8 +57,13 @@ abstract class AuthenticationPlugin */ public function storeCredentials() { + global $cfg; + $this->setSessionAccessTime(); + $cfg['Server']['user'] = $this->user; + $cfg['Server']['password'] = $this->password; + return true; } @@ -66,8 +85,7 @@ abstract class AuthenticationPlugin */ public function showFailure($failure) { - global $cfg; - Logging::logUser($cfg['Server']['user'], $failure); + Logging::logUser($this->user, $failure); } /** @@ -77,8 +95,6 @@ abstract class AuthenticationPlugin */ public function logOut() { - global $PHP_AUTH_USER, $PHP_AUTH_PW; - /* Obtain redirect URL (before doing logout) */ if (! empty($GLOBALS['cfg']['Server']['LogoutURL'])) { $redirect_url = $GLOBALS['cfg']['Server']['LogoutURL']; @@ -87,8 +103,8 @@ abstract class AuthenticationPlugin } /* Clear credentials */ - $PHP_AUTH_USER = ''; - $PHP_AUTH_PW = ''; + $this->user = ''; + $this->password = ''; /* * Get a logged-in server count in case of LoginCookieDeleteAll is disabled. diff --git a/test/classes/Plugins/Auth/AuthenticationConfigTest.php b/test/classes/Plugins/Auth/AuthenticationConfigTest.php index f5feb95a60..3fe0f16e7d 100644 --- a/test/classes/Plugins/Auth/AuthenticationConfigTest.php +++ b/test/classes/Plugins/Auth/AuthenticationConfigTest.php @@ -65,6 +65,10 @@ class AuthenticationConfigTest extends PmaTestCase */ public function testAuthCheck() { + $GLOBALS['cfg']['Server'] = array( + 'user' => 'username', + 'password' => 'password', + ); $this->assertTrue( $this->object->readCredentials() ); diff --git a/test/classes/Plugins/Auth/AuthenticationCookieTest.php b/test/classes/Plugins/Auth/AuthenticationCookieTest.php index b294125c1c..13dae42877 100644 --- a/test/classes/Plugins/Auth/AuthenticationCookieTest.php +++ b/test/classes/Plugins/Auth/AuthenticationCookieTest.php @@ -109,7 +109,7 @@ class AuthenticationCookieTest extends PmaTestCase $_REQUEST['old_usr'] = ''; $GLOBALS['cfg']['LoginCookieRecall'] = true; $GLOBALS['cfg']['blowfish_secret'] = 'secret'; - $GLOBALS['PHP_AUTH_USER'] = 'pmauser'; + $this->object->user = 'pmauser'; $GLOBALS['pma_auth_server'] = 'localhost'; // mock footer @@ -474,12 +474,12 @@ class AuthenticationCookieTest extends PmaTestCase $this->assertEquals( 'testPMAUser', - $GLOBALS['PHP_AUTH_USER'] + $this->object->user ); $this->assertEquals( 'testPMAPSWD', - $GLOBALS['PHP_AUTH_PW'] + $this->object->password ); $this->assertEquals( @@ -567,7 +567,7 @@ class AuthenticationCookieTest extends PmaTestCase $this->assertEquals( 'testBF', - $GLOBALS['PHP_AUTH_USER'] + $this->object->user ); } @@ -611,7 +611,7 @@ class AuthenticationCookieTest extends PmaTestCase $this->assertEquals( '', - $GLOBALS['PHP_AUTH_PW'] + $this->object->password ); } @@ -656,7 +656,7 @@ class AuthenticationCookieTest extends PmaTestCase */ public function testAuthSetUser() { - $GLOBALS['PHP_AUTH_USER'] = 'pmaUser2'; + $this->object->user = 'pmaUser2'; $arr = array( 'host' => 'a', 'port' => 1, @@ -670,21 +670,13 @@ class AuthenticationCookieTest extends PmaTestCase $GLOBALS['cfg']['Servers'][1] = $arr; $GLOBALS['cfg']['AllowArbitraryServer'] = true; $GLOBALS['pma_auth_server'] = 'b 2'; - $GLOBALS['PHP_AUTH_PW'] = $_SERVER['PHP_AUTH_PW'] = 'testPW'; + $this->object->password = 'testPW'; $GLOBALS['server'] = 2; $GLOBALS['cfg']['LoginCookieStore'] = true; $GLOBALS['from_cookie'] = true; $this->object->storeCredentials(); - $this->assertFalse( - isset($GLOBALS['PHP_AUTH_PW']) - ); - - $this->assertFalse( - isset($_SERVER['PHP_AUTH_PW']) - ); - $this->object->rememberCredentials(); $this->assertTrue( @@ -712,7 +704,7 @@ class AuthenticationCookieTest extends PmaTestCase */ public function testAuthSetUserWithHeaders() { - $GLOBALS['PHP_AUTH_USER'] = 'pmaUser2'; + $this->object->user = 'pmaUser2'; $arr = array( 'host' => 'a', 'port' => 1, @@ -727,7 +719,7 @@ class AuthenticationCookieTest extends PmaTestCase $GLOBALS['cfg']['Servers'][1] = $arr; $GLOBALS['cfg']['AllowArbitraryServer'] = true; $GLOBALS['pma_auth_server'] = 'b 2'; - $GLOBALS['PHP_AUTH_PW'] = $_SERVER['PHP_AUTH_PW'] = 'testPW'; + $this->object->password = 'testPW'; $GLOBALS['server'] = 2; $GLOBALS['cfg']['LoginCookieStore'] = true; $GLOBALS['from_cookie'] = false; diff --git a/test/classes/Plugins/Auth/AuthenticationHttpTest.php b/test/classes/Plugins/Auth/AuthenticationHttpTest.php index 9f1fc54ecc..4eb47772ab 100644 --- a/test/classes/Plugins/Auth/AuthenticationHttpTest.php +++ b/test/classes/Plugins/Auth/AuthenticationHttpTest.php @@ -186,9 +186,6 @@ class AuthenticationHttpTest extends PmaTestCase public function testAuthCheck($user, $pass, $userIndex, $passIndex, $expectedReturn, $expectedUser, $expectedPass, $old_usr = '' ) { - $GLOBALS['PHP_AUTH_USER'] = ''; - $GLOBALS['PHP_AUTH_PW'] = ''; - $_SERVER[$userIndex] = $user; $_SERVER[$passIndex] = $pass; @@ -201,12 +198,12 @@ class AuthenticationHttpTest extends PmaTestCase $this->assertEquals( $expectedUser, - $GLOBALS['PHP_AUTH_USER'] + $this->object->user ); $this->assertEquals( $expectedPass, - $GLOBALS['PHP_AUTH_PW'] + $this->object->password ); $_SERVER[$userIndex] = null; @@ -279,8 +276,8 @@ class AuthenticationHttpTest extends PmaTestCase { // case 1 - $GLOBALS['PHP_AUTH_USER'] = 'testUser'; - $GLOBALS['PHP_AUTH_PW'] = 'testPass'; + $this->object->user = 'testUser'; + $this->object->password = 'testPass'; $GLOBALS['server'] = 2; $GLOBALS['cfg']['Server']['user'] = 'testUser'; @@ -298,10 +295,6 @@ class AuthenticationHttpTest extends PmaTestCase $GLOBALS['cfg']['Server']['password'] ); - $this->assertFalse( - isset($GLOBALS['PHP_AUTH_PW']) - ); - $this->assertFalse( isset($_SERVER['PHP_AUTH_PW']) ); @@ -312,8 +305,8 @@ class AuthenticationHttpTest extends PmaTestCase ); // case 2 - $GLOBALS['PHP_AUTH_USER'] = 'testUser'; - $GLOBALS['PHP_AUTH_PW'] = 'testPass'; + $this->object->user = 'testUser'; + $this->object->password = 'testPass'; $GLOBALS['cfg']['Servers'][1] = array( 'host' => 'a', 'user' => 'testUser', @@ -345,8 +338,8 @@ class AuthenticationHttpTest extends PmaTestCase // case 3 $GLOBALS['server'] = 3; - $GLOBALS['PHP_AUTH_USER'] = 'testUser'; - $GLOBALS['PHP_AUTH_PW'] = 'testPass'; + $this->object->user = 'testUser'; + $this->object->password = 'testPass'; $GLOBALS['cfg']['Servers'][1] = array( 'host' => 'a', 'user' => 'testUsers', diff --git a/test/classes/Plugins/Auth/AuthenticationSignonTest.php b/test/classes/Plugins/Auth/AuthenticationSignonTest.php index 77042c18a2..a421b11d72 100644 --- a/test/classes/Plugins/Auth/AuthenticationSignonTest.php +++ b/test/classes/Plugins/Auth/AuthenticationSignonTest.php @@ -131,12 +131,12 @@ class AuthenticationSignonTest extends PmaTestCase $this->assertEquals( 'user', - $GLOBALS['PHP_AUTH_USER'] + $this->object->user ); $this->assertEquals( 'password', - $GLOBALS['PHP_AUTH_PW'] + $this->object->password ); $this->assertEquals( @@ -230,12 +230,12 @@ class AuthenticationSignonTest extends PmaTestCase $this->assertEquals( 'user123', - $GLOBALS['PHP_AUTH_USER'] + $this->object->user ); $this->assertEquals( 'pass123', - $GLOBALS['PHP_AUTH_PW'] + $this->object->password ); } @@ -246,8 +246,8 @@ class AuthenticationSignonTest extends PmaTestCase */ public function testAuthSetUser() { - $GLOBALS['PHP_AUTH_USER'] = 'testUser123'; - $GLOBALS['PHP_AUTH_PW'] = 'testPass123'; + $this->object->user = 'testUser123'; + $this->object->password = 'testPass123'; $this->assertTrue( $this->object->storeCredentials() From c1375f4e84f01c9f087e029e4fd15d2b214c8a00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Fri, 20 Oct 2017 17:05:01 +0200 Subject: [PATCH 08/12] Remove not needed access through globals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michal Čihař --- libraries/common.inc.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/common.inc.php b/libraries/common.inc.php index 90333d07c7..8e9e8ee2de 100644 --- a/libraries/common.inc.php +++ b/libraries/common.inc.php @@ -529,7 +529,7 @@ if (! defined('PMA_MINIMUM_COMMON')) { $userlink = $GLOBALS['dbi']->connect(DatabaseInterface::CONNECT_USER); if ($userlink === false) { - $GLOBALS['auth_plugin']->showFailure('mysql-denied'); + $auth_plugin->showFailure('mysql-denied'); } // Set timestamp for the session, if required. From e020dbfe16bb272a9de312a1b45cdc23bbebeb83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Fri, 20 Oct 2017 17:14:28 +0200 Subject: [PATCH 09/12] Add test for AuthenticationPlugin::authenticate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michal Čihař --- .../Plugins/Auth/AuthenticationCookieTest.php | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/classes/Plugins/Auth/AuthenticationCookieTest.php b/test/classes/Plugins/Auth/AuthenticationCookieTest.php index 13dae42877..1c410cfb71 100644 --- a/test/classes/Plugins/Auth/AuthenticationCookieTest.php +++ b/test/classes/Plugins/Auth/AuthenticationCookieTest.php @@ -1125,4 +1125,35 @@ class AuthenticationCookieTest extends PmaTestCase ), ); } + + /** + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationCookie::authenticate + * + * @return void + */ + public function testAuthenticate() + { + $GLOBALS['cfg']['CaptchaLoginPrivateKey'] = ''; + $GLOBALS['cfg']['CaptchaLoginPublicKey'] = ''; + $GLOBALS['cfg']['Server']['AllowRoot'] = false; + $GLOBALS['cfg']['Server']['AllowNoPassword'] = false; + $_REQUEST['old_usr'] = ''; + $_REQUEST['pma_username'] = 'testUser'; + $_REQUEST['pma_password'] = 'testPassword'; + + ob_start(); + $this->object->authenticate(); + $result = ob_get_clean(); + + /* Nothing should be printed */ + $this->assertEquals('', $result); + + /* Verify readCredentials worked */ + $this->assertEquals('testUser', $this->object->user); + $this->assertEquals('testPassword', $this->object->password); + + /* Verify storeCredentials worked */ + $this->assertEquals('testUser', $GLOBALS['cfg']['Server']['user']); + $this->assertEquals('testPassword', $GLOBALS['cfg']['Server']['password']); + } } From 3f8e1f19e053401f90c0b474845024fa5f91c879 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Fri, 20 Oct 2017 17:41:39 +0200 Subject: [PATCH 10/12] Add tests for AuthenticationPlugin::checkRules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michal Čihař --- .../Plugins/Auth/AuthenticationCookieTest.php | 219 ++++++++++++++++-- 1 file changed, 196 insertions(+), 23 deletions(-) diff --git a/test/classes/Plugins/Auth/AuthenticationCookieTest.php b/test/classes/Plugins/Auth/AuthenticationCookieTest.php index 1c410cfb71..3dd3b2c57a 100644 --- a/test/classes/Plugins/Auth/AuthenticationCookieTest.php +++ b/test/classes/Plugins/Auth/AuthenticationCookieTest.php @@ -91,13 +91,7 @@ class AuthenticationCookieTest extends PmaTestCase ); } - /** - * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::showLoginForm - * - * @return void - * @group medium - */ - public function testAuthError() + private function getAuthErrorMockResponse() { $mockResponse = $this->mockResponse(); @@ -106,12 +100,6 @@ class AuthenticationCookieTest extends PmaTestCase ->with() ->will($this->returnValue(false)); - $_REQUEST['old_usr'] = ''; - $GLOBALS['cfg']['LoginCookieRecall'] = true; - $GLOBALS['cfg']['blowfish_secret'] = 'secret'; - $this->object->user = 'pmauser'; - $GLOBALS['pma_auth_server'] = 'localhost'; - // mock footer $mockFooter = $this->getMockBuilder('PhpMyAdmin\Footer') ->disableOriginalConstructor() @@ -165,17 +153,7 @@ class AuthenticationCookieTest extends PmaTestCase ->will($this->returnValue($mockHeader)); $GLOBALS['pmaThemeImage'] = 'test'; - $GLOBALS['conn_error'] = true; - $GLOBALS['cfg']['Lang'] = 'en'; - $GLOBALS['cfg']['AllowArbitraryServer'] = true; $GLOBALS['cfg']['Servers'] = array(1, 2); - $GLOBALS['cfg']['CaptchaLoginPrivateKey'] = ''; - $GLOBALS['cfg']['CaptchaLoginPublicKey'] = ''; - $GLOBALS['target'] = 'testTarget'; - $GLOBALS['db'] = 'testDb'; - $GLOBALS['table'] = 'testTable'; - - file_put_contents('testlogo_right.png', ''); // mock error handler @@ -194,6 +172,35 @@ class AuthenticationCookieTest extends PmaTestCase ->with(); $GLOBALS['error_handler'] = $mockErrorHandler; + } + + /** + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationConfig::showLoginForm + * + * @return void + * @group medium + */ + public function testAuthError() + { + $this->getAuthErrorMockResponse(); + + $_REQUEST['old_usr'] = ''; + $GLOBALS['cfg']['LoginCookieRecall'] = true; + $GLOBALS['cfg']['blowfish_secret'] = 'secret'; + $this->object->user = 'pmauser'; + $GLOBALS['pma_auth_server'] = 'localhost'; + + + $GLOBALS['conn_error'] = true; + $GLOBALS['cfg']['Lang'] = 'en'; + $GLOBALS['cfg']['AllowArbitraryServer'] = true; + $GLOBALS['cfg']['CaptchaLoginPrivateKey'] = ''; + $GLOBALS['cfg']['CaptchaLoginPublicKey'] = ''; + $GLOBALS['target'] = 'testTarget'; + $GLOBALS['db'] = 'testDb'; + $GLOBALS['table'] = 'testTable'; + + file_put_contents('testlogo_right.png', ''); ob_start(); $this->object->showLoginForm(); @@ -1156,4 +1163,170 @@ class AuthenticationCookieTest extends PmaTestCase $this->assertEquals('testUser', $GLOBALS['cfg']['Server']['user']); $this->assertEquals('testPassword', $GLOBALS['cfg']['Server']['password']); } + + /** + * Test for PhpMyAdmin\Plugins\Auth\AuthenticationCookie::checkRules + * + * @return void + * + * @dataProvider checkRulesProvider + */ + public function testCheckRules($user, $pass, $ip, $root, $nopass, $rules, $expected) + { + $this->object->user = $user; + $this->object->password = $pass; + $this->object->storeCredentials(); + + $_SERVER['REMOTE_ADDR'] = $ip; + + $GLOBALS['cfg']['Server']['AllowRoot'] = $root; + $GLOBALS['cfg']['Server']['AllowNoPassword'] = $nopass; + $GLOBALS['cfg']['Server']['AllowDeny'] = $rules; + + if (! empty($expected)) { + $this->getAuthErrorMockResponse(); + } + + ob_start(); + $this->object->checkRules(); + $result = ob_get_clean(); + + if (empty($expected)) { + $this->assertEquals($expected, $result); + } else { + $this->assertContains($expected, $result); + } + } + + public function checkRulesProvider() + { + return array( + 'nopass-ok' => array( + 'testUser', + '', + '1.2.3.4', + true, + true, + array(), + '', + ), + 'nopass' => array( + 'testUser', + '', + '1.2.3.4', + true, + false, + array(), + 'Login without a password is forbidden', + ), + 'root-ok' => array( + 'root', + 'root', + '1.2.3.4', + true, + true, + array(), + '', + ), + 'root' => array( + 'root', + 'root', + '1.2.3.4', + false, + true, + array(), + 'Access denied!', + ), + 'rules-deny-allow-ok' => array( + 'root', + 'root', + '1.2.3.4', + true, + true, + array( + 'order' => 'deny,allow', + 'rules' => array( + 'allow root 1.2.3.4', + 'deny % from all', + ), + ), + '', + ), + 'rules-deny-allow-reject' => array( + 'user', + 'root', + '1.2.3.4', + true, + true, + array( + 'order' => 'deny,allow', + 'rules' => array( + 'allow root 1.2.3.4', + 'deny % from all', + ), + ), + 'Access denied!', + ), + 'rules-allow-deny-ok' => array( + 'root', + 'root', + '1.2.3.4', + true, + true, + array( + 'order' => 'allow,deny', + 'rules' => array( + 'deny user from all', + 'allow root 1.2.3.4', + ), + ), + '', + ), + 'rules-allow-deny-reject' => array( + 'user', + 'root', + '1.2.3.4', + true, + true, + array( + 'order' => 'allow,deny', + 'rules' => array( + 'deny user from all', + 'allow root 1.2.3.4', + ), + ), + 'Access denied!', + ), + 'rules-explicit-ok' => array( + 'root', + 'root', + '1.2.3.4', + true, + true, + array( + 'order' => 'explicit', + 'rules' => array( + 'deny user from all', + 'allow root 1.2.3.4', + ), + ), + '', + ), + 'rules-explicit-reject' => array( + 'user', + 'root', + '1.2.3.4', + true, + true, + array( + 'order' => 'explicit', + 'rules' => array( + 'deny user from all', + 'allow root 1.2.3.4', + ), + ), + 'Access denied!', + ), + ); + } } From 6cb2963dcf6b40e21d3416ced2f419cfd3e988c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Fri, 20 Oct 2017 18:14:30 +0200 Subject: [PATCH 11/12] Fix createIV documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michal Čihař --- libraries/classes/Plugins/Auth/AuthenticationCookie.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/classes/Plugins/Auth/AuthenticationCookie.php b/libraries/classes/Plugins/Auth/AuthenticationCookie.php index e387a7cfd8..86836fedac 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationCookie.php +++ b/libraries/classes/Plugins/Auth/AuthenticationCookie.php @@ -835,7 +835,7 @@ class AuthenticationCookie extends AuthenticationPlugin * further decryption. I don't think necessary to have one iv * per server so I don't put the server number in the cookie name. * - * @return void + * @return string */ public function createIV() { From 6619fe9196be13bb09accc5cf858b0e5f02eb763 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Fri, 20 Oct 2017 19:11:04 +0200 Subject: [PATCH 12/12] Check return value from cookieDecrypt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This can return false and we should fail early once this happens. Signed-off-by: Michal Čihař --- .../Plugins/Auth/AuthenticationCookie.php | 24 ++++++++++++------- .../Plugins/Auth/AuthenticationCookieTest.php | 7 +++++- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/libraries/classes/Plugins/Auth/AuthenticationCookie.php b/libraries/classes/Plugins/Auth/AuthenticationCookie.php index 86836fedac..6af0669f2f 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationCookie.php +++ b/libraries/classes/Plugins/Auth/AuthenticationCookie.php @@ -386,11 +386,16 @@ class AuthenticationCookie extends AuthenticationPlugin return false; } - $this->user = $this->cookieDecrypt( + $value = $this->cookieDecrypt( $_COOKIE['pmaUser-' . $GLOBALS['server']], $this->_getEncryptionSecret() ); + if ($value === false) { + return false; + } + + $this->user = $value; // user was never logged in since session start if (empty($_SESSION['browser_access_time'])) { return false; @@ -427,14 +432,15 @@ class AuthenticationCookie extends AuthenticationPlugin if (empty($_COOKIE['pmaAuth-' . $GLOBALS['server']])) { return false; } - - $auth_data = json_decode( - $this->cookieDecrypt( - $_COOKIE['pmaAuth-' . $GLOBALS['server']], - $this->_getSessionEncryptionSecret() - ), - true + $value = $this->cookieDecrypt( + $_COOKIE['pmaAuth-' . $GLOBALS['server']], + $this->_getSessionEncryptionSecret() ); + if ($value === false) { + return false; + } + + $auth_data = json_decode($value, true); if (! is_array($auth_data) || ! isset($auth_data['password'])) { return false; @@ -777,7 +783,7 @@ class AuthenticationCookie extends AuthenticationPlugin * @param string $encdata encrypted data * @param string $secret the secret * - * @return string|bool original data, false on error + * @return string|false original data, false on error */ public function cookieDecrypt($encdata, $secret) { diff --git a/test/classes/Plugins/Auth/AuthenticationCookieTest.php b/test/classes/Plugins/Auth/AuthenticationCookieTest.php index 3dd3b2c57a..e0e48f46ea 100644 --- a/test/classes/Plugins/Auth/AuthenticationCookieTest.php +++ b/test/classes/Plugins/Auth/AuthenticationCookieTest.php @@ -645,9 +645,14 @@ class AuthenticationCookieTest extends PmaTestCase // mock for blowfish function $this->object = $this->getMockBuilder('PhpMyAdmin\Plugins\Auth\AuthenticationCookie') ->disableOriginalConstructor() - ->setMethods(array('showFailure')) + ->setMethods(array('showFailure', 'cookieDecrypt')) ->getMock(); + $this->object->expects($this->once()) + ->method('cookieDecrypt') + ->will($this->returnValue('testBF')); + + $this->object->expects($this->once()) ->method('showFailure');