diff --git a/lib/ImportExport/AbstractImportExport.php b/lib/ImportExport/AbstractImportExport.php index 22c1f2b..7206482 100644 --- a/lib/ImportExport/AbstractImportExport.php +++ b/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 diff --git a/lib/ImportExport/OPML.php b/lib/ImportExport/OPML.php index 30a3cc5..30cb4f5 100644 --- a/lib/ImportExport/OPML.php +++ b/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 = []; diff --git a/lib/User.php b/lib/User.php index 37ae814..56e716b 100644 --- a/lib/User.php +++ b/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()); } diff --git a/lib/User/Driver.php b/lib/User/Driver.php index e36ca38..6bfa25b 100644 --- a/lib/User/Driver.php +++ b/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 user - public function userAdd(string $user, string $password = null); - // removes a user + + /** Adds a new user and returns their password + * + * 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 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); - // removes a user's password; this makes authentication fail unconditionally + + /** sets a user's password + * + * 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; } diff --git a/lib/User/ExceptionAuthz.php b/lib/User/ExceptionAuthz.php deleted file mode 100644 index 2d16f59..0000000 --- a/lib/User/ExceptionAuthz.php +++ /dev/null @@ -1,10 +0,0 @@ -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); + } } diff --git a/tests/cases/ImportExport/TestImportExport.php b/tests/cases/ImportExport/TestImportExport.php index 64eaf90..1a899c4 100644 --- a/tests/cases/ImportExport/TestImportExport.php +++ b/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 { diff --git a/tests/cases/ImportExport/TestOPML.php b/tests/cases/ImportExport/TestOPML.php index 1e65fa1..36caa77 100644 --- a/tests/cases/ImportExport/TestOPML.php +++ b/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::$user)->exists->thenReturn(true); + \Phake::when(Arsse::$db)->userExists->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"); } diff --git a/tests/cases/User/TestInternal.php b/tests/cases/User/TestInternal.php index 6a88b4d..21587f3 100644 --- a/tests/cases/User/TestInternal.php +++ b/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($drv)->userExists->thenReturn(true); - \Phake::verifyNoFurtherInteraction(Arsse::$db); - $this->assertTrue($drv->userPasswordUnset("john.doe@example.com")); + \Phake::when(Arsse::$db)->userExists->thenReturn(true); + $this->assertTrue((new Driver)->userPasswordUnset("john.doe@example.com")); + \Phake::verify(Arsse::$db, \Phake::times(0))->userPasswordUnset; } public function testUnsetAPasswordForAMssingUser(): void { - $drv = \Phake::partialMock(Driver::class); - \Phake::when($drv)->userExists->thenReturn(false); - \Phake::verifyNoFurtherInteraction(Arsse::$db); + \Phake::when(Arsse::$db)->userExists->thenReturn(false); $this->assertException("doesNotExist", "User"); - $drv->userPasswordUnset("john.doe@example.com"); + (new Driver)->userPasswordUnset("john.doe@example.com"); } } diff --git a/tests/cases/User/TestUser.php b/tests/cases/User/TestUser.php index 80be0e6..759241c 100644 --- a/tests/cases/User/TestUser.php +++ b/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);