diff --git a/ChangeLog b/ChangeLog index 521ef6873c..501172770d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,6 +12,7 @@ phpMyAdmin - ChangeLog - issue #11705 Fix occassional 200 errors on Windows - issue #12219 Fix locking issues when importing SQL - issue #12231 Avoid confusing warning when mysql extension is missing +- issue Improve handling of logout 4.6.1 (2016-05-02) - issue #12120 PMA_Util not found in insert_edit.lib.php diff --git a/libraries/navigation/NavigationHeader.php b/libraries/navigation/NavigationHeader.php index c0b24bc600..61c8cd20a6 100644 --- a/libraries/navigation/NavigationHeader.php +++ b/libraries/navigation/NavigationHeader.php @@ -167,8 +167,7 @@ class NavigationHeader if ($GLOBALS['server'] != 0) { // Logout for advanced authentication if ($GLOBALS['cfg']['Server']['auth_type'] != 'config') { - $link = 'index.php' . $GLOBALS['url_query']; - $link .= '&old_usr=' . urlencode($GLOBALS['PHP_AUTH_USER']); + $link = 'logout.php' . $GLOBALS['url_query']; $retval .= PMA\libraries\Util::getNavigationLink( $link, $showText, diff --git a/libraries/plugins/AuthenticationPlugin.php b/libraries/plugins/AuthenticationPlugin.php index 7d5a794015..d6753dd637 100644 --- a/libraries/plugins/AuthenticationPlugin.php +++ b/libraries/plugins/AuthenticationPlugin.php @@ -52,6 +52,46 @@ abstract class AuthenticationPlugin */ abstract public function authFails(); + /** + * Perform logout + * + * @return void + */ + 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']; + } else { + $redirect_url = $this->getLoginFormURL(); + } + + /* Clear credentials */ + $PHP_AUTH_USER = ''; + $PHP_AUTH_PW = ''; + + /* delete user's choices that were stored in session */ + $_SESSION = array(); + if (!defined('TESTSUITE')) { + session_destroy(); + } + + /* Redirect to login form (or configured URL) */ + PMA_sendHeaderLocation($redirect_url); + } + + /** + * Returns URL for login form. + * + * @return string + */ + public function getLoginFormURL() + { + return './index.php'; + } + /** * Returns error message for failed authentication. * diff --git a/libraries/plugins/auth/AuthenticationCookie.php b/libraries/plugins/auth/AuthenticationCookie.php index c4d5db6a0c..a4dec4aa2c 100644 --- a/libraries/plugins/auth/AuthenticationCookie.php +++ b/libraries/plugins/auth/AuthenticationCookie.php @@ -67,18 +67,6 @@ class AuthenticationCookie extends AuthenticationPlugin } } - /* Perform logout to custom URL */ - if (! empty($_REQUEST['old_usr']) - && ! empty($GLOBALS['cfg']['Server']['LogoutURL']) - ) { - PMA_sendHeaderLocation($GLOBALS['cfg']['Server']['LogoutURL']); - if (defined('TESTSUITE')) { - return true; - } else { - exit; - } - } - // No recall if blowfish secret is not configured as it would produce // garbage if ($GLOBALS['cfg']['LoginCookieRecall'] @@ -295,34 +283,6 @@ class AuthenticationCookie extends AuthenticationPlugin } // END Swekey Integration - if (! empty($_REQUEST['old_usr'])) { - // The user wants to be logged out - // -> delete his choices that were stored in session - - // according to the PHP manual we should do this before the destroy: - //$_SESSION = array(); - - if (! defined('TESTSUITE')) { - session_destroy(); - } - // -> delete password cookie(s) - if ($GLOBALS['cfg']['LoginCookieDeleteAll']) { - foreach ($GLOBALS['cfg']['Servers'] as $key => $val) { - $GLOBALS['PMA_Config']->removeCookie('pmaPass-' . $key); - if (isset($_COOKIE['pmaPass-' . $key])) { - unset($_COOKIE['pmaPass-' . $key]); - } - } - } else { - $GLOBALS['PMA_Config']->removeCookie( - 'pmaPass-' . $GLOBALS['server'] - ); - if (isset($_COOKIE['pmaPass-' . $GLOBALS['server']])) { - unset($_COOKIE['pmaPass-' . $GLOBALS['server']]); - } - } - } - if (! empty($_REQUEST['pma_username'])) { // Verify Captcha if it is required. @@ -831,4 +791,30 @@ class AuthenticationCookie extends AuthenticationPlugin { $this->storePasswordCookie($password); } + + /** + * Perform logout + * + * @return void + */ + public function logOut() + { + // -> delete password cookie(s) + if ($GLOBALS['cfg']['LoginCookieDeleteAll']) { + foreach ($GLOBALS['cfg']['Servers'] as $key => $val) { + $GLOBALS['PMA_Config']->removeCookie('pmaPass-' . $key); + if (isset($_COOKIE['pmaPass-' . $key])) { + unset($_COOKIE['pmaPass-' . $key]); + } + } + } else { + $GLOBALS['PMA_Config']->removeCookie( + 'pmaPass-' . $GLOBALS['server'] + ); + if (isset($_COOKIE['pmaPass-' . $GLOBALS['server']])) { + unset($_COOKIE['pmaPass-' . $GLOBALS['server']]); + } + } + parent::logOut(); + } } diff --git a/libraries/plugins/auth/AuthenticationHttp.php b/libraries/plugins/auth/AuthenticationHttp.php index 6c668d2526..62dcc01690 100644 --- a/libraries/plugins/auth/AuthenticationHttp.php +++ b/libraries/plugins/auth/AuthenticationHttp.php @@ -49,18 +49,6 @@ class AuthenticationHttp extends AuthenticationPlugin */ public function authForm() { - /* Perform logout to custom URL */ - if (!empty($_REQUEST['old_usr']) - && !empty($GLOBALS['cfg']['Server']['LogoutURL']) - ) { - PMA_sendHeaderLocation($GLOBALS['cfg']['Server']['LogoutURL']); - if (!defined('TESTSUITE')) { - exit; - } else { - return false; - } - } - if (empty($GLOBALS['cfg']['Server']['auth_http_realm'])) { if (empty($GLOBALS['cfg']['Server']['verbose'])) { $server_message = $GLOBALS['cfg']['Server']['host']; @@ -262,4 +250,14 @@ class AuthenticationHttp extends AuthenticationPlugin return true; } + + /** + * Returns URL for login form. + * + * @return string + */ + public function getLoginFormURL() + { + return './index.php?old_usr=' . $GLOBALS['PHP_AUTH_USER']; + } } diff --git a/libraries/plugins/auth/AuthenticationSignon.php b/libraries/plugins/auth/AuthenticationSignon.php index 9b30c8ecfa..fa60ed7894 100644 --- a/libraries/plugins/auth/AuthenticationSignon.php +++ b/libraries/plugins/auth/AuthenticationSignon.php @@ -28,11 +28,6 @@ class AuthenticationSignon extends AuthenticationPlugin unset($_SESSION['LAST_SIGNON_URL']); if (empty($GLOBALS['cfg']['Server']['SignonURL'])) { PMA_fatalError('You must set SignonURL!'); - } elseif (!empty($_REQUEST['old_usr']) - && !empty($GLOBALS['cfg']['Server']['LogoutURL']) - ) { - /* Perform logout to custom URL */ - PMA_sendHeaderLocation($GLOBALS['cfg']['Server']['LogoutURL']); } else { PMA_sendHeaderLocation($GLOBALS['cfg']['Server']['SignonURL']); } @@ -82,9 +77,6 @@ class AuthenticationSignon extends AuthenticationPlugin /* No configuration updates */ $single_signon_cfgupdate = array(); - /* Are we requested to do logout? */ - $do_logout = !empty($_REQUEST['old_usr']); - /* Handle script based auth */ if (!empty($script_name)) { if (!file_exists($script_name)) { @@ -117,18 +109,10 @@ class AuthenticationSignon extends AuthenticationPlugin /* Grab credentials if they exist */ if (isset($_SESSION['PMA_single_signon_user'])) { - if ($do_logout) { - $PHP_AUTH_USER = ''; - } else { - $PHP_AUTH_USER = $_SESSION['PMA_single_signon_user']; - } + $PHP_AUTH_USER = $_SESSION['PMA_single_signon_user']; } if (isset($_SESSION['PMA_single_signon_password'])) { - if ($do_logout) { - $PHP_AUTH_PW = ''; - } else { - $PHP_AUTH_PW = $_SESSION['PMA_single_signon_password']; - } + $PHP_AUTH_PW = $_SESSION['PMA_single_signon_password']; } if (isset($_SESSION['PMA_single_signon_host'])) { $single_signon_host = $_SESSION['PMA_single_signon_host']; diff --git a/logout.php b/logout.php new file mode 100644 index 0000000000..3bfa8f72b3 --- /dev/null +++ b/logout.php @@ -0,0 +1,15 @@ +logOut(); +} + diff --git a/test/classes/plugin/auth/AuthenticationCookieTest.php b/test/classes/plugin/auth/AuthenticationCookieTest.php index ec8693215e..62769dca33 100644 --- a/test/classes/plugin/auth/AuthenticationCookieTest.php +++ b/test/classes/plugin/auth/AuthenticationCookieTest.php @@ -373,6 +373,9 @@ class AuthenticationCookieTest extends PMATestCase */ public function testAuthHeader() { + $GLOBALS['cfg']['LoginCookieDeleteAll'] = false; + $GLOBALS['cfg']['Servers'] = array(1); + $restoreInstance = PMA\libraries\Response::getInstance(); $mockResponse = $this->getMockBuilder('PMA\libraries\Response') @@ -380,11 +383,6 @@ class AuthenticationCookieTest extends PMATestCase ->setMethods(array('isAjax', 'headersSent', 'header')) ->getMock(); - $mockResponse->expects($this->once()) - ->method('isAjax') - ->with() - ->will($this->returnValue(false)); - $mockResponse->expects($this->any()) ->method('headersSent') ->with() @@ -398,12 +396,9 @@ class AuthenticationCookieTest extends PMATestCase $attrInstance->setAccessible(true); $attrInstance->setValue($mockResponse); - $_REQUEST['old_usr'] = 'user1'; $GLOBALS['cfg']['Server']['LogoutURL'] = 'http://www.phpmyadmin.net/logout'; - $this->assertTrue( - $this->object->auth() - ); + $this->object->logOut(); $attrInstance->setValue($restoreInstance); } @@ -454,20 +449,40 @@ class AuthenticationCookieTest extends PMATestCase */ public function testLogoutDelete() { + $restoreInstance = PMA\libraries\Response::getInstance(); + + $mockResponse = $this->getMockBuilder('PMA\libraries\Response') + ->disableOriginalConstructor() + ->setMethods(array('isAjax', 'headersSent', 'header')) + ->getMock(); + + $mockResponse->expects($this->any()) + ->method('headersSent') + ->with() + ->will($this->returnValue(false)); + + $mockResponse->expects($this->once()) + ->method('header') + ->with('Location: ./index.php' . ((SID) ? '?' . SID : '')); + + $attrInstance = new ReflectionProperty('PMA\libraries\Response', '_instance'); + $attrInstance->setAccessible(true); + $attrInstance->setValue($mockResponse); + $GLOBALS['cfg']['Server']['auth_swekey_config'] = ''; $GLOBALS['cfg']['CaptchaLoginPrivateKey'] = ''; $GLOBALS['cfg']['CaptchaLoginPublicKey'] = ''; - $_REQUEST['old_usr'] = 'pmaolduser'; $GLOBALS['cfg']['LoginCookieDeleteAll'] = true; $GLOBALS['cfg']['Servers'] = array(1); $_COOKIE['pmaPass-0'] = 'test'; - $this->object->authCheck(); + $this->object->logOut(); $this->assertFalse( isset($_COOKIE['pmaPass-0']) ); + $attrInstance->setValue($restoreInstance); } /** @@ -477,21 +492,40 @@ class AuthenticationCookieTest extends PMATestCase */ public function testLogout() { + $restoreInstance = PMA\libraries\Response::getInstance(); + + $mockResponse = $this->getMockBuilder('PMA\libraries\Response') + ->disableOriginalConstructor() + ->setMethods(array('isAjax', 'headersSent', 'header')) + ->getMock(); + + $mockResponse->expects($this->any()) + ->method('headersSent') + ->with() + ->will($this->returnValue(false)); + + $mockResponse->expects($this->once()) + ->method('header') + ->with('Location: ./index.php' . ((SID) ? '?' . SID : '')); + + $attrInstance = new ReflectionProperty('PMA\libraries\Response', '_instance'); + $attrInstance->setAccessible(true); + $attrInstance->setValue($mockResponse); $GLOBALS['cfg']['Server']['auth_swekey_config'] = ''; $GLOBALS['cfg']['CaptchaLoginPrivateKey'] = ''; $GLOBALS['cfg']['CaptchaLoginPublicKey'] = ''; - $_REQUEST['old_usr'] = 'pmaolduser'; $GLOBALS['cfg']['LoginCookieDeleteAll'] = false; $GLOBALS['cfg']['Servers'] = array(1); $GLOBALS['server'] = 1; $_COOKIE['pmaPass-1'] = 'test'; - $this->object->authCheck(); + $this->object->logOut(); $this->assertFalse( isset($_COOKIE['pmaPass-1']) ); + $attrInstance->setValue($restoreInstance); } /** diff --git a/test/classes/plugin/auth/AuthenticationHttpTest.php b/test/classes/plugin/auth/AuthenticationHttpTest.php index 3d0dcd2f74..dd63588034 100644 --- a/test/classes/plugin/auth/AuthenticationHttpTest.php +++ b/test/classes/plugin/auth/AuthenticationHttpTest.php @@ -123,9 +123,13 @@ class AuthenticationHttpTest extends PMATestCase call_user_func_array(array($header_method, 'withConsecutive'), $headers); try { - $this->assertFalse( - $this->object->auth() - ); + if (!empty($_REQUEST['old_usr'])) { + $this->object->logOut(); + } else { + $this->assertFalse( + $this->object->auth() + ); + } } finally { $attrInstance->setValue($restoreInstance); } diff --git a/test/classes/plugin/auth/AuthenticationSignonTest.php b/test/classes/plugin/auth/AuthenticationSignonTest.php index 79d0f0f9f6..bab7437365 100644 --- a/test/classes/plugin/auth/AuthenticationSignonTest.php +++ b/test/classes/plugin/auth/AuthenticationSignonTest.php @@ -73,10 +73,9 @@ class AuthenticationSignonTest extends PMATestCase // case 2 $GLOBALS['cfg']['Server']['SignonURL'] = 'http://phpmyadmin.net/SignonURL'; - $_REQUEST['old_usr'] = 'oldUser'; $GLOBALS['cfg']['Server']['LogoutURL'] = 'http://phpmyadmin.net/logoutURL'; - $this->object->auth(); + $this->object->logOut(); $this->assertContains( 'Location: http://phpmyadmin.net/logoutURL?PHPSESSID=', @@ -87,10 +86,9 @@ class AuthenticationSignonTest extends PMATestCase $GLOBALS['header'] = array(); $GLOBALS['cfg']['Server']['SignonURL'] = 'http://phpmyadmin.net/SignonURL'; - $_REQUEST['old_usr'] = ''; $GLOBALS['cfg']['Server']['LogoutURL'] = ''; - $this->object->auth(); + $this->object->logOut(); $this->assertContains( 'Location: http://phpmyadmin.net/SignonURL?PHPSESSID=', @@ -155,6 +153,26 @@ class AuthenticationSignonTest extends PMATestCase */ public function testAuthCheckToken() { + $restoreInstance = PMA\libraries\Response::getInstance(); + + $mockResponse = $this->getMockBuilder('PMA\libraries\Response') + ->disableOriginalConstructor() + ->setMethods(array('isAjax', 'headersSent', 'header')) + ->getMock(); + + $mockResponse->expects($this->any()) + ->method('headersSent') + ->with() + ->will($this->returnValue(false)); + + $mockResponse->expects($this->once()) + ->method('header') + ->with('Location: ./index.php' . ((SID) ? '?' . SID : '')); + + $attrInstance = new ReflectionProperty('PMA\libraries\Response', '_instance'); + $attrInstance->setAccessible(true); + $attrInstance->setValue($mockResponse); + $GLOBALS['cfg']['Server']['SignonURL'] = 'http://phpmyadmin.net/SignonURL'; $GLOBALS['cfg']['Server']['SignonSession'] = 'session123'; $GLOBALS['cfg']['Server']['host'] = 'localhost'; @@ -162,7 +180,6 @@ class AuthenticationSignonTest extends PMATestCase $GLOBALS['cfg']['Server']['user'] = 'user'; $GLOBALS['cfg']['Server']['SignonScript'] = ''; $_COOKIE['session123'] = true; - $_REQUEST['old_usr'] = 'oldUser'; $_SESSION['PMA_single_signon_user'] = 'user123'; $_SESSION['PMA_single_signon_password'] = 'pass123'; $_SESSION['PMA_single_signon_host'] = 'local'; @@ -172,28 +189,20 @@ class AuthenticationSignonTest extends PMATestCase $sessionName = session_name(); $sessionID = session_id(); - $this->assertFalse( - $this->object->authCheck() - ); + $this->object->logOut(); $this->assertEquals( array( 'SignonURL' => 'http://phpmyadmin.net/SignonURL', 'SignonScript' => '', 'SignonSession' => 'session123', - 'host' => 'local', - 'port' => '12', + 'host' => 'localhost', + 'port' => '80', 'user' => 'user', - 'foo' => 'bar' ), $GLOBALS['cfg']['Server'] ); - $this->assertEquals( - 'pmaToken', - $_SESSION[' PMA_token '] - ); - $this->assertEquals( $sessionName, session_name() @@ -207,6 +216,7 @@ class AuthenticationSignonTest extends PMATestCase $this->assertFalse( isset($_SESSION['LAST_SIGNON_URL']) ); + $attrInstance->setValue($restoreInstance); } /**