0
0
mirror of https://github.com/postfixadmin/postfixadmin.git synced 2024-09-19 19:22:14 +02:00

refactor Login stuff out of Handler classes into Login... add tests

This commit is contained in:
David Goodwin 2020-09-25 21:32:53 +01:00
parent f091948381
commit b868f950bf
10 changed files with 93 additions and 141 deletions

View File

@ -80,7 +80,9 @@ class AdminpasswordHandler extends PFAHandler {
* check if old password is correct
*/
protected function _validate_oldpass($field, $val) {
if ($this->login($this->id, $val)) {
$l = new Login('admin', 'username');
if($l->login($this->id, $val)) {
return true;
}

View File

@ -736,49 +736,6 @@ class MailboxHandler extends PFAHandler {
return true;
}
/********************************************************************************************************************
old functions - we'll see what happens to them
(at least they should use the *Handler functions instead of doing SQL)
/********************************************************************************************************************/
/**
* @return boolean true on success; false on failure
* @param string $new_password
* @param string $old_password
* @param bool $match = true
*
* All passwords need to be plain text; they'll be hashed appropriately
* as per the configuration in config.inc.php
*/
public function change_pw($new_password, $old_password, $match = true) {
list(/*NULL*/, $domain) = explode('@', $this->id);
if ($match == true) {
if (!$this->login($this->id, $old_password)) {
db_log($domain, 'edit_password', "MATCH FAILURE: " . $this->id);
$this->errormsg[] = Config::Lang('pPassword_password_current_text_error');
return false;
}
}
$set = array(
'password' => pacrypt($new_password) ,
);
$result = db_update('mailbox', 'username', $this->id, $set);
if ($result != 1) {
db_log($domain, 'edit_password', "FAILURE: " . $this->id);
$this->errormsg[] = Config::lang('pEdit_mailbox_result_error');
return false;
}
db_log($domain, 'edit_password', $this->id);
return true;
}
#TODO: more self explaining language strings!
}

View File

@ -822,53 +822,6 @@ abstract class PFAHandler {
return true;
}
/**
* Attempt to log a user in.
* @param string $username
* @param string $password
* @return boolean true on successful login (i.e. password matches etc)
*/
public function login($username, $password) {
$table = table_by_key($this->db_table);
$active = db_get_boolean(true);
$query = "SELECT password FROM $table WHERE {$this->id_field} = :username AND active = :active";
$values = array('username' => $username, 'active' => $active);
$result = db_query_all($query,$values);
if (sizeof($result) == 1) {
$row = $result[0];
$crypt_password = pacrypt($password, $row['password']);
return hash_equals($row['password'], $crypt_password);
}
// try and be near constant time regardless of whether the db user exists or not
$x = pacrypt('abc', 'def');
return hash_equals('not', 'comparable');
}
/**
* Generate and store a unique password reset token valid for one hour
* @param string $username
* @return false|string
*/
public function getPasswordRecoveryCode($username) {
if ($this->init($username)) {
$token = generate_password();
$updatedRows = db_update($this->db_table, $this->id_field, $username, array(
'token' => pacrypt($token),
'token_validity' => date("Y-m-d H:i:s", strtotime('+ 1 hour')),
));
if ($updatedRows == 1) {
return $token;
}
}
return false;
}
/**
* Verify user's one time password reset token
* @param string $username

View File

@ -18,11 +18,8 @@ class VacationHandler extends PFAHandler {
*/
protected $domain_field = 'domain';
/**
* @return void
*/
public function init($id) {
die('VacationHandler is not yet ready to be used with *Handler methods'); # obvious TODO: remove when it's ready ;-)
public function init($id) : bool {
throw new \Exception('VacationHandler is not yet ready to be used with *Handler methods');
}
/**

View File

@ -55,7 +55,10 @@ if ($_SERVER['REQUEST_METHOD'] == "POST") {
}
$h = new AdminHandler();
if ($h->login($fUsername, $fPassword)) {
$login = new Login('admin', $h->getId_field());
if($login->login($fUsername, $fPassword)) {
init_session($fUsername, true);
# they've logged in, so see if they are a domain admin, as well.

View File

@ -47,9 +47,10 @@ if ($_SERVER['REQUEST_METHOD'] == "POST") {
}
$h = new MailboxHandler();
if ($h->login($fUsername, $fPassword)) {
init_session($fUsername, false);
$login = new Login('mailbox', 'username');;
if ($login->login($fUsername, $fPassword)) {
init_session($fUsername, false);
header("Location: main.php");
exit;
} else {

View File

@ -38,7 +38,8 @@ if ($context === 'admin' && !Config::read('forgotten_admin_password_reset') || $
die('Password reset is disabled by configuration option: forgotten_admin_password_reset');
}
function sendCodebyEmail($to, $username, $code) {
function sendCodebyEmail($to, $username, $code)
{
$https = isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on' ? 'https' : 'http';
$_SERVER['REQUEST_SCHEME'] = isset($_SERVER['REQUEST_SCHEME']) ? $_SERVER['REQUEST_SCHEME'] : $https;
@ -48,7 +49,8 @@ function sendCodebyEmail($to, $username, $code) {
return smtp_mail($to, Config::read('admin_email'), Config::Lang('pPassword_welcome'), Config::read('admin_smtp_password'), Config::lang_f('pPassword_recovery_email_body', $url));
}
function sendCodebySMS($to, $username, $code) {
function sendCodebySMS($to, $username, $code)
{
$text = Config::lang_f('pPassword_recovery_sms_body', $code);
$function = Config::read('sms_send_function');
@ -69,8 +71,13 @@ if ($_SERVER['REQUEST_METHOD'] === "POST") {
$tUsername = escape_string($username);
$table = $context === 'admin' ? 'admin' : 'mailbox';
$login = new Login($table, 'username');
$token = $login->generatePasswordRecoveryCode($tUsername);
$handler = $context === 'admin' ? new AdminHandler : new MailboxHandler;
$token = $handler->getPasswordRecoveryCode($tUsername);
if ($token !== false) {
$table = table_by_key($context === 'users' ? 'mailbox' : 'admin');
$row = db_query_one("SELECT * FROM $table WHERE username= :username", array('username' => $username));

View File

@ -56,26 +56,33 @@ if ($_SERVER['REQUEST_METHOD'] == "POST") {
flash_error($validpass[0]); # TODO: honor all error messages, not only the first one
$error += 1;
}
$mh = new MailboxHandler;
if (!$mh->login($username, $fPassword_current)) {
$mh = new MailboxHandler();
$login = new Login('mailbox', 'username');
if (!$login->login($username, $fPassword_current)) {
$error += 1;
$pPassword_password_current_text = $PALANG['pPassword_password_current_text_error'];
}
if (empty($fPassword) or ($fPassword != $fPassword2)) {
$error += 1;
$pPassword_password_text = $PALANG['pPassword_password_text_error'];
}
if ($error == 0) {
$mh->init($username); # TODO: error handling
if ($mh->change_pw($fPassword, $fPassword_current)) {
flash_info(Config::Lang_f('pPassword_result_success', $username));
header("Location: main.php");
exit(0);
} else {
flash_error(Config::Lang_f('pPassword_result_error', $username));
try {
if ($login->changePassword($username, $fPassword, $fPassword_current)) {
flash_info(Config::Lang_f('pPassword_result_success', $username));
header("Location: main.php");
exit(0);
} else {
flash_error(Config::Lang_f('pPassword_result_error', $username));
}
} catch (\Exception $e) {
flash_error($e->getMessage());
}
}
}

View File

@ -44,9 +44,11 @@ $server = new Zend_XmlRpc_Server();
* @param string $password
* @return boolean true on success, else false.
*/
function login($username, $password) {
function login($username, $password)
{
$h = new MailboxHandler();
if ($h->login($username, $password)) {
$login = new Login('mailbox', 'username');
if ($login->login($username, $password)) {
session_regenerate_id();
$_SESSION['authenticated'] = true;
$_SESSION['sessid'] = array();
@ -66,37 +68,56 @@ if (!isset($_SESSION['authenticated'])) {
echo $server->handle();
class UserProxy {
class UserProxy
{
/**
* @param string $old_password
* @param string $new_password
* @return boolean true on success
*/
public function changePassword($old_password, $new_password) {
public function changePassword($old_password, $new_password)
{
$uh = new MailboxHandler();
if (!$uh->init($_SESSION['sessid']['username'])) {
$username = $_SESSION['sessid']['username'] ?? '';
if (empty($username)) {
throw new \Exception("not logged in? invalid session");
}
if (!$uh->init($username)) {
return false; // user doesn't exist.
}
$login = new Login('mailbox', 'username');
try {
return $login->changePassword($username, $new_password, $old_password);
} catch (\Exception $e) {
return false;
}
return $uh->change_pw($new_password, $old_password);
}
/**
* @param string $username
* @param string $password
* @return boolean true if successful.
*/
public function login($username, $password) {
$uh = new MailboxHandler(); # $_SESSION['sessid']['username']);
return $uh->login($username, $password);
* @param string $username
* @param string $password
* @return boolean true if successful.
*/
public function login($username, $password)
{
$login = new Login('mailbox', 'username');
return $login->login($username, $password);
}
}
class VacationProxy {
class VacationProxy
{
/**
* @return boolean true if the vacation is removed successfully. Else false.
*/
public function remove() {
public function remove()
{
$vh = new VacationHandler($_SESSION['sessid']['username']);
return $vh->remove();
}
@ -105,7 +126,8 @@ class VacationProxy {
* @return boolean true if vacation stuff is enabled in this instance of postfixadmin
* and the user has the ability to make changes to it.
*/
public function isVacationSupported() {
public function isVacationSupported()
{
$vh = new VacationHandler($_SESSION['sessid']['username']);
return $vh->vacation_supported();
}
@ -113,7 +135,8 @@ class VacationProxy {
/**
* @return boolean true if the user has an active vacation record etc.
*/
public function checkVacation() {
public function checkVacation()
{
$vh = new VacationHandler($_SESSION['sessid']['username']);
return $vh->check_vacation();
}
@ -121,7 +144,8 @@ class VacationProxy {
/**
* @return array|bool - either array of vacation details or boolean false if the user has none.
*/
public function getDetails() {
public function getDetails()
{
$vh = new VacationHandler($_SESSION['sessid']['username']);
return $vh->get_details();
}
@ -135,16 +159,20 @@ class VacationProxy {
* @return boolean true on success.
* Whatiis @replyType?? for
*/
public function setAway($subject, $body, $interval_time = 0, $activeFrom = '2000-01-01', $activeUntil = '2099-12-31') {
public function setAway($subject, $body, $interval_time = 0, $activeFrom = '2000-01-01', $activeUntil = '2099-12-31')
{
$vh = new VacationHandler($_SESSION['sessid']['username']);
return $vh->set_away($subject, $body, $interval_time, $activeFrom, $activeUntil);
}
}
class AliasProxy {
class AliasProxy
{
/**
* @return array - array of aliases this user has. Array may be empty.
*/
public function get() {
public function get()
{
$ah = new AliasHandler();
$ah->init($_SESSION['sessid']['username']);
/* I see no point in returning special addresses to the user. */
@ -158,10 +186,11 @@ class AliasProxy {
* @param string flag to set ('forward_and_store' or 'remote_only')
* @return boolean true
*/
public function update($addresses, $flags) {
public function update($addresses, $flags)
{
$ah = new AliasHandler();
$ah->init($_SESSION['sessid']['username']);
$values = ['goto' => $addresses];
if ($flags == 'forward_and_store') {
@ -184,7 +213,8 @@ class AliasProxy {
* @return boolean true if the user has 'store_and_forward' set.
* (i.e. their email address is also in the alias table). IF it returns false, then it's 'remote_only'
*/
public function hasStoreAndForward() {
public function hasStoreAndForward()
{
$ah = new AliasHandler();
$ah->init($_SESSION['sessid']['username']);
$ah->view();

View File

@ -21,11 +21,6 @@ class MailboxHandlerTest extends \PHPUnit\Framework\TestCase {
$this->assertEmpty($results);
$this->assertFalse($x->checkPasswordRecoveryCode('test', 'fake'));
$token = $x->getPasswordRecoveryCode('test.person.does.not.exist@example.com');
$this->assertFalse($token);
}