From 7785eb072ba927aa1ccc8d63425fef3b479e4cc2 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Mon, 20 Feb 2017 17:04:13 -0500 Subject: [PATCH] Complete rewrite of User class and other changes - User-related database methods will now throw User\Exception upon errors - Internal userAdd method can now generate random passwords - Pursuant to above, dependency on password genrator has been added, and password-related methods now return strings instead of booleans - User class methods now all explicitly follow different branches for internal/external/missing implementations - various User class methods now perform auto-provisioning of the internal database when external implementations report success on users not in the database - Tests have been adjusted to account for the above changes - Lots is probably still broken --- composer.json | 5 +- composer.lock | 42 +++- lib/Conf.php | 1 + lib/Database.php | 29 +-- lib/User.php | 269 ++++++++++++++++++-------- lib/User/Driver.php | 4 +- lib/User/ExceptionNotImplemented.php | 6 + lib/User/InternalFunctions.php | 4 +- tests/User/TestUser.php | 53 ++++- tests/lib/User/DriverInternalMock.php | 49 ++--- 10 files changed, 334 insertions(+), 128 deletions(-) create mode 100644 lib/User/ExceptionNotImplemented.php diff --git a/composer.json b/composer.json index ead7a5b..1bdf628 100644 --- a/composer.json +++ b/composer.json @@ -22,7 +22,8 @@ "jkingweb/druuid": "^3.0.0", "phpseclib/phpseclib": "^2.0.4", "webmozart/glob": "^4.1.0", - "fguillot/picoFeed": ">=0.1.31" + "fguillot/picoFeed": ">=0.1.31", + "hosteurope/password-generator": "^1.0" }, "require-dev": { "mikey179/vfsStream": "^1.6.4" @@ -37,4 +38,4 @@ "JKingWeb\\NewsSync\\Test\\": "tests/lib/" } } -} \ No newline at end of file +} diff --git a/composer.lock b/composer.lock index 76c3192..54d1b21 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "content-hash": "953c6b705277c5b622d82a3d6f0c9dfd", + "content-hash": "264437f06f643a1413d45660c2a32124", "packages": [ { "name": "fguillot/picofeed", @@ -59,6 +59,46 @@ "homepage": "https://github.com/fguillot/picoFeed", "time": "2017-01-16T03:10:21+00:00" }, + { + "name": "hosteurope/password-generator", + "version": "v1.0.1", + "source": { + "type": "git", + "url": "https://github.com/hosteurope/password-generator.git", + "reference": "21bb99eb9ae47191d816368d3d9e54562e3f9a5f" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/hosteurope/password-generator/zipball/21bb99eb9ae47191d816368d3d9e54562e3f9a5f", + "reference": "21bb99eb9ae47191d816368d3d9e54562e3f9a5f", + "shasum": "" + }, + "require": { + "php": ">=7.0.0" + }, + "require-dev": { + "phpunit/phpunit": "^5.5" + }, + "type": "library", + "autoload": { + "psr-4": { + "PasswordGenerator\\": "src/", + "PasswordGeneratorTests\\": "tests/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Karim Geiger", + "email": "karim.geiger@heg.com" + } + ], + "description": "Password generator for generating policy-compliant passwords.", + "time": "2016-12-08T09:32:12+00:00" + }, { "name": "jkingweb/druuid", "version": "3.0.0", diff --git a/lib/Conf.php b/lib/Conf.php index 9ed9899..0c286eb 100644 --- a/lib/Conf.php +++ b/lib/Conf.php @@ -26,6 +26,7 @@ class Conf { public $userDriver = User\DriverInternal::class; public $userAuthPreferHTTP = false; public $userComposeNames = true; + public $userTempPasswordLength = 20; public $simplepieCache = BASE.".cache"; diff --git a/lib/Database.php b/lib/Database.php index 3318ba0..aa710f4 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -1,6 +1,7 @@ db->prepare("SELECT count(*) from newssync_users where id is ?", "str")->run($user)->getSingle(); } - public function userAdd(string $user, string $password = null): bool { + public function userAdd(string $user, string $password = null): string { if(!$this->data->user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if($this->userExists($user)) return false; - if(strlen($password) > 0) $password = password_hash($password, \PASSWORD_DEFAULT); - $this->db->prepare("INSERT INTO newssync_users(id,password) values(?,?)", "str", "str")->run($user,$password); - return true; + if($this->userExists($user)) throw new User\Exception("alreadyExists", ["action" => __FUNCTION__, "user" => $user]); + if($password===null) $password = (new PassGen)->length($this->data->conf->userTempPasswordLength)->get(); + $hash = ""; + if(strlen($password) > 0) $hash = password_hash($password, \PASSWORD_DEFAULT); + $this->db->prepare("INSERT INTO newssync_users(id,password) values(?,?)", "str", "str")->run($user,$hash); + return $password; } public function userRemove(string $user): bool { if(!$this->data->user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - $this->db->prepare("DELETE from newssync_users where id is ?", "str")->run($user); + if($this->db->prepare("DELETE from newssync_users where id is ?", "str")->run($user)->changes() < 1) throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); return true; } @@ -208,16 +211,18 @@ class Database { public function userPasswordGet(string $user): string { if(!$this->data->user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$this->userExists($user)) return ""; + if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); return (string) $this->db->prepare("SELECT password from newssync_users where id is ?", "str")->run($user)->getSingle(); } - public function userPasswordSet(string $user, string $password = null): bool { + public function userPasswordSet(string $user, string $password = null): string { if(!$this->data->user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$this->userExists($user)) return false; - if(strlen($password > 0)) $password = password_hash($password, \PASSWORD_DEFAULT); - $this->db->prepare("UPDATE newssync_users set password = ? where id is ?", "str", "str")->run($password, $user); - return true; + if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + if($password===null) $password = (new PassGen)->length($this->data->conf->userTempPasswordLength)->get(); + $hash = ""; + if(strlen($password > 0)) $hash = password_hash($password, \PASSWORD_DEFAULT); + $this->db->prepare("UPDATE newssync_users set password = ? where id is ?", "str", "str")->run($hash, $user); + return $password; } public function userPropertiesGet(string $user): array { diff --git a/lib/User.php b/lib/User.php index bdea2f1..a008b61 100644 --- a/lib/User.php +++ b/lib/User.php @@ -8,7 +8,6 @@ class User { protected $data; protected $u; protected $authz = true; - protected $existSupported = 0; protected $authzSupported = 0; static public function listDrivers(): array { @@ -29,7 +28,6 @@ class User { $this->data = $data; $driver = $data->conf->userDriver; $this->u = $driver::create($data); - $this->existSupported = $this->u->driverFunctions("userExists"); $this->authzSupported = $this->u->driverFunctions("authorize"); } @@ -72,27 +70,30 @@ class User { if($this->data->conf->userAuthPreferHTTP) return $this->authHTTP(); return $this->authForm(); } else { - if($this->u->auth($user, $password)) { - $this->authPostProcess($user, $password); - return true; + switch($this->u->driverFunctions("auth")) { + case User\Driver::FUNC_EXTERNAL: + $out = $this->u->auth($user, $password); + if($out && !$this->data->db->userExists($user)) $this->autoProvision($user, $password); + return $out; + case User\Driver::FUNC_INTERNAL: + return $this->u->auth($user, $password); + case User\Driver::FUNCT_NOT_IMPLEMENTED: + return false; } - return false; } } public function authForm(): bool { $cred = $this->credentialsForm(); if(!$cred["user"]) return $this->challengeForm(); - if(!$this->u->auth($cred["user"], $cred["password"])) return $this->challengeForm(); - $this->authPostProcess($cred["user"], $cred["password"]); + if(!$this->auth($cred["user"], $cred["password"])) return $this->challengeForm(); return true; } public function authHTTP(): bool { $cred = $this->credentialsHTTP(); if(!$cred["user"]) return $this->challengeHTTP(); - if(!$this->u->auth($cred["user"], $cred["password"])) return $this->challengeHTTP(); - $this->authPostProcess($cred["user"], $cred["password"]); + if(!$this->auth($cred["user"], $cred["password"])) return $this->challengeHTTP(); return true; } @@ -101,23 +102,31 @@ class User { } public function list(string $domain = null): array { - if($this->u->driverFunctions("userList")==User\Driver::FUNC_EXTERNAL) { - if($domain===null) { - if(!$this->data->user->authorize("@".$domain, "userList")) throw new User\ExceptionAuthz("notAuthorized", ["action" => "userList", "user" => $domain]); - } else { - if(!$this->data->user->authorize("", "userList")) throw new User\ExceptionAuthz("notAuthorized", ["action" => "userList", "user" => "all users"]); - } - return $this->u->userList($domain); - } else { - return $this->data->db->userList($domain); + $func = "userList"; + switch($this->u->driverFunctions($func)) { + case User\Driver::FUNC_EXTERNAL: + // we handle authorization checks for external drivers + if($domain===null) { + if(!$this->authorize("@".$domain, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $domain]); + } else { + if(!$this->authorize("", $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => "all users"]); + } + case User\Driver::FUNC_INTERNAL: + // internal functions handle their own authorization + return $this->u->userList($domain); + case User\Driver::FUNCT_NOT_IMPLEMENTED: + throw new User\ExceptionNotImplemented("notImplemented", ["action" => $func, "user" => $domain]); } } public function authorize(string $affectedUser, string $action, int $promoteLevel = 0): bool { + // if authorization checks are disabled (either because we're running the installer or the background updater) just return true if(!$this->authz) return true; + // if we don't have a logged-in user, fetch credentials if($this->id===null) $this->credentials(); + // if the driver implements authorization, return the result if($this->authzSupported) return $this->u->authorize($affectedUser, $action, $promoteLevel); - // if the driver does not implement authorization, only allow operation for the current user (this means no new users can be added) + // if the driver does not implement authorization, only allow operation for the logged-in user (this means no new users can be added) if($affectedUser==$this->id && $action != "userRightsSet") return true; return false; } @@ -129,58 +138,86 @@ class User { } public function exists(string $user): bool { - if($this->u->driverFunctions("userExists") != User\Driver::FUNC_INTERNAL) { - if(!$this->data->user->authorize($user, "userExists")) throw new User\ExceptionAuthz("notAuthorized", ["action" => "userExists", "user" => $user]); - } - if(!$this->existSupported) return true; - $out = $this->u->userExists($user); - if($out && $this->existSupported==User\Driver::FUNC_EXTERNAL && !$this->data->db->userExist($user)) { - try {$this->data->db->userAdd($user);} catch(\Throwable $e) {} + $func = "userExists"; + switch($this->u->driverFunctions($func)) { + case User\Driver::FUNC_EXTERNAL: + // we handle authorization checks for external drivers + if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + $out = $this->u->userExists($user); + if($out && !$this->data->db->userExist($user)) $this->autoProvision($user, ""); + return $out; + case User\Driver::FUNC_INTERNAL: + // internal functions handle their own authorization + return $this->u->userExists($user); + case User\Driver::FUNCT_NOT_IMPLEMENTED: + // throwing an exception here would break all kinds of stuff; we just report that the user exists + return true; } - return $out; } - public function add($user, $password = null): bool { - if($this->u->driverFunctions("userAdd") != User\Driver::FUNC_INTERNAL) { - if(!$this->data->user->authorize($user, "userAdd")) throw new User\ExceptionAuthz("notAuthorized", ["action" => "userAdd", "user" => $user]); + public function add($user, $password = null): string { + $func = "userAdd"; + switch($this->u->driverFunctions($func)) { + case User\Driver::FUNC_EXTERNAL: + // we handle authorization checks for external drivers + if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + $newPassword = $this->u->userAdd($user, $password); + // if there was no exception and we don't have the user in the internal database, add it + if(!$this->data->db->userExists($user)) $this->autoProvision($user, $newPassword); + return $newPassword; + case User\Driver::FUNC_INTERNAL: + // internal functions handle their own authorization + return $this->u->userAdd($user, $password); + case User\Driver::FUNCT_NOT_IMPLEMENTED: + throw new User\ExceptionNotImplemented("notImplemented", ["action" => $func, "user" => $user]); } - if($this->exists($user)) throw new User\Exception("alreadyExists", ["action" => "userAdd", "user" => $user]); - $out = $this->u->userAdd($user, $password); - if($out && $this->u->driverFunctions("userAdd") != User\Driver::FUNC_INTERNAL) { - try { - if(!$this->data->db->userExists($user)) $this->data->db->userAdd($user, $password); - } catch(\Throwable $e) {} - } - return $out; } public function remove(string $user): bool { - if($this->u->driverFunctions("userRemove") != User\Driver::FUNC_INTERNAL) { - if(!$this->data->user->authorize($user, "userRemove")) throw new User\ExceptionAuthz("notAuthorized", ["action" => "userRemove", "user" => $user]); + $func = "userRemove"; + switch($this->u->driverFunctions($func)) { + case User\Driver::FUNC_EXTERNAL: + // we handle authorization checks for external drivers + if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + $out = $this->u->userRemove($user); + if($out && $this->data->db->userExists($user)) { + // if the user was removed and we have it in our data, remove it there + if(!$this->data->db->userExists($user)) $this->data->db->userRemove($user); + } + return $out; + case User\Driver::FUNC_INTERNAL: + // internal functions handle their own authorization + return $this->u->userRemove($user); + case User\Driver::FUNCT_NOT_IMPLEMENTED: + throw new User\ExceptionNotImplemented("notImplemented", ["action" => $func, "user" => $user]); } - if(!$this->exists($user)) return false; - $out = $this->u->userRemove($user); - if($out && $this->u->driverFunctions("userRemove") != User\Driver::FUNC_INTERNAL) { - try { - if($this->data->db->userExists($user)) $this->data->db->userRemove($user); - } catch(\Throwable $e) {} - } - return $out; } - public function passwordSet(string $user, string $password): bool { - if($this->u->driverFunctions("userPasswordSet") != User\Driver::FUNC_INTERNAL) { - if(!$this->data->user->authorize($user, "userPasswordSet")) throw new User\ExceptionAuthz("notAuthorized", ["action" => "userPasswordSet", "user" => $user]); + public function passwordSet(string $user, string $newPassword = null, $oldPassword = null): bool { + $func = "userPasswordSet"; + switch($this->u->driverFunctions($func)) { + case User\Driver::FUNC_EXTERNAL: + // we handle authorization checks for external drivers + if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + $out = $this->u->userPasswordSet($user, $newPassword, $oldPassword); + if($this->data->db->userExists($user)) { + // if the password change was successful and the user exists, set the internal password to the same value + $this->data->db->userPasswordSet($user, $out); + } else { + // if the user does not exists in the internal database, create it + $this->autoProvision($user, $out); + } + return $out; + case User\Driver::FUNC_INTERNAL: + // internal functions handle their own authorization + return $this->u->userPasswordSet($user, $newPassword); + case User\Driver::FUNCT_NOT_IMPLEMENTED: + throw new User\ExceptionNotImplemented("notImplemented", ["action" => $func, "user" => $user]); } - if(!$this->exists($user)) return false; - return $this->u->userPasswordSet($user, $password); } public function propertiesGet(string $user): array { - if($this->u->driverFunctions("userPropertiesGet") != User\Driver::FUNC_INTERNAL) { - if(!$this->data->user->authorize($user, "userPropertiesGet")) throw new User\ExceptionAuthz("notAuthorized", ["action" => "userPropertiesGet", "user" => $user]); - } - if(!$this->exists($user)) return false; + // prepare default values $domain = null; if($this->data->conf->userComposeNames) $domain = substr($user,strrpos($user,"@")+1); $init = [ @@ -189,35 +226,95 @@ class User { "rights" => User\Driver::RIGHTS_NONE, "domain" => $domain ]; - if($this->u->driverFunctions("userPropertiesGet") != User\Driver::FUNC_NOT_IMPLEMENTED) { - return array_merge($init, $this->u->userPropertiesGet($user)); + $func = "userPropertiesGet"; + switch($this->u->driverFunctions($func)) { + case User\Driver::FUNC_EXTERNAL: + // we handle authorization checks for external drivers + if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + $out = array_merge($init, $this->u->userPropertiesGet($user)); + // remove password if it is return (not exhaustive, but...) + if(array_key_exists('password', $out)) unset($out['password']); + // if the user does not exist in the internal database, add it + if(!$this->data->db->userExists($user)) $this->autoProvision($user, "", $out); + return $out; + case User\Driver::FUNC_INTERNAL: + // internal functions handle their own authorization + return array_merge($init, $this->u->userPropertiesGet($user)); + case User\Driver::FUNCT_NOT_IMPLEMENTED: + // we can return generic values if the function is not implemented + return $init; } - return $init; } public function propertiesSet(string $user, array $properties): array { - if($this->u->driverFunctions("userPropertiesSet") != User\Driver::FUNC_INTERNAL) { - if(!$this->data->user->authorize($user, "userPropertiesSet")) throw new User\ExceptionAuthz("notAuthorized", ["action" => "userPropertiesSet", "user" => $user]); + // remove from the array any values which should be set specially + foreach(['password', 'rights'] as $key) { + if(array_key_exists($key, $properties)) unset($properties[$key]); + } + $func = "userPropertiesSet"; + switch($this->u->driverFunctions($func)) { + case User\Driver::FUNC_EXTERNAL: + // we handle authorization checks for external drivers + if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + $out = $this->u->userPropertiesSet($user, $properties); + if($this->data->db->userExists($user)) { + // if the property change was successful and the user exists, set the internal properties to the same values + $this->data->db->userPpropertiesSet($user, $out); + } else { + // if the user does not exists in the internal database, create it + $this->autoProvision($user, "", $out); + } + return $out; + case User\Driver::FUNC_INTERNAL: + // internal functions handle their own authorization + return $this->u->userPropertiesSet($user, $properties); + case User\Driver::FUNCT_NOT_IMPLEMENTED: + throw new User\ExceptionNotImplemented("notImplemented", ["action" => $func, "user" => $user]); } - if(!$this->exists($user)) throw new User\Exception("doesNotExist", ["user" => $user, "action" => "userPropertiesSet"]); - return $this->u->userPropertiesSet($user, $properties); } public function rightsGet(string $user): int { - if($this->u->driverFunctions("userRightsGet") != User\Driver::FUNC_INTERNAL) { - if(!$this->data->user->authorize($user, "userRightsGet")) throw new User\ExceptionAuthz("notAuthorized", ["action" => "userRightsGet", "user" => $user]); + $func = "userRightsGet"; + switch($this->u->driverFunctions($func)) { + case User\Driver::FUNC_EXTERNAL: + // we handle authorization checks for external drivers + if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + $out = $this->u->userRightsGet($user); + // if the user does not exist in the internal database, add it + if(!$this->data->db->userExists($user)) $this->autoProvision($user, "", null, $out); + return $out; + case User\Driver::FUNC_INTERNAL: + // internal functions handle their own authorization + return $this->u->userRightsGet($user); + case User\Driver::FUNCT_NOT_IMPLEMENTED: + // assume all users are unprivileged + return User\Driver::RIGHTS_NONE; } - // we do not throw an exception here if the user does not exist, because it makes no material difference - if(!$this->exists($user)) return User\Driver::RIGHTS_NONE; - return $this->u->userRightsGet($user); } public function rightsSet(string $user, int $level): bool { - if($this->u->driverFunctions("userRightsSet") != User\Driver::FUNC_INTERNAL) { - if(!$this->data->user->authorize($user, "userRightsSet")) throw new User\ExceptionAuthz("notAuthorized", ["action" => "userRightsSet", "user" => $user]); + $func = "userRightsSet"; + switch($this->u->driverFunctions($func)) { + case User\Driver::FUNC_EXTERNAL: + // we handle authorization checks for external drivers + if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + $out = $this->u->userRightsSet($user, $level); + // if the user does not exist in the internal database, add it + if($out && $this->data->db->userExists($user)) { + $authz = $this->authorizationEnabled(); + $this->authorizationEnabled(false); + $this->data->db->userRightsSet($user, $level); + $this->authorizationEnabled($authz); + } else if($out) { + $this->autoProvision($user, "", null, $level); + } + return $out; + case User\Driver::FUNC_INTERNAL: + // internal functions handle their own authorization + return $this->u->userRightsSet($user, $level); + case User\Driver::FUNCT_NOT_IMPLEMENTED: + throw new User\ExceptionNotImplemented("notImplemented", ["action" => $func, "user" => $user]); } - if(!$this->exists($user)) return false; - return $this->u->userRightsSet($user, $level); } // FIXME: stubs @@ -233,11 +330,25 @@ class User { } } - protected function authPostprocess(string $user, string $password): bool { - if($this->u->driverFunctions("auth") != User\Driver::FUNC_INTERNAL && !$this->data->db->userExists($user)) { - if($password=="") $password = null; - try {$this->data->db->userAdd($user, $password);} catch(\Throwable $e) {} + protected function autoProvision(string $user, string $password = null, array $properties = null, int $rights = 0): string { + // temporarily disable authorization checks, to avoid potential problems + $authz = $this->authorizationEnabled(); + $this->authorizationEnabled(false); + // create the user + $out = $this->data->db->userAdd($user, $password); + // set the user rights + $this->data->db->userRightsSet($user, $level); + // set the user properties... + if($properties===null) { + // if nothing is provided but the driver uses an external function, try to get the current values from the external source + try { + if($this->u->driverFunctions("userPropertiesGet")==User\Driver::FUNC_EXTERNAL) $this->data->db->userPropertiesSet($user, $this->u->userPropertiesGet($user)); + } catch(\Throwable $e) {} + } else { + // otherwise if values are provided, use those + $this->data->db->userPropertiesSet($user, $properties); } - return true; + $this->authorizationEnabled($authz); + return $out; } } \ No newline at end of file diff --git a/lib/User/Driver.php b/lib/User/Driver.php index 598d2f8..3bfc93e 100644 --- a/lib/User/Driver.php +++ b/lib/User/Driver.php @@ -26,13 +26,13 @@ Interface Driver { // checks whether a user exists function userExists(string $user): bool; // adds a user - function userAdd(string $user, string $password = null): bool; + function userAdd(string $user, string $password = null): string; // removes a user function userRemove(string $user): bool; // lists all users function userList(string $domain = null): array; // sets a user's password; if the driver does not require the old password, it may be ignored - function userPasswordSet(string $user, string $newPassword, string $oldPassword): bool; + function userPasswordSet(string $user, string $newPassword = null, string $oldPassword = null): string; // gets user metadata (currently not useful) function userPropertiesGet(string $user): array; // sets user metadata (currently not useful) diff --git a/lib/User/ExceptionNotImplemented.php b/lib/User/ExceptionNotImplemented.php new file mode 100644 index 0000000..b19394f --- /dev/null +++ b/lib/User/ExceptionNotImplemented.php @@ -0,0 +1,6 @@ +db->userExists($user); } - function userAdd(string $user, string $password = null): bool { + function userAdd(string $user, string $password = null): string { return $this->db->userAdd($user, $password); } @@ -62,7 +62,7 @@ trait InternalFunctions { return $this->db->userList($domain); } - function userPasswordSet(string $user, string $newPassword, string $oldPassword): bool { + function userPasswordSet(string $user, string $newPassword = null, string $oldPassword = null): bool { return $this->db->userPasswordSet($user, $newPassword); } diff --git a/tests/User/TestUser.php b/tests/User/TestUser.php index 2f7f5f8..609b1c8 100644 --- a/tests/User/TestUser.php +++ b/tests/User/TestUser.php @@ -6,6 +6,9 @@ namespace JKingWeb\NewsSync; class TestUser extends \PHPUnit\Framework\TestCase { use Test\Tools; + const USER1 = "john.doe@example.com"; + const USER2 = "jane.doe@example.com"; + protected $data; function setUp() { @@ -15,17 +18,55 @@ class TestUser extends \PHPUnit\Framework\TestCase { $conf->userAuthPreferHTTP = true; $this->data = new Test\RuntimeData($conf); $this->data->user = new User($this->data); - $this->data->db = new $drv($this->data); - Test\User\DriverInternalMock::$db = []; - $_SERVER['PHP_AUTH_USER'] = "john.doe@example.com"; + $_SERVER['PHP_AUTH_USER'] = self::USER1; $_SERVER['PHP_AUTH_PW'] = "secret"; } - function testAddingAUser() { + function testListUsers() { $this->assertCount(0,$this->data->user->list()); - $this->data->user->add($_SERVER['PHP_AUTH_USER'], $_SERVER['PHP_AUTH_PW']); + } + + function testCheckIfAUserDoesNotExist() { + $this->assertFalse($this->data->user->exists(self::USER1)); + } + + function testAddAUser() { + $this->data->user->add(self::USER1, "secret"); $this->assertCount(1,$this->data->user->list()); + } + + function testCheckIfAUserDoesExist() { + $this->data->user->add(self::USER1, "secret"); + $this->assertTrue($this->data->user->exists(self::USER1)); + } + + function testAddADuplicateUser() { + $this->data->user->add(self::USER1, "secret"); $this->assertException("alreadyExists", "User"); - $this->data->user->add($_SERVER['PHP_AUTH_USER'], $_SERVER['PHP_AUTH_PW']); + $this->data->user->add(self::USER1, "secret"); + } + + function testAddMultipleUsers() { + $this->data->user->add(self::USER1, "secret"); + $this->data->user->add(self::USER2, "secret"); + $this->assertCount(2,$this->data->user->list()); + } + + function testRemoveAUser() { + $this->data->user->add(self::USER1, "secret"); + $this->assertCount(1,$this->data->user->list()); + $this->data->user->remove(self::USER1); + $this->assertCount(0,$this->data->user->list()); + } + + function testRemoveAMissingUser() { + $this->assertException("doesNotExist", "User"); + $this->data->user->remove(self::USER1); + } + + function testAuthenticateAUser() { + $this->data->user->add(self::USER1, "secret"); + $this->assertTrue($this->data->user->auth(self::USER1, "secret")); + $this->assertFalse($this->data->user->auth(self::USER1, "superman")); } } diff --git a/tests/lib/User/DriverInternalMock.php b/tests/lib/User/DriverInternalMock.php index 8d0ad85..f592daf 100644 --- a/tests/lib/User/DriverInternalMock.php +++ b/tests/lib/User/DriverInternalMock.php @@ -1,11 +1,11 @@ Driver::FUNC_INTERNAL, @@ -44,7 +44,7 @@ final class DriverInternalMock implements Driver { function auth(string $user, string $password): bool { if(!$this->userExists($user)) return false; - if(password_verify($password, static::$db[$user]['password'])) return true; + if(password_verify($password, $this->db[$user]['password'])) return true; return false; } @@ -53,27 +53,28 @@ final class DriverInternalMock implements Driver { } function userExists(string $user): bool { - return array_key_exists($user, static::$db); + return array_key_exists($user, $this->db); } - function userAdd(string $user, string $password = null): bool { - if($this->userExists($user)) return false; + function userAdd(string $user, string $password = null): string { + if($this->userExists($user)) throw new Exception("alreadyExists", ["action" => __FUNCTION__, "user" => $user]); + if($password===null) $password = (new PassGen)->length($this->data->conf->userTempPasswordLength)->get(); $u = [ 'password' => $password ? password_hash($password, \PASSWORD_DEFAULT) : null, 'rights' => Driver::RIGHTS_NONE, ]; - static::$db[$user] = $u; - return true; + $this->db[$user] = $u; + return $password; } function userRemove(string $user): bool { - if(!$this->userExists($user)) return false; - unset(static::$db[$user]); + if(!$this->userExists($user)) throw new Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + unset($this->db[$user]); return true; } function userList(string $domain = null): array { - $list = array_keys(static::$db); + $list = array_keys($this->db); if($domain===null) { return $list; } else { @@ -85,32 +86,32 @@ final class DriverInternalMock implements Driver { } } - function userPasswordSet(string $user, string $newPassword, string $oldPassword): bool { - if(!$this->userExists($user)) return false; - if(!$this->auth($user, $oldPassword)) return false; - static::$db[$user]['password'] = password_hash($newPassword, \PASSWORD_DEFAULT); - return true; + function userPasswordSet(string $user, string $newPassword = null, string $oldPassword = null): string { + if(!$this->userExists($user)) throw new Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + if($newPassword===null) $newPassword = (new PassGen)->length($this->data->conf->userTempPasswordLength)->get(); + $this->db[$user]['password'] = password_hash($newPassword, \PASSWORD_DEFAULT); + return $newPassword; } function userPropertiesGet(string $user): array { - if(!$this->userExists($user)) return []; - return static::$db[$user]; + if(!$this->userExists($user)) throw new Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + return $this->db[$user]; } function userPropertiesSet(string $user, array $properties): array { - if(!$this->userExists($user)) return []; - static::$db[$user] = array_merge(static::$db[$user], $properties); + if(!$this->userExists($user)) throw new Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + $this->db[$user] = array_merge($this->db[$user], $properties); return $this->userPropertiesGet($user); } function userRightsGet(string $user): int { - if(!$this->userExists($user)) return Driver::RIGHTS_NONE; - return static::$db[$user]['rights']; + if(!$this->userExists($user)) throw new Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + return $this->db[$user]['rights']; } function userRightsSet(string $user, int $level): bool { - if(!$this->userExists($user)) return false; - static::$db[$user]['rights'] = $level; + if(!$this->userExists($user)) throw new Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + $this->db[$user]['rights'] = $level; return true; } } \ No newline at end of file