From 06dee77bac15a3fb0b91976d3b234c0e6af63eea Mon Sep 17 00:00:00 2001 From: "J. King" Date: Mon, 23 Nov 2020 09:31:50 -0500 Subject: [PATCH] First tests for Miniflux --- lib/Database.php | 9 +- lib/REST.php | 12 +- lib/REST/Miniflux/ErrorResponse.php | 19 +++ lib/REST/Miniflux/Status.php | 37 ++++++ lib/REST/Miniflux/V1.php | 56 +++++++-- lib/User.php | 3 +- locale/en.php | 3 + tests/cases/Database/SeriesToken.php | 21 +++- .../cases/REST/Miniflux/TestErrorResponse.php | 22 ++++ tests/cases/REST/Miniflux/TestStatus.php | 34 +++++ tests/cases/REST/Miniflux/TestV1.php | 118 ++++++++++++++++++ tests/cases/User/TestUser.php | 21 ++-- tests/phpunit.dist.xml | 6 +- 13 files changed, 328 insertions(+), 33 deletions(-) create mode 100644 lib/REST/Miniflux/ErrorResponse.php create mode 100644 lib/REST/Miniflux/Status.php create mode 100644 tests/cases/REST/Miniflux/TestErrorResponse.php create mode 100644 tests/cases/REST/Miniflux/TestStatus.php create mode 100644 tests/cases/REST/Miniflux/TestV1.php diff --git a/lib/Database.php b/lib/Database.php index ace75a8..760a0de 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -400,7 +400,7 @@ class Database { * @param \DateTimeInterface|null $expires An optional expiry date and time for the token * @param string $data Application-specific data associated with a token */ - public function tokenCreate(string $user, string $class, string $id = null, \DateTimeInterface $expires = null, string $data = null): string { + public function tokenCreate(string $user, string $class, string $id = null, ?\DateTimeInterface $expires = null, string $data = null): string { if (!$this->userExists($user)) { throw new User\ExceptionConflict("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); } @@ -418,7 +418,7 @@ class Database { * @param string $class The class of the token e.g. the protocol name * @param string|null $id The ID of a specific token, or null for all tokens in the class */ - public function tokenRevoke(string $user, string $class, string $id = null): bool { + public function tokenRevoke(string $user, string $class, ?string $id = null): bool { if (is_null($id)) { $out = $this->db->prepare("DELETE FROM arsse_tokens where \"user\" = ? and class = ?", "str", "str")->run($user, $class)->changes(); } else { @@ -436,6 +436,11 @@ class Database { return $out; } + /** List tokens associated with a user */ + public function tokenList(string $user, string $class): Db\Result { + return $this->db->prepare("SELECT id,created,expires,data from arsse_tokens where class = ? and user = ? and (expires is null or expires > CURRENT_TIMESTAMP)", "str", "str")->run($class, $user); + } + /** Deletes expires tokens from the database, returning the number of deleted tokens */ public function tokenCleanup(): int { return $this->db->query("DELETE FROM arsse_tokens where expires < CURRENT_TIMESTAMP")->changes(); diff --git a/lib/REST.php b/lib/REST.php index 011d27d..4f1f4bd 100644 --- a/lib/REST.php +++ b/lib/REST.php @@ -42,9 +42,19 @@ class REST { ], 'miniflux' => [ // Miniflux https://miniflux.app/docs/api.html 'match' => '/v1/', - 'strip' => '/v1', + 'strip' => '', 'class' => REST\Miniflux\V1::class, ], + 'miniflux-version' => [ // Miniflux version report + 'match' => '/version', + 'strip' => '', + 'class' => REST\Miniflux\Status::class, + ], + 'miniflux-healthcheck' => [ // Miniflux health check + 'match' => '/healthcheck', + 'strip' => '', + 'class' => REST\Miniflux\Status::class, + ], // Other candidates: // Microsub https://indieweb.org/Microsub // Google Reader http://feedhq.readthedocs.io/en/latest/api/index.html diff --git a/lib/REST/Miniflux/ErrorResponse.php b/lib/REST/Miniflux/ErrorResponse.php new file mode 100644 index 0000000..1cf467e --- /dev/null +++ b/lib/REST/Miniflux/ErrorResponse.php @@ -0,0 +1,19 @@ + Arsse::$lang->msg("API.Miniflux.Error.".$msg, $data)]; + parent::__construct($data, $status, $headers, $encodingOptions); + } +} diff --git a/lib/REST/Miniflux/Status.php b/lib/REST/Miniflux/Status.php new file mode 100644 index 0000000..367a7a6 --- /dev/null +++ b/lib/REST/Miniflux/Status.php @@ -0,0 +1,37 @@ +getRequestTarget())['path'] ?? ""; + if (!in_array($target, ["/version", "/healthcheck"])) { + return new EmptyResponse(404); + } + $method = $req->getMethod(); + if ($method === "OPTIONS") { + return new EmptyResponse(204, ['Allow' => "HEAD, GET"]); + } elseif ($method !== "GET") { + return new EmptyResponse(405, ['Allow' => "HEAD, GET"]); + } + $out = ""; + if ($target === "/version") { + $out = V1::VERSION; + } elseif ($target === "/healthcheck") { + $out = "OK"; + } + return new TextResponse($out); + } +} diff --git a/lib/REST/Miniflux/V1.php b/lib/REST/Miniflux/V1.php index 74873af..45d6191 100644 --- a/lib/REST/Miniflux/V1.php +++ b/lib/REST/Miniflux/V1.php @@ -13,6 +13,7 @@ use JKingWeb\Arsse\Misc\HTTP; use JKingWeb\Arsse\REST\Exception; use JKingWeb\Arsse\REST\Exception404; use JKingWeb\Arsse\REST\Exception405; +use JKingWeb\Arsse\User\ExceptionConflict as UserException; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; use Laminas\Diactoros\Response\EmptyResponse; @@ -20,6 +21,8 @@ use Laminas\Diactoros\Response\EmptyResponse; class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { protected const ACCEPTED_TYPES_OPML = ["text/xml", "application/xml", "text/x-opml"]; protected const ACCEPTED_TYPES_JSON = ["application/json", "text/json"]; + public const VERSION = "2.0.25"; + protected $paths = [ '/categories' => ['GET' => "getCategories", 'POST' => "createCategory"], '/categories/1' => ['PUT' => "updateCategory", 'DELETE' => "deleteCategory"], @@ -35,25 +38,41 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { '/feeds/1/icon' => ['GET' => "getFeedIcon"], '/feeds/1/refresh' => ['PUT' => "refreshFeed"], '/feeds/refresh' => ['PUT' => "refreshAllFeeds"], - '/healthcheck' => ['GET' => "healthCheck"], '/import' => ['POST' => "opmlImport"], '/me' => ['GET' => "getCurrentUser"], '/users' => ['GET' => "getUsers", 'POST' => "createUser"], '/users/1' => ['GET' => "getUser", 'PUT' => "updateUser", 'DELETE' => "deleteUser"], '/users/*' => ['GET' => "getUser"], - '/version' => ['GET' => "getVersion"], ]; public function __construct() { } - public function dispatch(ServerRequestInterface $req): ResponseInterface { - // try to authenticate + protected function authenticate(ServerRequestInterface $req): bool { + // first check any tokens; this is what Miniflux does + foreach ($req->getHeader("X-Auth-Token") as $t) { + if (strlen($t)) { + // a non-empty header is authoritative, so we'll stop here one way or the other + try { + $d = Arsse::$db->tokenLookup("miniflux.login", $t); + } catch (ExceptionInput $e) { + return false; + } + Arsse::$user->id = $d->user; + return true; + } + } + // next check HTTP auth if ($req->getAttribute("authenticated", false)) { Arsse::$user->id = $req->getAttribute("authenticatedUser"); - } else { - // TODO: Handle X-Auth-Token authentication - return new EmptyResponse(401); + } + return false; + } + + public function dispatch(ServerRequestInterface $req): ResponseInterface { + // try to authenticate + if (!$this->authenticate($req)) { + return new ErrorResponse("401", 401); } // get the request path only; this is assumed to already be normalized $target = parse_url($req->getRequestTarget())['path'] ?? ""; @@ -65,17 +84,14 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { $func = $this->chooseCall($target, $method); if ($func === "opmlImport") { if (!HTTP::matchType($req, "", ...[self::ACCEPTED_TYPES_OPML])) { - return new EmptyResponse(415, ['Accept' => implode(", ", self::ACCEPTED_TYPES_OPML)]); + return new ErrorResponse(415, ['Accept' => implode(", ", self::ACCEPTED_TYPES_OPML)]); } $data = (string) $req->getBody(); } elseif ($method === "POST" || $method === "PUT") { - if (!HTTP::matchType($req, "", ...[self::ACCEPTED_TYPES_JSON])) { - return new EmptyResponse(415, ['Accept' => implode(", ", self::ACCEPTED_TYPES_JSON)]); - } $data = @json_decode($data, true); if (json_last_error() !== \JSON_ERROR_NONE) { // if the body could not be parsed as JSON, return "400 Bad Request" - return new EmptyResponse(400); + return new ErrorResponse(["invalidBodyJSON", json_last_error_msg()], 400); } } else { $data = null; @@ -154,4 +170,20 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { throw new Exception404(); } } + + public static function tokenGenerate(string $user, string $label): string { + $t = base64_encode(random_bytes(24)); + return Arsse::$db->tokenCreate($user, "miniflux.login", $t, null, $label); + } + + public static function tokenList(string $user): array { + if (!Arsse::$db->userExists($user)) { + throw new UserException("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + } + $out = []; + foreach (Arsse::$db->tokenList($user, "miniflux.login") as $r) { + $out[] = ['label' => $r['data'], 'id' => $r['id']]; + } + return $out; + } } diff --git a/lib/User.php b/lib/User.php index 0a70e5d..e8359bc 100644 --- a/lib/User.php +++ b/lib/User.php @@ -69,7 +69,6 @@ class User { } return $out; } - public function remove(string $user): bool { try { @@ -143,7 +142,7 @@ class User { if (array_key_exists("tz", $data)) { if (!is_string($data['tz'])) { throw new User\ExceptionInput("invalidTimezone", ['field' => "tz", 'value' => ""]); - } elseif(!@timezone_open($data['tz'])) { + } elseif (!@timezone_open($data['tz'])) { throw new User\ExceptionInput("invalidTimezone", ['field' => "tz", 'value' => $data['tz']]); } $in['tz'] = $data['tz']; diff --git a/locale/en.php b/locale/en.php index 66a03ee..c0dea55 100644 --- a/locale/en.php +++ b/locale/en.php @@ -7,6 +7,9 @@ return [ 'CLI.Auth.Success' => 'Authentication successful', 'CLI.Auth.Failure' => 'Authentication failed', + 'API.Miniflux.Error.401' => 'Access Unauthorized', + 'API.Miniflux.Error.invalidBodyJSON' => 'Invalid JSON payload: {0}', + 'API.TTRSS.Category.Uncategorized' => 'Uncategorized', 'API.TTRSS.Category.Special' => 'Special', 'API.TTRSS.Category.Labels' => 'Labels', diff --git a/tests/cases/Database/SeriesToken.php b/tests/cases/Database/SeriesToken.php index 3f766aa..7a14ed0 100644 --- a/tests/cases/Database/SeriesToken.php +++ b/tests/cases/Database/SeriesToken.php @@ -33,12 +33,16 @@ trait SeriesToken { 'class' => "str", 'user' => "str", 'expires' => "datetime", + 'data' => "str", ], 'rows' => [ - ["80fa94c1a11f11e78667001e673b2560", "fever.login", "jane.doe@example.com", $faroff], - ["27c6de8da13311e78667001e673b2560", "fever.login", "jane.doe@example.com", $past], // expired - ["ab3b3eb8a13311e78667001e673b2560", "class.class", "jane.doe@example.com", null], - ["da772f8fa13c11e78667001e673b2560", "class.class", "john.doe@example.com", $future], + ["80fa94c1a11f11e78667001e673b2560", "fever.login", "jane.doe@example.com", $faroff, null], + ["27c6de8da13311e78667001e673b2560", "fever.login", "jane.doe@example.com", $past, null], // expired + ["ab3b3eb8a13311e78667001e673b2560", "class.class", "jane.doe@example.com", null, null], + ["da772f8fa13c11e78667001e673b2560", "class.class", "john.doe@example.com", $future, null], + ["A", "miniflux.login", "jane.doe@example.com", null, "Label 1"], + ["B", "miniflux.login", "jane.doe@example.com", null, "Label 2"], + ["C", "miniflux.login", "john.doe@example.com", null, "Label 1"], ], ], ]; @@ -127,4 +131,13 @@ trait SeriesToken { // revoking tokens which do not exist is not an error $this->assertFalse(Arsse::$db->tokenRevoke($user, "unknown.class")); } + + public function testListTokens(): void { + $user = "jane.doe@example.com"; + $exp = [ + ['id' => "A", 'data' => "Label 1"], + ['id' => "B", 'data' => "Label 2"], + ]; + $this->assertResult($exp, Arsse::$db->tokenList($user, "miniflux.login")); + } } diff --git a/tests/cases/REST/Miniflux/TestErrorResponse.php b/tests/cases/REST/Miniflux/TestErrorResponse.php new file mode 100644 index 0000000..23d6e28 --- /dev/null +++ b/tests/cases/REST/Miniflux/TestErrorResponse.php @@ -0,0 +1,22 @@ +assertSame('{"error_message":"Access Unauthorized"}', (string) $act->getBody()); + } + + public function testCreateVariableResponse(): void { + $act = new ErrorResponse(["invalidBodyJSON", "Doh!"], 401); + $this->assertSame('{"error_message":"Invalid JSON payload: Doh!"}', (string) $act->getBody()); + } +} diff --git a/tests/cases/REST/Miniflux/TestStatus.php b/tests/cases/REST/Miniflux/TestStatus.php new file mode 100644 index 0000000..bcf81d1 --- /dev/null +++ b/tests/cases/REST/Miniflux/TestStatus.php @@ -0,0 +1,34 @@ +dispatch($this->serverRequest($method, $url, "")); + $this->assertMessage($exp, $act); + } + + public function provideRequests(): iterable { + return [ + ["/version", "GET", new TextResponse(V1::VERSION)], + ["/version", "POST", new EmptyResponse(405, ['Allow' => "HEAD, GET"])], + ["/version", "OPTIONS", new EmptyResponse(204, ['Allow' => "HEAD, GET"])], + ["/healthcheck", "GET", new TextResponse("OK")], + ["/healthcheck", "POST", new EmptyResponse(405, ['Allow' => "HEAD, GET"])], + ["/healthcheck", "OPTIONS", new EmptyResponse(204, ['Allow' => "HEAD, GET"])], + ["/version/", "GET", new EmptyResponse(404)], + ]; + } +} diff --git a/tests/cases/REST/Miniflux/TestV1.php b/tests/cases/REST/Miniflux/TestV1.php new file mode 100644 index 0000000..7d20a0a --- /dev/null +++ b/tests/cases/REST/Miniflux/TestV1.php @@ -0,0 +1,118 @@ + */ +class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest { + protected $h; + protected $transaction; + + protected function req(string $method, string $target, $data = "", array $headers = [], bool $authenticated = true, bool $body = true): ResponseInterface { + $prefix = "/v1"; + $url = $prefix.$target; + if ($body) { + $params = []; + } else { + $params = $data; + $data = []; + } + $req = $this->serverRequest($method, $url, $prefix, $headers, [], $data, "application/json", $params, $authenticated ? "john.doe@example.com" : ""); + return $this->h->dispatch($req); + } + + public function setUp(): void { + self::clearData(); + self::setConf(); + // create a mock user manager + Arsse::$user = \Phake::mock(User::class); + Arsse::$user->id = "john.doe@example.com"; + // create a mock database interface + Arsse::$db = \Phake::mock(Database::class); + $this->transaction = \Phake::mock(Transaction::class); + \Phake::when(Arsse::$db)->begin->thenReturn($this->transaction); + //initialize a handler + $this->h = new V1(); + } + + public function tearDown(): void { + self::clearData(); + } + + protected function v($value) { + return $value; + } + + public function testSendAuthenticationChallenge(): void { + $exp = new EmptyResponse(401); + $this->assertMessage($exp, $this->req("GET", "/", "", [], false)); + } + + /** @dataProvider provideInvalidPaths */ + public function testRespondToInvalidPaths($path, $method, $code, $allow = null): void { + $exp = new EmptyResponse($code, $allow ? ['Allow' => $allow] : []); + $this->assertMessage($exp, $this->req($method, $path)); + } + + public function provideInvalidPaths(): array { + return [ + ["/", "GET", 404], + ["/", "POST", 404], + ["/", "PUT", 404], + ["/", "DELETE", 404], + ["/", "OPTIONS", 404], + ["/version/invalid", "GET", 404], + ["/version/invalid", "POST", 404], + ["/version/invalid", "PUT", 404], + ["/version/invalid", "DELETE", 404], + ["/version/invalid", "OPTIONS", 404], + ["/folders/1/invalid", "GET", 404], + ["/folders/1/invalid", "POST", 404], + ["/folders/1/invalid", "PUT", 404], + ["/folders/1/invalid", "DELETE", 404], + ["/folders/1/invalid", "OPTIONS", 404], + ["/version", "POST", 405, "GET"], + ["/version", "PUT", 405, "GET"], + ["/version", "DELETE", 405, "GET"], + ["/folders", "PUT", 405, "GET, POST"], + ["/folders", "DELETE", 405, "GET, POST"], + ["/folders/1", "GET", 405, "PUT, DELETE"], + ["/folders/1", "POST", 405, "PUT, DELETE"], + ]; + } + + public function testRespondToInvalidInputTypes(): void { + $exp = new EmptyResponse(415, ['Accept' => "application/json"]); + $this->assertMessage($exp, $this->req("PUT", "/folders/1", '', ['Content-Type' => "application/xml"])); + $exp = new EmptyResponse(400); + $this->assertMessage($exp, $this->req("PUT", "/folders/1", '')); + $this->assertMessage($exp, $this->req("PUT", "/folders/1", '', ['Content-Type' => null])); + } + + /** @dataProvider provideOptionsRequests */ + public function testRespondToOptionsRequests(string $url, string $allow, string $accept): void { + $exp = new EmptyResponse(204, [ + 'Allow' => $allow, + 'Accept' => $accept, + ]); + $this->assertMessage($exp, $this->req("OPTIONS", $url)); + } + + public function provideOptionsRequests(): array { + return [ + ["/feeds", "HEAD,GET,POST", "application/json"], + ["/feeds/2112", "DELETE", "application/json"], + ["/user", "HEAD,GET", "application/json"], + ]; + } +} diff --git a/tests/cases/User/TestUser.php b/tests/cases/User/TestUser.php index 310a0a3..8863d5f 100644 --- a/tests/cases/User/TestUser.php +++ b/tests/cases/User/TestUser.php @@ -9,7 +9,6 @@ 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; @@ -25,7 +24,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); @@ -43,7 +42,7 @@ 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(); @@ -166,7 +165,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"; @@ -178,7 +177,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"; @@ -189,7 +188,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"; @@ -205,7 +204,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"; @@ -381,7 +380,7 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { \Phake::verify($this->drv)->userPropertiesGet($user); } } - + /** @dataProvider providePropertyChanges */ public function testSetThePropertiesOfAUser(array $in, $out): void { $user = "john.doe@example.com"; @@ -399,7 +398,7 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { \Phake::verify(Arsse::$db)->userExists($user); } } - + /** @dataProvider providePropertyChanges */ public function testSetThePropertiesOfAUserWeDoNotKnow(array $in, $out): void { $user = "john.doe@example.com"; @@ -418,7 +417,7 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { \Phake::verify(Arsse::$db)->userAdd($user, null); } } - + public function providePropertyChanges(): iterable { return [ [['admin' => true], ['admin' => true]], @@ -431,7 +430,7 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { [['lang' => null], ['lang' => null]], ]; } - + public function testSetThePropertiesOfAMissingUser(): void { $user = "john.doe@example.com"; $in = ['admin' => true]; diff --git a/tests/phpunit.dist.xml b/tests/phpunit.dist.xml index 0c6f8a7..a46fe77 100644 --- a/tests/phpunit.dist.xml +++ b/tests/phpunit.dist.xml @@ -112,10 +112,14 @@ cases/REST/TestREST.php + + cases/REST/Miniflux/TestErrorResponse.php + cases/REST/Miniflux/TestStatus.php + cases/REST/NextcloudNews/TestVersions.php cases/REST/NextcloudNews/TestV1_2.php - cases/REST/NextcloudNews/PDO/TestV1_2.php + cases/REST/NextcloudNews/PDO/TestV1_2.php cases/REST/TinyTinyRSS/TestSearch.php