model/UserHandler.php: proofreading, lots of changes and added TODO comments

- change_pass() now returns the return value of change_pw()
- made most variables un-escaped (as expected by db_insert/db_update/
  db_delete). This means they HAVE TO BE ESCAPED when using them in a
  hand-written query. (Error messages also need escaping - the
  backslashes from escape_string wouldn't help there anyways.)
- escaped variables renamed to $E_whatever
- changed all return values to true (success) and false (failure)
  instead of shell-like 0 (success) and 1 (failure)
- removed unused global $config at various places
- removed timestamp columns from db_insert and db_update call where they
  match the default
- added db_log() for some (but not all) failures
- honor $CONF[maildir_name_hook] when creating a mailbox
- splitting a mail address is now done with list(...) = explode(...)
- switched to db_begin(), db_commit(), db_rollback()
- let smtp_mail create the mail header
- added missing db_commit in delete()
- various minor fixes, code cleanup, comments and TODO notes

To sum it up: all code in UserHandler.php that does not have a TODO
attached should be fine :-)


git-svn-id: https://svn.code.sf.net/p/postfixadmin/code/trunk@908 a1433add-5e2c-0410-b055-b7f2511e0802
pull/2/head
Christian Boltz 14 years ago
parent 900615a5ec
commit 1a0d584bf9

