From 6d5e93b07c4ee9a0961421d8efaddc8cd0c846c6 Mon Sep 17 00:00:00 2001 From: Barnaby Walters Date: Wed, 9 Jun 2021 00:06:35 +0200 Subject: [PATCH] Refactored Exception Handling, mostly tested authorization request handler * Internal error conditions now raise IndieAuthException * Bubbled unknown exceptions converted to generic IndieAuthException * Exceptions passed to overridable handler, turned into response * Wrote many more tests, fixed a variety of problems --- src/Callback/DefaultAuthorizationForm.php | 8 +- src/IndieAuthException.php | 60 +++ src/Server.php | 350 ++++++++++------- src/Storage/FilesystemJsonStorage.php | 2 +- src/functions.php | 23 +- .../default_authorization_page.html.php | 5 +- .../default_exception_response.html.php | 32 ++ tests/ServerTest.php | 363 +++++++++++++++++- .../authorization_form_json_response.json.php | 21 + .../templates/code_exception_response.txt.php | 4 + 10 files changed, 700 insertions(+), 168 deletions(-) create mode 100644 src/IndieAuthException.php create mode 100644 src/templates/default_exception_response.html.php create mode 100644 tests/templates/authorization_form_json_response.json.php create mode 100644 tests/templates/code_exception_response.txt.php diff --git a/src/Callback/DefaultAuthorizationForm.php b/src/Callback/DefaultAuthorizationForm.php index 2eae915..81f70d1 100644 --- a/src/Callback/DefaultAuthorizationForm.php +++ b/src/Callback/DefaultAuthorizationForm.php @@ -33,6 +33,13 @@ class DefaultAuthorizationForm implements AuthorizationFormInterface, LoggerAwar $scopes[$s] = null; // Ideally there would be a description of the scope here, we don’t have one though. } + if (is_null($clientHApp)) { + $clientHApp = [ + 'type' => ['h-app'], + 'properties' => [] + ]; + } + $hApp = [ 'name' => M\getProp($clientHApp, 'name'), 'url' => M\getProp($clientHApp, 'url'), @@ -62,7 +69,6 @@ class DefaultAuthorizationForm implements AuthorizationFormInterface, LoggerAwar // You may wish to additionally make any other necessary changes to the the code based on // the form submission, e.g. if the user set a custom token lifetime, or wanted extra data // stored on the token to affect how it behaves. - return $code; } diff --git a/src/IndieAuthException.php b/src/IndieAuthException.php new file mode 100644 index 0000000..5f9c055 --- /dev/null +++ b/src/IndieAuthException.php @@ -0,0 +1,60 @@ + ['statusCode' => 500, 'name' => 'Internal Server Error', 'explanation' => 'An internal server error occurred.'], + self::REQUEST_MISSING_PARAMETER => ['statusCode' => 400, 'name' => 'Request Missing Parameter', 'explanation' => 'The request was missing one or more required query string parameters.'], + self::AUTHENTICATION_CALLBACK_MISSING_ME_PARAM => ['statusCode' => 500, 'name' => 'Internal Server Error', 'explanation' => 'The user data returned from handleAuthenticationRequestCallback was missing a “me” parameter.'], + self::AUTHORIZATION_APPROVAL_REQUEST_MISSING_HASH => ['statusCode' => 400, 'name' => 'Request Missing Hash', 'explanation' => 'An authentication form submission request was missing the hash parameter.'], + self::AUTHORIZATION_APPROVAL_REQUEST_INVALID_HASH => ['statusCode' => 400, 'name' => 'Request Hash Invalid', 'explanation' => 'The hash protecting the query string parameters from tampering was invalid. Your form submission may have been altered by malicious client-side code.'], + // TODO: should this one be a 500 because it’s an internal server error, or a 400 because the client_id was likely invalid? Is anyone ever going to notice, or care? + self::HTTP_EXCEPTION_FETCHING_CLIENT_ID => ['statusCode' => 500, 'name' => 'Error Fetching Client App URL', 'explanation' => 'Fetching the client app (client_id) failed.'], + self::INTERNAL_EXCEPTION_FETCHING_CLIENT_ID => ['statusCode' => 500, 'name' => 'Internal Error fetching client app URI', 'explanation' => 'Fetching the client app (client_id) failed due to an internal error.'], + self::INVALID_REDIRECT_URI => ['statusCode' => 400, 'name' => 'Invalid Client App Redirect URI', 'explanation' => 'The client app redirect URI (redirect_uri) did not sufficiently match client_id, or exactly match any redirect URIs parsed from fetching the client_id.'] + ]; + + protected ServerRequestInterface $request; + + public static function create(int $code, ServerRequestInterface $request, ?Throwable $previous=null): self { + // Only accept known codes. Default to 0 (generic internal error) on an unrecognised code. + if (!in_array($code, array_keys(self::EXC_INFO))) { + $code = 0; + } + $message = self::EXC_INFO[$code]['name']; + $e = new self($message, $code, $previous); + $e->request = $request; + return $e; + } + + public function getStatusCode() { + return self::EXC_INFO[$this->code]['statusCode'] ?? 500; + } + + public function getExplanation() { + return self::EXC_INFO[$this->code]['explanation'] ?? 'An unknown error occured.'; + } + + public function trustQueryParams() { + return $this->code == self::AUTHORIZATION_APPROVAL_REQUEST_INVALID_HASH + || $this->code == self::AUTHORIZATION_APPROVAL_REQUEST_MISSING_HASH; + } + + public function getRequest() { + return $this->request; + } +} diff --git a/src/Server.php b/src/Server.php index bd7fb4b..a61d355 100644 --- a/src/Server.php +++ b/src/Server.php @@ -35,29 +35,31 @@ class Server { const HANDLE_AUTHENTICATION_REQUEST = 'handleAuthenticationRequestCallback'; const HASH_QUERY_STRING_KEY = 'taproot_indieauth_server_hash'; const DEFAULT_CSRF_KEY = 'taproot_indieauth_server_csrf'; + const APPROVE_ACTION_KEY = 'taproot_indieauth_action'; + const APPROVE_ACTION_VALUE = 'approve'; - public $callbacks; + protected Storage\TokenStorageInterface $authorizationCodeStorage; - public Storage\TokenStorageInterface $authorizationCodeStorage; + protected Storage\TokenStorageInterface $accessTokenStorage; - public Storage\TokenStorageInterface $accessTokenStorage; + protected AuthorizationFormInterface $authorizationForm; - public AuthorizationFormInterface $authorizationForm; + protected MiddlewareInterface $csrfMiddleware; - public MiddlewareInterface $csrfMiddleware; + protected LoggerInterface $logger; - public LoggerInterface $logger; + protected $httpGetWithEffectiveUrl; - public HttpClientInterface $httpClient; + protected $handleAuthenticationRequestCallback; - public $httpGetWithEffectiveUrl; + protected $handleNonIndieAuthRequest; - public $handleAuthenticationRequestCallback; - - public $handleNonIndieAuthRequest; + protected string $exceptionTemplatePath; protected string $secret; + protected int $tokenLength; + public function __construct(array $config) { $config = array_merge([ 'csrfMiddleware' => null, @@ -67,8 +69,20 @@ class Server { 'accessTokenStorage' => null, 'httpGetWithEffectiveUrl' => null, 'authorizationForm' => new DefaultAuthorizationForm(), + 'exceptionTemplatePath' => __DIR__ . '/templates/default_exception_response.html.php', + 'tokenLength' => 64 ], $config); + if (!is_int($config['tokenLength'])) { + throw new Exception("\$config['tokenLength'] must be an int!"); + } + $this->tokenLength = $config['tokenLength']; + + if (!is_string($config['exceptionTemplatePath'])) { + throw new Exception("\$config['secret'] must be a string (path)."); + } + $this->exceptionTemplatePath = $config['exceptionTemplatePath']; + $secret = $config['secret'] ?? ''; if (!is_string($secret) || strlen($secret) < 64) { throw new Exception("\$config['secret'] must be a string with a minimum length of 64 characters."); @@ -153,6 +167,12 @@ class Server { trySetLogger($this->authorizationForm, $this->logger); } + /** + * Handle Authorization Endpoint Request + * + * @param ServerRequestInterface $request + * @return ResponseInterface + */ public function handleAuthorizationEndpointRequest(ServerRequestInterface $request): ResponseInterface { $this->logger->info('Handling an IndieAuth Authorization Endpoint request.'); @@ -175,163 +195,188 @@ class Server { } // Because the special case above isn’t allowed to be CSRF-protected, we have to do some rather silly - // gymnastics here to selectively-CSRF-protect requests which do need it. + // closure gymnastics here to selectively-CSRF-protect requests which do need it. return $this->csrfMiddleware->process($request, new Middleware\ClosureRequestHandler(function (ServerRequestInterface $request) { - // If this is an authorization or approval request (allowing POST requests as well to accommodate - // approval requests and custom auth form submission. - if (isIndieAuthAuthorizationRequest($request, ['get', 'post'])) { - $this->logger->info('Handling an authorization request', ['method' => $request->getMethod()]); + // Wrap the entire user-facing handler in a try/catch block which catches any exception, converts it + // to IndieAuthException if necessary, then passes it to $this->handleException() to be turned into a + // response. + try { + // If this is an authorization or approval request (allowing POST requests as well to accommodate + // approval requests and custom auth form submission. + if (isIndieAuthAuthorizationRequest($request, ['get', 'post'])) { + $this->logger->info('Handling an authorization request', ['method' => $request->getMethod()]); - $queryParams = $request->getQueryParams(); - // Return an error if we’re missing required parameters. - $requiredParameters = ['client_id', 'redirect_uri', 'state', 'code_challenge', 'code_challenge_method']; - $missingRequiredParameters = array_filter($requiredParameters, function ($p) use ($queryParams) { - return !array_key_exists($p, $queryParams) || empty($queryParams[$p]); - }); - if (!empty($missingRequiredParameters)) { - $this->logger->warning('The authorization request was missing required parameters. Returning an error response.', ['missing' => $missingRequiredParameters]); - // TODO: return a better response, or at least allow the library consumer to configure a better response. - return new Response(400, ['content-type' => 'text/plain'], 'The IndieAuth request was missing the following parameters: ' . join("\n", $missingRequiredParameters)); - } - - // Normalise the me parameter, if it exists. - if (array_key_exists('me', $queryParams)) { - $queryParams['me'] = IndieAuthClient::normalizeMeURL($queryParams['me']); - } - - // Build a URL containing the indieauth authorization request parameters, hashing them - // to protect them from being changed. - // Make a hash of the protected indieauth-specific parameters. - $hash = hashAuthorizationRequestParameters($request, $this->secret); - $queryParams[self::HASH_QUERY_STRING_KEY] = $hash; - $authenticationRedirect = $request->getUri()->withQuery(buildQueryString($queryParams))->__toString(); - - // User-facing requests always start by calling the authentication request callback. - $this->logger->info('Calling handle_authentication_request callback'); - $authenticationResult = call_user_func($this->handleAuthenticationRequestCallback, $request, $authenticationRedirect); - - // If the authentication handler returned a Response, return that as-is. - if ($authenticationResult instanceof ResponseInterface) { - return $authenticationResult; - } elseif (is_array($authenticationResult)) { - // Check the resulting array for errors. - if (!array_key_exists('me', $authenticationResult)) { - $this->logger->error('The handle_authentication_request callback returned an array with no me key.', ['array' => $authenticationResult]); - return new Response(500, ['content-type' => 'text/plain'], 'An internal error occurred.'); + $queryParams = $request->getQueryParams(); + // Return an error if we’re missing required parameters. + $requiredParameters = ['client_id', 'redirect_uri', 'state', 'code_challenge', 'code_challenge_method']; + $missingRequiredParameters = array_filter($requiredParameters, function ($p) use ($queryParams) { + return !array_key_exists($p, $queryParams) || empty($queryParams[$p]); + }); + if (!empty($missingRequiredParameters)) { + $this->logger->warning('The authorization request was missing required parameters. Returning an error response.', ['missing' => $missingRequiredParameters]); + throw IndieAuthException::create(IndieAuthException::REQUEST_MISSING_PARAMETER, $request); } - // Fetch the client_id URL to find information about the client to present to the user. - try { - /** @var ResponseInterface $clientIdResponse */ - list($clientIdResponse, $clientIdEffectiveUrl) = call_user_func($this->httpGetWithEffectiveUrl, $queryParams['client_id']); - $clientIdMf2 = Mf2\parse((string) $clientIdResponse->getBody(), $clientIdEffectiveUrl); - } catch (ClientExceptionInterface | RequestExceptionInterface | NetworkExceptionInterface $e) { - $this->logger->error("Caught an HTTP exception while trying to fetch the client_id. Returning an error response.", [ - 'client_id' => $queryParams['client_id'], - 'exception' => $e->__toString() - ]); - - return new Response(500, ['content-type' => 'text/plain'], 'An internal error occurred.'); - } catch (Exception $e) { - $this->logger->error("Caught an unknown exception while trying to fetch the client_id. Returning an error response.", [ - 'exception' => $e->__toString() - ]); - - return new Response(500, ['content-type' => 'text/plain'], 'An internal error occurred.'); + // Normalise the me parameter, if it exists. + if (array_key_exists('me', $queryParams)) { + $queryParams['me'] = IndieAuthClient::normalizeMeURL($queryParams['me']); } + + // Build a URL containing the indieauth authorization request parameters, hashing them + // to protect them from being changed. + // Make a hash of the protected indieauth-specific parameters. + $hash = hashAuthorizationRequestParameters($request, $this->secret); + // Operate on a copy of $queryParams, otherwise requests will always have a valid hash! + $redirectQueryParams = $queryParams; + $redirectQueryParams[self::HASH_QUERY_STRING_KEY] = $hash; + $authenticationRedirect = $request->getUri()->withQuery(buildQueryString($redirectQueryParams))->__toString(); - // Search for an h-app with u-url matching the client_id. - $clientHApps = M\findMicroformatsByProperty(M\findMicroformatsByType($clientIdMf2, 'h-app'), 'url', $queryParams['client-id']); - $clientHApp = empty($clientHApps) ? null : $clientHApps[0]; - - // Search for all link@rel=redirect_uri at the client_id. - $clientIdRedirectUris = []; - if (array_key_exists('redirect_uri', $clientIdMf2['rels'])) { - $clientIdRedirectUris = array_merge($clientIdRedirectUris, $clientIdMf2['rels']); - } - foreach (HeaderParser::parse($clientIdResponse->getHeader('Link')) as $link) { - if (array_key_exists('rel', $link) and str_contains(" {$link['rel']} ", " redirect_uri ")) { - // Strip off the < > which surround the link URL for some reason. - $clientIdRedirectUris[] = substr($link[0], 1, strlen($link[0]) - 2); + // User-facing requests always start by calling the authentication request callback. + $this->logger->info('Calling handle_authentication_request callback'); + $authenticationResult = call_user_func($this->handleAuthenticationRequestCallback, $request, $authenticationRedirect); + + // If the authentication handler returned a Response, return that as-is. + if ($authenticationResult instanceof ResponseInterface) { + return $authenticationResult; + } elseif (is_array($authenticationResult)) { + // Check the resulting array for errors. + if (!array_key_exists('me', $authenticationResult)) { + $this->logger->error('The handle_authentication_request callback returned an array with no me key.', ['array' => $authenticationResult]); + throw IndieAuthException::create(IndieAuthException::AUTHENTICATION_CALLBACK_MISSING_ME_PARAM, $request); } - } - // If the authority of the redirect_uri does not match the client_id, or exactly match one of their redirect URLs, return an error. - $cidComponents = M\parseUrl($queryParams['client_id']); - $ruriComponents = M\parseUrl($queryParams['redirect_uri']); - $clientIdMatchesRedirectUri = $cidComponents['scheme'] == $ruriComponents['scheme'] - && $cidComponents['host'] == $ruriComponents['host'] - && $cidComponents['port'] == $ruriComponents['port']; - $redirectUriValid = $clientIdMatchesRedirectUri || in_array($queryParams['redirect_uri'], $clientIdRedirectUris); + // If this is a POST request sent from the authorization (i.e. scope-choosing) form: + if (isAuthorizationApprovalRequest($request)) { + // Authorization approval requests MUST include a hash protecting the sensitive IndieAuth + // authorization request parameters from being changed, e.g. by a malicious script which + // found its way onto the authorization form. + $expectedHash = hashAuthorizationRequestParameters($request, $this->secret); + if (is_null($expectedHash)) { + // In theory this code should never be reached, as we already checked the request for valid parameters. + // However, it’s possible for hashAuthorizationRequestParameters() to return null, and if for whatever + // reason it does, the library should handle that case as elegantly as possible. + $this->logger->warning("Calculating the expected hash for an authorization approval request failed. This SHOULD NOT happen; if you encounter this error please contact the maintainers of taproot/indieauth."); + throw IndieAuthException::create(IndieAuthException::REQUEST_MISSING_PARAMETER, $request); + } + + if (!array_key_exists(self::HASH_QUERY_STRING_KEY, $queryParams)) { + $this->logger->warning("An authorization approval request did not have a " . self::HASH_QUERY_STRING_KEY . " parameter."); + throw IndieAuthException::create(IndieAuthException::AUTHORIZATION_APPROVAL_REQUEST_MISSING_HASH, $request); + } - if (!$redirectUriValid) { - $this->logger->warning("The provided redirect_uri did not match either the client_id, nor the discovered redirect URIs.", [ - 'provided_redirect_uri' => $queryParams['redirect_uri'], - 'provided_client_id' => $queryParams['client_id'], - 'discovered_redirect_uris' => $clientIdRedirectUris - ]); - - return new Response(500, ['content-type' => 'text/plain'], 'An internal error occurred.'); - } - - // If this is a POST request sent from the authorization (i.e. scope-choosing) form: - if (isAuthorizationApprovalRequest($request)) { - // Authorization approval requests MUST include a hash protecting the sensitive IndieAuth - // authorization request parameters from being changed, e.g. by a malicious script which - // found its way onto the authorization form. - $expectedHash = hashAuthorizationRequestParameters($request, $this->secret); - if (is_null($expectedHash)) { - $this->logger->warning("An authorization approval request did not have a " . self::HASH_QUERY_STRING_KEY . " parameter."); - return new Response(400, ['content-type' => 'text/plain'], 'The ' . self::HASH_QUERY_STRING_KEY . ' parameter was missing!'); - } - if (!hash_equals($expectedHash, $queryParams[self::HASH_QUERY_STRING_KEY])) { - $this->logger->warning("The hash provided in the URL was invalid!", [ - 'expected' => $expectedHash, - 'actual' => $queryParams[self::HASH_QUERY_STRING_KEY] + if (!hash_equals($expectedHash, $queryParams[self::HASH_QUERY_STRING_KEY])) { + $this->logger->warning("The hash provided in the URL was invalid!", [ + 'expected' => $expectedHash, + 'actual' => $queryParams[self::HASH_QUERY_STRING_KEY] + ]); + throw IndieAuthException::create(IndieAuthException::AUTHORIZATION_APPROVAL_REQUEST_INVALID_HASH, $request); + } + + // Assemble the data for the authorization code, store it somewhere persistent. + $code = array_merge($authenticationResult, [ + 'client_id' => $queryParams['client_id'], + 'redirect_uri' => $queryParams['redirect_uri'], + 'state' => $queryParams['state'], + 'code_challenge' => $queryParams['code_challenge'], + 'code_challenge_method' => $queryParams['code_challenge_method'], + 'requested_scope' => $queryParams['scope'] ?? '', + 'code' => generateRandomString($this->tokenLength) ]); - return new Response(400, ['content-type' => 'text/plain'], 'Invalid hash!'); + + // Pass it to the auth code customisation callback. + $code = $this->authorizationForm->transformAuthorizationCode($request, $code); + + // Store the authorization code. + $success = $this->authorizationCodeStorage->put($code['code'], $code); + if (!$success) { + // 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); + } + + // Return a redirect to the client app. + return new Response(302, ['Location' => appendQueryParams($queryParams['redirect_uri'], [ + 'code' => $code['code'], + 'state' => $code['state'] + ])]); } - // Assemble the data for the authorization code, store it somewhere persistent. - $code = array_merge($authenticationResult, [ - 'client_id' => $queryParams['client_id'], - 'redirect_uri' => $queryParams['redirect_uri'], - 'state' => $queryParams['state'], - 'code_challenge' => $queryParams['code_challenge'], - 'code_challenge_method' => $queryParams['code_challenge_method'], - 'requested_scope' => $queryParams['scope'] ?? '', - 'code' => generateRandomString(256) - ]); + // Otherwise, the user is authenticated and needs to authorize the client app + choose scopes. - // Pass it to the auth code customisation callback, if any. + // Fetch the client_id URL to find information about the client to present to the user. + try { + /** @var ResponseInterface $clientIdResponse */ + list($clientIdResponse, $clientIdEffectiveUrl) = call_user_func($this->httpGetWithEffectiveUrl, $queryParams['client_id']); + $clientIdMf2 = Mf2\parse((string) $clientIdResponse->getBody(), $clientIdEffectiveUrl); + } catch (ClientExceptionInterface | RequestExceptionInterface | NetworkExceptionInterface $e) { + $this->logger->error("Caught an HTTP exception while trying to fetch the client_id. Returning an error response.", [ + 'client_id' => $queryParams['client_id'], + 'exception' => $e->__toString() + ]); + + throw IndieAuthException::create(IndieAuthException::HTTP_EXCEPTION_FETCHING_CLIENT_ID, $request, $e); + } catch (Exception $e) { + $this->logger->error("Caught an unknown exception while trying to fetch the client_id. Returning an error response.", [ + 'exception' => $e->__toString() + ]); + + throw IndieAuthException::create(IndieAuthException::INTERNAL_EXCEPTION_FETCHING_CLIENT_ID, $request, $e); + } - $code = $this->authorizationForm->transformAuthorizationCode($request, $code); - // Store the authorization code. - $this->authorizationCodeStorage->put($code['code'], $code); + // Search for an h-app with u-url matching the client_id. + $clientHApps = M\findMicroformatsByProperty(M\findMicroformatsByType($clientIdMf2, 'h-app'), 'url', $queryParams['client_id']); + $clientHApp = empty($clientHApps) ? null : $clientHApps[0]; + + // Search for all link@rel=redirect_uri at the client_id. + $clientIdRedirectUris = []; + if (array_key_exists('redirect_uri', $clientIdMf2['rels'])) { + $clientIdRedirectUris = array_merge($clientIdRedirectUris, $clientIdMf2['rels']['redirect_uri']); + } + + foreach (HeaderParser::parse($clientIdResponse->getHeader('Link')) as $link) { + if (array_key_exists('rel', $link) && mb_strpos(" {$link['rel']} ", " redirect_uri ") !== false) { + // Strip off the < > which surround the link URL for some reason. + $clientIdRedirectUris[] = substr($link[0], 1, strlen($link[0]) - 2); + } + } - // Return a redirect to the client app. - return new Response(302, ['Location' => appendQueryParams($queryParams['redirect_uri'], [ - 'code' => $code['code'], - 'state' => $code['state'] - ])]); + // If the authority of the redirect_uri does not match the client_id, or exactly match one of their redirect URLs, return an error. + $clientIdMatchesRedirectUri = urlComponentsMatch($queryParams['client_id'], $queryParams['redirect_uri'], [PHP_URL_SCHEME, PHP_URL_HOST, PHP_URL_PORT]); + $redirectUriValid = $clientIdMatchesRedirectUri || in_array($queryParams['redirect_uri'], $clientIdRedirectUris); + + if (!$redirectUriValid) { + $this->logger->warning("The provided redirect_uri did not match either the client_id, nor the discovered redirect URIs.", [ + 'provided_redirect_uri' => $queryParams['redirect_uri'], + 'provided_client_id' => $queryParams['client_id'], + 'discovered_redirect_uris' => $clientIdRedirectUris + ]); + + throw IndieAuthException::create(IndieAuthException::INVALID_REDIRECT_URI, $request); + } + + // Present the authorization UI. + return $this->authorizationForm->showForm($request, $authenticationResult, $authenticationRedirect, $clientHApp); } - - // Otherwise, the user is authenticated and needs to authorize the client app + choose scopes. - - // Present the authorization UI. - return $this->authorizationForm->showForm($request, $authenticationResult, $authenticationRedirect, $clientHApp); } - } - // If the request isn’t an IndieAuth Authorization or Code-redeeming request, it’s either an invalid - // request or something to do with a custom auth handler (e.g. sending a one-time code in an email.) - $nonIndieAuthRequestResult = call_user_func($this->handleNonIndieAuthRequest, $request); - if ($nonIndieAuthRequestResult instanceof ResponseInterface) { - return $nonIndieAuthRequestResult; - } else { - return new Response(400, ['content-type' => 'text/plain'], 'Invalid request!'); + // If the request isn’t an IndieAuth Authorization or Code-redeeming request, it’s either an invalid + // request or something to do with a custom auth handler (e.g. sending a one-time code in an email.) + $nonIndieAuthRequestResult = call_user_func($this->handleNonIndieAuthRequest, $request); + if ($nonIndieAuthRequestResult instanceof ResponseInterface) { + return $nonIndieAuthRequestResult; + } else { + throw IndieAuthException::create(IndieAuthException::INTERNAL_ERROR, $request); + } + } catch (IndieAuthException $e) { + // All IndieAuthExceptions will already have been logged. + return $this->handleException($e); + } catch (Exception $e) { + // Unknown exceptions will not have been logged; do so now. + $this->logger->error("Caught unknown exception: {$e}"); + return $this->handleException(IndieAuthException::create(0, $request, $e)); } - })); + })); } public function handleTokenEndpointRequest(ServerRequestInterface $request): ResponseInterface { @@ -347,4 +392,11 @@ class Server { // If everything checks out, generate an access token and return it. } + + protected function handleException(IndieAuthException $exception): ResponseInterface { + return new Response($exception->getStatusCode(), ['content-type' => 'text/html'], renderTemplate($this->exceptionTemplatePath, [ + 'request' => $exception->getRequest(), + 'exception' => $exception + ])); + } } diff --git a/src/Storage/FilesystemJsonStorage.php b/src/Storage/FilesystemJsonStorage.php index e09113c..6956b41 100644 --- a/src/Storage/FilesystemJsonStorage.php +++ b/src/Storage/FilesystemJsonStorage.php @@ -53,7 +53,7 @@ class FilesystemJsonStorage implements TokenStorageInterface { public function put(string $key, array $data): bool { // Ensure that the containing folder exists. @mkdir($this->path, 0777, true); - + return file_put_contents($this->getPath($key), json_encode($data)) !== false; } diff --git a/src/functions.php b/src/functions.php index c70a44a..d8f284e 100644 --- a/src/functions.php +++ b/src/functions.php @@ -2,10 +2,14 @@ namespace Taproot\IndieAuth; +use Exception; +use IndieAuth\Client; use Psr\Http\Message\ServerRequestInterface; use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerInterface; +use function BarnabyWalters\Mf2\parseUrl; + // From https://github.com/indieweb/indieauth-client-php/blob/main/src/IndieAuth/Client.php, thanks aaronpk. function generateRandomString($numBytes) { if (function_exists('random_bytes')) { @@ -51,7 +55,7 @@ function isIndieAuthAuthorizationRequest(ServerRequestInterface $request, $permi function isAuthorizationApprovalRequest(ServerRequestInterface $request) { return strtolower($request->getMethod()) == 'post' && array_key_exists('taproot_indieauth_action', $request->getParsedBody()) - && $request->getParsedBody()['taproot_indieauth_action'] == 'approve'; + && $request->getParsedBody()[Server::APPROVE_ACTION_KEY] == Server::APPROVE_ACTION_VALUE; } function buildQueryString(array $parameters) { @@ -62,6 +66,23 @@ function buildQueryString(array $parameters) { return join('&', $qs); } +function urlComponentsMatch($url1, $url2, ?array $components=null): bool { + $validComponents = [PHP_URL_HOST, PHP_URL_PASS, PHP_URL_PATH, PHP_URL_PORT, PHP_URL_USER, PHP_URL_QUERY, PHP_URL_SCHEME, PHP_URL_FRAGMENT]; + $components = $components ?? $validComponents; + + foreach ($components as $cmp) { + if (!in_array($cmp, $validComponents)) { + throw new Exception("Invalid parse_url() component passed: $cmp"); + } + + if (parse_url($url1, $cmp) !== parse_url($url2, $cmp)) { + return false; + } + } + + return true; +} + /** * Append Query Parameters * diff --git a/src/templates/default_authorization_page.html.php b/src/templates/default_authorization_page.html.php index 7cb7ee7..5748c6d 100644 --- a/src/templates/default_authorization_page.html.php +++ b/src/templates/default_authorization_page.html.php @@ -1,4 +1,7 @@ ">Cancel (back to ) - +

