diff --git a/src/Server.php b/src/Server.php index 4b531d7..91007b6 100644 --- a/src/Server.php +++ b/src/Server.php @@ -36,9 +36,7 @@ class Server { const APPROVE_ACTION_KEY = 'taproot_indieauth_action'; const APPROVE_ACTION_VALUE = 'approve'; - protected Storage\TokenStorageInterface $authorizationCodeStorage; - - protected Storage\TokenStorageInterface $accessTokenStorage; + protected Storage\TokenStorageInterface $tokenStorage; protected AuthorizationFormInterface $authorizationForm; @@ -61,8 +59,7 @@ class Server { 'csrfMiddleware' => null, 'logger' => null, self::HANDLE_NON_INDIEAUTH_REQUEST => function (ServerRequestInterface $request) { return null; }, // Default to no-op. - 'authorizationCodeStorage' => null, - 'accessTokenStorage' => null, + 'tokenStorage' => null, 'httpGetWithEffectiveUrl' => null, 'authorizationForm' => new DefaultAuthorizationForm(), 'exceptionTemplatePath' => __DIR__ . '/../templates/default_exception_response.html.php', @@ -75,7 +72,7 @@ class Server { $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 Exception("\$config['secret'] must be a string with a minimum length of 64 characters. Make one with Taproot\IndieAuth\generateRandomString(64)"); } $this->secret = $secret; @@ -94,28 +91,17 @@ class Server { } $this->handleNonIndieAuthRequest = $config[self::HANDLE_NON_INDIEAUTH_REQUEST]; - $authorizationCodeStorage = $config['authorizationCodeStorage']; - if (!$authorizationCodeStorage instanceof Storage\TokenStorageInterface) { - if (is_string($authorizationCodeStorage)) { - $authorizationCodeStorage = new Storage\FilesystemJsonStorage($authorizationCodeStorage, 600, true); - } else { - throw new Exception("\$config['authorizationCodeStorage'] must be either a string (path) or an instance of Taproot\IndieAuth\TokenStorageInterface."); - } - } - trySetLogger($authorizationCodeStorage, $this->logger); - $this->authorizationCodeStorage = $authorizationCodeStorage; - - $accessTokenStorage = $config['accessTokenStorage']; - if (!$accessTokenStorage instanceof Storage\TokenStorageInterface) { - if (is_string($accessTokenStorage)) { + $tokenStorage = $config['tokenStorage']; + if (!$tokenStorage instanceof Storage\TokenStorageInterface) { + if (is_string($tokenStorage)) { // Create a default access token storage with a TTL of 7 days. - $accessTokenStorage = new Storage\FilesystemJsonStorage($accessTokenStorage, 60 * 60 * 24 * 7, true); + $tokenStorage = new Storage\FilesystemJsonStorage($tokenStorage, $this->secret); } else { - throw new Exception('$accessTokenStorage parameter must be either a string (path) or an instance of Taproot\IndieAuth\TokenStorageInterface.'); + throw new Exception("\$config['tokenStorage'] parameter must be either a string (path) or an instance of Taproot\IndieAuth\TokenStorageInterface."); } } - trySetLogger($accessTokenStorage, $this->logger); - $this->accessTokenStorage = $accessTokenStorage; + trySetLogger($tokenStorage, $this->logger); + $this->tokenStorage = $tokenStorage; $csrfMiddleware = $config['csrfMiddleware']; if (!$csrfMiddleware instanceof MiddlewareInterface) { @@ -281,8 +267,8 @@ class Server { $code = $this->authorizationForm->transformAuthorizationCode($request, $code); // Store the authorization code. - $success = $this->authorizationCodeStorage->put($code['code'], $code); - if (!$success) { + $authCode = $this->tokenStorage->createAuthCode($code); + if (is_null($authCode)) { // 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."); @@ -291,7 +277,7 @@ class Server { // Return a redirect to the client app. return new Response(302, ['Location' => appendQueryParams($queryParams['redirect_uri'], [ - 'code' => $code['code'], + 'code' => $authCode->getKey(), 'state' => $code['state'] ])]); } diff --git a/src/Storage/FilesystemJsonStorage.php b/src/Storage/FilesystemJsonStorage.php index 18ab93a..b3c8389 100644 --- a/src/Storage/FilesystemJsonStorage.php +++ b/src/Storage/FilesystemJsonStorage.php @@ -9,7 +9,6 @@ use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use function Taproot\IndieAuth\generateRandomString; -use function Taproot\IndieAuth\trySetLogger; class FilesystemJsonStorage implements TokenStorageInterface, LoggerAwareInterface { const DEFAULT_AUTH_CODE_TTL = 60 * 5; // Five minutes. @@ -24,23 +23,24 @@ class FilesystemJsonStorage implements TokenStorageInterface, LoggerAwareInterfa protected LoggerInterface $logger; + public function __construct(string $path, string $secret, ?int $authCodeTtl=null, ?int $accessTokenTtl=null, $cleanUpNow=false, ?LoggerInterface $logger=null) { $this->logger = $logger ?? new NullLogger(); if (!is_string($secret) || strlen($secret) < 64) { - throw new Exception("\$secret must be a string with a minimum length of 64 characters."); + throw new Exception("\$secret must be a string with a minimum length of 64 characters. Make one with Taproot\IndieAuth\generateRandomString(64)"); } $this->secret = $secret; $this->path = rtrim($path, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR; - $this->authCodeTtl == $authCodeTtl ?? self::DEFAULT_AUTH_CODE_TTL; + $this->authCodeTtl = $authCodeTtl ?? self::DEFAULT_AUTH_CODE_TTL; $this->accessTokenTtl = $accessTokenTtl ?? self::DEFAULT_ACCESS_TOKEN_TTL; @mkdir($this->path, 0777, true); if ($cleanUpNow) { - $this->cleanUp(); + $this->deleteExpiredTokens(); } } @@ -204,7 +204,7 @@ class FilesystemJsonStorage implements TokenStorageInterface, LoggerAwareInterfa return $this->path . "$key.json"; } - protected function withLock(string $path, string $mode, callable $callback): mixed { + protected function withLock(string $path, string $mode, callable $callback) { $fp = fopen($path, $mode); if ($fp === false) { diff --git a/tests/FilesystemJsonStorageTest.php b/tests/FilesystemJsonStorageTest.php index 4851c0e..2948908 100644 --- a/tests/FilesystemJsonStorageTest.php +++ b/tests/FilesystemJsonStorageTest.php @@ -6,6 +6,7 @@ use DirectoryIterator; use PHPUnit\Framework\TestCase; use Taproot\IndieAuth\Storage\FilesystemJsonStorage; +const SECRET = '1111111111111111111111111111111111111111111111111111111111111111'; const TMP_DIR = __DIR__ . '/tmp'; class FilesystemJsonStorageTest extends TestCase { @@ -29,7 +30,7 @@ class FilesystemJsonStorageTest extends TestCase { } public function testCrud() { - $s = new FilesystemJsonStorage(TMP_DIR, 0, false); + $s = new FilesystemJsonStorage(TMP_DIR, SECRET); $t1data = ['example' => 'data']; $t1path = $s->getPath('t1'); @@ -48,10 +49,11 @@ class FilesystemJsonStorageTest extends TestCase { } public function testCleanUp() { - $s = new FilesystemJsonStorage(TMP_DIR, 1, false); - $s->put('t1', ['example' => 'data']); - sleep(2); - $s->cleanUp(); - $this->assertNull($s->get('t1'), 't1 was not cleaned up after expiring.'); + $s = new FilesystemJsonStorage(TMP_DIR, SECRET); + $s->put('t1', ['valid_until' => time() + 10]); + $s->put('t2', ['valid_until' => time() - 10]); + $s->deleteExpiredTokens(); + $this->assertIsArray($s->get('t1'), 't1 was not expired and should not have been deleted.'); + $this->assertNull($s->get('t2'), 't2 was not cleaned up after expiring.'); } } diff --git a/tests/ServerTest.php b/tests/ServerTest.php index 3844bd6..d4efb24 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -2,6 +2,7 @@ namespace Taproot\IndieAuth\Test; +use DirectoryIterator; use Exception; use Nyholm\Psr7\Response; use Nyholm\Psr7\ServerRequest; @@ -18,8 +19,7 @@ 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 TOKEN_STORAGE_PATH = __DIR__ . '/tmp/tokens'; 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'; @@ -28,8 +28,7 @@ class ServerTest extends TestCase { protected function getDefaultServer(array $config=[]) { return new Server(array_merge([ 'secret' => SERVER_SECRET, - 'authorizationCodeStorage' => AUTH_CODE_STORAGE_PATH, - 'accessTokenStorage' => ACCESS_TOKEN_STORAGE_PATH, + 'tokenStorage' => TOKEN_STORAGE_PATH, // 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. @@ -81,18 +80,24 @@ class ServerTest extends TestCase { protected function setUp(): void { // Clean up tmp folder. - new FilesystemJsonStorage(AUTH_CODE_STORAGE_PATH, -1, true); - new FilesystemJsonStorage(ACCESS_TOKEN_STORAGE_PATH, -1, true); - @rmdir(AUTH_CODE_STORAGE_PATH); - @rmdir(ACCESS_TOKEN_STORAGE_PATH); + @mkdir(TOKEN_STORAGE_PATH, 0777, true); + foreach (new DirectoryIterator(TOKEN_STORAGE_PATH) as $fileInfo) { + if ($fileInfo->isFile()) { + unlink($fileInfo->getPathname()); + } + } + @rmdir(TOKEN_STORAGE_PATH); } protected function tearDown(): void { // Clean up tmp folder. - new FilesystemJsonStorage(AUTH_CODE_STORAGE_PATH, -1, true); - new FilesystemJsonStorage(ACCESS_TOKEN_STORAGE_PATH, -1, true); - @rmdir(AUTH_CODE_STORAGE_PATH); - @rmdir(ACCESS_TOKEN_STORAGE_PATH); + @mkdir(TOKEN_STORAGE_PATH, 0777, true); + foreach (new DirectoryIterator(TOKEN_STORAGE_PATH) as $fileInfo) { + if ($fileInfo->isFile()) { + unlink($fileInfo->getPathname()); + } + } + @rmdir(TOKEN_STORAGE_PATH); } public function testAuthorizationRequestMissingParametersReturnsError() { @@ -220,8 +225,8 @@ class ServerTest extends TestCase { $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']); + $storage = new FilesystemJsonStorage(TOKEN_STORAGE_PATH, SECRET); + $storedCode = $storage->get(hash_hmac('sha256', $redirectUriQueryParams['code'], SECRET)); $this->assertNotNull($storedCode, 'An authorization code should be stored after a successful approval request.');