diff options
author | diosmosis <diosmosis@users.noreply.github.com> | 2018-07-27 00:24:52 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-07-27 00:24:52 +0300 |
commit | 35bd641d69235420f69da986d77437af700a6164 (patch) | |
tree | b83792eba151782f3a4776217e1c96d511577719 | |
parent | c5b46d7e61bd169b169470f8df9247e6f6781cc0 (diff) |
Sessions with more security (#12208)
* Modifying "cookie authentication" to be more secure.
Instead of authenticating by token auth if it exists in the cookie, validate an existing session. If the session
has the user name stored as a session var, it has been authenticated. If the request has the same IP address and
user agent as the request that created the session, the request is from the user that created the session. If
both of these are true, then the session is valid, and we don't need a token auth to authenticate.
If the session is deleted before the Piwik auth cookie expires (due to garbage collection), we attempt to
re-authenticate using a secure hash of the token auth. We don't do this on every request since password_verify()
will, at BEST, add 3.5ms to every request.
* Invalidate existing sessions after user password change.
Invalidation is accomplished w/o having to individually touch sessions by:
1. Using the password hash as the piwik_auth key secret, instead of the token auth. So when a password changes, existing piwik_auth keys are no longer valid. This affects session re-authentication.
2. Saving the session start time & the last time a user's password was modified, and checking that the session start time is always newer than the password modification time.
* Set session.gc_maxlifetime to login_cookie_expire time so session data does not disappear, remove session re-auth functionality & tie cookie hash to password modified time instead of password hash to retain automatic session invalidation on password change.
* In SessionInitializer, clear other cookie values so previously stored token auths will be removed.
* Make sure anonymous user is still default user whan authenticating.
* fixing test failures
* Remove hash checking in piwik_auth cookie.
piwik_auth cookie still required since it's presence indicates we should use SessionAuth instead of the normal authentication mechanism. Since there's always a session, even if you're not logged in, PIWIK_SESSID can't be used by itself to determine this.
* Make sure session auth doesnt break in edge case where ts_password_modified column does not exist.
* Clarify session destruction/invalidation logic in SessionAuth.
* Make UsersManagerTest slightly more comprehensive.
* Use Date::now()->getTimestampUTC() instead of time() in SessionFingerprint::initialize().
* Check getUser returns correct user info in SessionAuth for sanity.
* Add SessionInitializer::getAuthCookie() back since it is @api.
* Remove IP address from session auth info + check.
* Refactor session start changes so it is started in one place only.
* Remove SessionAuthCookieFactory & deprecate auth cookie INI config vars (still needed for SessionInitializer deprectaed method).
* Make sure user can still login if ts_password_modified column is not present in database.
* Rename ts_password_modified Update class.
* Update comment in SessionAuth to include why Piwik tries to create another session.
* Restore 3.x-dev SessionInitializer for BC (deprecated), move new SessionInitializer to core, add tests for both SessionInitializers.
* Change update to 3.5 version.
* Make sure normal auth implementation is used if sessionauth fails so anonymous user can be logged in.
* On logout clear session fingerprint so same session cannot be used to login.
* Change update name + bump version, and make sure Session::rememberMe() is called before session is started (otherwise it has no effect).
* Fixing tests.
* apply review fixes
* remove test
33 files changed, 1205 insertions, 155 deletions
diff --git a/config/global.ini.php b/config/global.ini.php index 2de8bf6704..a2eaafd7b9 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -370,15 +370,14 @@ session_save_handler = files ; it is recommended for security reasons to always use Matomo over https force_ssl = 0 -; login cookie name +; (DEPRECATED) has no effect login_cookie_name = piwik_auth ; By default, the auth cookie is set only for the duration of session. ; if "Remember me" is checked, the auth cookie will be valid for 14 days by default login_cookie_expire = 1209600 -; The path on the server in which the cookie will be available on. -; Defaults to empty. See spec in https://curl.haxx.se/rfc/cookie_spec.html +; (DEPRECATED) has no effect login_cookie_path = ; email address that appears as a Sender in the password recovery email diff --git a/core/Cookie.php b/core/Cookie.php index 24f8a3f6c1..ec6710befd 100644 --- a/core/Cookie.php +++ b/core/Cookie.php @@ -105,7 +105,7 @@ class Cookie */ public function isCookieFound() { - return isset($_COOKIE[$this->name]); + return self::isCookieInRequest($this->name); } /** @@ -257,7 +257,7 @@ class Cookie * * @return string Cookie content */ - protected function generateContentString() + public function generateContentString() { $cookieStr = ''; @@ -370,6 +370,14 @@ class Cookie } /** + * Removes all values from the cookie. + */ + public function clear() + { + $this->value = []; + } + + /** * Returns an easy to read cookie dump * * @return string The cookie dump @@ -394,4 +402,16 @@ class Cookie { return Common::sanitizeInputValues($value); } + + /** + * Returns true if a cookie named '$name' is in the current HTTP request, + * false if otherwise. + * + * @param string $name the name of the cookie + * @return boolean + */ + public static function isCookieInRequest($name) + { + return isset($_COOKIE[$name]); + } } diff --git a/core/Db/Schema/Mysql.php b/core/Db/Schema/Mysql.php index 4984eefd5b..f517edf6cc 100644 --- a/core/Db/Schema/Mysql.php +++ b/core/Db/Schema/Mysql.php @@ -41,6 +41,7 @@ class Mysql implements SchemaInterface token_auth CHAR(32) NOT NULL, superuser_access TINYINT(2) unsigned NOT NULL DEFAULT '0', date_registered TIMESTAMP NULL, + ts_password_modified TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, PRIMARY KEY(login), UNIQUE KEY uniq_keytoken(token_auth) ) ENGINE=$engine DEFAULT CHARSET=utf8 @@ -457,11 +458,13 @@ class Mysql implements SchemaInterface */ public function createAnonymousUser() { + $now = Date::factory('now')->getDatetime(); + // The anonymous user is the user that is assigned by default // note that the token_auth value is anonymous, which is assigned by default as well in the Login plugin $db = $this->getDb(); $db->query("INSERT IGNORE INTO " . Common::prefixTable("user") . " - VALUES ( 'anonymous', '', 'anonymous', 'anonymous@example.org', 'anonymous', 0, '" . Date::factory('now')->getDatetime() . "' );"); + VALUES ( 'anonymous', '', 'anonymous', 'anonymous@example.org', 'anonymous', 0, '$now', '$now' );"); } /** diff --git a/core/FrontController.php b/core/FrontController.php index c7b75a84b6..7a069a043f 100644 --- a/core/FrontController.php +++ b/core/FrontController.php @@ -20,6 +20,7 @@ use Piwik\Exception\StylesheetLessCompileException; use Piwik\Http\ControllerResolver; use Piwik\Http\Router; use Piwik\Plugins\CoreAdminHome\CustomLogo; +use Piwik\Session\SessionAuth; /** * This singleton dispatches requests to the appropriate plugin Controller. @@ -60,6 +61,8 @@ use Piwik\Plugins\CoreAdminHome\CustomLogo; class FrontController extends Singleton { const DEFAULT_MODULE = 'CoreHome'; + const DEFAULT_LOGIN = 'anonymous'; + const DEFAULT_TOKEN_AUTH = 'anonymous'; /** * Set to false and the Front Controller will not dispatch the request @@ -348,34 +351,19 @@ class FrontController extends Singleton SettingsPiwik::getPiwikUrl(); } - /** - * Triggered before the user is authenticated, when the global authentication object - * should be created. - * - * Plugins that provide their own authentication implementation should use this event - * to set the global authentication object (which must derive from {@link Piwik\Auth}). - * - * **Example** - * - * Piwik::addAction('Request.initAuthenticationObject', function() { - * StaticContainer::getContainer()->set('Piwik\Auth', new MyAuthImplementation()); - * }); - */ - Piwik::postEvent('Request.initAuthenticationObject'); - try { - $authAdapter = StaticContainer::get('Piwik\Auth'); - } catch (Exception $e) { - $message = "Authentication object cannot be found in the container. Maybe the Login plugin is not activated? - <br />You can activate the plugin by adding:<br /> - <code>Plugins[] = Login</code><br /> - under the <code>[Plugins]</code> section in your config/config.ini.php"; + $loggedIn = false; - $ex = new AuthenticationFailedException($message); - $ex->setIsHtmlMessage(); + // try authenticating w/ session first... + $sessionAuth = $this->makeSessionAuthenticator(); + if ($sessionAuth) { + $loggedIn = Access::getInstance()->reloadAccess($sessionAuth); + } - throw $ex; + // ... if session auth fails try normal auth (which will login the anonymous user) + if (!$loggedIn) { + $authAdapter = $this->makeAuthenticator(); + Access::getInstance()->reloadAccess($authAdapter); } - Access::getInstance()->reloadAccess($authAdapter); // Force the auth to use the token_auth if specified, so that embed dashboard // and all other non widgetized controller methods works fine @@ -405,11 +393,7 @@ class FrontController extends Singleton $action = Common::getRequestVar('action', false); } - if (SettingsPiwik::isPiwikInstalled() - && ($module !== 'API' || ($action && $action !== 'index')) - ) { - Session::start(); - + if (Session::isSessionStarted()) { $this->closeSessionEarlyForFasterUI(); } @@ -615,4 +599,62 @@ class FrontController extends Singleton throw new DatabaseSchemaIsNewerThanCodebaseException(implode(" ", $messages)); } } + + private function makeSessionAuthenticator() + { + $module = Common::getRequestVar('module', self::DEFAULT_MODULE, 'string'); + $action = Common::getRequestVar('action', false); + + // the session must be started before using the session authenticator, + // so we do it here, if this is not an API request. + if (SettingsPiwik::isPiwikInstalled() + && ($module !== 'API' || ($action && $action !== 'index')) + ) { + /** + * @ignore + */ + Piwik::postEvent('Session.beforeSessionStart'); + + Session::start(); + return StaticContainer::get(SessionAuth::class); + } + + return null; + } + + private function makeAuthenticator() + { + /** + * Triggered before the user is authenticated, when the global authentication object + * should be created. + * + * Plugins that provide their own authentication implementation should use this event + * to set the global authentication object (which must derive from {@link Piwik\Auth}). + * + * **Example** + * + * Piwik::addAction('Request.initAuthenticationObject', function() { + * StaticContainer::getContainer()->set('Piwik\Auth', new MyAuthImplementation()); + * }); + */ + Piwik::postEvent('Request.initAuthenticationObject'); + try { + $authAdapter = StaticContainer::get('Piwik\Auth'); + } catch (Exception $e) { + $message = "Authentication object cannot be found in the container. Maybe the Login plugin is not activated? + <br />You can activate the plugin by adding:<br /> + <code>Plugins[] = Login</code><br /> + under the <code>[Plugins]</code> section in your config/config.ini.php"; + + $ex = new AuthenticationFailedException($message); + $ex->setIsHtmlMessage(); + + throw $ex; + } + + $authAdapter->setLogin(self::DEFAULT_LOGIN); + $authAdapter->setTokenAuth(self::DEFAULT_TOKEN_AUTH); + + return $authAdapter; + } } diff --git a/core/Session.php b/core/Session.php index e9369b97d0..3987b1012e 100644 --- a/core/Session.php +++ b/core/Session.php @@ -52,6 +52,8 @@ class Session extends Zend_Session } self::$sessionStarted = true; + $config = Config::getInstance(); + // use cookies to store session id on the client side @ini_set('session.use_cookies', '1'); @@ -73,8 +75,11 @@ class Session extends Zend_Session // incorrectly invalidate the session @ini_set('session.referer_check', ''); + // to preserve previous behavior piwik_auth provided when it contained a token_auth, we ensure + // the session data won't be deleted until the cookie expires. + @ini_set('session.gc_maxlifetime', $config->General['login_cookie_expire']); + $currentSaveHandler = ini_get('session.save_handler'); - $config = Config::getInstance(); if (self::isFileBasedSessions()) { // Note: this handler doesn't work well in load-balanced environments and may have a concurrency issue with locked session files @@ -153,4 +158,9 @@ class Session extends Zend_Session { parent::writeClose(); } + + public static function isSessionStarted() + { + return self::$sessionStarted; + } } diff --git a/core/Session/SessionAuth.php b/core/Session/SessionAuth.php new file mode 100644 index 0000000000..3756f38998 --- /dev/null +++ b/core/Session/SessionAuth.php @@ -0,0 +1,168 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + * + */ + +namespace Piwik\Session; + +use Piwik\Auth; +use Piwik\AuthResult; +use Piwik\Date; +use Piwik\Plugins\UsersManager\Model as UsersModel; +use Piwik\Session; + +/** + * Validates already authenticated sessions. + * + * See {@link \Piwik\Session\SessionFingerprint} for more info. + */ +class SessionAuth implements Auth +{ + /** + * For tests, since there's no actual session there. + * + * @var bool + */ + private $shouldDestroySession; + + /** + * @var UsersModel + */ + private $userModel; + + public function __construct(UsersModel $userModel = null, $shouldDestroySession = true) + { + $this->userModel = $userModel ?: new UsersModel(); + $this->shouldDestroySession = $shouldDestroySession; + } + + public function getName() + { + // empty + } + + public function setTokenAuth($token_auth) + { + // empty + } + + public function getLogin() + { + // empty + } + + public function getTokenAuthSecret() + { + // empty + } + + public function setLogin($login) + { + // empty + } + + public function setPassword($password) + { + // empty + } + + public function setPasswordHash($passwordHash) + { + // empty + } + + public function authenticate() + { + $sessionFingerprint = new SessionFingerprint(); + $userModel = $this->userModel; + + $userForSession = $sessionFingerprint->getUser(); + if (empty($userForSession)) { + return $this->makeAuthFailure(); + } + + $user = $userModel->getUser($userForSession); + if (empty($user) + || $user['login'] !== $userForSession // sanity check in case there's a bug in getUser() + ) { + return $this->makeAuthFailure(); + } + + if (!$sessionFingerprint->isMatchingCurrentRequest()) { + $this->initNewBlankSession($sessionFingerprint); + return $this->makeAuthFailure(); + } + + $tsPasswordModified = !empty($user['ts_password_modified']) ? $user['ts_password_modified'] : null; + if ($this->isSessionStartedBeforePasswordChange($sessionFingerprint, $tsPasswordModified)) { + $this->destroyCurrentSession($sessionFingerprint); + return $this->makeAuthFailure(); + } + + return $this->makeAuthSuccess($user); + } + + private function isSessionStartedBeforePasswordChange(SessionFingerprint $sessionFingerprint, $tsPasswordModified) + { + // sanity check, make sure users can still login if the ts_password_modified column does not exist + if ($tsPasswordModified === null) { + return false; + } + + // if the session start time doesn't exist for some reason, log the user out + $sessionStartTime = $sessionFingerprint->getSessionStartTime(); + if (empty($sessionStartTime)) { + return true; + } + + return $sessionStartTime < Date::factory($tsPasswordModified)->getTimestampUTC(); + } + + private function makeAuthFailure() + { + return new AuthResult(AuthResult::FAILURE, null, null); + } + + private function makeAuthSuccess($user) + { + $this->setTokenAuth($user['token_auth']); + + $isSuperUser = (int) $user['superuser_access']; + $code = $isSuperUser ? AuthResult::SUCCESS_SUPERUSER_AUTH_CODE : AuthResult::SUCCESS; + + return new AuthResult($code, $user['login'], $user['token_auth']); + } + + private function initNewBlankSession(SessionFingerprint $sessionFingerprint) + { + // this user should be using a different session, so generate a new ID + // NOTE: Zend_Session cannot be used since it will destroy the old + // session. + if ($this->shouldDestroySession) { + session_regenerate_id(); + } + + // regenerating the ID will create a new session w/ a new ID, but will + // copy over the existing session data. we want the new session for the + // unauthorized user to be different, so we clear the session fingerprint. + $sessionFingerprint->clear(); + } + + private function destroyCurrentSession(SessionFingerprint $sessionFingerprint) + { + // Note: Piwik will attempt to create another session in the LoginController + // when rendering the login form (the nonce for the form is stored in the session). + // So we can't use Session::destroy() since Zend prohibits starting a new session + // after session_destroy() is called. Instead we clear the session fingerprint for + // the existing session and generate a new session. Both the old session & + // new session should have no stored data. + $sessionFingerprint->clear(); + if ($this->shouldDestroySession) { + Session::regenerateId(); + } + } +} diff --git a/core/Session/SessionFingerprint.php b/core/Session/SessionFingerprint.php new file mode 100644 index 0000000000..2239661fb9 --- /dev/null +++ b/core/Session/SessionFingerprint.php @@ -0,0 +1,102 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + * + */ + +namespace Piwik\Session; + +use Piwik\Date; + +/** + * Manages session information that is used to identify who the session + * is for. + * + * Once a session is authenticated using either a user name & password or + * token auth, some information about the user is stored in the session. + * This info includes the user name and the user agent + * string of the user's client, and a random session secret. + * + * In subsequent requests that use this session, we use the above information + * to verify that the session is allowed to be used by the person sending the + * request. + * + * This is accomplished by checking the request's user agent + * against what is stored in the session. If it doesn't then this is a + * session hijacking attempt. + * + * We also check that a hash in the piwik_auth cookie matches the hash + * of the time the user last changed their password + the session secret. + * If they don't match, the password has been changed since this session + * started, and is no longer valid. + */ +class SessionFingerprint +{ + const USER_NAME_SESSION_VAR_NAME = 'user.name'; + const SESSION_INFO_SESSION_VAR_NAME = 'session.info'; + + public function getUser() + { + if (isset($_SESSION[self::USER_NAME_SESSION_VAR_NAME])) { + return $_SESSION[self::USER_NAME_SESSION_VAR_NAME]; + } + + return null; + } + + public function getUserInfo() + { + if (isset($_SESSION[self::SESSION_INFO_SESSION_VAR_NAME])) { + return $_SESSION[self::SESSION_INFO_SESSION_VAR_NAME]; + } + + return null; + } + + public function initialize($userName, $time = null, $userAgent = null) + { + $_SESSION[self::USER_NAME_SESSION_VAR_NAME] = $userName; + $_SESSION[self::SESSION_INFO_SESSION_VAR_NAME] = [ + 'ts' => $time ?: Date::now()->getTimestampUTC(), + 'ua' => $userAgent ?: $this->getUserAgent(), + ]; + } + + public function clear() + { + unset($_SESSION[self::USER_NAME_SESSION_VAR_NAME]); + unset($_SESSION[self::SESSION_INFO_SESSION_VAR_NAME]); + } + + public function isMatchingCurrentRequest() + { + $requestUa = $this->getUserAgent(); + + $userInfo = $this->getUserInfo(); + if (empty($userInfo)) { + return false; + } + + return $userInfo['ua'] == $requestUa; + } + + public function getSessionStartTime() + { + $userInfo = $this->getUserInfo(); + if (empty($userInfo) + || empty($userInfo['ts']) + ) { + return null; + } + + return $userInfo['ts']; + } + + private function getUserAgent() + { + return array_key_exists('HTTP_USER_AGENT', $_SERVER) ? $_SERVER['HTTP_USER_AGENT'] : null; + } +} diff --git a/core/Session/SessionInitializer.php b/core/Session/SessionInitializer.php new file mode 100644 index 0000000000..1089b3c40e --- /dev/null +++ b/core/Session/SessionInitializer.php @@ -0,0 +1,106 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + * + */ +namespace Piwik\Session; + +use Exception; +use Piwik\Auth as AuthInterface; +use Piwik\AuthResult; +use Piwik\Piwik; +use Piwik\Session; + +/** + * Initializes authenticated sessions using an Auth implementation. + */ +class SessionInitializer +{ + /** + * Authenticates the user and, if successful, initializes an authenticated session. + * + * @param \Piwik\Auth $auth The Auth implementation to use. + * @throws Exception If authentication fails or the user is not allowed to login for some reason. + */ + public function initSession(AuthInterface $auth) + { + $this->regenerateSessionId(); + + $authResult = $this->doAuthenticateSession($auth); + + if (!$authResult->wasAuthenticationSuccessful()) { + + Piwik::postEvent('Login.authenticate.failed', array($auth->getLogin())); + + $this->processFailedSession(); + } else { + + Piwik::postEvent('Login.authenticate.successful', array($auth->getLogin())); + + $this->processSuccessfulSession($authResult); + } + } + + /** + * Authenticates the user. + * + * Derived classes can override this method to customize authentication logic or impose + * extra requirements on the user trying to login. + * + * @param AuthInterface $auth The Auth implementation to use when authenticating. + * @return AuthResult + */ + protected function doAuthenticateSession(AuthInterface $auth) + { + Piwik::postEvent( + 'Login.authenticate', + array( + $auth->getLogin(), + ) + ); + + return $auth->authenticate(); + } + + /** + * Executed when the session could not authenticate. + * + * @throws Exception always. + */ + protected function processFailedSession() + { + throw new Exception(Piwik::translate('Login_LoginPasswordNotCorrect')); + } + + /** + * Executed when the session was successfully authenticated. + * + * @param AuthResult $authResult The successful authentication result. + */ + protected function processSuccessfulSession(AuthResult $authResult) + { + $sessionIdentifier = new SessionFingerprint(); + $sessionIdentifier->initialize($authResult->getIdentity()); + } + + protected function regenerateSessionId() + { + Session::regenerateId(); + } + + /** + * Accessor to compute the hashed authentication token. + * + * @param string $login user login + * @param string $token_auth authentication token + * @return string hashed authentication token + * @deprecated + */ + public static function getHashTokenAuth($login, $token_auth) + { + return md5($login . $token_auth); + } +} diff --git a/core/Updater/Migration/Db/AddColumn.php b/core/Updater/Migration/Db/AddColumn.php index 5626c7a4fd..3e655fb433 100644 --- a/core/Updater/Migration/Db/AddColumn.php +++ b/core/Updater/Migration/Db/AddColumn.php @@ -23,5 +23,4 @@ class AddColumn extends Sql parent::__construct($sql, static::ERROR_CODE_DUPLICATE_COLUMN); } - } diff --git a/core/Updates/3.6.0-b1.php b/core/Updates/3.6.0-b1.php index c0185f3e1f..63e76231c4 100644 --- a/core/Updates/3.6.0-b1.php +++ b/core/Updates/3.6.0-b1.php @@ -9,18 +9,17 @@ namespace Piwik\Updates; -use Piwik\Common; -use Piwik\Db; -use Piwik\DbHelper; use Piwik\Updater; +use Piwik\Updates as PiwikUpdates; use Piwik\Updater\Migration; use Piwik\Updater\Migration\Factory as MigrationFactory; -use Piwik\Updates; +use Piwik\Common; +use Piwik\DbHelper; /** * Update for version 3.6.0-b1. */ -class Updates_3_6_0_b1 extends Updates +class Updates_3_6_0_b1 extends PiwikUpdates { /** * @var MigrationFactory @@ -36,7 +35,7 @@ class Updates_3_6_0_b1 extends Updates $this->migration = $factory; } - /** + /** * Here you can define one or multiple SQL statements that should be executed during the update. * @param Updater $updater * @return Migration[] @@ -69,6 +68,10 @@ class Updates_3_6_0_b1 extends Updates $migrations[] = $this->migration->db->addIndex('access', array('login', 'idsite'), 'index_loginidsite'); } + // changes for session auth + $migrations[] = $this->migration->db->addColumn('user', 'ts_password_modified', + 'TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP'); + return $migrations; } diff --git a/plugins/Login/Auth.php b/plugins/Login/Auth.php index 08bb693108..ab827bf12c 100644 --- a/plugins/Login/Auth.php +++ b/plugins/Login/Auth.php @@ -58,7 +58,7 @@ class Auth implements \Piwik\Auth } elseif (is_null($this->login)) { return $this->authenticateWithToken($this->token_auth); } elseif (!empty($this->login)) { - return $this->authenticateWithTokenOrHashToken($this->token_auth, $this->login); + return $this->authenticateWithLoginAndToken($this->token_auth, $this->login); } return new AuthResult(AuthResult::FAILURE, $this->login, $this->token_auth); @@ -96,14 +96,13 @@ class Auth implements \Piwik\Auth return new AuthResult(AuthResult::FAILURE, null, $token); } - private function authenticateWithTokenOrHashToken($token, $login) + private function authenticateWithLoginAndToken($token, $login) { $user = $this->userModel->getUser($login); if (!empty($user['token_auth']) // authenticate either with the token or the "hash token" - && ((SessionInitializer::getHashTokenAuth($login, $user['token_auth']) === $token) - || $user['token_auth'] === $token) + && $user['token_auth'] === $token ) { return $this->authenticationSuccess($user); } diff --git a/plugins/Login/Controller.php b/plugins/Login/Controller.php index c933214b01..3e7eac57a6 100644 --- a/plugins/Login/Controller.php +++ b/plugins/Login/Controller.php @@ -39,7 +39,7 @@ class Controller extends \Piwik\Plugin\Controller protected $auth; /** - * @var SessionInitializer + * @var \Piwik\Session\SessionInitializer */ protected $sessionInitializer; @@ -65,7 +65,7 @@ class Controller extends \Piwik\Plugin\Controller $this->auth = $auth; if (empty($sessionInitializer)) { - $sessionInitializer = new SessionInitializer(); + $sessionInitializer = new \Piwik\Session\SessionInitializer(); } $this->sessionInitializer = $sessionInitializer; } @@ -100,9 +100,8 @@ class Controller extends \Piwik\Plugin\Controller $login = $this->getLoginFromLoginOrEmail($loginOrEmail); $password = $form->getSubmitValue('form_password'); - $rememberMe = $form->getSubmitValue('form_rememberme') == '1'; try { - $this->authenticateAndRedirect($login, $password, $rememberMe); + $this->authenticateAndRedirect($login, $password); } catch (Exception $e) { $messageNoAccess = $e->getMessage(); } @@ -173,7 +172,7 @@ class Controller extends \Piwik\Plugin\Controller $urlToRedirect = Common::getRequestVar('url', $currentUrl, 'string'); $urlToRedirect = Common::unsanitizeInputValue($urlToRedirect); - $this->authenticateAndRedirect($login, $password, false, $urlToRedirect, $passwordHashed = true); + $this->authenticateAndRedirect($login, $password, $urlToRedirect, $passwordHashed = true); } /** @@ -201,12 +200,11 @@ class Controller extends \Piwik\Plugin\Controller * * @param string $login user name * @param string $password plain-text or hashed password - * @param bool $rememberMe Remember me? * @param string $urlToRedirect URL to redirect to, if successfully authenticated * @param bool $passwordHashed indicates if $password is hashed * @return string failure message if unable to authenticate */ - protected function authenticateAndRedirect($login, $password, $rememberMe, $urlToRedirect = false, $passwordHashed = false) + protected function authenticateAndRedirect($login, $password, $urlToRedirect = false, $passwordHashed = false) { Nonce::discardNonce('Login.login'); @@ -217,7 +215,7 @@ class Controller extends \Piwik\Plugin\Controller $this->auth->setPasswordHash($password); } - $this->sessionInitializer->initSession($this->auth, $rememberMe); + $this->sessionInitializer->initSession($this->auth); // remove password reset entry if it exists $this->passwordResetter->removePasswordResetInfo($login); @@ -357,14 +355,12 @@ class Controller extends \Piwik\Plugin\Controller /** * Clear session information * - * @param none * @return void */ public static function clearSession() { - $authCookieName = Config::getInstance()->General['login_cookie_name']; - $cookie = new Cookie($authCookieName); - $cookie->delete(); + $sessionFingerprint = new Session\SessionFingerprint(); + $sessionFingerprint->clear(); Session::expireSessionCookie(); } diff --git a/plugins/Login/Login.php b/plugins/Login/Login.php index c88756a160..9a88e9f48a 100644 --- a/plugins/Login/Login.php +++ b/plugins/Login/Login.php @@ -15,6 +15,7 @@ use Piwik\Container\StaticContainer; use Piwik\Cookie; use Piwik\FrontController; use Piwik\Piwik; +use Piwik\Session; /** * @@ -27,11 +28,11 @@ class Login extends \Piwik\Plugin public function registerEvents() { $hooks = array( - 'Request.initAuthenticationObject' => 'initAuthenticationObject', 'User.isNotAuthorized' => 'noAccess', 'API.Request.authenticate' => 'ApiRequestAuthenticate', 'AssetManager.getJavaScriptFiles' => 'getJsFiles', - 'AssetManager.getStylesheetFiles' => 'getStylesheetFiles' + 'AssetManager.getStylesheetFiles' => 'getStylesheetFiles', + 'Session.beforeSessionStart' => 'beforeSessionStart', ); return $hooks; } @@ -47,6 +48,26 @@ class Login extends \Piwik\Plugin $stylesheetFiles[] = "plugins/Login/stylesheets/variables.less"; } + public function beforeSessionStart() + { + if (!$this->shouldHandleRememberMe()) { + return; + } + + // if this is a login request & form_rememberme was set, change the session cookie expire time before starting the session + $rememberMe = isset($_POST['form_rememberme']) ? $_POST['form_rememberme'] : null; + if ($rememberMe == '1') { + Session::rememberMe(Config::getInstance()->General['login_cookie_expire']); + } + } + + private function shouldHandleRememberMe() + { + $module = Common::getRequestVar('module', false); + $action = Common::getRequestVar('action', false); + return $module == 'Login' && (empty($action) || $action == 'login'); + } + /** * Redirects to Login form with error message. * Listens to User.isNotAuthorized hook. @@ -82,35 +103,12 @@ class Login extends \Piwik\Plugin } /** - * Initializes the authentication object. - * Listens to Request.initAuthenticationObject hook. - */ - function initAuthenticationObject($activateCookieAuth = false) - { - $this->initAuthenticationFromCookie(StaticContainer::getContainer()->get('Piwik\Auth'), $activateCookieAuth); - } - - /** * @param $auth + * @deprecated authenticating via cookie is handled in core by SessionAuth */ public static function initAuthenticationFromCookie(\Piwik\Auth $auth, $activateCookieAuth) { - if (self::isModuleIsAPI() && !$activateCookieAuth) { - return; - } - - $authCookieName = Config::getInstance()->General['login_cookie_name']; - $authCookieExpiry = 0; - $authCookiePath = Config::getInstance()->General['login_cookie_path']; - $authCookie = new Cookie($authCookieName, $authCookieExpiry, $authCookiePath); - $defaultLogin = 'anonymous'; - $defaultTokenAuth = 'anonymous'; - if ($authCookie->isCookieFound()) { - $defaultLogin = $authCookie->get('login'); - $defaultTokenAuth = $authCookie->get('token_auth'); - } - $auth->setLogin($defaultLogin); - $auth->setTokenAuth($defaultTokenAuth); + // empty } } diff --git a/plugins/Login/SessionInitializer.php b/plugins/Login/SessionInitializer.php index 9db9da7dec..e011186ae7 100644 --- a/plugins/Login/SessionInitializer.php +++ b/plugins/Login/SessionInitializer.php @@ -21,15 +21,10 @@ use Piwik\ProxyHttp; use Piwik\Session; /** - * Initializes authenticated sessions using an Auth implementation. - * - * If a user is authenticated, a browser cookie is created so the user will be remembered - * until the cookie is destroyed. - * - * Plugins can override SessionInitializer behavior by extending this class and - * overriding methods. In order for these changes to have effect, however, an instance of - * the derived class must be used by the Login/Controller. + * This SessionInitializer is no longer used, but is kept for backwards compatibility. + * Session management no longer uses the piwik_auth cookie. * + * @deprecated * @api */ class SessionInitializer @@ -192,6 +187,8 @@ class SessionInitializer $cookie->setSecure(ProxyHttp::isHttps()); $cookie->setHttpOnly(true); $cookie->save(); + + return $cookie; } protected function regenerateSessionId() diff --git a/plugins/Login/tests/Integration/LoginTest.php b/plugins/Login/tests/Integration/LoginTest.php index 9a80b2aaf8..9f43512f0b 100644 --- a/plugins/Login/tests/Integration/LoginTest.php +++ b/plugins/Login/tests/Integration/LoginTest.php @@ -261,16 +261,6 @@ class LoginTest extends IntegrationTestCase $this->assertSuperUserLogin($rc, 'user'); } - public function test_authenticate_successLoginAndHashedTokenAuth() - { - $user = $this->_setUpUser(); - $hash = \Piwik\Plugins\Login\SessionInitializer::getHashTokenAuth($user['login'], $user['tokenAuth']); - - // valid login & hashed token auth - $rc = $this->authenticate($user['login'], $tokenAuth = $hash); - $this->assertUserLogin($rc); - } - public function test_authenticate_successWithValidPassword() { $user = $this->_setUpUser(); diff --git a/plugins/Login/tests/Integration/SessionInitializerTest.php b/plugins/Login/tests/Integration/SessionInitializerTest.php new file mode 100644 index 0000000000..9d2f7bb098 --- /dev/null +++ b/plugins/Login/tests/Integration/SessionInitializerTest.php @@ -0,0 +1,154 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + * + */ + +namespace Piwik\Plugins\Login\tests\Integration; + +use Piwik\Auth; +use Piwik\AuthResult; +use Piwik\Container\StaticContainer; +use Piwik\Cookie; +use Piwik\Plugins\Login\SessionInitializer; +use Piwik\Tests\Framework\TestCase\IntegrationTestCase; + +/** + * Since the original SessionInitializer is still in use, it needs to + * work. These light tests ensure it's still working. + */ +class SessionInitializerTest extends IntegrationTestCase +{ + public function setUp() + { + parent::setUp(); + + // AuthResult is in Auth.php, so we need to make sure that class gets loaded + // by loading Auth. + StaticContainer::get(Auth::class); + } + + public function test_initSession_CreatesCookie_WhenAuthenticationIsSuccessful() + { + $this->assertAuthCookieIsAbsent(); + + $sessionInitializer = new TestSessionInitializer(); + $sessionInitializer->initSession($this->makeMockAuth(AuthResult::SUCCESS), true); + + $this->assertAuthCookieIsCreated($sessionInitializer->cookie); + } + + public function test_initSession_DeletesCookie_WhenAuthenticationFailed() + { + $this->createAuthCookie(); + + try { + $sessionInitializer = new TestSessionInitializer(); + $sessionInitializer->initSession($this->makeMockAuth(AuthResult::FAILURE), true); + + $this->fail('Expected exception to be thrown.'); + } catch (\Exception $ex) { + // empty + } + + $this->assertAuthCookieIsDeleted($sessionInitializer->cookie); + } + + private function makeMockAuth($resultCode) + { + return new MockAuth($resultCode); + } + + private function assertAuthCookieIsAbsent() + { + $this->assertArrayNotHasKey('piwik_auth', $_COOKIE); + } + + private function assertAuthCookieIsCreated(Cookie $cookie) + { + $this->assertContains('login=czo5OiJ0ZXN0bG9naW4iOw==:token_auth=czozMjoiOWU5MDYxZjk2MDI0YTY3NWFmOGFkNWZmNmNiZGY2ZGMiOw==', + $cookie->generateContentString()); + } + + private function createAuthCookie() + { + $_COOKIE['piwik_auth'] = 'login=czo5OiJ0ZXN0bG9naW4iOw==:token_auth=czozMjoiOWU5MDYxZjk2MDI0YTY3NWFmOGFkNWZmNmNiZGY2ZGMiOw=='; + } + + private function assertAuthCookieIsDeleted(Cookie $cookie) + { + $this->assertEquals('', $cookie->generateContentString()); + } +} + +class TestSessionInitializer extends SessionInitializer +{ + /** + * @var Cookie + */ + public $cookie; + + protected function regenerateSessionId() + { + // empty + } + + protected function getAuthCookie($rememberMe) + { + $this->cookie = parent::getAuthCookie($rememberMe); + return $this->cookie; + } +} + +class MockAuth implements Auth +{ + private $result; + + public function __construct($resultCode) + { + $this->result = new AuthResult($resultCode, 'testlogin', 'dummytokenauth'); + } + + public function getName() + { + // empty + } + + public function setTokenAuth($token_auth) + { + // empty + } + + public function getLogin() + { + // empty + } + + public function getTokenAuthSecret() + { + // empty + } + + public function setLogin($login) + { + // empty + } + + public function setPassword($password) + { + // empty + } + + public function setPasswordHash($passwordHash) + { + // empty + } + + public function authenticate() + { + return $this->result; + } +}
\ No newline at end of file diff --git a/plugins/Overlay/API.php b/plugins/Overlay/API.php index 22f11eba95..0d9ae9cdcb 100644 --- a/plugins/Overlay/API.php +++ b/plugins/Overlay/API.php @@ -9,11 +9,8 @@ namespace Piwik\Plugins\Overlay; use Exception; -use Piwik\Access; use Piwik\Config; -use Piwik\Container\StaticContainer; use Piwik\DataTable; -use Piwik\Piwik; use Piwik\Plugins\SitesManager\API as APISitesManager; use Piwik\Plugins\SitesManager\SitesManager; use Piwik\Plugins\Transitions\API as APITransitions; @@ -30,8 +27,6 @@ class API extends \Piwik\Plugin\API */ public function getTranslations($idSite) { - $this->authenticate($idSite); - $translations = array( 'oneClick' => 'Overlay_OneClick', 'clicks' => 'Overlay_Clicks', @@ -48,8 +43,6 @@ class API extends \Piwik\Plugin\API */ public function getExcludedQueryParameters($idSite) { - $this->authenticate($idSite); - $sitesManager = APISitesManager::getInstance(); $site = $sitesManager->getSiteFromId($idSite); @@ -71,8 +64,6 @@ class API extends \Piwik\Plugin\API */ public function getFollowingPages($url, $idSite, $period, $date, $segment = false) { - $this->authenticate($idSite); - $url = PageUrl::excludeQueryParametersFromUrl($url, $idSite); // we don't unsanitize $url here. it will be done in the Transitions plugin. @@ -100,42 +91,4 @@ class API extends \Piwik\Plugin\API return $resultDataTable; } - - /** Do cookie authentication. This way, the token can remain secret. */ - private function authenticate($idSite) - { - if (Piwik::isUserHasViewAccess($idSite)) { - return; // user is already authenticated (e.g. API call with token_auth) - } - - /** - * Triggered immediately before the user is authenticated. - * - * This event can be used by plugins that provide their own authentication mechanism - * to make that mechanism available. Subscribers should set the `'Piwik\Auth'` object in - * the container to an object that implements the {@link Piwik\Auth} interface. - * - * **Example** - * - * use Piwik\Container\StaticContainer; - * - * public function initAuthenticationObject($activateCookieAuth) - * { - * StaticContainer::getContainer()->set('Piwik\Auth', new LDAPAuth($activateCookieAuth)); - * } - * - * @param bool $activateCookieAuth Whether authentication based on `$_COOKIE` values should - * be allowed. - */ - Piwik::postEvent('Request.initAuthenticationObject', array($activateCookieAuth = true)); - - $auth = StaticContainer::get('Piwik\Auth'); - $success = Access::getInstance()->reloadAccess($auth); - - if (!$success) { - throw new Exception('Authentication failed'); - } - - Piwik::checkUserHasViewAccess($idSite); - } } diff --git a/plugins/UsersManager/Controller.php b/plugins/UsersManager/Controller.php index 0be04abda4..6b59d05437 100644 --- a/plugins/UsersManager/Controller.php +++ b/plugins/UsersManager/Controller.php @@ -19,7 +19,6 @@ use Piwik\Piwik; use Piwik\Plugin\ControllerAdmin; use Piwik\Plugins\LanguagesManager\API as APILanguagesManager; use Piwik\Plugins\LanguagesManager\LanguagesManager; -use Piwik\Plugins\Login\SessionInitializer; use Piwik\Plugins\UsersManager\API as APIUsersManager; use Piwik\SettingsPiwik; use Piwik\Site; @@ -27,6 +26,7 @@ use Piwik\Tracker\IgnoreCookie; use Piwik\Translation\Translator; use Piwik\Url; use Piwik\View; +use Piwik\Session\SessionInitializer; class Controller extends ControllerAdmin { diff --git a/plugins/UsersManager/Model.php b/plugins/UsersManager/Model.php index d33c808c48..f986333bc0 100644 --- a/plugins/UsersManager/Model.php +++ b/plugins/UsersManager/Model.php @@ -10,6 +10,7 @@ namespace Piwik\Plugins\UsersManager; use Piwik\Auth\Password; use Piwik\Common; +use Piwik\Date; use Piwik\Db; use Piwik\Piwik; @@ -199,7 +200,8 @@ class Model 'email' => $email, 'token_auth' => $tokenAuth, 'date_registered' => $dateRegistered, - 'superuser_access' => 0 + 'superuser_access' => 0, + 'ts_password_modified' => Date::now()->getDatetime(), ); $db = $this->getDb(); @@ -223,6 +225,11 @@ class Model $bind[] = $val; } + if (!empty($fields['password'])) { + $set[] = "ts_password_modified = ?"; + $bind[] = Date::now()->getDatetime(); + } + $bind[] = $userLogin; $db = $this->getDb(); diff --git a/plugins/UsersManager/tests/Integration/UsersManagerTest.php b/plugins/UsersManager/tests/Integration/UsersManagerTest.php index 790387ce8b..8bc80b436a 100644 --- a/plugins/UsersManager/tests/Integration/UsersManagerTest.php +++ b/plugins/UsersManager/tests/Integration/UsersManagerTest.php @@ -26,6 +26,8 @@ use Exception; */ class UsersManagerTest extends IntegrationTestCase { + const DATETIME_REGEX = '/\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}/'; + /** * @var API */ @@ -74,8 +76,20 @@ class UsersManagerTest extends IntegrationTestCase if (is_null($newAlias)) { $newAlias = $user['alias']; } + $userAfter = $this->api->getUser($user["login"]); + + $this->assertArrayHasKey('date_registered', $userAfter); + $this->assertRegExp(self::DATETIME_REGEX, $userAfter['date_registered']); + + $this->assertArrayHasKey('ts_password_modified', $userAfter); + $this->assertRegExp(self::DATETIME_REGEX, $userAfter['date_registered']); + + $this->assertArrayHasKey('password', $userAfter); + $this->assertNotEmpty($userAfter['password']); + unset($userAfter['date_registered']); + unset($userAfter['ts_password_modified']); unset($userAfter['password']); // implicitly checks password! @@ -425,6 +439,7 @@ class UsersManagerTest extends IntegrationTestCase unset($user['password']); unset($user['token_auth']); unset($user['date_registered']); + unset($user['ts_password_modified']); } return $users; } diff --git a/plugins/UsersManager/tests/System/ApiTest.php b/plugins/UsersManager/tests/System/ApiTest.php index 4192b62d0c..8c1baaec80 100644 --- a/plugins/UsersManager/tests/System/ApiTest.php +++ b/plugins/UsersManager/tests/System/ApiTest.php @@ -38,7 +38,7 @@ class ApiTest extends SystemTestCase // login1 = super user, login2 = some admin access, login4 = only view access foreach ($logins as $login => $appendix) { $params['token_auth'] = self::$fixture->users[$login]['token']; - $xmlFieldsToRemove = array('date_registered', 'password', 'token_auth'); + $xmlFieldsToRemove = array('date_registered', 'password', 'token_auth', 'ts_password_modified'); $this->runAnyApiTest($api, $apiId . '_' . $appendix, $params, array('xmlFieldsToRemove' => $xmlFieldsToRemove)); } diff --git a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login1_when_superuseraccess.xml b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login1_when_superuseraccess.xml index e233776f69..4b1a43a0ee 100644 --- a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login1_when_superuseraccess.xml +++ b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login1_when_superuseraccess.xml @@ -7,5 +7,6 @@ <email>login1@example.com</email> <superuser_access>1</superuser_access> + </row> </result>
\ No newline at end of file diff --git a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login2_when_adminaccess.xml b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login2_when_adminaccess.xml index 7b9a2b37ca..b394489b94 100644 --- a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login2_when_adminaccess.xml +++ b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login2_when_adminaccess.xml @@ -7,5 +7,6 @@ <email>login2@example.com</email> <superuser_access>0</superuser_access> + </row> </result>
\ No newline at end of file diff --git a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login2_when_superuseraccess.xml b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login2_when_superuseraccess.xml index 7b9a2b37ca..b394489b94 100644 --- a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login2_when_superuseraccess.xml +++ b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login2_when_superuseraccess.xml @@ -7,5 +7,6 @@ <email>login2@example.com</email> <superuser_access>0</superuser_access> + </row> </result>
\ No newline at end of file diff --git a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login4_when_superuseraccess.xml b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login4_when_superuseraccess.xml index 28d1197732..caa6b7267e 100644 --- a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login4_when_superuseraccess.xml +++ b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login4_when_superuseraccess.xml @@ -7,5 +7,6 @@ <email>login4@example.com</email> <superuser_access>0</superuser_access> + </row> </result>
\ No newline at end of file diff --git a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login4_when_viewaccess.xml b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login4_when_viewaccess.xml index 28d1197732..caa6b7267e 100644 --- a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login4_when_viewaccess.xml +++ b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login4_when_viewaccess.xml @@ -7,5 +7,6 @@ <email>login4@example.com</email> <superuser_access>0</superuser_access> + </row> </result>
\ No newline at end of file diff --git a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login6_when_superuseraccess.xml b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login6_when_superuseraccess.xml index ebf69209da..beeca3f033 100644 --- a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login6_when_superuseraccess.xml +++ b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUser_login6_when_superuseraccess.xml @@ -7,5 +7,6 @@ <email>login6@example.com</email> <superuser_access>0</superuser_access> + </row> </result>
\ No newline at end of file diff --git a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUsersWithSiteAccess_3_admin_when_superuseraccess.xml b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUsersWithSiteAccess_3_admin_when_superuseraccess.xml index aa93d367ad..df237acece 100644 --- a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUsersWithSiteAccess_3_admin_when_superuseraccess.xml +++ b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUsersWithSiteAccess_3_admin_when_superuseraccess.xml @@ -7,6 +7,7 @@ <email>login5@example.com</email> <superuser_access>0</superuser_access> + </row> <row> <login>login6</login> @@ -15,5 +16,6 @@ <email>login6@example.com</email> <superuser_access>0</superuser_access> + </row> </result>
\ No newline at end of file diff --git a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUsers__when_superuseraccess.xml b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUsers__when_superuseraccess.xml index e8eb10f4b1..35351a9999 100644 --- a/plugins/UsersManager/tests/System/expected/test___UsersManager.getUsers__when_superuseraccess.xml +++ b/plugins/UsersManager/tests/System/expected/test___UsersManager.getUsers__when_superuseraccess.xml @@ -7,6 +7,7 @@ <email>login1@example.com</email> <superuser_access>1</superuser_access> + </row> <row> <login>login2</login> @@ -15,6 +16,7 @@ <email>login2@example.com</email> <superuser_access>0</superuser_access> + </row> <row> <login>login3</login> @@ -23,6 +25,7 @@ <email>login3@example.com</email> <superuser_access>0</superuser_access> + </row> <row> <login>login4</login> @@ -31,6 +34,7 @@ <email>login4@example.com</email> <superuser_access>0</superuser_access> + </row> <row> <login>login5</login> @@ -39,6 +43,7 @@ <email>login5@example.com</email> <superuser_access>0</superuser_access> + </row> <row> <login>login6</login> @@ -47,6 +52,7 @@ <email>login6@example.com</email> <superuser_access>0</superuser_access> + </row> <row> <login>login7</login> @@ -55,6 +61,7 @@ <email>login7@example.com</email> <superuser_access>0</superuser_access> + </row> <row> <login>login8</login> @@ -63,6 +70,7 @@ <email>login8@example.com</email> <superuser_access>0</superuser_access> + </row> <row> <login>superUserLogin</login> @@ -71,5 +79,6 @@ <email>hello@example.org</email> <superuser_access>1</superuser_access> + </row> </result>
\ No newline at end of file diff --git a/tests/PHPUnit/Integration/Session/SessionAuthTest.php b/tests/PHPUnit/Integration/Session/SessionAuthTest.php new file mode 100644 index 0000000000..cc4790b7b4 --- /dev/null +++ b/tests/PHPUnit/Integration/Session/SessionAuthTest.php @@ -0,0 +1,171 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + * + */ + +namespace Piwik\Tests\Integration\Session; + +use Piwik\AuthResult; +use Piwik\Container\StaticContainer; +use Piwik\Session\SessionAuth; +use Piwik\Session\SessionFingerprint; +use Piwik\Tests\Framework\Fixture; +use Piwik\Tests\Framework\TestCase\IntegrationTestCase; +use Piwik\Plugins\UsersManager\API as UsersManagerAPI; +use Piwik\Plugins\UsersManager\Model as UsersModel; + +class SessionAuthTest extends IntegrationTestCase +{ + const TEST_UA = 'test-user-agent'; + const TEST_OTHER_USER = 'testuser'; + + /** + * @var SessionAuth + */ + private $testInstance; + + public function setUp() + { + parent::setUp(); + + UsersManagerAPI::getInstance()->addUser(self::TEST_OTHER_USER, 'testpass', 'test@example.com'); + + $this->testInstance = StaticContainer::get(SessionAuth::class); + } + + public function test_authenticate_ReturnsFailure_IfRequestUserAgentDiffersFromSessionUserAgent() + { + $this->initializeSession(self::TEST_UA, Fixture::ADMIN_USER_LOGIN); + $this->initializeRequest('some-other-user-agent'); + + $result = $this->testInstance->authenticate(); + $this->assertEquals(AuthResult::FAILURE, $result->getCode()); + } + + public function test_authenticate_ReturnsSuccess_IfRequestUserAgentMatchSession() + { + $this->initializeSession(self::TEST_UA, self::TEST_OTHER_USER); + $this->initializeRequest(self::TEST_UA); + + $result = $this->testInstance->authenticate(); + $this->assertEquals(AuthResult::SUCCESS, $result->getCode()); + } + + public function test_authenticate_ReturnsSuperUserSuccess_IfRequestUserAgentMatchSession() + { + $this->initializeSession(self::TEST_UA, Fixture::ADMIN_USER_LOGIN); + $this->initializeRequest(self::TEST_UA); + + $result = $this->testInstance->authenticate(); + $this->assertEquals(AuthResult::SUCCESS_SUPERUSER_AUTH_CODE, $result->getCode()); + } + + public function test_authenticate_ReturnsFailure_IfNoSessionExists() + { + $this->initializeSession(self::TEST_UA, Fixture::ADMIN_USER_LOGIN); + $this->initializeRequest(self::TEST_UA); + + $this->destroySession(); + + $result = $this->testInstance->authenticate(); + $this->assertEquals(AuthResult::FAILURE, $result->getCode()); + } + + public function test_authenticate_ReturnsFailure_IfAuthenticatedSession_AndPasswordChangedAfterSessionCreated() + { + $this->initializeSession(self::TEST_UA, self::TEST_OTHER_USER); + $this->initializeRequest(self::TEST_UA); + + sleep(1); + + UsersManagerAPI::getInstance()->updateUser(self::TEST_OTHER_USER, 'testpass2'); + + $result = $this->testInstance->authenticate(); + $this->assertEquals(AuthResult::FAILURE, $result->getCode()); + + $this->assertEmpty($_SESSION); + } + + public function test_authenticate_ReturnsFailure_IfUsersModelReturnsIncorrectUser() + { + $this->initializeSession(self::TEST_UA, self::TEST_OTHER_USER); + $this->initializeRequest(self::TEST_UA); + + $sessionAuth = new SessionAuth(new MockUsersModel([ + 'login' => 'wronguser', + ])); + $result = $sessionAuth->authenticate(); + + $this->assertEquals(AuthResult::FAILURE, $result->getCode()); + } + + public function test_authenticate_ReturnsSuccess_IfUserDataHasNoPasswordModifiedTimestamp() + { + $this->initializeSession(self::TEST_UA, self::TEST_OTHER_USER); + $this->initializeRequest(self::TEST_UA); + + $usersModel = new UsersModel(); + $user = $usersModel->getUser(self::TEST_OTHER_USER); + unset($user['ts_password_modified']); + + $sessionAuth = new SessionAuth(new MockUsersModel($user)); + $result = $sessionAuth->authenticate(); + + $this->assertEquals(AuthResult::SUCCESS, $result->getCode()); + } + + private function initializeRequest($userAgent) + { + $_SERVER['HTTP_USER_AGENT'] = $userAgent; + } + + private function initializeSession($userAgent, $userLogin) + { + $sessionFingerprint = new SessionFingerprint(); + $sessionFingerprint->initialize($userLogin, $time = null, $userAgent); + } + + protected static function configureFixture($fixture) + { + parent::configureFixture($fixture); + + $fixture->createSuperUser = true; + } + + private function destroySession() + { + unset($_SESSION[SessionFingerprint::SESSION_INFO_SESSION_VAR_NAME]); + unset($_SESSION[SessionFingerprint::USER_NAME_SESSION_VAR_NAME]); + } + + public function provideContainerConfig() + { + return [ + SessionAuth::class => \DI\object() + ->constructorParameter('shouldDestroySession', false), + ]; + } +} + +class MockUsersModel extends UsersModel +{ + /** + * @var array + */ + private $userData; + + public function __construct(array $userData) + { + parent::__construct(); + $this->userData = $userData; + } + + public function getUser($userLogin) + { + return $this->userData; + } +}
\ No newline at end of file diff --git a/tests/PHPUnit/Unit/CookieTest.php b/tests/PHPUnit/Unit/CookieTest.php index 52adec4d0e..e09cb1f4a4 100644 --- a/tests/PHPUnit/Unit/CookieTest.php +++ b/tests/PHPUnit/Unit/CookieTest.php @@ -8,6 +8,8 @@ namespace Piwik\Tests\Unit; +use Piwik\Cookie; + class CookieTest extends \PHPUnit_Framework_TestCase { /** @@ -152,4 +154,15 @@ class CookieTest extends \PHPUnit_Framework_TestCase $a = 'a:2:{i:0;a:0:{}O:28:"Test_Piwik_Cookie_Mock_Class":0:{}s:4:"test";'; $this->assertFalse(safe_unserialize($a), "test: unserializing with illegal key"); } + + public function test_isCookieInRequest_ReturnsTrueIfCookieExists() + { + $_COOKIE['abc'] = 'value'; + $this->assertTrue(Cookie::isCookieInRequest('abc')); + } + + public function test_isCookieInRequest_ReturnsFalseIfCookieExists() + { + $this->assertFalse(Cookie::isCookieInRequest('abc')); + } } diff --git a/tests/PHPUnit/Unit/Session/SessionFingerprintTest.php b/tests/PHPUnit/Unit/Session/SessionFingerprintTest.php new file mode 100644 index 0000000000..047fdd80df --- /dev/null +++ b/tests/PHPUnit/Unit/Session/SessionFingerprintTest.php @@ -0,0 +1,153 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + * + */ + +namespace Piwik\Tests\Unit\Session; + + +use Piwik\Session\SessionFingerprint; + +class SessionFingerprintTest extends \PHPUnit_Framework_TestCase +{ + const TEST_TIME_VALUE = 4567; + + /** + * @var SessionFingerprint + */ + private $testInstance; + + public function setUp() + { + parent::setUp(); + + $this->testInstance = new SessionFingerprint(); + } + + public function test_getUser_ReturnsUserNameSessionVar_WhenSessionVarIsSet() + { + $_SESSION[SessionFingerprint::USER_NAME_SESSION_VAR_NAME] = 'testuser'; + $this->assertEquals('testuser', $this->testInstance->getUser()); + } + + public function test_getUser_ReturnsNull_WhenSessionVarIsNotSet() + { + $this->assertNull($this->testInstance->getUser()); + } + + public function test_getUserInfo_ReturnsUserInfoSessionVar_WhenSessionVarIsSet() + { + $sessionVarValue = [ + 'ip' => 'someip', + 'ua' => 'someua', + ]; + + $_SESSION[SessionFingerprint::SESSION_INFO_SESSION_VAR_NAME] = $sessionVarValue; + $this->assertEquals($sessionVarValue, $this->testInstance->getUserInfo()); + } + + public function test_getUserInfo_ReturnsNull_WhenSessionVarIsNotSet() + { + $this->assertNull($this->testInstance->getUserInfo()); + } + + public function test_initialize_SetsSessionVarsToCurrentRequest() + { + $_SERVER['HTTP_USER_AGENT'] = 'test-user-agent'; + $this->testInstance->initialize('testuser', self::TEST_TIME_VALUE); + + $this->assertEquals('testuser', $_SESSION[SessionFingerprint::USER_NAME_SESSION_VAR_NAME]); + $this->assertEquals( + ['ts' => self::TEST_TIME_VALUE, 'ua' => 'test-user-agent'], + $_SESSION[SessionFingerprint::SESSION_INFO_SESSION_VAR_NAME] + ); + } + + public function test_initialize_DoesNotSetUserAgent_IfUserAgentIsNotInHttpRequest() + { + unset($_SERVER['HTTP_USER_AGENT']); + $this->testInstance->initialize('testuser', self::TEST_TIME_VALUE); + + $this->assertEquals('testuser', $_SESSION[SessionFingerprint::USER_NAME_SESSION_VAR_NAME]); + $this->assertEquals( + ['ts' => self::TEST_TIME_VALUE, 'ua' => null], + $_SESSION[SessionFingerprint::SESSION_INFO_SESSION_VAR_NAME] + ); + } + + /** + * @dataProvider getTestDataForIsMatchingCurrentRequest + */ + public function test_isMatchingCurrentRequest_ChecksIfSessionVarsMatchRequest( + $sessionUa, $requestUa, $expectedResult + ) { + $_SESSION[SessionFingerprint::SESSION_INFO_SESSION_VAR_NAME] = [ + 'ua' => $sessionUa, + ]; + + $_SERVER['HTTP_USER_AGENT'] = $requestUa; + + $this->assertEquals($expectedResult, $this->testInstance->isMatchingCurrentRequest()); + } + + public function getTestDataForIsMatchingCurrentRequest() + { + return [ + ['test ua', 'test ua', true], + ['nontest ua', 'test ua', false], + [null, 'test ua', false], + ]; + } + + public function test_isMatchingCurrentRequest_ReturnsFalse_IfUserInfoSessionVarDoesNotExist() + { + $_SERVER['HTTP_USER_AGENT'] = 'test-ua'; + + $this->assertEquals(false, $this->testInstance->isMatchingCurrentRequest()); + } + + public function test_isMatchingCurrentRequest_ReturnsFalse_IfRequestDetailsDoNotExist() + { + $_SESSION[SessionFingerprint::SESSION_INFO_SESSION_VAR_NAME] = [ + 'ua' => 'test-ua', + ]; + + $this->assertEquals(false, $this->testInstance->isMatchingCurrentRequest()); + } + + public function test_getSessionStartTime_() + { + $_SESSION[SessionFingerprint::SESSION_INFO_SESSION_VAR_NAME] = [ + 'ts' => 123. + ]; + $this->assertEquals(123, $this->testInstance->getSessionStartTime()); + } + + public function test_getSessionStartTime_ReturnsNull_IfThereIsNoSessionInfo() + { + $this->assertNull($this->testInstance->getSessionStartTime()); + } + + public function test_getSessionStartTime_ReturnsNull_IfThereIsNoSessionStartTime() + { + $_SESSION[SessionFingerprint::SESSION_INFO_SESSION_VAR_NAME] = []; + $this->assertNull($this->testInstance->getSessionStartTime()); + } + + public function test_destroy_RemovesSessionFingerprintSessionVars() + { + $_SESSION['someotherdata'] = 'somedata'; + $_SESSION[SessionFingerprint::USER_NAME_SESSION_VAR_NAME] = 'someuser'; + $_SESSION[SessionFingerprint::SESSION_INFO_SESSION_VAR_NAME] = [ + 'some' => 'data', + ]; + + $this->testInstance->clear(); + + $this->assertEquals(['someotherdata' => 'somedata'], $_SESSION); + } +} diff --git a/tests/PHPUnit/Unit/Session/SessionInitializerTest.php b/tests/PHPUnit/Unit/Session/SessionInitializerTest.php new file mode 100644 index 0000000000..7b3c2a206a --- /dev/null +++ b/tests/PHPUnit/Unit/Session/SessionInitializerTest.php @@ -0,0 +1,135 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + * + */ + +namespace Piwik\Tests\Unit\Session; + +use Piwik\Auth; +use Piwik\AuthResult; +use Piwik\Session\SessionFingerprint; +use Piwik\Session\SessionInitializer; + +class SessionInitializerTest extends \PHPUnit_Framework_TestCase +{ + private $oldUnitTestValue; + + public function setUp() + { + parent::setUp(); + + $this->oldUnitTestValue = \Zend_Session::$_unitTestEnabled; + \Zend_Session::$_unitTestEnabled = true; + } + + public function tearDown() + { + \Zend_Session::$_unitTestEnabled = $this->oldUnitTestValue; + + parent::tearDown(); + } + + /** + * @dataProvider getTestDataForInitSession + * @expectedExceptionMessage Login_LoginPasswordNotCorrect + */ + public function test_initSession_Throws_IfAuthenticationFailed($rememberMe) + { + $sessionInitializer = new TestSessionInitializer(); + $sessionInitializer->initSession($this->makeMockAuth(AuthResult::SUCCESS), $rememberMe); + } + + /** + * @dataProvider getTestDataForInitSession + */ + public function test_initSession_InitializesTheSessionCorrectly_IfAuthenticationSucceeds($rememberMe) + { + $sessionInitializer = new TestSessionInitializer(); + $sessionInitializer->initSession($this->makeMockAuth(AuthResult::SUCCESS), $rememberMe); + + $this->assertSessionCreatedCorrectly(); + } + + public function getTestDataForInitSession() + { + return [ + [true], + [false], + ]; + } + + private function makeMockAuth($resultCode) + { + return new MockAuth($resultCode); + } + + private function assertSessionCreatedCorrectly() + { + $fingerprint = new SessionFingerprint(); + $this->assertEquals('testlogin', $fingerprint->getUser()); + $this->assertNotEmpty($fingerprint->getSessionStartTime()); + $this->assertEquals(['ts', 'ua'], array_keys($fingerprint->getUserInfo())); + } +} + +class TestSessionInitializer extends SessionInitializer +{ + protected function regenerateSessionId() + { + // empty + } +} + +class MockAuth implements Auth +{ + private $result; + + public function __construct($resultCode) + { + $this->result = new AuthResult($resultCode, 'testlogin', 'dummytokenauth'); + } + + public function getName() + { + // empty + } + + public function setTokenAuth($token_auth) + { + // empty + } + + public function getLogin() + { + // empty + } + + public function getTokenAuthSecret() + { + // empty + } + + public function setLogin($login) + { + // empty + } + + public function setPassword($password) + { + // empty + } + + public function setPasswordHash($passwordHash) + { + // empty + } + + public function authenticate() + { + return $this->result; + } +}
\ No newline at end of file |