diff --git a/model/AdminHandler.php b/model/AdminHandler.php index 9f898d65..9a35294f 100644 --- a/model/AdminHandler.php +++ b/model/AdminHandler.php @@ -123,7 +123,7 @@ class AdminHandler extends PFAHandler { * called by $this->store() after storing $this->values in the database * can be used to update additional tables, call scripts etc. */ - protected function storemore() { + protected function postSave() : bool { # store list of allowed domains in the domain_admins table if (isset($this->values['domains'])) { if (is_array($this->values['domains'])) { diff --git a/model/AdminpasswordHandler.php b/model/AdminpasswordHandler.php index 3f03376d..5799e22e 100644 --- a/model/AdminpasswordHandler.php +++ b/model/AdminpasswordHandler.php @@ -35,7 +35,7 @@ class AdminpasswordHandler extends PFAHandler { ); } - public function init($id) { + public function init($id) :bool { # hardcode to logged in admin if ($this->admin_username == '') { die("No admin logged in"); diff --git a/model/AliasHandler.php b/model/AliasHandler.php index 9119cfdd..d1f69111 100644 --- a/model/AliasHandler.php +++ b/model/AliasHandler.php @@ -136,7 +136,7 @@ class AliasHandler extends PFAHandler { * AliasHandler needs some special handling in init() and therefore overloads the function. * It also calls parent::init() */ - public function init($id) { + public function init(string $id) : bool { $bits = explode('@', $id); if (sizeof($bits) == 2) { $local_part = $bits[0]; @@ -303,7 +303,7 @@ class AliasHandler extends PFAHandler { $this->values['goto'] = join(',', $values['goto']); } - protected function storemore() { + protected function postSave() : bool { # TODO: if alias belongs to a mailbox, update mailbox active status return true; } @@ -357,7 +357,7 @@ class AliasHandler extends PFAHandler { return array($condition, $searchmode); } - public function getList($condition, $searchmode = array(), $limit=-1, $offset=-1) { + public function getList($condition, $searchmode = array(), $limit=-1, $offset=-1) : bool { list($condition, $searchmode) = $this->condition_ignore_mailboxes($condition, $searchmode); $this->set_is_mailbox_extrafrom($condition, $searchmode); $result = parent::getList($condition, $searchmode, $limit, $offset); diff --git a/model/AliasdomainHandler.php b/model/AliasdomainHandler.php index e2f3cb81..5b419632 100644 --- a/model/AliasdomainHandler.php +++ b/model/AliasdomainHandler.php @@ -51,7 +51,7 @@ class AliasdomainHandler extends PFAHandler { } } - public function init($id) { + public function init($id) : bool { $success = parent::init($id); if ($success) { if (count($this->struct['alias_domain']['options']) == 0 && $this->new) { diff --git a/model/DomainHandler.php b/model/DomainHandler.php index c919c3b9..4bdc48ba 100644 --- a/model/DomainHandler.php +++ b/model/DomainHandler.php @@ -148,7 +148,7 @@ class DomainHandler extends PFAHandler { } - protected function beforestore() { + protected function preSave() : bool { # TODO: is this function superfluous? _can_edit should already cover this if ($this->is_superadmin) { return true; @@ -161,7 +161,7 @@ class DomainHandler extends PFAHandler { * called by $this->store() after storing $this->values in the database * can be used to update additional tables, call scripts etc. */ - protected function storemore() { + protected function postSave() : bool { if ($this->new && $this->values['default_aliases']) { foreach (Config::read_array('default_aliases') as $address=>$goto) { $address = $address . "@" . $this->id; diff --git a/model/MailboxHandler.php b/model/MailboxHandler.php index 85d0c9b4..596eccd4 100644 --- a/model/MailboxHandler.php +++ b/model/MailboxHandler.php @@ -60,7 +60,7 @@ class MailboxHandler extends PFAHandler { } } - public function init($id) { + public function init($id) : bool { if (!parent::init($id)) { return false; } @@ -217,7 +217,7 @@ class MailboxHandler extends PFAHandler { } - protected function beforestore() { + protected function preSave() : bool { if (isset($this->values['quota']) && $this->values['quota'] != -1 && is_numeric($this->values['quota'])) { $multiplier = Config::read_string('quota_multiplier'); if ($multiplier == 0 || !is_numeric($multiplier)) { // or empty string, or null, or false... @@ -256,7 +256,7 @@ class MailboxHandler extends PFAHandler { return false; } - if (!$ah->store()) { + if (!$ah->save()) { $this->errormsg[] = $ah->errormsg[0]; return false; } @@ -292,7 +292,7 @@ class MailboxHandler extends PFAHandler { return $ok && parent::set($values); } - protected function storemore() { + protected function postSave() : bool { if ($this->new) { if (!$this->mailbox_post_script()) { # return false; # TODO: should this be fatal? @@ -649,7 +649,7 @@ class MailboxHandler extends PFAHandler { /** - * Called by storemore() after a mailbox has been created. + * Called by postSave() after a mailbox has been created. * Immediately returns, unless configuration indicates * that one or more sub-folders should be created. * diff --git a/model/PFAHandler.php b/model/PFAHandler.php index 9bf5dc42..f763dde0 100644 --- a/model/PFAHandler.php +++ b/model/PFAHandler.php @@ -337,7 +337,7 @@ abstract class PFAHandler { * initialize with $id and check if it is valid * @param string $id */ - public function init($id) { + public function init(string $id) : bool { // postfix treats address lookups (aliases, mailboxes) as if they were lowercase. // MySQL is normally case insenstive, PostgreSQL is case sensitive. @@ -526,7 +526,7 @@ abstract class PFAHandler { } /** - * store $this->values in the database + * save $this->values to the database * * converts values based on $this->struct[*][type] (boolean, password encryption) * @@ -534,13 +534,13 @@ abstract class PFAHandler { * @return bool - true if all values were stored in the database, otherwise false * error messages (if any) are stored in $this->errormsg */ - public function store() { + public function save() : bool { if ($this->values_valid == false) { $this->errormsg[] = "one or more values are invalid!"; return false; } - if (!$this->beforestore()) { + if (!$this->preSave()) { return false; } @@ -584,9 +584,9 @@ abstract class PFAHandler { return false; } - $result = $this->storemore(); + $result = $this->postSave(); - # db_log() even if storemore() failed + # db_log() even if postSave() failed db_log($this->domain, $this->msg['logname'], $this->id); if ($result) { @@ -600,17 +600,17 @@ abstract class PFAHandler { /** * called by $this->store() before storing the values in the database - * @return bool - if false, store() will abort + * @return bool - if false, save() will abort */ - protected function beforestore() { + protected function preSave() : bool { return true; # do nothing, successfully ;-) } /** - * called by $this->store() after storing $this->values in the database + * called by $this->save() after storing $this->values in the database * can be used to update additional tables, call scripts etc. */ - protected function storemore() { + protected function postSave() : bool { return true; # do nothing, successfully ;-) } @@ -735,7 +735,7 @@ abstract class PFAHandler { * @param int $offset - number of first row to return * @return array - rows (as associative array, with the ID as key) */ - protected function read_from_db($condition, $searchmode = array(), $limit=-1, $offset=-1) { + protected function read_from_db($condition, $searchmode = array(), $limit=-1, $offset=-1) : array { $queryparts = $this->build_select_query($condition, $searchmode); $query = $queryparts['select_cols'] . $queryparts['from_where_order']; @@ -800,7 +800,7 @@ abstract class PFAHandler { * @return bool - always true, no need to check ;-) (if $result is not an array, getList die()s) * The data is stored in $this->result (as array of rows, each row is an associative array of column => value) */ - public function getList($condition, $searchmode = array(), $limit=-1, $offset=-1) { + public function getList($condition, $searchmode = array(), $limit=-1, $offset=-1) : bool { if (is_array($condition)) { $real_condition = array(); foreach ($condition as $key => $value) { diff --git a/model/VacationHandler.php b/model/VacationHandler.php index fcdc900d..1b5bce31 100644 --- a/model/VacationHandler.php +++ b/model/VacationHandler.php @@ -279,7 +279,7 @@ class VacationHandler extends PFAHandler { # TODO: supress logging in AliasHandler if called from VacationHandler (VacationHandler should log itsself) - if (!$handler->store()) { + if (!$handler->save()) { print_r($handler->errormsg); # TODO: error handling return false; } diff --git a/public/setup.php b/public/setup.php index a47a6be8..cd488dc1 100644 --- a/public/setup.php +++ b/public/setup.php @@ -458,7 +458,7 @@ function create_admin($values) { return array(1, "", $handler->errormsg); } - if (!$handler->store()) { + if (!$handler->save()) { return array(1, "", $handler->errormsg); } diff --git a/public/users/edit-alias.php b/public/users/edit-alias.php index ea4f4916..3d59e91d 100644 --- a/public/users/edit-alias.php +++ b/public/users/edit-alias.php @@ -121,7 +121,7 @@ if ($_SERVER['REQUEST_METHOD'] == "POST") { flash_error($errormsg[0]); } - $updated = $ah->store(); + $updated = $ah->save(); if ($updated) { header("Location: main.php"); diff --git a/public/users/password-change.php b/public/users/password-change.php index e54f8b64..2085da34 100644 --- a/public/users/password-change.php +++ b/public/users/password-change.php @@ -70,7 +70,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { $values = $handler->result; $values['password'] = $fPassword; $values['password2'] = $fPassword2; - if ($handler->set($values) && $handler->store()) { + if ($handler->set($values) && $handler->save()) { flash_info(Config::lang_f('pPassword_result_success', $tUsername)); header('Location: main.php'); exit(0); diff --git a/public/xmlrpc.php b/public/xmlrpc.php index 95cc923a..035f240c 100644 --- a/public/xmlrpc.php +++ b/public/xmlrpc.php @@ -176,7 +176,7 @@ class AliasProxy { //error_log('ah->set failed' . print_r($values, true)); return false; } - $store = $ah->store(); + $store = $ah->save(); return $store; } diff --git a/tests/AliasHandlerTest.php b/tests/AliasHandlerTest.php index d75a7922..ce4a9c1e 100644 --- a/tests/AliasHandlerTest.php +++ b/tests/AliasHandlerTest.php @@ -1,15 +1,21 @@ getList(""); - $this->assertTrue($list); - $results = $x->result(); - $this->assertEmpty($results); +class AliasHandlerTest extends \PHPUnit\Framework\TestCase +{ + + protected function setUp(): void + { + // Fake being an admin. + $_SESSION = [ + 'sessid' => [ + 'roles' => ['global-admin'] + ] + ]; + parent::setUp(); } - public function tearDown(): void { + protected function tearDown(): void + { $_SESSION = []; db_query('DELETE FROM alias'); db_query('DELETE FROM domain_admins'); @@ -18,7 +24,18 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase { parent::tearDown(); } - public function testCannotAddAliasUntilDomainIsThere() { + public function testBasic() + { + $x = new AliasHandler(); + $list = $x->getList(""); + $this->assertTrue($list); + $results = $x->result(); + $this->assertEmpty($results); + } + + + public function testCannotAddAliasUntilDomainIsThere() + { // Fake us being an admin. @@ -52,7 +69,8 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase { /** * @see https://github.com/postfixadmin/postfixadmin/pull/375 and https://github.com/postfixadmin/postfixadmin/issues/358 */ - public function testCannotAddAliasThatPointsToItself() { + public function testCannotAddAliasThatPointsToItself() + { // Fake being an admin. $_SESSION = [ 'sessid' => [ @@ -82,7 +100,7 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase { $this->assertTrue($ret); - $ret = $dh->store(); + $ret = $dh->save(); $this->assertTrue($ret); @@ -125,7 +143,7 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase { $this->assertEquals(4, count($list)); // default aliases. $x->set($values); - $x->store(); + $x->save(); $this->assertNotEmpty($x->errormsg); $this->assertEquals( @@ -135,61 +153,18 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase { ], $x->errormsg); } - public function testAddingDataEtc() { + public function testAddingDataEtc() + { + // Fake being an admin. $_SESSION = [ 'sessid' => [ 'roles' => ['global-admin'] ] ]; - // Add example.com - $dh = new DomainHandler(1, 'admin', true); - $dh->init('example.com'); + $this->addDomain('example.com', 'admin'); - $ret = $dh->set( - [ - 'domain' => 'example.com', - 'description' => 'test domain', - 'aliases' => 11, - 'mailboxes' => 12, - 'active' => 1, - 'backupmx' => 0, - 'default_aliases' => 1 - ] - ); - - - $this->assertEmpty($dh->errormsg); - $this->assertEmpty($dh->infomsg); - - $this->assertTrue($ret); - - $ret = $dh->store(); - - $this->assertTrue($ret); - - // Need to add 'admin' as a domain_admin - db_insert('domain_admins', ['username' => 'admin', 'domain' => 'example.com', 'created' => '2020-01-01', 'active' => 1], ['created'], true); - - $dh = new DomainHandler(0, 'admin', true); - $dh->getList(''); - $result = $dh->result(); - - $this->assertEmpty($dh->infomsg); - $this->assertEmpty($dh->errormsg); - - $this->assertNotEmpty($result); - - $this->assertEquals('example.com', $result['example.com']['domain']); - $this->assertEquals('test domain', $result['example.com']['description']); - - $this->assertEquals(11, $result['example.com']['aliases']); - $this->assertEquals(12, $result['example.com']['mailboxes']); // default aliases. - - $this->assertEquals(4, $result['example.com']['alias_count']); // default aliases. - $this->assertEquals(0, $result['example.com']['mailbox_count']); - $this->assertEquals(1, $result['example.com']['active']); $x = new AliasHandler(1, 'admin', true); @@ -208,7 +183,7 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase { $this->assertEquals(4, count($list)); // default aliases. $x->set($values); - $x->store(); + $x->save(); $x->getList(''); @@ -230,4 +205,202 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase { $this->assertTrue($found, "check output : " . json_encode($list)); } + + + private function addDomain(string $domain, string $username): void + { + // Fake being an admin. + $_SESSION = [ + 'sessid' => [ + 'roles' => ['global-admin'] + ] + ]; + // Add example.com + $dh = new DomainHandler(1, $username, true); + + $dh->init('example.com'); + + $ret = $dh->set( + [ + 'domain' => $domain, + 'description' => 'test domain', + 'aliases' => 11, + 'mailboxes' => 12, + 'active' => 1, + 'backupmx' => 0, + 'default_aliases' => 1 + ] + ); + + + $this->assertEmpty($dh->errormsg); + $this->assertEmpty($dh->infomsg); + $this->assertTrue($ret); + $ret = $dh->save(); + $this->assertTrue($ret); + + // Need to add 'admin' as a domain_admin + db_insert('domain_admins', ['username' => $username, 'domain' => $domain, 'created' => '2020-01-01', 'active' => 1], ['created'], true); + + + $dh = new DomainHandler(0, $username, true); + $dh->getList(''); + $result = $dh->result(); + $this->assertEmpty($dh->infomsg); + $this->assertEmpty($dh->errormsg); + + $this->assertNotEmpty($result); + + $expected = [ + 'domain' => 'example.com', + 'description' => 'test domain', + 'aliases' => 11, + 'alias_count' => 4, + 'mailboxes' => 12, + 'mailbox_count' => 0, + 'backupmx' => 0, + 'active' => 1, + ]; + + foreach ($expected as $k => $v) { + $this->assertEquals($v, $result[$domain][$k]); + } + + } + + public function testYouCannotAddMoreAliasesThanTheDomainLimit() + { + + $this->addDomain('example.com', 'admin'); + + // default limit is 11 aliases.... so it should exit once we get past that. + + + $dh = new DomainHandler(0, 'admin', true); + $this->assertTrue($dh->getList('')); + $result = $dh->result(); + + // alias count limit is 11. + $this->assertEquals(11, $result['example.com']['aliases']); + + // 4 default aliases were added. + $this->assertEquals(4, $result['example.com']['alias_count']); + + + foreach (range(1, 7) as $char) { + + $newAlias = $char . '-test@example.com'; + + $x = new AliasHandler(1, 'admin', true); + $values = [ + 'localpart' => explode('@', $newAlias)[0], + 'domain' => 'example.com', + 'active' => 1, + 'address' => $newAlias, + 'goto' => ['dest@example.com'] + ]; + + $r = $x->init($newAlias); + + $this->assertTrue($r); + + $x->set($values); + $this->assertTrue($x->save()); + $this->assertTrue($x->getList('')); + $list = $x->result(); + $this->assertArrayHasKey($newAlias, $list); + $this->assertEquals(1, $list[$newAlias]['active']); + } + + // try and add one more - it should fail. + $x = new AliasHandler(1, 'admin', true); + $values = [ + 'localpart' => 'z-david.test', + 'domain' => 'example.com', + 'active' => 1, + 'address' => 'z-david.test@example.com', + 'goto' => ['dest@example.com'] + ]; + + $r = $x->init('z-david.test@example.com'); + + // doesn't already exist. + $this->assertFalse($r); + + // try saving .... + $x->set($values); + $this->assertFalse($x->save()); + + $this->assertEquals([ + 'address' => "You have reached your limit to create aliases!", + 0 => "one or more values are invalid!" + ], $x->errormsg); + + } + + + public function testLoadsOfAliasesGetHandledByPager() + { + + $this->addDomain('example.com', 'admin'); + + // default limit is 11 aliases.... so it should exit once we get past that. + + $dh = new DomainHandler(0, 'admin', true); + $dh->init('example.com'); + + $this->assertTrue($dh->set( + [ + //'domain' => 'example.com', + 'aliases' => 99, + 'mailboxes' => 88, + 'backupmx' => 0, + 'active' => 1, + ] + )); + + $this->assertTrue($dh->save()); + + $dh->getList(''); + + $domain = $dh->result()['example.com']; + + $this->assertEquals(99, $domain['aliases']); + $this->assertEquals(88, $domain['mailboxes']); + + foreach (range(1, 80) as $char) { + + $newAlias = $char . '-test@example.com'; + + $x = new AliasHandler(1, 'admin', true); + $values = [ + 'localpart' => explode('@', $newAlias)[0], + 'domain' => 'example.com', + 'active' => 1, + 'address' => $newAlias, + 'goto' => ['dest@example.com'] + ]; + + $r = $x->init($newAlias); + + $this->assertTrue($r); + + $x->set($values); + $this->assertTrue($x->save()); + $this->assertTrue($x->getList('')); + $list = $x->result(); + $this->assertArrayHasKey($newAlias, $list); + $this->assertEquals(1, $list[$newAlias]['active']); + } + + // try and add one more - it should fail. + $x = new AliasHandler(0, 'admin', true); + + $x->getList('', [], 5, 20); + $results = $x->result(); + + $this->assertEquals(5, count($results)); + $this->assertTrue(isset($results['31-test@example.com'])); + + } } diff --git a/tests/DomainHandlerTest.php b/tests/DomainHandlerTest.php index 91145611..cbb45fcc 100644 --- a/tests/DomainHandlerTest.php +++ b/tests/DomainHandlerTest.php @@ -48,7 +48,7 @@ class DomainHandlerTest extends \PHPUnit\Framework\TestCase $this->assertEmpty($dh->errormsg); $this->assertEmpty($dh->infomsg); $this->assertTrue($ret); - $ret = $dh->store(); + $ret = $dh->save(); $this->assertTrue($ret); // Need to add 'admin' as a domain_admin @@ -93,7 +93,7 @@ class DomainHandlerTest extends \PHPUnit\Framework\TestCase ); $this->assertTrue($ret); - $this->assertTrue($dh->store()); + $this->assertTrue($dh->save()); $this->assertEmpty($dh->errormsg); $dh->getList(''); diff --git a/tests/MailboxHandlerTest.php b/tests/MailboxHandlerTest.php index e2597d03..98813e2c 100644 --- a/tests/MailboxHandlerTest.php +++ b/tests/MailboxHandlerTest.php @@ -60,7 +60,7 @@ class MailboxHandlerTest extends \PHPUnit\Framework\TestCase { $this->assertTrue($ret); - $ret = $dh->store(); + $ret = $dh->save(); $this->assertTrue($ret); @@ -109,7 +109,7 @@ class MailboxHandlerTest extends \PHPUnit\Framework\TestCase { $this->assertEquals(0, count($list)); $x->set($values); - $x->store(); + $x->save(); $x->getList(''); @@ -149,7 +149,7 @@ class MailboxHandlerTest extends \PHPUnit\Framework\TestCase { $this->assertEmpty($h->errormsg); $this->assertEmpty($h->infomsg); $this->assertTrue($r); - $this->assertTrue($h->store()); + $this->assertTrue($h->save()); $h->getList(''); $list = $h->result();