From def07bb1ad2a16178e3915ba3132d30321dc3bd3 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Mon, 30 Nov 2020 10:52:32 -0500 Subject: [PATCH] Tests for Miniflux authentication This appears to match Miniflux's behaviour --- lib/REST/Miniflux/V1.php | 21 ++++++----- tests/cases/REST/Miniflux/TestV1.php | 52 ++++++++++++++++++++++++---- tests/phpunit.dist.xml | 1 + 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/lib/REST/Miniflux/V1.php b/lib/REST/Miniflux/V1.php index 45d61914..3860c20b 100644 --- a/lib/REST/Miniflux/V1.php +++ b/lib/REST/Miniflux/V1.php @@ -7,12 +7,14 @@ declare(strict_types=1); namespace JKingWeb\Arsse\REST\Miniflux; use JKingWeb\Arsse\Arsse; -use JKingWeb\Arsse\Misc\ValueInfo; use JKingWeb\Arsse\AbstractException; +use JKingWeb\Arsse\Db\ExceptionInput; use JKingWeb\Arsse\Misc\HTTP; +use JKingWeb\Arsse\Misc\ValueInfo; use JKingWeb\Arsse\REST\Exception; use JKingWeb\Arsse\REST\Exception404; use JKingWeb\Arsse\REST\Exception405; +use JKingWeb\Arsse\REST\Exception501; use JKingWeb\Arsse\User\ExceptionConflict as UserException; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; @@ -21,6 +23,7 @@ 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"]; + protected const TOKEN_LENGTH = 32; public const VERSION = "2.0.25"; protected $paths = [ @@ -50,21 +53,22 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { 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 + if ($req->hasHeader("X-Auth-Token")) { + $t = $req->getHeader("X-Auth-Token")[0]; // consider only the first token + if (strlen($t)) { // and only if it is not blank try { $d = Arsse::$db->tokenLookup("miniflux.login", $t); } catch (ExceptionInput $e) { return false; } - Arsse::$user->id = $d->user; + Arsse::$user->id = $d['user']; return true; } } // next check HTTP auth if ($req->getAttribute("authenticated", false)) { Arsse::$user->id = $req->getAttribute("authenticatedUser"); + return true; } return false; } @@ -84,11 +88,11 @@ 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 ErrorResponse(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") { - $data = @json_decode($data, true); + $data = @json_decode((string) $req->getBody(), true); if (json_last_error() !== \JSON_ERROR_NONE) { // if the body could not be parsed as JSON, return "400 Bad Request" return new ErrorResponse(["invalidBodyJSON", json_last_error_msg()], 400); @@ -172,7 +176,8 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler { } public static function tokenGenerate(string $user, string $label): string { - $t = base64_encode(random_bytes(24)); + // Miniflux produces tokens in base64url alphabet + $t = str_replace(["+", "/"], ["-", "_"], base64_encode(random_bytes(self::TOKEN_LENGTH))); return Arsse::$db->tokenCreate($user, "miniflux.login", $t, null, $label); } diff --git a/tests/cases/REST/Miniflux/TestV1.php b/tests/cases/REST/Miniflux/TestV1.php index db8c34d9..de35c276 100644 --- a/tests/cases/REST/Miniflux/TestV1.php +++ b/tests/cases/REST/Miniflux/TestV1.php @@ -10,13 +10,19 @@ use JKingWeb\Arsse\Arsse; use JKingWeb\Arsse\User; use JKingWeb\Arsse\Database; use JKingWeb\Arsse\Db\Transaction; +use JKingWeb\Arsse\Db\ExceptionInput; +use JKingWeb\Arsse\REST\Exception404; use JKingWeb\Arsse\REST\Miniflux\V1; +use JKingWeb\Arsse\REST\Miniflux\ErrorResponse; use Psr\Http\Message\ResponseInterface; +use Laminas\Diactoros\Response\JsonResponse as Response; +use Laminas\Diactoros\Response\EmptyResponse; /** @covers \JKingWeb\Arsse\REST\Miniflux\V1 */ class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest { protected $h; protected $transaction; + protected $token = "Tk2o9YubmZIL2fm2w8Z4KlDEQJz532fNSOcTG0s2_xc="; protected function req(string $method, string $target, $data = "", array $headers = [], bool $authenticated = true, bool $body = true): ResponseInterface { $prefix = "/v1"; @@ -54,13 +60,47 @@ class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest { } /** @dataProvider provideAuthResponses */ - public function testAuthenticateAUser(): void { - $exp = new EmptyResponse(401); - $this->assertMessage($exp, $this->req("GET", "/", "", [], false)); + public function testAuthenticateAUser($token, bool $auth, bool $success): void { + $exp = new ErrorResponse("401", 401); + $user = "john.doe@example.com"; + if ($token !== null) { + $headers = ['X-Auth-Token' => $token]; + } else { + $headers = []; + } + Arsse::$user->id = null; + \Phake::when(Arsse::$db)->tokenLookup->thenThrow(new ExceptionInput("subjectMissing")); + \Phake::when(Arsse::$db)->tokenLookup("miniflux.login", $this->token)->thenReturn(['user' => $user]); + if ($success) { + $this->expectExceptionObject(new Exception404); + try { + $this->req("GET", "/", "", $headers, $auth); + } finally { + $this->assertSame($user, Arsse::$user->id); + } + } else { + $this->assertMessage($exp, $this->req("GET", "/", "", $headers, $auth)); + $this->assertNull(Arsse::$user->id); + } + } + + public function provideAuthResponses(): iterable { + return [ + [null, false, false], + [null, true, true], + [$this->token, false, true], + [[$this->token, "BOGUS"], false, true], + ["", true, true], + [["", "BOGUS"], true, true], + ["NOT A TOKEN", false, false], + ["NOT A TOKEN", true, false], + [["BOGUS", $this->token], false, false], + [["", $this->token], false, false], + ]; } /** @dataProvider provideInvalidPaths */ - public function testRespondToInvalidPaths($path, $method, $code, $allow = null): void { + public function xtestRespondToInvalidPaths($path, $method, $code, $allow = null): void { $exp = new EmptyResponse($code, $allow ? ['Allow' => $allow] : []); $this->assertMessage($exp, $this->req($method, $path)); } @@ -72,7 +112,7 @@ class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest { ]; } - public function testRespondToInvalidInputTypes(): void { + public function xtestRespondToInvalidInputTypes(): void { $exp = new EmptyResponse(415, ['Accept' => "application/json"]); $this->assertMessage($exp, $this->req("PUT", "/folders/1", '', ['Content-Type' => "application/xml"])); $exp = new EmptyResponse(400); @@ -81,7 +121,7 @@ class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest { } /** @dataProvider provideOptionsRequests */ - public function testRespondToOptionsRequests(string $url, string $allow, string $accept): void { + public function xtestRespondToOptionsRequests(string $url, string $allow, string $accept): void { $exp = new EmptyResponse(204, [ 'Allow' => $allow, 'Accept' => $accept, diff --git a/tests/phpunit.dist.xml b/tests/phpunit.dist.xml index a46fe77d..18486652 100644 --- a/tests/phpunit.dist.xml +++ b/tests/phpunit.dist.xml @@ -115,6 +115,7 @@ cases/REST/Miniflux/TestErrorResponse.php cases/REST/Miniflux/TestStatus.php + cases/REST/Miniflux/TestV1.php cases/REST/NextcloudNews/TestVersions.php