diff --git a/lib/Database.php b/lib/Database.php index 1b2ddf2b..37d4ab63 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -278,10 +278,6 @@ class Database { if(!Data::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } - // If the user doesn't exist throw an exception. - if(!$this->userExists($user)) { - throw new User\Exception("doesNotExist", ["user" => $user, "action" => __FUNCTION__]); - } // if the desired folder name is missing or invalid, throw an exception if(!array_key_exists("name", $data) || $data['name']=="") { throw new Db\ExceptionInput("missing", ["action" => __FUNCTION__, "field" => "name"]); @@ -312,10 +308,6 @@ class Database { if(!Data::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } - // if the user doesn't exist throw an exception. - if(!$this->userExists($user)) { - throw new User\Exception("doesNotExist", ["user" => $user, "action" => __FUNCTION__]); - } // check to make sure the parent exists, if one is specified if(!is_null($parent)) { if(!$this->db->prepare("SELECT count(*) from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $parent)->getValue()) { @@ -335,7 +327,6 @@ class Database { public function folderRemove(string $user, int $id): bool { if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["user" => $user, "action" => __FUNCTION__]); $changes = $this->db->prepare("DELETE FROM arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $id)->changes(); if(!$changes) throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id]); return true; @@ -343,7 +334,6 @@ class Database { public function folderPropertiesGet(string $user, int $id): array { if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["user" => $user, "action" => __FUNCTION__]); $props = $this->db->prepare("SELECT id,name,parent from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $id)->getRow(); if(!$props) throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id]); return $props; @@ -351,7 +341,6 @@ class Database { public function folderPropertiesSet(string $user, int $id, array $data): bool { if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["user" => $user, "action" => __FUNCTION__]); // layer the existing folder properties onto the new desired ones; this also has the effect of checking whether the folder is valid $data = array_merge($this->folderPropertiesGet($user, $id), $data); // if the desired folder name is missing or invalid, throw an exception @@ -394,7 +383,6 @@ class Database { public function subscriptionAdd(string $user, string $url, string $fetchUser = "", string $fetchPassword = ""): int { if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["user" => $user, "action" => __FUNCTION__]); // check to see if the feed exists $feedID = $this->db->prepare("SELECT id from arsse_feeds where url is ? and username is ? and password is ?", "str", "str", "str")->run($url, $fetchUser, $fetchPassword)->getValue(); if(is_null($feedID)) { @@ -415,7 +403,6 @@ class Database { public function subscriptionList(string $user, int $folder = null, int $id = null): Db\Result { if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["user" => $user, "action" => __FUNCTION__]); // check to make sure the folder exists, if one is specified $query = "SELECT @@ -442,7 +429,9 @@ class Database { public function subscriptionRemove(string $user, int $id): bool { if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - return (bool) $this->db->prepare("DELETE from arsse_subscriptions where owner is ? and id is ?", "str", "int")->run($user, $id)->changes(); + $changes = $this->db->prepare("DELETE from arsse_subscriptions where owner is ? and id is ?", "str", "int")->run($user, $id)->changes(); + if(!$changes) throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id]); + return true; } public function subscriptionPropertiesGet(string $user, int $id): array { @@ -457,7 +446,6 @@ class Database { public function subscriptionPropertiesSet(string $user, int $id, array $data): bool { if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["user" => $user, "action" => __FUNCTION__]); $tr = $this->db->begin(); if(!$this->db->prepare("SELECT count(*) from arsse_subscriptions where owner is ? and id is ?", "str", "int")->run($user, $id)->getValue()) { // if the ID doesn't exist or doesn't belong to the user, throw an exception diff --git a/lib/User.php b/lib/User.php index 20c347e9..7ea4b852 100644 --- a/lib/User.php +++ b/lib/User.php @@ -6,7 +6,7 @@ class User { public $id = null; protected $u; - protected $authz = true; + protected $authz = 0; protected $authzSupported = 0; protected $actor = []; @@ -36,14 +36,18 @@ class User { // checks whether the logged in user is authorized to act for the affected user (used especially when granting rights) function authorize(string $affectedUser, string $action, int $newRightsLevel = 0): bool { // if authorization checks are disabled (either because we're running the installer or the background updater) just return true - if(!$this->authz) return true; + if(!$this->authorizationEnabled()) return true; // if we don't have a logged-in user, fetch credentials if($this->id===null) $this->credentials(); // if the affected user is the actor and the actor is not trying to grant themselves rights, accept the request if($affectedUser==Data::$user->id && $action != "userRightsSet") return true; + // if we're authorizing something other than a user function and the affected user is not the actor, make sure the affected user exists + $this->authorizationEnabled(false); + if(Data::$user->id != $affectedUser && strpos($action, "user")!==0 && !$this->exists($affectedUser)) throw new User\Exception("doesNotExist", ["action" => $action, "user" => $affectedUser]); + $this->authorizationEnabled(true); // get properties of actor if not already available if(!sizeof($this->actor)) $this->actor = $this->propertiesGet(Data::$user->id); - $rights =& $this->actor["rights"]; + $rights = $this->actor["rights"]; // if actor is a global admin, accept the request if($rights==User\Driver::RIGHTS_GLOBAL_ADMIN) return true; // if actor is a common user, deny the request @@ -162,9 +166,10 @@ class User { } public function authorizationEnabled(bool $setting = null): bool { - if($setting===null) return $this->authz; - $this->authz = $setting; - return $setting; + if(is_null($setting)) return !$this->authz; + $this->authz += ($setting ? -1 : 1); + if($this->authz < 0) $this->authz = 0; + return !$this->authz; } public function exists(string $user): bool { diff --git a/tests/User/TestAuthorization.php b/tests/User/TestAuthorization.php index 058b2413..c7a68568 100644 --- a/tests/User/TestAuthorization.php +++ b/tests/User/TestAuthorization.php @@ -1,6 +1,7 @@ authorizationEnabled(false); + Data::$user = Phake::PartialMock(User::class); + Phake::when(Data::$user)->authorize->thenReturn(true); foreach(self::USERS as $user => $level) { Data::$user->add($user, ""); Data::$user->rightsSet($user, $level); } - Data::$user->authorizationEnabled(true); + Phake::reset(Data::$user); } function tearDown() { $this->clearData(); } + function testToggleLogic() { + $this->assertTrue(Data::$user->authorizationEnabled()); + $this->assertTrue(Data::$user->authorizationEnabled(true)); + $this->assertFalse(Data::$user->authorizationEnabled(false)); + $this->assertFalse(Data::$user->authorizationEnabled(false)); + $this->assertFalse(Data::$user->authorizationEnabled(true)); + $this->assertTrue(Data::$user->authorizationEnabled(true)); + } + function testSelfActionLogic() { foreach(array_keys(self::USERS) as $user) { Data::$user->auth($user, ""); @@ -298,4 +308,11 @@ class TestAuthorization extends \PHPUnit\Framework\TestCase { } return $err; } + + function testMissingUserLogic() { + Data::$user->auth("gadm@example.com", ""); + $this->assertTrue(Data::$user->authorize("user@example.com", "someFunction")); + $this->assertException("doesNotExist", "User"); + Data::$user->authorize("this_user_does_not_exist@example.org", "someFunction"); + } } \ No newline at end of file diff --git a/tests/lib/Database/SeriesFolder.php b/tests/lib/Database/SeriesFolder.php index c21cf34a..2042c490 100644 --- a/tests/lib/Database/SeriesFolder.php +++ b/tests/lib/Database/SeriesFolder.php @@ -45,11 +45,6 @@ trait SeriesFolder { Data::$db->folderAdd("john.doe@example.com", ['name' => "Sociology", 'parent' => 4]); // folder ID 4 belongs to Jane } - function testAddAFolderForAMissingUser() { - $this->assertException("doesNotExist", "User"); - Data::$db->folderAdd("john.doe@example.org", ['name' => "Sociology"]); - } - function testAddAFolderWithAMissingName() { $this->assertException("missing", "Db", "ExceptionInput"); Data::$db->folderAdd("john.doe@example.com", []); @@ -119,11 +114,6 @@ trait SeriesFolder { Data::$db->folderList("john.doe@example.com", 4); // folder ID 4 belongs to Jane } - function testListFoldersForAMissingUser() { - $this->assertException("doesNotExist", "User"); - Data::$db->folderList("john.doe@example.org"); - } - function testListFoldersWithoutAuthority() { Phake::when(Data::$user)->authorize->thenReturn(false); $this->assertException("notAuthorized", "User", "ExceptionAuthz"); @@ -158,11 +148,6 @@ trait SeriesFolder { Data::$db->folderRemove("john.doe@example.com", 4); // folder ID 4 belongs to Jane } - function testRemoveAFolderForAMissingUser() { - $this->assertException("doesNotExist", "User"); - Data::$db->folderRemove("john.doe@example.org", 1); - } - function testRemoveAFolderWithoutAuthority() { Phake::when(Data::$user)->authorize->thenReturn(false); $this->assertException("notAuthorized", "User", "ExceptionAuthz"); @@ -189,11 +174,6 @@ trait SeriesFolder { Data::$db->folderPropertiesGet("john.doe@example.com", 4); // folder ID 4 belongs to Jane } - function testGetThePropertiesOfAFolderForAMissingUser() { - $this->assertException("doesNotExist", "User"); - Data::$db->folderPropertiesGet("john.doe@example.org", 1); - } - function testGetThePropertiesOfAFolderWithoutAuthority() { Phake::when(Data::$user)->authorize->thenReturn(false); $this->assertException("notAuthorized", "User", "ExceptionAuthz"); @@ -246,11 +226,6 @@ trait SeriesFolder { Data::$db->folderPropertiesSet("john.doe@example.com", 4, ['parent' => null]); // folder ID 4 belongs to Jane } - function testSetThePropertiesOfAFolderForAMissingUser() { - $this->assertException("doesNotExist", "User"); - Data::$db->folderPropertiesSet("john.doe@example.org", 1, ['parent' => null]); - } - function testSetThePropertiesOfAFolderWithoutAuthority() { Phake::when(Data::$user)->authorize->thenReturn(false); $this->assertException("notAuthorized", "User", "ExceptionAuthz"); diff --git a/tests/lib/Database/SeriesSubscription.php b/tests/lib/Database/SeriesSubscription.php index e62ea4e2..825ba756 100644 --- a/tests/lib/Database/SeriesSubscription.php +++ b/tests/lib/Database/SeriesSubscription.php @@ -20,6 +20,7 @@ trait SeriesSubscription { ], 'rows' => [ [1,"http://example.com/feed1", "Ook", "", ""], + [2,"http://example.com/feed2", "Eek", "", ""], ] ], 'arsse_subscriptions' => [ @@ -27,13 +28,14 @@ trait SeriesSubscription { 'id' => "int", 'owner' => "str", 'feed' => "int", + 'title' => "str", ], 'rows' => [ - + [1,"john.doe@example.com",2,null], ] ] ]; - // merge folder table with base user table + // merge tables $this->data = array_merge($this->data, $data); $this->primeDatabase($this->data); // initialize a partial mock of the Database object to later manipulate the feedUpdate method @@ -78,7 +80,6 @@ trait SeriesSubscription { $user = "john.doe@example.com"; $url = "http://example.org/feed1"; $feedID = $this->nextID("arsse_feeds"); - $subID = $this->nextID("arsse_subscriptions"); Phake::when(Data::$db)->feedUpdate->thenThrow(new FeedException($url, new \PicoFeed\Client\InvalidUrlException())); try { Data::$db->subscriptionAdd($user, $url); @@ -94,4 +95,19 @@ trait SeriesSubscription { throw $e; } } + + function testAddADuplicateSubscription() { + $user = "john.doe@example.com"; + $url = "http://example.com/feed2"; + $this->assertException("constraintViolation", "Db", "ExceptionInput"); + Data::$db->subscriptionAdd($user, $url); + } + + function testAddAFeedWithoutAuthority() { + $user = "john.doe@example.com"; + $url = "http://example.com/feed1"; + Phake::when(Data::$user)->authorize->thenReturn(false); + $this->assertException("notAuthorized", "User", "ExceptionAuthz"); + Data::$db->subscriptionAdd($user, $url); + } } \ No newline at end of file diff --git a/tests/lib/User/CommonTests.php b/tests/lib/User/CommonTests.php index 7122fd26..48275445 100644 --- a/tests/lib/User/CommonTests.php +++ b/tests/lib/User/CommonTests.php @@ -5,6 +5,7 @@ use JKingWeb\Arsse\Data; use JKingWeb\Arsse\Conf; use JKingWeb\Arsse\User; use JKingWeb\Arsse\User\Driver; +use Phake; trait CommonTests { @@ -15,8 +16,8 @@ trait CommonTests { $conf->userAuthPreferHTTP = true; Data::$conf = $conf; Data::$db = new Database(); - Data::$user = new User(); - Data::$user->authorizationEnabled(false); + Data::$user = Phake::PartialMock(User::class); + Phake::when(Data::$user)->authorize->thenReturn(true); $_SERVER['PHP_AUTH_USER'] = self::USER1; $_SERVER['PHP_AUTH_PW'] = "secret"; // call the additional setup method if it exists