diff --git a/src/IndieAuthException.php b/src/IndieAuthException.php index e3f881f..10f89ba 100644 --- a/src/IndieAuthException.php +++ b/src/IndieAuthException.php @@ -19,6 +19,7 @@ class IndieAuthException extends Exception { const INVALID_STATE = 9; const INVALID_CODE_CHALLENGE = 10; const INVALID_SCOPE = 11; + const INTERNAL_ERROR_REDIRECT = 12; const EXC_INFO = [ self::INTERNAL_ERROR => ['statusCode' => 500, 'name' => 'Internal Server Error', 'explanation' => 'An internal server error occurred.'], @@ -34,6 +35,7 @@ class IndieAuthException extends Exception { self::INVALID_STATE => ['statusCode' => 302, 'name' => 'Invalid state Parameter', 'error' => 'invalid_request'], self::INVALID_CODE_CHALLENGE => ['statusCode' => 302, 'name' => 'Invalid code_challenge Parameter', 'error' => 'invalid_request'], self::INVALID_SCOPE => ['statusCode' => 302, 'name' => 'Invalid scope Parameter', 'error' => 'invalid_request'], + self::INTERNAL_ERROR_REDIRECT => ['statusCode' => 302, 'name' => 'Internal Server Error', 'error' => 'internal_error'], ]; protected ServerRequestInterface $request; diff --git a/src/Server.php b/src/Server.php index 6bb4a85..04c851c 100644 --- a/src/Server.php +++ b/src/Server.php @@ -66,7 +66,7 @@ class Server { ], $config); if (!is_string($config['exceptionTemplatePath'])) { - throw new Exception("\$config['secret'] must be a string (path)."); + throw new Exception("\$config['exceptionTemplatePath'] must be a string (path)."); } $this->exceptionTemplatePath = $config['exceptionTemplatePath']; @@ -130,10 +130,10 @@ class Server { $effectiveUrl = empty($rdh) ? $uri : array_values($rdh)[count($rdh) - 1]; return [$resp, $effectiveUrl]; - // @codeCoverageIgnoreEnd }; } else { throw new Exception('No valid $httpGetWithEffectiveUrl was provided, and guzzlehttp/guzzle was not installed. Either require guzzlehttp/guzzle, or provide a valid callable.'); + // @codeCoverageIgnoreEnd } } trySetLogger($httpGetWithEffectiveUrl, $this->logger); @@ -360,7 +360,7 @@ class Server { // If saving the authorization code failed silently, there isn’t much we can do about it, // but should at least log and return an error. $this->logger->error("Saving the authorization code failed and returned false without raising an exception."); - throw IndieAuthException::create(IndieAuthException::INTERNAL_ERROR, $request); + throw IndieAuthException::create(IndieAuthException::INTERNAL_ERROR_REDIRECT, $request); } // Return a redirect to the client app. diff --git a/src/Storage/FilesystemJsonStorage.php b/src/Storage/FilesystemJsonStorage.php index 4aa25de..c8b914e 100644 --- a/src/Storage/FilesystemJsonStorage.php +++ b/src/Storage/FilesystemJsonStorage.php @@ -209,10 +209,10 @@ class FilesystemJsonStorage implements TokenStorageInterface, LoggerAwareInterfa } protected function withLock(string $path, string $mode, callable $callback) { - $fp = fopen($path, $mode); + $fp = @fopen($path, $mode); if ($fp === false) { - return false; + return null; } // Wait for a lock. diff --git a/src/functions.php b/src/functions.php index f82bd65..feca81c 100644 --- a/src/functions.php +++ b/src/functions.php @@ -37,7 +37,7 @@ function hashAuthorizationRequestParameters(ServerRequestInterface $request, str $hashedParameters = $hashedParameters ?? ['client_id', 'redirect_uri', 'code_challenge', 'code_challenge_method']; $algo = $algo ?? 'sha256'; - $queryParams = $request->getQueryParams(); + $queryParams = $request->getQueryParams() ?? []; $data = ''; foreach ($hashedParameters as $key) { if (!array_key_exists($key, $queryParams)) { @@ -50,19 +50,19 @@ function hashAuthorizationRequestParameters(ServerRequestInterface $request, str function isIndieAuthAuthorizationCodeRedeemingRequest(ServerRequestInterface $request) { return strtolower($request->getMethod()) == 'post' - && array_key_exists('grant_type', $request->getParsedBody()) + && array_key_exists('grant_type', $request->getParsedBody() ?? []) && $request->getParsedBody()['grant_type'] == 'authorization_code'; } function isIndieAuthAuthorizationRequest(ServerRequestInterface $request, $permittedMethods=['get']) { return in_array(strtolower($request->getMethod()), array_map('strtolower', $permittedMethods)) - && array_key_exists('response_type', $request->getQueryParams()) + && array_key_exists('response_type', $request->getQueryParams() ?? []) && $request->getQueryParams()['response_type'] == 'code'; } function isAuthorizationApprovalRequest(ServerRequestInterface $request) { return strtolower($request->getMethod()) == 'post' - && array_key_exists('taproot_indieauth_action', $request->getParsedBody()) + && array_key_exists('taproot_indieauth_action', $request->getParsedBody() ?? []) && $request->getParsedBody()[Server::APPROVE_ACTION_KEY] == Server::APPROVE_ACTION_VALUE; } diff --git a/tests/ServerTest.php b/tests/ServerTest.php index 2f13931..4c78f0c 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -15,8 +15,10 @@ use Taproot\IndieAuth\Callback\SingleUserPasswordAuthenticationCallback; use Taproot\IndieAuth\IndieAuthException; use Taproot\IndieAuth\Server; use Taproot\IndieAuth\Storage\FilesystemJsonStorage; +use Taproot\IndieAuth\Storage\Token; use Taproot\IndieAuth\Storage\TokenStorageInterface; +use function GuzzleHttp\Promise\exception_for; use function Taproot\IndieAuth\generatePKCECodeChallenge; use function Taproot\IndieAuth\generateRandomString; use function Taproot\IndieAuth\hashAuthorizationRequestParameters; @@ -109,6 +111,34 @@ class ServerTest extends TestCase { @rmdir(TOKEN_STORAGE_PATH); } + /** + * Configuration/Instantiation Tests + */ + + public function testInvalidConfigRaisesException() { + $badConfigs = [ + ['exceptionTemplatePath' => 21], + ['secret' => 12], + ['secret' => 'too short'], + ['logger' => 'not a logger'], + [Server::HANDLE_AUTHENTICATION_REQUEST => 'not a callable'], + [Server::HANDLE_NON_INDIEAUTH_REQUEST => 'again, not a callable'], + ['tokenStorage' => 34], + ['csrfMiddleware' => 'not a middleware object'], + ['httpGetWithEffectiveUrl' => 'not even a callable this time'], + ['authorizationForm' => 'not an auth form instance'] + ]; + + foreach ($badConfigs as $badConfig) { + try { + $this->getDefaultServer($badConfig); + $this->fail(); + } catch (Exception $e) { + $this->assertTrue(true); + } + } + } + /** * Authorization Request Tests */ @@ -504,11 +534,35 @@ EOT $this->assertEquals($authenticationResponse['profile'], $storedCode['profile'], "The “profile” value in the stored code did not match the “profile” value from the authentication response."); } + public function testReturnsInternalServerErrorIfAuthCodeCannotBeStored() { + $s = $this->getDefaultServer([ + 'tokenStorage' => new NullTokenStorage(), + Server::HANDLE_AUTHENTICATION_REQUEST => function (ServerRequestInterface $request, string $formAction): array { + return ['me' => 'https://me.example.com']; + }, + 'httpGetWithEffectiveUrl' => function ($url): array { + return [ + new Response(200, ['content-type' => 'text/html'], ''), + $url + ]; + } + ]); + + $req = $this->getApprovalRequest(true, true); + + $res = $s->handleAuthorizationEndpointRequest($req); + + $this->assertEquals(302, $res->getStatusCode()); + $expectedErrorName = IndieAuthException::EXC_INFO[IndieAuthException::INTERNAL_ERROR_REDIRECT]['error']; + parse_str(parse_url($res->getHeaderLine('location'), PHP_URL_QUERY), $redirectQueryParams); + $this->assertEquals($expectedErrorName, $redirectQueryParams['error']); + } + /** * Test Authorization Token Exchange Requests */ - public function testExchangePathsReturnErrorsIfParametersAreMissing() { + public function testExchangeFlowsReturnErrorsIfParametersAreMissing() { $s = $this->getDefaultServer(); $req = (new ServerRequest('POST', 'https://example.com'))->withParsedBody([ @@ -526,7 +580,7 @@ EOT $this->assertEquals('invalid_request', $tokenEndpointJson['error']); } - public function testExchangePathsReturnErrorOnInvalidParameters() { + public function testExchangeFlowsReturnErrorOnInvalidParameters() { $s = $this->getDefaultServer(); $storage = new FilesystemJsonStorage(TOKEN_STORAGE_PATH, SERVER_SECRET); @@ -534,6 +588,7 @@ EOT 'Mismatched client_id' => ['client_id' => 'https://invalid-client.example.com/'], 'Mismatched redirect_uri' => ['redirect_uri' => 'https://invalid-client.example.com/auth'], 'Invalid code_verifier' => ['code_verifier' => 'definitely_not_the_randomly_generated_string'], + 'Wrong auth code' => ['code' => 'yeah_thats_not_the_right_code'] ]; foreach ($testCases as $name => $params) { @@ -640,6 +695,22 @@ EOT ], $resJson); } + public function testTokenEndpointReturnsErrorOnNonIndieauthRequest() { + $s = $this->getDefaultServer(); + + $badRequests = [ + new ServerRequest('GET', 'https://example.com/'), + new ServerRequest('POST', 'https://example.com/') + ]; + + foreach ($badRequests as $badRequest) { + $res = $s->handleTokenEndpointRequest($badRequest); + $this->assertEquals(400, $res->getStatusCode()); + $resJson = json_decode((string) $res->getBody(), true); + $this->assertEquals('invalid_request', $resJson['error']); + } + } + public function testTokenEndpointReturnsErrorIfAccessCodeGrantsNoScopes() { $s = $this->getDefaultServer(); $storage = new FilesystemJsonStorage(TOKEN_STORAGE_PATH, SERVER_SECRET); @@ -732,6 +803,10 @@ EOT } } +/** + * Utility functions and classes + */ + function scopeEquals($scope1, $scope2): bool { $scope1 = is_string($scope1) ? explode(' ', $scope1) : $scope1; $scope2 = is_string($scope2) ? explode(' ', $scope2) : $scope2; @@ -739,3 +814,21 @@ function scopeEquals($scope1, $scope2): bool { sort($scope2); return $scope1 == $scope2; } + +class NullTokenStorage implements TokenStorageInterface { + public function createAuthCode(array $data): ?Token { + return null; + } + + public function getAccessToken(string $token): ?Token { + return null; + } + + public function exchangeAuthCodeForAccessToken(string $code): ?Token { + return null; + } + + public function revokeAccessToken(string $token): bool { + return false; + } +}