From d3ebb1bd56cd85d055ccc042faeae4a1279e68fa Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 17 Nov 2020 16:23:36 -0500 Subject: [PATCH] Last set of tests for user management. Fixes #180 --- lib/User.php | 11 ++-- locale/en.php | 2 + tests/cases/User/TestUser.php | 111 ++++++++++++++++++++++++++++++++-- 3 files changed, 114 insertions(+), 10 deletions(-) diff --git a/lib/User.php b/lib/User.php index c44a2068..0a70e5d2 100644 --- a/lib/User.php +++ b/lib/User.php @@ -69,6 +69,7 @@ class User { } return $out; } + public function remove(string $user): bool { try { @@ -141,15 +142,15 @@ class User { $in = []; if (array_key_exists("tz", $data)) { if (!is_string($data['tz'])) { - throw new User\ExceptionInput("invalidTimezone"); - } elseif (!in_array($data['tz'], \DateTimeZone::listIdentifiers())) { - throw new User\ExceptionInput("invalidTimezone", $data['tz']); + throw new User\ExceptionInput("invalidTimezone", ['field' => "tz", 'value' => ""]); + } elseif(!@timezone_open($data['tz'])) { + throw new User\ExceptionInput("invalidTimezone", ['field' => "tz", 'value' => $data['tz']]); } $in['tz'] = $data['tz']; } foreach (["admin", "sort_asc"] as $k) { if (array_key_exists($k, $data)) { - if (($v = V::normalize($data[$k], V::T_BOOL)) === null) { + if (($v = V::normalize($data[$k], V::T_BOOL | V::M_DROP)) === null) { throw new User\ExceptionInput("invalidBoolean", $k); } $in[$k] = $v; @@ -161,7 +162,7 @@ class User { $out = $this->u->userPropertiesSet($user, $in); // synchronize the internal database if (!Arsse::$db->userExists($user)) { - Arsse::$db->userAdd($user, $this->generatePassword()); + Arsse::$db->userAdd($user, null); } Arsse::$db->userPropertiesSet($user, $out); return $out; diff --git a/locale/en.php b/locale/en.php index 2791c5b1..66a03eee 100644 --- a/locale/en.php +++ b/locale/en.php @@ -140,6 +140,8 @@ return [ 'Exception.JKingWeb/Arsse/User/Exception.authFailed' => 'Authentication failed', 'Exception.JKingWeb/Arsse/User/ExceptionSession.invalid' => 'Session with ID {0} does not exist', 'Exception.JKingWeb/Arsse/User/ExceptionInput.invalidUsername' => 'User names may not contain the Unicode character {0}', + 'Exception.JKingWeb/Arsse/User/ExceptionInput.invalidBoolean' => 'User property "{0}" must be a boolean value (true or false)', + 'Exception.JKingWeb/Arsse/User/ExceptionInput.invalidTimezone' => 'User property "{field}" must be a valid zoneinfo timezone', 'Exception.JKingWeb/Arsse/Feed/Exception.internalError' => 'Could not download feed "{url}" because of an internal error which is probably a bug', 'Exception.JKingWeb/Arsse/Feed/Exception.invalidCertificate' => 'Could not download feed "{url}" because its server is serving an invalid SSL certificate', 'Exception.JKingWeb/Arsse/Feed/Exception.invalidUrl' => 'Feed URL "{url}" is invalid', diff --git a/tests/cases/User/TestUser.php b/tests/cases/User/TestUser.php index e958c557..310a0a33 100644 --- a/tests/cases/User/TestUser.php +++ b/tests/cases/User/TestUser.php @@ -9,6 +9,7 @@ namespace JKingWeb\Arsse\TestCase\User; use JKingWeb\Arsse\Arsse; use JKingWeb\Arsse\Database; use JKingWeb\Arsse\User; +use JKingWeb\Arsse\AbstractException as Exception; use JKingWeb\Arsse\User\ExceptionConflict; use JKingWeb\Arsse\User\ExceptionInput; use JKingWeb\Arsse\User\Driver; @@ -24,7 +25,7 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { // create a mock user driver $this->drv = \Phake::mock(Driver::class); } - + public function tearDown(): void { \Phake::verifyNoOtherInteractions($this->drv); \Phake::verifyNoOtherInteractions(Arsse::$db); @@ -42,6 +43,13 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { $u->id = null; $this->assertSame("", (string) $u); } + + public function testGeneratePasswords(): void { + $u = new User($this->drv); + $pass1 = $u->generatePassword(); + $pass2 = $u->generatePassword(); + $this->assertNotEquals($pass1, $pass2); + } /** @dataProvider provideAuthentication */ public function testAuthenticateAUser(bool $preAuth, string $user, string $password, bool $exp): void { @@ -158,7 +166,7 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { \Phake::verify($this->drv)->userAdd($user, $pass); \Phake::verify(Arsse::$db)->userExists($user); } - + public function testRemoveAUser(): void { $user = "john.doe@example.com"; $pass = "secret"; @@ -170,7 +178,7 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { \Phake::verify(Arsse::$db)->userRemove($user); \Phake::verify($this->drv)->userRemove($user); } - + public function testRemoveAUserWeDoNotKnow(): void { $user = "john.doe@example.com"; $pass = "secret"; @@ -181,7 +189,7 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { \Phake::verify(Arsse::$db)->userExists($user); \Phake::verify($this->drv)->userRemove($user); } - + public function testRemoveAMissingUser(): void { $user = "john.doe@example.com"; $pass = "secret"; @@ -197,7 +205,7 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { \Phake::verify($this->drv)->userRemove($user); } } - + public function testRemoveAMissingUserWeDoNotKnow(): void { $user = "john.doe@example.com"; $pass = "secret"; @@ -343,4 +351,97 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { [['num' => 1, 'admin' => true, 'lang' => null, 'tz' => "America/Toronto", 'sort_asc' => true], ['num' => 1, 'admin' => true, 'lang' => "fr", 'tz' => "America/Toronto", 'sort_asc' => true], ['lang' => null]], ]; } + + public function testGetThePropertiesOfAUserWeDoNotKnow(): void { + $user = "john.doe@example.com"; + $extra = ['tz' => "Europe/Istanbul"]; + $base = ['num' => 47, 'admin' => false, 'lang' => null, 'tz' => "Etc/UTC", 'sort_asc' => false]; + $exp = ['num' => 47, 'admin' => false, 'lang' => null, 'tz' => "Europe/Istanbul", 'sort_asc' => false]; + $u = new User($this->drv); + \Phake::when($this->drv)->userPropertiesGet->thenReturn($extra); + \Phake::when(Arsse::$db)->userPropertiesGet->thenReturn($base); + \Phake::when(Arsse::$db)->userAdd->thenReturn(true); + \Phake::when(Arsse::$db)->userExists->thenReturn(false); + $this->assertSame($exp, $u->propertiesGet($user)); + \Phake::verify($this->drv)->userPropertiesGet($user); + \Phake::verify(Arsse::$db)->userPropertiesGet($user); + \Phake::verify(Arsse::$db)->userPropertiesSet($user, $extra); + \Phake::verify(Arsse::$db)->userAdd($user, null); + \Phake::verify(Arsse::$db)->userExists($user); + } + + public function testGetThePropertiesOfAMissingUser(): void { + $user = "john.doe@example.com"; + $u = new User($this->drv); + \Phake::when($this->drv)->userPropertiesGet->thenThrow(new ExceptionConflict("doesNotExist")); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); + try { + $u->propertiesGet($user); + } finally { + \Phake::verify($this->drv)->userPropertiesGet($user); + } + } + + /** @dataProvider providePropertyChanges */ + public function testSetThePropertiesOfAUser(array $in, $out): void { + $user = "john.doe@example.com"; + $u = new User($this->drv); + if ($out instanceof \Exception) { + $this->assertException($out); + $u->propertiesSet($user, $in); + } else { + \Phake::when(Arsse::$db)->userExists->thenReturn(true); + \Phake::when($this->drv)->userPropertiesSet->thenReturn($out); + \Phake::when(Arsse::$db)->userPropertiesSet->thenReturn(true); + $this->assertSame($out, $u->propertiesSet($user, $in)); + \Phake::verify($this->drv)->userPropertiesSet($user, $in); + \Phake::verify(Arsse::$db)->userPropertiesSet($user, $out); + \Phake::verify(Arsse::$db)->userExists($user); + } + } + + /** @dataProvider providePropertyChanges */ + public function testSetThePropertiesOfAUserWeDoNotKnow(array $in, $out): void { + $user = "john.doe@example.com"; + $u = new User($this->drv); + if ($out instanceof \Exception) { + $this->assertException($out); + $u->propertiesSet($user, $in); + } else { + \Phake::when(Arsse::$db)->userExists->thenReturn(false); + \Phake::when($this->drv)->userPropertiesSet->thenReturn($out); + \Phake::when(Arsse::$db)->userPropertiesSet->thenReturn(true); + $this->assertSame($out, $u->propertiesSet($user, $in)); + \Phake::verify($this->drv)->userPropertiesSet($user, $in); + \Phake::verify(Arsse::$db)->userPropertiesSet($user, $out); + \Phake::verify(Arsse::$db)->userExists($user); + \Phake::verify(Arsse::$db)->userAdd($user, null); + } + } + + public function providePropertyChanges(): iterable { + return [ + [['admin' => true], ['admin' => true]], + [['admin' => 2], new ExceptionInput("invalidBoolean")], + [['sort_asc' => 2], new ExceptionInput("invalidBoolean")], + [['tz' => "Etc/UTC"], ['tz' => "Etc/UTC"]], + [['tz' => "Etc/blah"], new ExceptionInput("invalidTimezone")], + [['tz' => false], new ExceptionInput("invalidTimezone")], + [['lang' => "en-ca"], ['lang' => "en-CA"]], + [['lang' => null], ['lang' => null]], + ]; + } + + public function testSetThePropertiesOfAMissingUser(): void { + $user = "john.doe@example.com"; + $in = ['admin' => true]; + $u = new User($this->drv); + \Phake::when($this->drv)->userPropertiesSet->thenThrow(new ExceptionConflict("doesNotExist")); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); + try { + $u->propertiesSet($user, $in); + } finally { + \Phake::verify($this->drv)->userPropertiesSet($user, $in); + } + } }