From aa572270979b2322e0951d1e1a3d85da70b3f3b3 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 11 Jan 2018 15:48:29 -0500 Subject: [PATCH] Use PSR-7 for authentication; fixes #53 --- lib/Conf.php | 4 ++- lib/REST.php | 35 ++++++++++++++++++++++ lib/REST/NextCloudNews/V1_2.php | 6 ++-- tests/cases/REST/TestREST.php | 53 +++++++++++++++++++++++++++++++++ tests/lib/AbstractTest.php | 2 +- 5 files changed, 96 insertions(+), 4 deletions(-) diff --git a/lib/Conf.php b/lib/Conf.php index bd046ff9..cc0a1834 100644 --- a/lib/Conf.php +++ b/lib/Conf.php @@ -72,10 +72,12 @@ class Conf { * @see https://en.wikipedia.org/wiki/ISO_8601#Durations */ public $purgeArticlesUnread = "P21D"; + /** @var string Application name to present to clients during authentication */ + public $httpRealm = "The Advanced RSS Environment"; /** @var string Space-separated list of origins from which to allow cross-origin resource sharing */ public $httpOriginsAllowed = "*"; /** @var string Space-separated list of origins from which to deny cross-origin resource sharing */ - public $httpOriginsDenied = ""; + public $httpOriginsDenied = ""; /** Creates a new configuration object * @param string $import_file Optional file to read configuration data from diff --git a/lib/REST.php b/lib/REST.php index fc4cfacc..3820308e 100644 --- a/lib/REST.php +++ b/lib/REST.php @@ -6,6 +6,8 @@ declare(strict_types=1); namespace JKingWeb\Arsse; + +use JKingWeb\Arsse\Arsse; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; @@ -68,6 +70,8 @@ class REST { // find the API to handle try { list ($api, $target, $class) = $this->apiMatch($req->getRequestTarget(), $this->apis); + // authenticate the request pre-emptively + $req = $this->authenticateRequest($req); // modify the request to have an uppercase method and a stripped target $req = $req->withMethod(strtoupper($req->getMethod()))->withRequestTarget($target); // fetch the correct handler @@ -119,7 +123,38 @@ class REST { throw new REST\Exception501(); } + public function authenticateRequest(ServerRequestInterface $req): ServerRequestInterface { + $user = ""; + $password = ""; + $env = $req->getServerParams(); + if (isset($env['PHP_AUTH_USER'])) { + $user = $env['PHP_AUTH_USER']; + if (isset($env['PHP_AUTH_PW'])) { + $password = $env['PHP_AUTH_PW']; + } + } elseif (isset($env['REMOTE_USER'])) { + $user = $env['REMOTE_USER']; + } + if (strlen($user)) { + $valid = Arsse::$user->auth($user, $password); + } + if ($valid) { + $req = $req->withAttribute("authenticated", true); + $req = $req->withAttribute("authenticatedUser", $user); + } + return $req; + } + + public function challenge(ResponseInterface $res, string $realm = null): ResponseInterface { + $realm = $realm ?? Arsse::$conf->httpRealm ?? "Default"; + return $res->withAddedHeader("WWW-Authenticate", 'Basic realm="'.$realm.'"'); + } + public function normalizeResponse(ResponseInterface $res, RequestInterface $req = null): ResponseInterface { + // if the response code is 401, issue an HTTP authentication challenge + if ($res->getStatusCode()==401) { + $res = $this->challenge($res); + } // set or clear the Content-Length header field $body = $res->getBody(); $bodySize = $body->getSize(); diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index 0983ebd6..6b556ffd 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -80,8 +80,10 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { public function dispatch(ServerRequestInterface $req): ResponseInterface { // try to authenticate - if (!Arsse::$user->authHTTP()) { - return new EmptyResponse(401, ['WWW-Authenticate' => 'Basic realm="'.self::REALM.'"']); + if ($req->getAttribute("authenticated", false)) { + Arsse::$user->id = $req->getAttribute("authenticatedUser"); + } else { + return new EmptyResponse(401); } // explode and normalize the URL path $target = new Target($req->getRequestTarget()); diff --git a/tests/cases/REST/TestREST.php b/tests/cases/REST/TestREST.php index 7851c1ea..6cb7d91a 100644 --- a/tests/cases/REST/TestREST.php +++ b/tests/cases/REST/TestREST.php @@ -6,6 +6,8 @@ declare(strict_types=1); namespace JKingWeb\Arsse\TestCase\REST; +use JKingWeb\Arsse\Arsse; +use JKingWeb\Arsse\User; use JKingWeb\Arsse\REST; use JKingWeb\Arsse\REST\Handler; use JKingWeb\Arsse\REST\Exception501; @@ -58,7 +60,50 @@ class TestREST extends \JKingWeb\Arsse\Test\AbstractTest { [$fake, "/full/url-not", []], ]; } + + /** @dataProvider provideAuthenticableRequests */ + public function testAuthenticateRequests(array $serverParams, array $expAttr) { + $r = new REST(); + // create a mock user manager + Arsse::$user = Phake::mock(User::class); + Phake::when(Arsse::$user)->auth->thenReturn(true); + Phake::when(Arsse::$user)->auth($this->anything(), "superman")->thenReturn(false); + Phake::when(Arsse::$user)->auth("jane.doe@example.com", $this->anything())->thenReturn(false); + // create an input server request + $req = new ServerRequest($serverParams); + // create the expected output + $exp = $req; + foreach ($expAttr as $key => $value) { + $exp = $exp->withAttribute($key, $value); + } + $act = $r->authenticateRequest($req); + $this->assertMessage($exp, $act); + } + public function provideAuthenticableRequests() { + return [ + [['PHP_AUTH_USER' => "john.doe@example.com", 'PHP_AUTH_PW' => "secret"], ['authenticated' => true, 'authenticatedUser' => "john.doe@example.com"]], + [['PHP_AUTH_USER' => "john.doe@example.com", 'PHP_AUTH_PW' => "secret", 'REMOTE_USER' => "jane.doe@example.com"], ['authenticated' => true, 'authenticatedUser' => "john.doe@example.com"]], + [['PHP_AUTH_USER' => "jane.doe@example.com", 'PHP_AUTH_PW' => "secret"], []], + [['PHP_AUTH_USER' => "john.doe@example.com", 'PHP_AUTH_PW' => "superman"], []], + [['REMOTE_USER' => "john.doe@example.com"], ['authenticated' => true, 'authenticatedUser' => "john.doe@example.com"]], + [['REMOTE_USER' => "someone.else@example.com"], ['authenticated' => true, 'authenticatedUser' => "someone.else@example.com"]], + [['REMOTE_USER' => "jane.doe@example.com"], []], + ]; + } + + public function testSendAuthenticationChallenges() { + $this->setConf(); + $r = new REST(); + $in = new EmptyResponse(401); + $exp = $in->withHeader("WWW-Authenticate", 'Basic realm="OOK"'); + $act = $r->challenge($in, "OOK"); + $this->assertMessage($exp, $act); + $exp = $in->withHeader("WWW-Authenticate", 'Basic realm="'.Arsse::$conf->httpRealm.'"'); + $act = $r->challenge($in); + $this->assertMessage($exp, $act); + } + /** @dataProvider provideUnnormalizedOrigins */ public function testNormalizeOrigins(string $origin, string $exp, array $ports = null) { $r = new REST(); @@ -207,6 +252,9 @@ class TestREST extends \JKingWeb\Arsse\Test\AbstractTest { public function testNormalizeHttpResponses(ResponseInterface $res, ResponseInterface $exp, RequestInterface $req = null) { $r = Phake::partialMock(REST::class); Phake::when($r)->corsNegotiate->thenReturn(true); + Phake::when($r)->challenge->thenReturnCallback(function ($res) { + return $res->withHeader("WWW-Authenticate", "Fake Value"); + }); Phake::when($r)->corsApply->thenReturnCallback(function ($res) { return $res; }); @@ -219,6 +267,7 @@ class TestREST extends \JKingWeb\Arsse\Test\AbstractTest { fwrite($stream,"ook"); return [ [new EmptyResponse(204), new EmptyResponse(204)], + [new EmptyResponse(401), new EmptyResponse(401, ['WWW-Authenticate' => "Fake Value"])], [new EmptyResponse(204, ['Allow' => "PUT"]), new EmptyResponse(204, ['Allow' => "PUT, OPTIONS"])], [new EmptyResponse(204, ['Allow' => "PUT, OPTIONS"]), new EmptyResponse(204, ['Allow' => "PUT, OPTIONS"])], [new EmptyResponse(204, ['Allow' => "PUT,OPTIONS"]), new EmptyResponse(204, ['Allow' => "PUT, OPTIONS"])], @@ -249,6 +298,9 @@ class TestREST extends \JKingWeb\Arsse\Test\AbstractTest { Phake::when($r)->normalizeResponse->thenReturnCallback(function ($res) { return $res; }); + Phake::when($r)->authenticateRequest->thenReturnCallback(function ($req) { + return $req; + }); if ($called) { $h = Phake::mock($class); Phake::when($r)->getHandler($class)->thenReturn($h); @@ -257,6 +309,7 @@ class TestREST extends \JKingWeb\Arsse\Test\AbstractTest { $out = $r->dispatch($req); $this->assertInstanceOf(ResponseInterface::class, $out); if ($called) { + Phake::verify($r)->authenticateRequest; Phake::verify($h)->dispatch(Phake::capture($in)); $this->assertSame($method, $in->getMethod()); $this->assertSame($target, $in->getRequestTarget()); diff --git a/tests/lib/AbstractTest.php b/tests/lib/AbstractTest.php index fbcb227c..4986c1aa 100644 --- a/tests/lib/AbstractTest.php +++ b/tests/lib/AbstractTest.php @@ -58,7 +58,7 @@ abstract class AbstractTest extends \PHPUnit\Framework\TestCase { $this->assertEquals($exp->getAttributes(), $act->getAttributes(), $text); } $this->assertInstanceOf(RequestInterface::class, $act, $text); - $this->assertSame($exp->getRequestMethod(), $act->getRequestMethod(), $text); + $this->assertSame($exp->getMethod(), $act->getMethod(), $text); $this->assertSame($exp->getRequestTarget(), $act->getRequestTarget(), $text); } if ($exp instanceof JsonResponse) {