From 63a66747d2584548204e72d8ccfe15b8ec228ef6 Mon Sep 17 00:00:00 2001
From: Joey3000 <Joey3000@users.noreply.github.com>
Date: Thu, 7 Jan 2016 12:17:10 +0100
Subject: [PATCH] Standardize usage of **password** hash function
As preparation for introduction of a stronger hash function in https://github.com/piwik/piwik/issues/5728, this standardizes the usage of the current **password** hash function across the Login and UsersManager plugins. Specifically:
* Use UsersManager::getPasswordHash() throughout, instead of previous direct use of md5() in some places
* Concentrate the hash length sanity check in the newly created UsersManager::checkPasswordHash(). Previously, UsersManager\Auth and Login\PasswordResetter classes did it separately. Both checks were moved into the newly created UsersManager class function, as that is also where other public static password check functions reside.
* Replaced the "md5" string with "hash" in affected variable names and comments
No functional change is intended.
---
plugins/Login/Auth.php | 21 ++++++++++-----------
plugins/Login/Controller.php | 3 ++-
plugins/Login/PasswordResetter.php | 12 +++++-------
plugins/UsersManager/API.php | 15 ++++++---------
plugins/UsersManager/UsersManager.php | 14 ++++++++++++++
5 files changed, 37 insertions(+), 28 deletions(-)
diff --git a/plugins/Login/Auth.php b/plugins/Login/Auth.php
index b05d31f32e..302c28f7f8 100644
--- a/plugins/Login/Auth.php
+++ b/plugins/Login/Auth.php
@@ -12,13 +12,14 @@ use Exception;
use Piwik\AuthResult;
use Piwik\Db;
use Piwik\Plugins\UsersManager\Model;
+use Piwik\Plugins\UsersManager\UsersManager;
use Piwik\Session;
class Auth implements \Piwik\Auth
{
protected $login;
protected $token_auth;
- protected $md5Password;
+ protected $hashedPassword;
/**
* @var Model
@@ -47,7 +48,7 @@ class Auth implements \Piwik\Auth
*/
public function authenticate()
{
- if (!empty($this->md5Password)) { // favor authenticating by password
+ if (!empty($this->hashedPassword)) { // favor authenticating by password
return $this->authenticateWithPassword($this->login, $this->getTokenAuthSecret());
} elseif (is_null($this->login)) {
return $this->authenticateWithToken($this->token_auth);
@@ -132,7 +133,7 @@ class Auth implements \Piwik\Auth
*/
public function getTokenAuthSecret()
{
- return $this->md5Password;
+ return $this->hashedPassword;
}
/**
@@ -153,9 +154,9 @@ class Auth implements \Piwik\Auth
public function setPassword($password)
{
if (empty($password)) {
- $this->md5Password = null;
+ $this->hashedPassword = null;
} else {
- $this->md5Password = md5($password);
+ $this->hashedPassword = UsersManager::getPasswordHash($password);
}
}
@@ -163,19 +164,17 @@ class Auth implements \Piwik\Auth
* Sets the password hash to use when authentication.
*
* @param string $passwordHash The password hash.
- * @throws Exception if $passwordHash does not have 32 characters in it.
*/
public function setPasswordHash($passwordHash)
{
if ($passwordHash === null) {
- $this->md5Password = null;
+ $this->hashedPassword = null;
return;
}
- if (strlen($passwordHash) != 32) {
- throw new Exception("Invalid hash: incorrect length " . strlen($passwordHash));
- }
+ // check that the password hash is valid (sanity check)
+ UsersManager::checkPasswordHash($passwordHash, Piwik::translate('Login_ExceptionPasswordMD5HashExpected'))
- $this->md5Password = $passwordHash;
+ $this->hashedPassword = $passwordHash;
}
}
diff --git a/plugins/Login/Controller.php b/plugins/Login/Controller.php
index 3e5fe60faa..1b18bcbef6 100644
--- a/plugins/Login/Controller.php
+++ b/plugins/Login/Controller.php
@@ -184,9 +184,10 @@ class Controller extends \Piwik\Plugin\Controller
* Authenticate user and password. Redirect if successful.
*
* @param string $login user name
- * @param string $password md5 password
+ * @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)
diff --git a/plugins/Login/PasswordResetter.php b/plugins/Login/PasswordResetter.php
index d2631abdf9..e2030bf504 100644
--- a/plugins/Login/PasswordResetter.php
+++ b/plugins/Login/PasswordResetter.php
@@ -378,14 +378,12 @@ class PasswordResetter
*
* Derived classes can override this method to provide fewer or more checks.
*
- * @param string $password The password to check.
- * @throws Exception if the password is not 32 bytes long.
+ * @param string $passwordHash The password hash to check.
+ * @throws Exception if the password hash length is incorrect.
*/
- protected function checkPasswordHash($password)
+ protected function checkPasswordHash($passwordHash)
{
- if (strlen($password) != 32) {
- throw new Exception(Piwik::translate('Login_ExceptionPasswordMD5HashExpected'));
- }
+ UsersManager::checkPasswordHash($passwordHash, Piwik::translate('Login_ExceptionPasswordMD5HashExpected'))
}
/**
@@ -477,4 +475,4 @@ class PasswordResetter
{
return $login . '_reset_password_info';
}
-}
\ No newline at end of file
+}
diff --git a/plugins/UsersManager/API.php b/plugins/UsersManager/API.php
index ec900ff65d..95ea28974b 100644
--- a/plugins/UsersManager/API.php
+++ b/plugins/UsersManager/API.php
@@ -342,7 +342,7 @@ class API extends \Piwik\Plugin\API
}
/**
- * Returns the user information (login, password md5, alias, email, date_registered, etc.)
+ * Returns the user information (login, password hash, alias, email, date_registered, etc.)
*
* @param string $userLogin the user login
*
@@ -359,7 +359,7 @@ class API extends \Piwik\Plugin\API
}
/**
- * Returns the user information (login, password md5, alias, email, date_registered, etc.)
+ * Returns the user information (login, password hash, alias, email, date_registered, etc.)
*
* @param string $userEmail the user email
*
@@ -783,16 +783,13 @@ class API extends \Piwik\Plugin\API
* Generates a unique MD5 for the given login & password
*
* @param string $userLogin Login
- * @param string $md5Password MD5ied string of the password
- * @throws Exception
+ * @param string $passwordHash hashed string of the password
* @return string
*/
- public function getTokenAuth($userLogin, $md5Password)
+ public function getTokenAuth($userLogin, $passwordHash)
{
- if (strlen($md5Password) != 32) {
- throw new Exception(Piwik::translate('UsersManager_ExceptionPasswordMD5HashExpected'));
- }
+ UsersManager::checkPasswordHash($passwordHash, Piwik::translate('UsersManager_ExceptionPasswordMD5HashExpected'))
- return md5($userLogin . $md5Password);
+ return md5($userLogin . $passwordHash);
}
}
diff --git a/plugins/UsersManager/UsersManager.php b/plugins/UsersManager/UsersManager.php
index 344faf107a..1ab37c82bf 100644
--- a/plugins/UsersManager/UsersManager.php
+++ b/plugins/UsersManager/UsersManager.php
@@ -157,6 +157,20 @@ class UsersManager extends \Piwik\Plugin
return md5($password);
}
+ /**
+ * Checks the password hash length. Used as a sanity check.
+ *
+ * @param string $passwordHash The password hash to check.
+ * @param string $exceptionMessage Message of the exception thrown.
+ * @throws Exception if the password hash length is incorrect.
+ */
+ public static function checkPasswordHash($passwordHash, $exceptionMessage)
+ {
+ if (strlen($passwordHash) !== strlen(static::getPasswordHash('teststring'))) {
+ throw new Exception($exceptionMessage);
+ }
+ }
+
public function getClientSideTranslationKeys(&$translationKeys)
{
$translationKeys[] = "General_OrCancel";
--
GitLab