Move password generation to the User class

This allows user drivers which wish to generate their own passwords to
do so, and those which do not to defer to the built-in generator

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 {

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();
+    }
 }

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);
 }

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;
     }
 }

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");