diff --git a/src/templates/default_exception_response.html.php b/src/templates/default_exception_response.html.php new file mode 100644 index 0000000..38f1bd9 --- /dev/null +++ b/src/templates/default_exception_response.html.php @@ -0,0 +1,32 @@ + + + + + + IndieAuth • Error! + + + + +

Error: getMessage()) ?>

+ +

getExplanation()) ?>

+ + + trustQueryParams()): ?> +

Return to app (getQueryParams()['client_id']) ?> ?>) + + + + +

+ + diff --git a/tests/ServerTest.php b/tests/ServerTest.php index 2fd718a..7cea1f1 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -2,17 +2,30 @@ namespace Taproot\IndieAuth\Test; +use Exception; use Nyholm\Psr7\Response; use Nyholm\Psr7\ServerRequest; +use Nyholm\Psr7\Request; +use PDO; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Taproot\IndieAuth\Callback\DefaultAuthorizationForm; use Taproot\IndieAuth\Callback\SingleUserPasswordAuthenticationCallback; +use Taproot\IndieAuth\IndieAuthException; +use Taproot\IndieAuth\Middleware\NoOpMiddleware; use Taproot\IndieAuth\Server; use Taproot\IndieAuth\Storage\FilesystemJsonStorage; +use function Taproot\IndieAuth\hashAuthorizationRequestParameters; +use function Taproot\IndieAuth\urlComponentsMatch; + const SERVER_SECRET = '1111111111111111111111111111111111111111111111111111111111111111'; const AUTH_CODE_STORAGE_PATH = __DIR__ . '/tmp/authorization_codes'; const ACCESS_TOKEN_STORAGE_PATH = __DIR__ . '/tmp/authorization_codes'; +const CODE_EXCEPTION_TEMPLATE_PATH = __DIR__ . '/templates/code_exception_response.txt.php'; +const AUTHORIZATION_FORM_JSON_RESPONSE_TEMPLATE_PATH = __DIR__ . '/templates/authorization_form_json_response.json.php'; const TMP_DIR = __DIR__ . '/tmp'; class ServerTest extends TestCase { @@ -21,11 +34,15 @@ class ServerTest extends TestCase { 'secret' => SERVER_SECRET, 'authorizationCodeStorage' => AUTH_CODE_STORAGE_PATH, 'accessTokenStorage' => ACCESS_TOKEN_STORAGE_PATH, - Server::HANDLE_AUTHENTICATION_REQUEST => new SingleUserPasswordAuthenticationCallback(['me' => 'https://example.com/'], password_hash('password', PASSWORD_DEFAULT)) + // With this template, IndieAuthException response bodies will contain only their IndieAuthException error code, for ease of comparison. + 'exceptionTemplatePath' => CODE_EXCEPTION_TEMPLATE_PATH, + // Default to a simple single-user password authentication handler. + Server::HANDLE_AUTHENTICATION_REQUEST => new SingleUserPasswordAuthenticationCallback(['me' => 'https://example.com/'], password_hash('password', PASSWORD_DEFAULT), Server::DEFAULT_CSRF_KEY), + 'authorizationForm' => new DefaultAuthorizationForm(AUTHORIZATION_FORM_JSON_RESPONSE_TEMPLATE_PATH), ], $config)); } - protected function getIARequest(array $params=[]) { + protected function getIARequest(array $params=[]): ServerRequestInterface { return (new ServerRequest('GET', 'https://example.com/'))->withQueryParams(array_merge([ 'response_type' => 'code', 'client_id' => 'https://app.example.com/', @@ -36,6 +53,36 @@ class ServerTest extends TestCase { ], $params)); } + protected function getApprovalRequest(bool $validCsrf=false, bool $addValidHash=false, ?array $queryParams=null, ?array $parsedBody=null): ServerRequestInterface { + $queryParams = $queryParams ?? []; + $parsedBody = $parsedBody ?? []; + $cookieParams = []; + + $parsedBody[Server::APPROVE_ACTION_KEY] = Server::APPROVE_ACTION_VALUE; + + // Assume Middleware\DoubleSubmitCookieCsrfMiddleware is being used. + $csrfVal = 'random_and_secure_csrf_value'; + if ($validCsrf) { + $parsedBody[Server::DEFAULT_CSRF_KEY] = $csrfVal; + $cookieParams = [ + Server::DEFAULT_CSRF_KEY => $csrfVal + ]; + } + + $req = $this->getIARequest($queryParams) + ->withMethod('POST') + ->withParsedBody($parsedBody) + ->withCookieParams($cookieParams); + + if ($addValidHash) { + $req = $req->withQueryParams(array_merge($req->getQueryParams(), [ + Server::HASH_QUERY_STRING_KEY => hashAuthorizationRequestParameters($req, SERVER_SECRET) + ])); + } + + return $req; + } + protected function setUp(): void { // Clean up tmp folder. new FilesystemJsonStorage(AUTH_CODE_STORAGE_PATH, -1, true); @@ -55,9 +102,11 @@ class ServerTest extends TestCase { public function testAuthorizationRequestMissingParametersReturnsError() { $s = $this->getDefaultServer(); - $req = (new ServerRequest('GET', 'https://example.com/')); + $req = (new ServerRequest('GET', 'https://example.com/'))->withQueryParams([ + 'response_type' => 'code' // This param is required to identify the request as an IA authorization request. + ]); $res = $s->handleAuthorizationEndpointRequest($req); - $this->assertEquals(400, $res->getStatusCode()); + $this->assertEquals((string) IndieAuthException::REQUEST_MISSING_PARAMETER, (string) $res->getBody()); } public function testUnauthenticatedRequestReturnsAuthenticationResponse() { @@ -83,27 +132,311 @@ class ServerTest extends TestCase { $res = $s->handleAuthorizationEndpointRequest($this->getIARequest()); - $this->assertEquals(500, $res->getStatusCode()); + $this->assertEquals((string) IndieAuthException::AUTHENTICATION_CALLBACK_MISSING_ME_PARAM, (string) $res->getBody()); } - public function testReturnServerErrorIfFetchingClientIdThrowsException() { - $exceptionClasses = ['GuzzleHttp\Exception\ConnectException', 'GuzzleHttp\Exception\RequestException']; - foreach ($exceptionClasses as $eClass) { - $req = $this->getIARequest(); + public function testReturnErrorIfFetchingClientIdThrowsException() { + $exceptionClasses = [ + 'GuzzleHttp\Exception\ConnectException' => (string) IndieAuthException::HTTP_EXCEPTION_FETCHING_CLIENT_ID, + 'GuzzleHttp\Exception\RequestException' => (string) IndieAuthException::HTTP_EXCEPTION_FETCHING_CLIENT_ID, + 'Exception' => (string) IndieAuthException::INTERNAL_EXCEPTION_FETCHING_CLIENT_ID + ]; + foreach ($exceptionClasses as $eClass => $expectedResponse) { $s = $this->getDefaultServer([ Server::HANDLE_AUTHENTICATION_REQUEST => function (ServerRequestInterface $request, string $formAction) { return ['me' => 'https://example.com/']; }, - 'httpGetWithEffectiveUrl' => function ($url) use ($eClass, $req) { - throw new $eClass($eClass, $req); + 'httpGetWithEffectiveUrl' => function ($url) use ($eClass) { + if ($eClass == 'Exception') { throw new Exception(); } + throw new $eClass($eClass, new Request('GET', $url)); } ]); - - $res = $s->handleAuthorizationEndpointRequest($req); - $this->assertEquals(500, $res->getStatusCode()); + $res = $s->handleAuthorizationEndpointRequest($this->getIARequest()); + + $this->assertEquals($expectedResponse, (string) $res->getBody()); } } - + public function testReturnsErrorIfApprovalRequestHasNoHash() { + $s = $this->getDefaultServer([ + Server::HANDLE_AUTHENTICATION_REQUEST => function (ServerRequestInterface $request, string $formAction) { + return ['me' => 'https://example.com']; + } + ]); + $res = $s->handleAuthorizationEndpointRequest($this->getApprovalRequest(true, false)); + + $this->assertEquals((string) IndieAuthException::AUTHORIZATION_APPROVAL_REQUEST_MISSING_HASH, (string) $res->getBody()); + } + + public function testReturnsErrorIfApprovalRequestHasInvalidHash() { + $s = $this->getDefaultServer([ + Server::HANDLE_AUTHENTICATION_REQUEST => function (ServerRequestInterface $request, string $formAction) { + return ['me' => 'https://example.com']; + } + ]); + $req = $this->getApprovalRequest(true, false); + $req = $req->withQueryParams(array_merge($req->getQueryParams(), [ + Server::HASH_QUERY_STRING_KEY => 'clearly not a valid hash' + ])); + $res = $s->handleAuthorizationEndpointRequest($req); + + $this->assertEquals((string) IndieAuthException::AUTHORIZATION_APPROVAL_REQUEST_INVALID_HASH, (string) $res->getBody()); + } + + public function testValidApprovalRequestIsHandledCorrectly() { + // Make a valid authentication response with additional information, to make sure that it’s saved + // in the authorization code. + $authenticationResponse = [ + 'me' => 'https://me.example.com/', + 'profile' => [ + 'name' => 'Example Name' + ] + ]; + + $s = $this->getDefaultServer([ + Server::HANDLE_AUTHENTICATION_REQUEST => function (ServerRequestInterface $request, string $formAction) use ($authenticationResponse) { + return $authenticationResponse; + } + ]); + + // Make an approval request with valid CSRF tokens, a valid query parameter hash, one requested scope + // (different from the two granted scopes, so that we can test that requested and granted scopes are + // stored separately) and a redirect URI with a query string (so we can test that our IA query string + // parameters are appended correctly). + $grantedScopes = ['create', 'update']; + $req = $this->getApprovalRequest(true, true, [ + 'scope' => 'create', + 'redirect_uri' => 'https://app.example.com/indieauth?client_redirect_query_string_param=value' + ], [ + 'taproot_indieauth_server_scope[]' => $grantedScopes + ]); + + $res = $s->handleAuthorizationEndpointRequest($req); + + $this->assertEquals(302, $res->getStatusCode(), 'The Response from a successful approval request must be a 302 redirect.'); + + $responseLocation = $res->getHeaderLine('location'); + $queryParams = $req->getQueryParams(); + parse_str(parse_url($responseLocation, PHP_URL_QUERY), $redirectUriQueryParams); + + $this->assertTrue(urlComponentsMatch($responseLocation, $queryParams['redirect_uri'], [PHP_URL_SCHEME, PHP_URL_HOST, PHP_URL_USER, PHP_URL_PORT, PHP_URL_HOST, PHP_URL_PORT, PHP_URL_PATH]), 'The successful redirect response location did not match the redirect URI up to the path.'); + $this->assertEquals($redirectUriQueryParams['state'], $queryParams['state'], 'The redirect URI state parameter did not match the authorization request state parameter.'); + $this->assertEquals('value', $redirectUriQueryParams['client_redirect_query_string_param'], 'Query string params in the client app redirect_uri were not correctly preserved.'); + + $storage = new FilesystemJsonStorage(AUTH_CODE_STORAGE_PATH); + $storedCode = $storage->get($redirectUriQueryParams['code']); + + $this->assertNotNull($storedCode, 'An authorization code should be stored after a successful approval request.'); + + foreach (['client_id', 'redirect_uri', 'state', 'code_challenge', 'code_challenge_method'] as $p) { + $this->assertEquals($queryParams[$p], $storedCode[$p], "Parameter $p in the stored code ({$storedCode[$p]}) was not the same as the request parameter ($queryParams[$p])."); + } + + $this->assertTrue(scopeEquals($queryParams['scope'], $storedCode['requested_scope']), "The requested scopes in the stored code ({$storedCode['requested_scope']}) did not match the scopes in the scope query parameter ({$queryParams['scope']})."); + $this->assertTrue(scopeEquals($grantedScopes, $storedCode['scope']), "The granted scopes in the stored code ({$storedCode['scope']}) did not match the granted scopes from the authorization form response (" . join(' ', $grantedScopes) . ")."); + + $this->assertEquals($authenticationResponse['me'], $storedCode['me'], "The “me” value in the stored code ({$storedCode['me']}) did not match the “me” value from the authentication response ({$authenticationResponse['me']})."); + $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 testReturnsErrorIfRedirectUriDoesntMatchClientIdWithNoParsedRedirectUris() { + $s = $this->getDefaultServer([ + Server::HANDLE_AUTHENTICATION_REQUEST => function (ServerRequestInterface $request, string $formAction): array { + return ['me' => 'https://me.example.com']; + }, + 'httpGetWithEffectiveUrl' => function ($url): array { + // An empty response suffices for this test. + return [ + new Response(200, ['content-type' => 'text/html'], '' ), + $url + ]; + } + ]); + + $req = $this->getIARequest([ + 'client_id' => 'https://client.example.com/', + 'redirect_uri' => 'https://not-the-client.example.com/auth' + ]); + + $res = $s->handleAuthorizationEndpointRequest($req); + + $this->assertEquals((string) IndieAuthException::INVALID_REDIRECT_URI, (string) $res->getBody()); + } + + public function testReturnsErrorIfRedirectUriDoesntMatchClientIdOrParsedRedirectUris() { + $s = $this->getDefaultServer([ + Server::HANDLE_AUTHENTICATION_REQUEST => function (ServerRequestInterface $request, string $formAction): array { + return ['me' => 'https://me.example.com']; + }, + 'httpGetWithEffectiveUrl' => function ($url): array { + // Pass some tricky values to test for correct rel parsing. + return [ + new Response(200, [ + 'content-type' => 'text/html', + 'link' => [ + '; rel="wrong_redirect_uri_rel"', // Matches redirect_uri but has wrong rel + '; rel="redirect_uri"' // redirect_uri is correct but url is invalid. + ] + ], + << +href matches redirect_uri, wrong rel: +EOT + ), + $url + ]; + } + ]); + + $req = $this->getIARequest([ + 'client_id' => 'https://client.example.com/', + 'redirect_uri' => 'https://not-the-client.example.com/auth' + ]); + + $res = $s->handleAuthorizationEndpointRequest($req); + + $this->assertEquals((string) IndieAuthException::INVALID_REDIRECT_URI, (string) $res->getBody()); + } + + public function testReturnsAuthorizationFormIfClientIdSufficientlyMatchesRedirectUri() { + $s = $this->getDefaultServer([ + 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'], ''), // An empty response suffices for this test. + $url + ]; + } + ]); + + $req = $this->getIARequest([ + 'client_id' => 'https://client.example.com/', + 'redirect_uri' => 'https://client.example.com/auth' + ]); + + $res = $s->handleAuthorizationEndpointRequest($req); + + $this->assertEquals(200, $res->getStatusCode()); + } + + public function testReturnsAuthorizationFormIfClientIdExactlyMatchesParsedLinkHeaderRedirectUri() { + $s = $this->getDefaultServer([ + 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', + 'link' => '; rel="another_rel redirect_uri"' + ], + '' + ), + $url + ]; + } + ]); + + $req = $this->getIARequest([ + 'client_id' => 'https://client.example.com/', + 'redirect_uri' => 'https://link-header.example.com/auth' + ]); + + $res = $s->handleAuthorizationEndpointRequest($req); + + $this->assertEquals(200, $res->getStatusCode()); + } + + public function testReturnsAuthorizationFormIfClientIdExactlyMatchesParsedLinkElementRedirectUri() { + $s = $this->getDefaultServer([ + 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->getIARequest([ + 'client_id' => 'https://client.example.com/', + 'redirect_uri' => 'https://link-element.example.com/auth' + ]); + + $res = $s->handleAuthorizationEndpointRequest($req); + + $this->assertEquals(200, $res->getStatusCode()); + } + + public function testFindsFirstHAppExactlyMatchingClientId() { + $correctHAppName = 'Correct h-app!'; + $correctHAppUrl = 'https://client.example.com/'; + $correctHAppPhoto = 'https://client.example.com/logo.png'; + + $s = $this->getDefaultServer([ + Server::HANDLE_AUTHENTICATION_REQUEST => function (ServerRequestInterface $request, string $formAction) { + return ['me' => 'https://me.example.com']; + }, + 'httpGetWithEffectiveUrl' => function ($url) use ($correctHAppPhoto, $correctHAppName, $correctHAppUrl) { + return [ + new Response(200, ['content-type' => 'text/html'], + <<Wrong + +{$correctHAppName} +EOT + ), + $url + ]; + } + ]); + + $req = $this->getIARequest([ + 'client_id' => $correctHAppUrl, + 'redirect_uri' => 'https://client.example.com/auth' + ]); + + $res = $s->handleAuthorizationEndpointRequest($req); + + $this->assertEquals(200, $res->getStatusCode()); + + $parsedResponse = json_decode((string) $res->getBody(), true); + $flatHApp = $parsedResponse['clientHApp']; + $this->assertEquals($correctHAppUrl, $flatHApp['url']); + $this->assertEquals($correctHAppName, $flatHApp['name']); + $this->assertEquals($correctHAppPhoto, $flatHApp['photo']); + } + + public function testNonIndieAuthRequestWithDefaultHandlerReturnsError() { + $res = $this->getDefaultServer()->handleAuthorizationEndpointRequest(new ServerRequest('GET', 'https://example.com')); + + $this->assertEquals((string) IndieAuthException::INTERNAL_ERROR, (string) $res->getBody()); + } + + public function testResponseReturnedFromNonIndieAuthRequestHandler() { + $responseBody = 'A response to a non-indieauth request.'; + + $res = $this->getDefaultServer([ + Server::HANDLE_NON_INDIEAUTH_REQUEST => function (ServerRequestInterface $request) use ($responseBody) { + return new Response(200, ['content-type' => 'text/plain'], $responseBody); + } + ])->handleAuthorizationEndpointRequest(new ServerRequest('GET', 'https://example.com')); + + $this->assertEquals($responseBody, (string) $res->getBody()); + } +} + +function scopeEquals($scope1, $scope2): bool { + $scope1 = is_string($scope1) ? explode(' ', $scope1) : $scope1; + $scope2 = is_string($scope2) ? explode(' ', $scope2) : $scope2; + sort($scope1); + sort($scope2); + return $scope1 == $scope2; } diff --git a/tests/templates/authorization_form_json_response.json.php b/tests/templates/authorization_form_json_response.json.php new file mode 100644 index 0000000..731640d --- /dev/null +++ b/tests/templates/authorization_form_json_response.json.php @@ -0,0 +1,21 @@ + $formAction, + 'clientHApp' => $clientHApp, + 'user' => $user, + 'scopes' => $scopes, + 'clientId' => $clientId, + 'clientRedirectUri' => $clientRedirectUri, + 'csrfFormElement' => $csrfFormElement +]); +?> \ No newline at end of file diff --git a/tests/templates/code_exception_response.txt.php b/tests/templates/code_exception_response.txt.php new file mode 100644 index 0000000..e79f0b2 --- /dev/null +++ b/tests/templates/code_exception_response.txt.php @@ -0,0 +1,4 @@ +getCode() ?> \ No newline at end of file