From ca1819776eb1e9478f05ca8fc04f303b912804e4 Mon Sep 17 00:00:00 2001 From: Barnaby Walters Date: Sun, 13 Jun 2021 15:24:17 +0200 Subject: [PATCH] Tested SingleUserPasswordAuthCallback, improved ServerTest --- ...ngleUserPasswordAuthenticationCallback.php | 12 +++++--- src/Server.php | 28 +++++++++++-------- src/Storage/TokenStorageInterface.php | 2 +- tests/ServerTest.php | 7 +++-- 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/Callback/SingleUserPasswordAuthenticationCallback.php b/src/Callback/SingleUserPasswordAuthenticationCallback.php index b2df3b1..7b1fba8 100644 --- a/src/Callback/SingleUserPasswordAuthenticationCallback.php +++ b/src/Callback/SingleUserPasswordAuthenticationCallback.php @@ -2,7 +2,7 @@ namespace Taproot\IndieAuth\Callback; -use Exception; +use BadMethodCallException; use Nyholm\Psr7\Response; use Psr\Http\Message\ServerRequestInterface; @@ -59,8 +59,12 @@ class SingleUserPasswordAuthenticationCallback { * @param string|null $csrfKey The key under which to fetch a CSRF token from `$request` attributes, and as the CSRF token name in submitted form data. Defaults to the Server default, only change if you’re using a custom CSRF middleware. */ public function __construct(array $user, string $hashedPassword, ?string $formTemplate=null, ?string $csrfKey=null) { - if (!array_key_exists('me', $user) || !is_string($user['me'])) { - throw new Exception('The $user array MUST contain a “me” key, the value which must be the user’s canonical URL as a string.'); + if (!isset($user['me'])) { + throw new BadMethodCallException('The $user array MUST contain a “me” key, the value which must be the user’s canonical URL as a string.'); + } + + if (is_null(password_get_info($hashedPassword)['algo'])) { + throw new BadMethodCallException('The provided $hashedPassword was not a valid hash created by the password_hash() function.'); } $this->user = $user; $this->hashedPassword = $hashedPassword; @@ -68,7 +72,7 @@ class SingleUserPasswordAuthenticationCallback { $this->csrfKey = $csrfKey ?? \Taproot\IndieAuth\Server::DEFAULT_CSRF_KEY; } - public function __invoke(ServerRequestInterface $request, string $formAction, ?string $normalizedMeUrl) { + public function __invoke(ServerRequestInterface $request, string $formAction, ?string $normalizedMeUrl=null) { // If the request is a form submission with a matching password, return the corresponding // user data. if ($request->getMethod() == 'POST' && password_verify($request->getParsedBody()[self::PASSWORD_FORM_PARAMETER] ?? '', $this->hashedPassword)) { diff --git a/src/Server.php b/src/Server.php index 411f57a..eaa8613 100644 --- a/src/Server.php +++ b/src/Server.php @@ -2,6 +2,7 @@ namespace Taproot\IndieAuth; +use BadMethodCallException; use BarnabyWalters\Mf2 as M; use Exception; use GuzzleHttp\Psr7\Header as HeaderParser; @@ -186,7 +187,7 @@ class Server { */ public function __construct(array $config) { $config = array_merge([ - 'csrfMiddleware' => null, + 'csrfMiddleware' => new Middleware\DoubleSubmitCookieCsrfMiddleware(self::DEFAULT_CSRF_KEY), 'logger' => null, self::HANDLE_NON_INDIEAUTH_REQUEST => function (ServerRequestInterface $request) { return null; }, // Default to no-op. 'tokenStorage' => null, @@ -196,28 +197,28 @@ class Server { ], $config); if (!is_string($config['exceptionTemplatePath'])) { - throw new Exception("\$config['exceptionTemplatePath'] must be a string (path)."); + throw new BadMethodCallException("\$config['exceptionTemplatePath'] 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."); + throw new BadMethodCallException("\$config['secret'] must be a string with a minimum length of 64 characters."); } $this->secret = $secret; if (!is_null($config['logger']) && !$config['logger'] instanceof LoggerInterface) { - throw new Exception("\$config['logger'] must be an instance of \\Psr\\Log\\LoggerInterface or null."); + throw new BadMethodCallException("\$config['logger'] must be an instance of \\Psr\\Log\\LoggerInterface or null."); } $this->logger = $config['logger'] ?? new NullLogger(); if (!(array_key_exists(self::HANDLE_AUTHENTICATION_REQUEST, $config) and is_callable($config[self::HANDLE_AUTHENTICATION_REQUEST]))) { - throw new Exception('$callbacks[\'' . self::HANDLE_AUTHENTICATION_REQUEST .'\'] must be present and callable.'); + throw new BadMethodCallException('$callbacks[\'' . self::HANDLE_AUTHENTICATION_REQUEST .'\'] must be present and callable.'); } $this->handleAuthenticationRequestCallback = $config[self::HANDLE_AUTHENTICATION_REQUEST]; if (!is_callable($config[self::HANDLE_NON_INDIEAUTH_REQUEST])) { - throw new Exception("\$config['" . self::HANDLE_NON_INDIEAUTH_REQUEST . "'] must be callable"); + throw new BadMethodCallException("\$config['" . self::HANDLE_NON_INDIEAUTH_REQUEST . "'] must be callable"); } $this->handleNonIndieAuthRequest = $config[self::HANDLE_NON_INDIEAUTH_REQUEST]; @@ -227,7 +228,7 @@ class Server { // Create a default access token storage with a TTL of 7 days. $tokenStorage = new Storage\FilesystemJsonStorage($tokenStorage, $this->secret); } else { - throw new Exception("\$config['tokenStorage'] parameter must be either a string (path) or an instance of Taproot\IndieAuth\TokenStorageInterface."); + throw new BadMethodCallException("\$config['tokenStorage'] parameter must be either a string (path) or an instance of Taproot\IndieAuth\TokenStorageInterface."); } } trySetLogger($tokenStorage, $this->logger); @@ -235,14 +236,13 @@ class Server { $csrfMiddleware = $config['csrfMiddleware']; if (!$csrfMiddleware instanceof MiddlewareInterface) { - // Default to the statless Double-Submit Cookie CSRF Middleware, with default settings. - $csrfMiddleware = new Middleware\DoubleSubmitCookieCsrfMiddleware(self::DEFAULT_CSRF_KEY); + throw new BadMethodCallException("\$config['csrfMiddleware'] must be null or implement MiddlewareInterface."); } trySetLogger($csrfMiddleware, $this->logger); $this->csrfMiddleware = $csrfMiddleware; $httpGetWithEffectiveUrl = $config['httpGetWithEffectiveUrl']; - if (!is_callable($httpGetWithEffectiveUrl)) { + if (is_null($httpGetWithEffectiveUrl)) { if (class_exists('\GuzzleHttp\Client')) { $httpGetWithEffectiveUrl = function (string $uri) { // This code can’t be tested, ignore it for coverage purposes. @@ -262,15 +262,19 @@ class Server { return [$resp, $effectiveUrl]; }; } else { - throw new Exception('No valid $httpGetWithEffectiveUrl was provided, and guzzlehttp/guzzle was not installed. Either require guzzlehttp/guzzle, or provide a valid callable.'); + throw new BadMethodCallException("\$config['httpGetWithEffectiveUrl'] was not provided, and guzzlehttp/guzzle was not installed. Either require guzzlehttp/guzzle, or provide a valid callable."); // @codeCoverageIgnoreEnd } + } else { + if (!is_callable($httpGetWithEffectiveUrl)) { + throw new BadMethodCallException("\$config['httpGetWithEffectiveUrl'] must be callable."); + } } trySetLogger($httpGetWithEffectiveUrl, $this->logger); $this->httpGetWithEffectiveUrl = $httpGetWithEffectiveUrl; if (!$config['authorizationForm'] instanceof AuthorizationFormInterface) { - throw new Exception("When provided, \$config['authorizationForm'] must implement Taproot\IndieAuth\Callback\AuthorizationForm."); + throw new BadMethodCallException("When provided, \$config['authorizationForm'] must implement Taproot\IndieAuth\Callback\AuthorizationForm."); } $this->authorizationForm = $config['authorizationForm']; trySetLogger($this->authorizationForm, $this->logger); diff --git a/src/Storage/TokenStorageInterface.php b/src/Storage/TokenStorageInterface.php index c1e6836..b1c3082 100644 --- a/src/Storage/TokenStorageInterface.php +++ b/src/Storage/TokenStorageInterface.php @@ -36,7 +36,7 @@ namespace Taproot\IndieAuth\Storage; */ interface TokenStorageInterface { /** - * Create Auth Code + * Create Authorization Code * * This method is called on a valid authorization token request. The `$data` * array is guaranteed to have the following keys: diff --git a/tests/ServerTest.php b/tests/ServerTest.php index a162eeb..88202c1 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -2,6 +2,7 @@ namespace Taproot\IndieAuth\Test; +use BadMethodCallException; use DirectoryIterator; use Exception; use Nyholm\Psr7\Response; @@ -129,11 +130,11 @@ class ServerTest extends TestCase { ['authorizationForm' => 'not an auth form instance'] ]; - foreach ($badConfigs as $badConfig) { + foreach ($badConfigs as $testId => $badConfig) { try { $this->getDefaultServer($badConfig); - $this->fail(); - } catch (Exception $e) { + $this->fail("Test case {$testId} failed."); + } catch (BadMethodCallException $e) { $this->assertTrue(true); } }