diff --git a/model/AdminpasswordHandler.php b/model/AdminpasswordHandler.php index 5799e22e..badd63a3 100644 --- a/model/AdminpasswordHandler.php +++ b/model/AdminpasswordHandler.php @@ -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; } diff --git a/model/MailboxHandler.php b/model/MailboxHandler.php index 596eccd4..9ac6fd45 100644 --- a/model/MailboxHandler.php +++ b/model/MailboxHandler.php @@ -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! } diff --git a/model/PFAHandler.php b/model/PFAHandler.php index f763dde0..74cbae4e 100644 --- a/model/PFAHandler.php +++ b/model/PFAHandler.php @@ -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 diff --git a/model/VacationHandler.php b/model/VacationHandler.php index 1b5bce31..6126daf3 100644 --- a/model/VacationHandler.php +++ b/model/VacationHandler.php @@ -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'); } /** diff --git a/public/login.php b/public/login.php index 39a36330..b2e3a4e7 100644 --- a/public/login.php +++ b/public/login.php @@ -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. diff --git a/public/users/login.php b/public/users/login.php index 59c64fe8..35f34ddc 100644 --- a/public/users/login.php +++ b/public/users/login.php @@ -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 { diff --git a/public/users/password-recover.php b/public/users/password-recover.php index ff5c4c61..687d6b91 100644 --- a/public/users/password-recover.php +++ b/public/users/password-recover.php @@ -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)); diff --git a/public/users/password.php b/public/users/password.php index cd27a6a2..6f57e8cd 100644 --- a/public/users/password.php +++ b/public/users/password.php @@ -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()); } } } diff --git a/public/xmlrpc.php b/public/xmlrpc.php index 035f240c..ce0421a7 100644 --- a/public/xmlrpc.php +++ b/public/xmlrpc.php @@ -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(); diff --git a/tests/MailboxHandlerTest.php b/tests/MailboxHandlerTest.php index 98813e2c..051f161a 100644 --- a/tests/MailboxHandlerTest.php +++ b/tests/MailboxHandlerTest.php @@ -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); }