Clean up user driver API

- It is no longer assumed a driver knows whether a user exists
- The $password param is now required (but nullable when setting

lib/ImportExport/AbstractImportExport.php

@@ -13,7 +13,7 @@ use JKingWeb\Arsse\User\Exception as UserException;
 
 abstract class AbstractImportExport {
     public function import(string $user, string $data, bool $flat = false, bool $replace = false): bool {
-        if (!Arsse::$user->exists($user)) {
+        if (!Arsse::$db->userExists($user)) {
             throw new UserException("doesNotExist", ["action" => __FUNCTION__, "user" => $user]);
         }
         // first extract useful information from the input

lib/ImportExport/OPML.php

@@ -91,7 +91,7 @@ class OPML extends AbstractImportExport {
     }
 
     public function export(string $user, bool $flat = false): string {
-        if (!Arsse::$user->exists($user)) {
+        if (!Arsse::$db->userExists($user)) {
             throw new UserException("doesNotExist", ["action" => __FUNCTION__, "user" => $user]);
         }
         $tags = [];

lib/User.php

@@ -48,10 +48,6 @@ class User {
         return $this->u->userList();
     }
 
-    public function exists(string $user): bool {
-        return $this->u->userExists($user);
-    }
-
     public function add($user, $password = null): string {
         return $this->u->userAdd($user, $password) ?? $this->u->userAdd($user, $this->generatePassword());
     }

lib/User/Driver.php

@@ -7,26 +7,49 @@ declare(strict_types=1);
 namespace JKingWeb\Arsse\User;
 
 interface Driver {
-    public const FUNC_NOT_IMPLEMENTED = 0;
-    public const FUNC_INTERNAL = 1;
-    public const FUNC_EXTERNAL = 2;
-
-    // returns an instance of a class implementing this interface.
     public function __construct();
-    // returns a human-friendly name for the driver (for display in installer, for example)
+
+    /** Returns a human-friendly name for the driver (for display in installer, for example) */
     public static function driverName(): string;
-    // authenticates a user against their name and password
+
+    /** Authenticates a user against their name and password */
     public function auth(string $user, string $password): bool;
-    // checks whether a user exists
+
-    public function userExists(string $user): bool;
+    /** Adds a new user and returns their password
-    // adds a user
+     * 
-    public function userAdd(string $user, string $password = null);
+     * When given no password the implementation may return null; the user
-    // removes a user
+     * manager will then generate a random password and try again with that
+     * password. Alternatively the implementation may generate its own 
+     * password if desired
+     * 
+     * @param string $user The username to create
+     * @param string|null $password The cleartext password to assign to the user, or null to generate a random password
+     */
+    public function userAdd(string $user, string $password = null): ?string;
+
+    /** Removes a user */
     public function userRemove(string $user): bool;
-    // lists all users
+
+    /** 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);
+    /** sets a user's password
-    // removes a user's password; this makes authentication fail unconditionally
+     * 
+     * When given no password the implementation may return null; the user
+     * manager will then generate a random password and try again with that
+     * password. Alternatively the implementation may generate its own 
+     * password if desired
+     * 
+     * @param string $user The user for whom to change the password
+     * @param string|null $password The cleartext password to assign to the user, or null to generate a random password
+     * @param string|null $oldPassword The user's previous password, if known
+     */
+    public function userPasswordSet(string $user, ?string $newPassword, string $oldPassword = null);
+
+    /** removes a user's password; this makes authentication fail unconditionally 
+     * 
+     * @param string $user The user for whom to change the password
+     * @param string|null $oldPassword The user's previous password, if known
+     */
     public function userPasswordUnset(string $user, string $oldPassword = null): bool;
 }

lib/User/ExceptionAuthz.php

@@ -1,10 +0,0 @@
-<?php
-/** @license MIT
- * Copyright 2017 J. King, Dustin Wilson et al.
- * See LICENSE and AUTHORS files for details */
-
-declare(strict_types=1);
-namespace JKingWeb\Arsse\User;
-
-class ExceptionAuthz extends Exception {
-}

lib/User/Internal/Driver.php

@@ -32,10 +32,6 @@ class Driver implements \JKingWeb\Arsse\User\Driver {
         return password_verify($password, $hash);
     }
 
-    public function userExists(string $user): bool {
-        return Arsse::$db->userExists($user);
-    }
-
     public function userAdd(string $user, string $password = null): ?string {
         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
@@ -52,7 +48,7 @@ class Driver implements \JKingWeb\Arsse\User\Driver {
         return Arsse::$db->userList();
     }
 
-    public function userPasswordSet(string $user, string $newPassword = null, string $oldPassword = null): ?string {
+    public function userPasswordSet(string $user, ?string $newPassword, string $oldPassword = null): ?string {
         // do nothing: the internal database is updated regardless of what the driver does (assuming it does not throw an exception)
         return $newPassword;
     }
@@ -70,4 +66,8 @@ class Driver implements \JKingWeb\Arsse\User\Driver {
     protected function userPasswordGet(string $user): ?string {
         return Arsse::$db->userPasswordGet($user);
     }
+
+    protected function userExists(string $user): bool {
+        return Arsse::$db->userExists($user);
+    }
 }

tests/cases/ImportExport/TestImportExport.php

@@ -27,7 +27,6 @@ class TestImportExport extends \JKingWeb\Arsse\Test\AbstractTest {
         self::clearData();
         // create a mock user manager
         Arsse::$user = \Phake::mock(\JKingWeb\Arsse\User::class);
-        \Phake::when(Arsse::$user)->exists->thenReturn(true);
         // create a mock Import/Export processor
         $this->proc = \Phake::partialMock(AbstractImportExport::class);
         // initialize an SQLite memeory database
@@ -147,9 +146,8 @@ class TestImportExport extends \JKingWeb\Arsse\Test\AbstractTest {
     }
 
     public function testImportForAMissingUser(): void {
-        \Phake::when(Arsse::$user)->exists->thenReturn(false);
         $this->assertException("doesNotExist", "User");
-        $this->proc->import("john.doe@example.com", "", false, false);
+        $this->proc->import("no.one@example.com", "", false, false);
     }
 
     public function testImportWithInvalidFolder(): void {

tests/cases/ImportExport/TestOPML.php

@@ -82,8 +82,7 @@ OPML_EXPORT_SERIALIZATION;
     public function setUp(): void {
         self::clearData();
         Arsse::$db = \Phake::mock(\JKingWeb\Arsse\Database::class);
-        Arsse::$user = \Phake::mock(\JKingWeb\Arsse\User::class);
+        \Phake::when(Arsse::$db)->userExists->thenReturn(true);
-        \Phake::when(Arsse::$user)->exists->thenReturn(true);
     }
 
     public function testExportToOpml(): void {
@@ -101,7 +100,7 @@ OPML_EXPORT_SERIALIZATION;
     }
 
     public function testExportToOpmlAMissingUser(): void {
-        \Phake::when(Arsse::$user)->exists->thenReturn(false);
+        \Phake::when(Arsse::$db)->userExists->thenReturn(false);
         $this->assertException("doesNotExist", "User");
         (new OPML)->export("john.doe@example.com");
     }

tests/cases/User/TestInternal.php

@@ -75,18 +75,6 @@ class TestInternal extends \JKingWeb\Arsse\Test\AbstractTest {
         \Phake::verify(Arsse::$db, \Phake::times(2))->userList;
     }
 
-    public function testCheckThatAUserExists(): void {
-        $john = "john.doe@example.com";
-        $jane = "jane.doe@example.com";
-        \Phake::when(Arsse::$db)->userExists($john)->thenReturn(true);
-        \Phake::when(Arsse::$db)->userExists($jane)->thenReturn(false);
-        $driver = new Driver;
-        $this->assertTrue($driver->userExists($john));
-        \Phake::verify(Arsse::$db)->userExists($john);
-        $this->assertFalse($driver->userExists($jane));
-        \Phake::verify(Arsse::$db)->userExists($jane);
-    }
-
     public function testAddAUser(): void {
         $john = "john.doe@example.com";
         \Phake::when(Arsse::$db)->userAdd->thenReturnCallback(function($user, $pass) {
@@ -119,20 +107,18 @@ class TestInternal extends \JKingWeb\Arsse\Test\AbstractTest {
         \Phake::verifyNoFurtherInteraction(Arsse::$db);
         $this->assertSame("superman", (new Driver)->userPasswordSet($john, "superman"));
         $this->assertSame(null, (new Driver)->userPasswordSet($john, null));
+        \Phake::verify(Arsse::$db, \Phake::times(0))->userPasswordSet;
     }
 
     public function testUnsetAPassword(): void {
-        $drv = \Phake::partialMock(Driver::class);
+        \Phake::when(Arsse::$db)->userExists->thenReturn(true);
-        \Phake::when($drv)->userExists->thenReturn(true);
+        $this->assertTrue((new Driver)->userPasswordUnset("john.doe@example.com"));
-        \Phake::verifyNoFurtherInteraction(Arsse::$db);
+        \Phake::verify(Arsse::$db, \Phake::times(0))->userPasswordUnset;
-        $this->assertTrue($drv->userPasswordUnset("john.doe@example.com"));
     }
 
     public function testUnsetAPasswordForAMssingUser(): void {
-        $drv = \Phake::partialMock(Driver::class);
+        \Phake::when(Arsse::$db)->userExists->thenReturn(false);
-        \Phake::when($drv)->userExists->thenReturn(false);
-        \Phake::verifyNoFurtherInteraction(Arsse::$db);
         $this->assertException("doesNotExist", "User");
-        $drv->userPasswordUnset("john.doe@example.com");
+        (new Driver)->userPasswordUnset("john.doe@example.com");
     }
 }

tests/cases/User/TestUser.php

@@ -83,23 +83,6 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest {
         ];
     }
 
-    /** @dataProvider provideExistence */
-    public function testCheckThatAUserExists(string $user, $exp): void {
-        $u = new User($this->drv);
-        \Phake::when($this->drv)->userExists("john.doe@example.com")->thenReturn(true);
-        \Phake::when($this->drv)->userExists("jane.doe@example.com")->thenReturn(false);
-        $this->assertSame($exp, $u->exists($user));
-    }
-
-    public function provideExistence(): iterable {
-        $john = "john.doe@example.com";
-        $jane = "jane.doe@example.com";
-        return [
-            [$john, true],
-            [$jane, false],
-        ];
-    }
-
     /** @dataProvider provideAdditions */
     public function testAddAUser(string $user, $password, $exp): void {
         $u = new User($this->drv);