diff --git a/lib/Database.php b/lib/Database.php index 03300502..7182c6f7 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -6,7 +6,6 @@ declare(strict_types=1); namespace JKingWeb\Arsse; -use PasswordGenerator\Generator as PassGen; use JKingWeb\DrUUID\UUID; use JKingWeb\Arsse\Misc\Query; use JKingWeb\Arsse\Misc\Context; @@ -83,7 +82,7 @@ class Database { return $out; } - protected function generateIn(array $values, string $type) { + protected function generateIn(array $values, string $type): array { $out = [ [], // query clause [], // binding types @@ -122,21 +121,15 @@ class Database { return (bool) $this->db->prepare("SELECT count(*) from arsse_users where id = ?", "str")->run($user)->getValue(); } - public function userAdd(string $user, string $password = null): string { + public function userAdd(string $user, string $password): bool { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } elseif ($this->userExists($user)) { throw new User\Exception("alreadyExists", ["action" => __FUNCTION__, "user" => $user]); } - if ($password===null) { - $password = (new PassGen)->length(Arsse::$conf->userTempPasswordLength)->get(); - } - $hash = ""; - if (strlen($password) > 0) { - $hash = password_hash($password, \PASSWORD_DEFAULT); - } + $hash = (strlen($password) > 0) ? password_hash($password, \PASSWORD_DEFAULT) : ""; $this->db->prepare("INSERT INTO arsse_users(id,password) values(?,?)", "str", "str")->runArray([$user,$hash]); - return $password; + return true; } public function userRemove(string $user): bool { @@ -169,21 +162,15 @@ class Database { return (string) $this->db->prepare("SELECT password from arsse_users where id = ?", "str")->run($user)->getValue(); } - public function userPasswordSet(string $user, string $password = null): string { + public function userPasswordSet(string $user, string $password): bool { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } elseif (!$this->userExists($user)) { throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); } - if ($password===null) { - $password = (new PassGen)->length(Arsse::$conf->userTempPasswordLength)->get(); - } - $hash = ""; - if (strlen($password) > 0) { - $hash = password_hash($password, \PASSWORD_DEFAULT); - } + $hash = (strlen($password) > 0) ? password_hash($password, \PASSWORD_DEFAULT) : ""; $this->db->prepare("UPDATE arsse_users set password = ? where id = ?", "str", "str")->run($hash, $user); - return $password; + return true; } public function sessionCreate(string $user): string { diff --git a/lib/User.php b/lib/User.php index 6e474645..b0b05bcd 100644 --- a/lib/User.php +++ b/lib/User.php @@ -6,7 +6,7 @@ declare(strict_types=1); namespace JKingWeb\Arsse; -use JKingWeb\Arsse\User\Internal\Driver as InternalDriver; +use PasswordGenerator\Generator as PassGen; class User { @@ -80,7 +80,7 @@ class User { if (!$this->authorize($user, $func)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); } - return $this->u->userAdd($user, $password); + return $this->u->userAdd($user, $password) ?? $this->u->userAdd($user, $this->generatePassword()); } public function remove(string $user): bool { @@ -103,11 +103,15 @@ class User { if (!$this->authorize($user, $func)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); } - $out = $this->u->userPasswordSet($user, $newPassword, $oldPassword); - if (!$this->u instanceof InternalDriver && Arsse::$db->userExists($user)) { + $out = $this->u->userPasswordSet($user, $newPassword, $oldPassword) ?? $this->u->userPasswordSet($user, $this->generatePassword(), $oldPassword); + if (Arsse::$db->userExists($user)) { // if the password change was successful and the user exists, set the internal password to the same value Arsse::$db->userPasswordSet($user, $out); } return $out; } + + protected function generatePassword(): string { + return (new PassGen)->length(Arsse::$conf->userTempPasswordLength)->get(); + } } diff --git a/lib/User/Driver.php b/lib/User/Driver.php index 91aeaa0e..50ef8f3b 100644 --- a/lib/User/Driver.php +++ b/lib/User/Driver.php @@ -22,11 +22,11 @@ interface Driver { // checks whether a user exists public function userExists(string $user): bool; // adds a user - public function userAdd(string $user, string $password = null): string; + public function userAdd(string $user, string $password = null); // removes a user public function userRemove(string $user): bool; // lists all users public function userList(): array; // sets a user's password; if the driver does not require the old password, it may be ignored - public function userPasswordSet(string $user, string $newPassword = null, string $oldPassword = null): string; + public function userPasswordSet(string $user, string $newPassword = null, string $oldPassword = null); } diff --git a/lib/User/Internal/Driver.php b/lib/User/Internal/Driver.php index 5d65b764..b6dfe96c 100644 --- a/lib/User/Internal/Driver.php +++ b/lib/User/Internal/Driver.php @@ -34,8 +34,12 @@ class Driver implements \JKingWeb\Arsse\User\Driver { return Arsse::$db->userExists($user); } - public function userAdd(string $user, string $password = null): string { - return Arsse::$db->userAdd($user, $password); + public function userAdd(string $user, string $password = null) { + if (isset($password)) { + // only add the user if the password is not null; the user manager will retry with a generated password if null is returned + Arsse::$db->userAdd($user, $password); + } + return $password; } public function userRemove(string $user): bool { @@ -46,7 +50,8 @@ class Driver implements \JKingWeb\Arsse\User\Driver { return Arsse::$db->userList(); } - public function userPasswordSet(string $user, string $newPassword = null, string $oldPassword = null): string { - return Arsse::$db->userPasswordSet($user, $newPassword); + public function userPasswordSet(string $user, string $newPassword = null, string $oldPassword = null) { + // do nothing: the internal database is updated regardless of what the driver does (assuming it does not throw an exception) + return $newPassword; } } diff --git a/tests/lib/Database/SeriesUser.php b/tests/lib/Database/SeriesUser.php index ef704069..78d1f81b 100644 --- a/tests/lib/Database/SeriesUser.php +++ b/tests/lib/Database/SeriesUser.php @@ -60,35 +60,13 @@ trait SeriesUser { } public function testAddANewUser() { - $this->assertSame("", Arsse::$db->userAdd("john.doe@example.org", "")); + $this->assertTrue(Arsse::$db->userAdd("john.doe@example.org", "")); Phake::verify(Arsse::$user)->authorize("john.doe@example.org", "userAdd"); $state = $this->primeExpectations($this->data, ['arsse_users' => ['id','name','rights']]); $state['arsse_users']['rows'][] = ["john.doe@example.org", null, 0]; $this->compareExpectations($state); } - /** - * @depends testGetAPassword - * @depends testAddANewUser - */ - public function testAddANewUserWithARandomPassword() { - $user1 = "john.doe@example.org"; - $user2 = "john.doe@example.net"; - $pass1 = Arsse::$db->userAdd($user1); - $pass2 = Arsse::$db->userAdd($user2); - $this->assertSame(Arsse::$conf->userTempPasswordLength, strlen($pass1)); - $this->assertSame(Arsse::$conf->userTempPasswordLength, strlen($pass2)); - $this->assertNotEquals($pass1, $pass2); - $hash1 = Arsse::$db->userPasswordGet($user1); - $hash2 = Arsse::$db->userPasswordGet($user2); - Phake::verify(Arsse::$user)->authorize($user1, "userAdd"); - Phake::verify(Arsse::$user)->authorize($user2, "userAdd"); - Phake::verify(Arsse::$user)->authorize($user1, "userPasswordGet"); - Phake::verify(Arsse::$user)->authorize($user2, "userPasswordGet"); - $this->assertTrue(password_verify($pass1, $hash1), "Failed verifying password of $user1 '$pass1' against hash '$hash1'."); - $this->assertTrue(password_verify($pass2, $hash2), "Failed verifying password of $user2 '$pass2' against hash '$hash2'."); - } - public function testAddAnExistingUser() { $this->assertException("alreadyExists", "User"); Arsse::$db->userAdd("john.doe@example.com", ""); @@ -136,19 +114,14 @@ trait SeriesUser { */ public function testSetAPassword() { $user = "john.doe@example.com"; + $pass = "secret"; $this->assertEquals("", Arsse::$db->userPasswordGet($user)); - $pass = Arsse::$db->userPasswordSet($user, "secret"); + $this->assertTrue(Arsse::$db->userPasswordSet($user, $pass)); $hash = Arsse::$db->userPasswordGet($user); $this->assertNotEquals("", $hash); Phake::verify(Arsse::$user)->authorize($user, "userPasswordSet"); $this->assertTrue(password_verify($pass, $hash), "Failed verifying password of $user '$pass' against hash '$hash'."); } - public function testSetARandomPassword() { - $user = "john.doe@example.com"; - $this->assertEquals("", Arsse::$db->userPasswordGet($user)); - $pass = Arsse::$db->userPasswordSet($user); - $hash = Arsse::$db->userPasswordGet($user); - } public function testSetThePasswordOfAMissingUser() { $this->assertException("doesNotExist", "User");