@ -11,59 +11,56 @@ class UserHandler {
public function __construct($username) { public function __construct($username) {
$this->username = strtolower($username); $this->username = strtolower($username);
} }
public function change_pass($old_password, $new_password) { public function change_pass($old_password, $new_password) {
error_log('UserHandler->change_pass is deprecated. Please use UserHandler->change_pw!'); error_log('UserHandler->change_pass is deprecated. Please use UserHandler->change_pw!');
$this->change_pw($new_password, $old_password); return $this->change_pw($new_password, $old_password);
} }
/** /**
* @return boolean true on success; false on failure * @return boolean true on success; false on failure
* @param string $username
* @param string $old_password * @param string $old_password
* @param string $new_passwords * @param string $new_passwords
* @param bool $match = true
* *
* All passwords need to be plain text; they'll be hashed appropriately * All passwords need to be plain text; they'll be hashed appropriately
* as per the configuration in config.inc.php * as per the configuration in config.inc.php
*/ */
public function change_pw($new_password, $old_password, $match = true) { public function change_pw($new_password, $old_password, $match = true) {
global $config;
$username = $this->username; $username = $this->username;
$tmp = preg_split ('/@/', $username); list(/*NULL*/,$domain) = explode('@', $username);
$domain = $tmp[1];
$username = escape_string($username); $username = escape_string($username);
$table_mailbox = table_by_key('mailbox'); $table_mailbox = table_by_key('mailbox');
$new_db_password = escape_string(pacrypt($new_password)); $new_db_password = pacrypt($new_password);
if ($match == true) { if ($match == true) {
$active = db_get_boolean(True); $active = db_get_boolean(True);
$result = db_query("SELECT * FROM $table_mailbox WHERE username='$username' AND active='$active'"); $result = db_query("SELECT * FROM $table_mailbox WHERE username='$username' AND active='$active'");
$result = $result['result']; $result = $result['result'];
if ($new_db_password != $result['password']) { if ($new_db_password != $result['password']) { # TODO: comparison might fail because pacrypt() didn't know the salt above (separate pacrypt call?)
$this->errormsg[] = 'Passwords do not Match'; db_log ('CONSOLE', $domain, 'edit_password', "FAILURE: " . $this->username); # TODO: replace hardcoded CONSOLE - class is used by XMLRPC and users/
return 1; $this->errormsg[] = 'Passwords do not match'; # TODO: make translatable
return false;
} }
} }
$set = array( $set = array(
'password' => $new_db_password 'password' => $new_db_password
); );
$result = db_update('mailbox', 'username=\''.$username.'\'', $set, array('modified') ); $result = db_update('mailbox', 'username=\''.$username.'\'', $set );
db_log ('CONSOLE', $domain, 'edit_password', "$username");
if ($result != 1) { if ($result != 1) {
db_log ('CONSOLE', $domain, 'edit_password', "FAILURE: " . $this->username); # TODO: replace hardcoded CONSOLE - class is used by XMLRPC and users/
$this->errormsg[] = Lang::read('pEdit_mailbox_result_error'); $this->errormsg[] = Lang::read('pEdit_mailbox_result_error');
return 1; return false;
} }
return 0; db_log ('CONSOLE', $domain, 'edit_password', $this->username); # TODO: replace hardcoded CONSOLE - class is used by XMLRPC and users/
return true;
} }
/** /**
@ -73,7 +70,6 @@ class UserHandler {
* @return boolean true on successful login (i.e. password matches etc) * @return boolean true on successful login (i.e. password matches etc)
*/ */
public static function login($username, $password) { public static function login($username, $password) {
global $config;
$username = escape_string($username); $username = escape_string($username);
$table_mailbox = table_by_key('mailbox'); $table_mailbox = table_by_key('mailbox');
@ -99,94 +95,81 @@ class UserHandler {
* @param name string * @param name string
* *
*/ */
public function add($password, $name = '', $quota = 0, $active = true, $mail = true ) { public function add($password, $name = '', $quota = -999, $active = true, $mail = true ) {
# FIXME: change default value of $quota to something that is not an allowed value, like "-9" (0 is "unlimited", and I don't like that as default) # FIXME: default value of $quota (-999) is intentionally invalid. Add fallback to default quota.
# FIXME: Should the parameters be optional at all? # FIXME: Should the parameters be optional at all?
# TODO: check if parameters are valid/allowed (quota?). Checks should live in a separate function that can be used by add and edit. # TODO: check if parameters are valid/allowed (quota?).
# TODO: most code should live in a separate function that can be used by add and edit.
# TODO: On the longer term, the web interface should also use this class. # TODO: On the longer term, the web interface should also use this class.
global $config;
$username = $this->username;
$tmp = preg_split ('/@/', $username);
$domain = $tmp[1];
$address = escape_string($username);
$username = $tmp[0];
$table_mailbox = table_by_key('mailbox'); # TODO: copy/move all checks and validations from create-mailbox.php here
$table_alias = table_by_key('alias');
$active = db_get_boolean($active); $username = $this->username;
list($local_part,$domain) = explode ('@', $username);
if(!check_mailbox ($domain)) { if(!check_mailbox ($domain)) {
$this->errormsg[] = Lang::read('pCreate_mailbox_username_text_error3'); $this->errormsg[] = Lang::read('pCreate_mailbox_username_text_error3');
return 1; return false;
} }
$result = db_query ("SELECT * FROM $table_alias WHERE address='$address'");
# check if an alias with this name already exists
$result = db_query ("SELECT * FROM " . table_by_key('alias') . " WHERE address='" . escape_string($username) . "'");
if ($result['rows'] == 1){ if ($result['rows'] == 1){
$this->errormsg[] = Lang::read('pCreate_mailbox_username_text_error2'); $this->errormsg[] = Lang::read('pCreate_mailbox_username_text_error2');
return 1; return false;
} }
$plain = $password; $plain = $password;
$password = pacrypt ($password); $password = pacrypt ($password);
# TODO: Decide if we want to have the encryption method in the encrypted password string, and edit pacrypt() accordingly. No special handling here, please! # TODO: if we want to have the encryption method in the encrypted password string, it should be done in pacrypt(). No special handling here!
if ( preg_match("/^dovecot:/", Config::read('encrypt')) ) { # if ( preg_match("/^dovecot:/", Config::read('encrypt')) ) {
$split_method = preg_split ('/:/', Config::read('encrypt')); # $split_method = preg_split ('/:/', Config::read('encrypt'));
$method = strtoupper($split_method[1]); # $method = strtoupper($split_method[1]);
$password = '{' . $method . '}' . $password; # $password = '{' . $method . '}' . $password;
} # }
if (Config::read('domain_path') == "YES") if(Config::read('maildir_name_hook') != 'NO' && function_exists(Config::read('maildir_name_hook')) ) {
$hook_func = $CONF['maildir_name_hook'];
$maildir = $hook_func ($fDomain, $fUsername);
}
elseif (Config::read('domain_path') == "YES")
{ {
if (Config::read('domain_in_mailbox') == "YES") if (Config::read('domain_in_mailbox') == "YES")
{ {
$maildir = $domain . "/" . $address . "/"; $maildir = $domain . "/" . $username . "/";
} }
else else
{ {
$maildir = $domain . "/" . $username . "/"; $maildir = $domain . "/" . $local_part . "/";
} }
} }
else else
{ {
$maildir = $address . "/"; $maildir = $username . "/";
} }
$quota = multiply_quota ($quota); db_begin();
if ('pgsql'== Config::read('database_type')) $active = db_get_boolean($active);
{ $quota = multiply_quota ($quota);
db_query('BEGIN');
}
//$result = db_query ("INSERT INTO $table_alias (address,goto,domain,created,modified,active) VALUES ('$address','$address','$domain',NOW(),NOW(),'$active')"); $alias_data = array(
$arr = array( 'address' => $username,
'address' => $address, 'goto' => $username,
'goto' => $address,
'domain' => $domain, 'domain' => $domain,
'active' => $active, 'active' => $active,
); );
$result = db_insert('alias', $arr, array('created', 'modified') ); $result = db_insert('alias', $alias_data);
if ($result != 1) if ($result != 1)
{ {
$this->errormsg[] = Lang::read('pAlias_result_error') . "\n($address -> $address)\n"; $this->errormsg[] = Lang::read('pAlias_result_error') . "\n($username -> $username)\n";
return 1; return false;
}
// apparently uppercase usernames really confuse some IMAP clients.
$local_part = '';
if(preg_match('/^(.*)@/', $address, $matches)) {
$local_part = $matches[1];
} }
//$result = db_query ("INSERT INTO $table_mailbox (username,password,name,maildir,local_part,quota,domain,created,modified,active) VALUES ('$username','$password','$name','$maildir','$local_part','$quota','$domain',NOW(),NOW(),'$active')"); $mailbox_data = array(
'username' => $username,
$arr2 = array(
'username' => $address,
'password' => $password, 'password' => $password,
'name' => $name, 'name' => $name,
'maildir' => $maildir, 'maildir' => $maildir,
@ -194,123 +177,124 @@ class UserHandler {
'quota' => $quota, 'quota' => $quota,
'domain' => $domain, 'domain' => $domain,
'active' => $active, 'active' => $active,
); );
$result = db_insert('mailbox', $arr2, array('created', 'modified') ); $result = db_insert('mailbox', $mailbox_data);
if ($result != 1 || !mailbox_postcreation($address,$domain,$maildir, $quota)) if ($result != 1 || !mailbox_postcreation($username,$domain,$maildir, $quota)) {
{ $this->errormsg[] = Lang::read('pCreate_mailbox_result_error') . "\n($username)\n";
$this->errormsg[] = Lang::read('pCreate_mailbox_result_error') . "\n($address)\n"; db_rollback();
db_query('ROLLBACK'); return false;
return 1; } else {
} db_commit();
else db_log ('CONSOLE', $domain, 'create_mailbox', $username); # TODO: remove hardcoded CONSOLE
{
db_query('COMMIT');
db_log ('CONSOLE', $domain, 'create_mailbox', "$address");
if ($mail == true) if ($mail == true)
{ {
$fTo = $address; # TODO: move "send the mail" to a function
$fTo = $username;
$fFrom = Config::read('admin_email'); $fFrom = Config::read('admin_email');
$fHeaders = "To: " . $fTo . "\n"; $fSubject = Lang::read('pSendmail_subject_text');
$fHeaders .= "From: " . $fFrom . "\n"; $fBody = Config::read('welcome_text');
$fHeaders .= "Subject: " . encode_header (Lang::read('pSendmail_subject_text')) . "\n"; if (!smtp_mail ($fTo, $fFrom, $fSubject, $fBody))
$fHeaders .= "MIME-Version: 1.0\n";
$fHeaders .= "Content-Type: text/plain; charset=utf-8\n";
$fHeaders .= "Content-Transfer-Encoding: 8bit\n";
$fHeaders .= Config::read('welcome_text');
if (!smtp_mail ($fTo, $fFrom, $fHeaders))
{ {
$this->errormsg[] = Lang::read('pSendmail_result_error'); $this->errormsg[] = Lang::read('pSendmail_result_error');
return 1; return false;
} }
} }
create_mailbox_subfolders($address,$plain); create_mailbox_subfolders($username,$plain);
} }
return 0; return true;
} }
public function view() { public function view() {
global $config;
$username = $this->username; $username = $this->username;
$table_mailbox = table_by_key('mailbox'); $table_mailbox = table_by_key('mailbox');
# TODO: check if DATE_FORMAT works in MySQL and PostgreSQL
# TODO: maybe a more fine-grained date format would be better for non-CLI usage
$result = db_query("SELECT username, name, maildir, quota, local_part, domain, DATE_FORMAT(created, '%d.%m.%y') AS created, DATE_FORMAT(modified, '%d.%m.%y') AS modified, active FROM $table_mailbox WHERE username='$username'"); $result = db_query("SELECT username, name, maildir, quota, local_part, domain, DATE_FORMAT(created, '%d.%m.%y') AS created, DATE_FORMAT(modified, '%d.%m.%y') AS modified, active FROM $table_mailbox WHERE username='$username'");
if ($result['rows'] != 0) { if ($result['rows'] != 0) {
$this->return = db_array($result['result']); $this->return = db_array($result['result']);
return 0; return true;
} }
$this->errormsg = $result['error']; $this->errormsg = $result['error'];
return 1; return false;
} }
public function delete() { public function delete() {
global $config;
$username = $this->username; $username = $this->username;
$tmp = preg_split ('/@/', $username); list(/*$local_part*/,$domain) = explode ('@', $username);
$domain = $tmp[1];
$username = escape_string($username);
$E_username = escape_string($username);
$E_domain = escape_string($domain);
$table_mailbox = table_by_key('mailbox'); $table_mailbox = table_by_key('mailbox');
$table_alias = table_by_key('alias'); $table_alias = table_by_key('alias');
$table_vacation = table_by_key('vacation'); $table_vacation = table_by_key('vacation');
$table_vacation_notification = table_by_key('vacation_notification'); $table_vacation_notification = table_by_key('vacation_notification');
if (Config::read('database_type') == "pgsql") db_query('BEGIN'); db_begin();
/* there may be no aliases to delete */
$result = db_query("SELECT * FROM $table_alias WHERE address = '$username' AND domain = '$domain'"); $error = 0;
$result = db_query("SELECT * FROM $table_alias WHERE address = '$E_username' AND domain = '$domain'");
if($result['rows'] == 1) { if($result['rows'] == 1) {
//$result = db_query ("DELETE FROM $table_alias WHERE address='$username' AND domain='$domain'");
$result = db_delete('alias', 'address', $username); $result = db_delete('alias', 'address', $username);
db_log ('CONSOLE', $domain, 'delete_alias', $username); db_log ('CONSOLE', $domain, 'delete_alias', $username); # TODO: remove hardcoded CONSOLE
} else {
$this->errormsg[] = "no alias $username"; # todo: better message, make translatable
$error = 1;
} }
/* is there a mailbox? if do delete it from orbit; it's the only way to be sure */ /* is there a mailbox? if do delete it from orbit; it's the only way to be sure */
$result = db_query ("SELECT * FROM $table_mailbox WHERE username='$username' AND domain='$domain'"); $result = db_query ("SELECT * FROM $table_mailbox WHERE username='$E_username' AND domain='$domain'");
if ($result['rows'] == 1) if ($result['rows'] == 1)
{ {
//$result = db_query ("DELETE FROM $table_mailbox WHERE username='$username' AND domain='$domain'");
$result = db_delete('mailbox', 'username', $username); $result = db_delete('mailbox', 'username', $username);
$postdel_res=mailbox_postdeletion($username,$domain); $postdel_res=mailbox_postdeletion($username,$domain);
if ($result != 1 || !$postdel_res) if ($result != 1 || !$postdel_res)
{ {
$tMessage = Lang::read('pDelete_delete_error') . "$username ("; $tMessage = Lang::read('pDelete_delete_error') . "$username (";
if ($result['rows']!=1) if ($result['rows']!=1) # TODO: invalid test, $result is from db_delete and only contains the number of deleted rows
{ {
$tMessage.='mailbox'; $tMessage.='mailbox';
if (!$postdel_res) $tMessage.=', '; if (!$postdel_res) $tMessage.=', ';
$this->errormsg[] = "no mailbox $username"; # todo: better message, make translatable
$error = 1;
} }
if (!$postdel_res) if (!$postdel_res)
{ {
$tMessage.='post-deletion'; $tMessage.='post-deletion';
$this->errormsg[] = "post-deletion script failed"; # todo: better message, make translatable
$error = 1;
} }
$this->errormsg[] = $tMessage.')'; $this->errormsg[] = $tMessage.')';
return 1; # TODO: does db_rollback(); make sense? Not sure because mailbox_postdeletion was already called (move the call checking the db_delete result?)
# TODO: maybe mailbox_postdeletion should be run after all queries, just before commit/rollback
$error = 1;
# return false; # TODO: does this make sense? Or should we still cleanup vacation and vacation_notification?
} }
db_log ('CONSOLE', $domain, 'delete_mailbox', $username); db_log ('CONSOLE', $domain, 'delete_mailbox', $username);
} else {
$this->errormsg[] = "no mailbox $username"; # TODO: better message, make translatable
$error = 1;
} }
$result = db_query("SELECT * FROM $table_vacation WHERE email = '$username' AND domain = '$domain'"); $result = db_query("SELECT * FROM $table_vacation WHERE email = '$E_username' AND domain = '$domain'");
if($result['rows'] == 1) { if($result['rows'] == 1) {
//db_query ("DELETE FROM $table_vacation WHERE email='$username' AND domain='$domain'");
db_delete('vacation', 'email', $username); db_delete('vacation', 'email', $username);
//db_query ("DELETE FROM $table_vacation_notification WHERE on_vacation ='$username' "); /* should be caught by cascade, if PgSQL */ db_delete('vacation_notification', 'on_vacation', $username); # TODO: delete vacation_notification independent of vacation? (in case of "forgotten" vacation_notification entries)
db_delete('vacation_notification', 'on_vacation', $username);
} }
return 0; db_commit();
if ($error != 0) return false;
return true;
} }
} }

Loading…
Cancel
Save