diff --git a/README.md b/README.md index c5fe5bd..c5ca60a 100644 --- a/README.md +++ b/README.md @@ -791,6 +791,12 @@ The plugin supports multiple JWT generation strategies: **1. Secret-based (default):** +> [!IMPORTANT] +> When using HMAC algorithms, ensure your secret meets minimum lengths: +> - `HS256`: at least 32 bytes +> - `HS384`: at least 48 bytes +> - `HS512`: at least 64 bytes + ```php 'jwt' => [ 'secret' => env('MERCURE_JWT_SECRET'), diff --git a/composer.json b/composer.json index a6aea1a..40b53e1 100644 --- a/composer.json +++ b/composer.json @@ -6,10 +6,10 @@ "require": { "php": ">=8.2", "cakephp/cakephp": "^5.0.1", - "firebase/php-jwt": "^6.11" + "firebase/php-jwt": "^7.0" }, "require-dev": { - "phpunit/phpunit": "^11.1.3 || ^12.0", + "phpunit/phpunit": "^11.1.3 || ^12.0 || ^13.0", "cakephp/plugin-installer": "^2.0.1", "cakephp/cakephp-codesniffer": "^5.2", "phpstan/phpstan": "^2.1", diff --git a/config/mercure.php b/config/mercure.php index 7c432e7..2206cf3 100644 --- a/config/mercure.php +++ b/config/mercure.php @@ -43,6 +43,11 @@ * * Configure JWT authentication for the Mercure hub. Multiple strategies supported: * + * Important for HMAC algorithms: + * - HS256 requires at least 32-byte secret + * - HS384 requires at least 48-byte secret + * - HS512 requires at least 64-byte secret + * * 1. Secret-based (recommended): * - 'secret' => Your JWT signing secret * - 'algorithm' => Signing algorithm (default: HS256) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 9f282ef..c9ac8fb 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -7,7 +7,6 @@ > - diff --git a/src/Authorization.php b/src/Authorization.php index ea8aaad..a149716 100644 --- a/src/Authorization.php +++ b/src/Authorization.php @@ -5,6 +5,7 @@ use Cake\Http\Response; use Cake\Http\ServerRequest; +use InvalidArgumentException; use Mercure\Exception\MercureException; use Mercure\Internal\ConfigurationHelper; use Mercure\Jwt\FirebaseTokenFactory; @@ -57,13 +58,22 @@ public static function create(): AuthorizationInterface throw new MercureException(sprintf("Token factory class '%s' not found", $factoryClass)); } - $factory = new $factoryClass($secret, $algorithm); + try { + $factory = new $factoryClass($secret, $algorithm); + } catch (InvalidArgumentException $e) { + throw new MercureException($e->getMessage(), $e->getCode(), previous: $e); + } + if (!$factory instanceof TokenFactoryInterface) { throw new MercureException('Token factory must implement TokenFactoryInterface'); } } else { // Use default Firebase factory - $factory = new FirebaseTokenFactory($secret, $algorithm); + try { + $factory = new FirebaseTokenFactory($secret, $algorithm); + } catch (InvalidArgumentException $e) { + throw new MercureException($e->getMessage(), $e->getCode(), previous: $e); + } } self::$instance = new AuthorizationService($factory, $cookieConfig); diff --git a/src/Jwt/FirebaseTokenFactory.php b/src/Jwt/FirebaseTokenFactory.php index 783b06c..6efb50f 100644 --- a/src/Jwt/FirebaseTokenFactory.php +++ b/src/Jwt/FirebaseTokenFactory.php @@ -4,6 +4,7 @@ namespace Mercure\Jwt; use Firebase\JWT\JWT; +use InvalidArgumentException; /** * Firebase Token Factory @@ -13,6 +14,17 @@ */ class FirebaseTokenFactory implements TokenFactoryInterface { + /** + * Minimum key length in bytes for HMAC algorithms. + * + * @var array + */ + private const MIN_HMAC_KEY_LENGTH = [ + 'HS256' => 32, + 'HS384' => 48, + 'HS512' => 64, + ]; + /** * Constructor * @@ -23,6 +35,7 @@ public function __construct( private string $secret, private string $algorithm = 'HS256', ) { + $this->validateSymmetricKeyLength(); } /** @@ -60,4 +73,25 @@ public function create( return JWT::encode($payload, $this->secret, $this->algorithm); } + + /** + * Validate HMAC key lengths to match the minimum security requirements. + * + * @throws \InvalidArgumentException When the configured key is too short + */ + private function validateSymmetricKeyLength(): void + { + $minimumLength = self::MIN_HMAC_KEY_LENGTH[$this->algorithm] ?? null; + if ($minimumLength === null) { + return; + } + + if (strlen($this->secret) < $minimumLength) { + throw new InvalidArgumentException(sprintf( + 'JWT secret is too short for %s. Expected at least %d bytes.', + $this->algorithm, + $minimumLength, + )); + } + } } diff --git a/src/Publisher.php b/src/Publisher.php index 89de079..c4fa469 100644 --- a/src/Publisher.php +++ b/src/Publisher.php @@ -3,6 +3,7 @@ namespace Mercure; +use InvalidArgumentException; use Mercure\Exception\MercureException; use Mercure\Internal\ConfigurationHelper; use Mercure\Jwt\FactoryTokenProvider; @@ -97,13 +98,22 @@ private static function createTokenProvider(): TokenProviderInterface throw new MercureException(sprintf("Token factory class '%s' not found", $factoryClass)); } - $factory = new $factoryClass($secret, $algorithm); + try { + $factory = new $factoryClass($secret, $algorithm); + } catch (InvalidArgumentException $e) { + throw new MercureException($e->getMessage(), $e->getCode(), previous: $e); + } + if (!$factory instanceof TokenFactoryInterface) { throw new MercureException('Token factory must implement TokenFactoryInterface'); } } else { // Option 3b: Default Firebase factory - $factory = new FirebaseTokenFactory($secret, $algorithm); + try { + $factory = new FirebaseTokenFactory($secret, $algorithm); + } catch (InvalidArgumentException $e) { + throw new MercureException($e->getMessage(), $e->getCode(), previous: $e); + } } return new FactoryTokenProvider($factory, $publish, $subscribe, $additionalClaims); diff --git a/src/View/Helper/MercureHelper.php b/src/View/Helper/MercureHelper.php index 38d41d6..1959bcd 100644 --- a/src/View/Helper/MercureHelper.php +++ b/src/View/Helper/MercureHelper.php @@ -48,6 +48,8 @@ * $this->Mercure->addTopic('/user/123/messages'); * $this->Mercure->addTopics(['/books/456', '/comments/789']); * ``` + * + * @extends \Cake\View\Helper<\Cake\View\View> */ class MercureHelper extends Helper { diff --git a/tests/TestCase/AuthorizationTest.php b/tests/TestCase/AuthorizationTest.php index 74532a8..a283145 100644 --- a/tests/TestCase/AuthorizationTest.php +++ b/tests/TestCase/AuthorizationTest.php @@ -34,7 +34,7 @@ protected function setUp(): void Configure::write('Mercure', [ 'url' => 'https://mercure.example.com/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', 'algorithm' => 'HS256', ], 'cookie' => [ @@ -143,7 +143,7 @@ public function testGetInstanceThrowsExceptionWithNonExistentFactory(): void */ public function testSetInstanceAllowsCustomInstance(): void { - $tokenFactory = new FirebaseTokenFactory('custom-secret', 'HS256'); + $tokenFactory = new FirebaseTokenFactory('custom-secret-key-with-32-bytes-min!!', 'HS256'); $customService = new AuthorizationService($tokenFactory, ['name' => 'customCookie']); Authorization::setInstance($customService); @@ -434,7 +434,7 @@ public function testAddDiscoveryHeaderThrowsExceptionWhenNoUrlConfigured(): void 'url' => '', 'public_url' => '', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', 'algorithm' => 'HS256', ], ]); @@ -530,4 +530,19 @@ public function testAddDiscoveryHeaderWithOptionalParametersDelegatesToService() $mercureHeader = array_filter($linkHeaders, fn(string $h): bool => str_contains($h, 'rel="mercure"')); $this->assertNotEmpty($mercureHeader); } + + /** + * Test getInstance wraps invalid JWT secret length in MercureException + */ + public function testGetInstanceThrowsMercureExceptionWhenJwtSecretTooShort(): void + { + Configure::write('Mercure.jwt.secret', 'too-short-secret'); + Configure::write('Mercure.jwt.algorithm', 'HS256'); + Authorization::clear(); + + $this->expectException(MercureException::class); + $this->expectExceptionMessage('JWT secret is too short for HS256'); + + Authorization::create(); + } } diff --git a/tests/TestCase/Controller/Component/MercureComponentTest.php b/tests/TestCase/Controller/Component/MercureComponentTest.php index a5caeb2..fd53095 100644 --- a/tests/TestCase/Controller/Component/MercureComponentTest.php +++ b/tests/TestCase/Controller/Component/MercureComponentTest.php @@ -47,7 +47,7 @@ protected function setUp(): void 'url' => 'https://mercure.example.com/.well-known/mercure', 'public_url' => 'https://public.mercure.example.com/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', 'algorithm' => 'HS256', ], 'cookie' => [ diff --git a/tests/TestCase/Http/Middleware/MercureDiscoveryMiddlewareTest.php b/tests/TestCase/Http/Middleware/MercureDiscoveryMiddlewareTest.php index 6cf76d8..0ebdb68 100644 --- a/tests/TestCase/Http/Middleware/MercureDiscoveryMiddlewareTest.php +++ b/tests/TestCase/Http/Middleware/MercureDiscoveryMiddlewareTest.php @@ -37,7 +37,7 @@ protected function setUp(): void 'url' => 'https://mercure.example.com/.well-known/mercure', 'public_url' => 'https://public.mercure.example.com/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', 'algorithm' => 'HS256', ], ]); @@ -90,7 +90,7 @@ public function testMiddlewareUsesPublicUrl(): void $request = new ServerRequest(); $response = new Response(); - $handler = $this->createMock(RequestHandlerInterface::class); + $handler = $this->createStub(RequestHandlerInterface::class); $handler->method('handle')->willReturn($response); $result = $this->middleware->process($request, $handler); @@ -111,7 +111,7 @@ public function testMiddlewareFallsBackToUrl(): void $request = new ServerRequest(); $response = new Response(); - $handler = $this->createMock(RequestHandlerInterface::class); + $handler = $this->createStub(RequestHandlerInterface::class); $handler->method('handle')->willReturn($response); $result = $this->middleware->process($request, $handler); @@ -129,7 +129,7 @@ public function testMiddlewareThrowsExceptionWhenNoUrlConfigured(): void 'url' => '', 'public_url' => '', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', 'algorithm' => 'HS256', ], ]); @@ -138,7 +138,7 @@ public function testMiddlewareThrowsExceptionWhenNoUrlConfigured(): void $request = new ServerRequest(); $response = new Response(); - $handler = $this->createMock(RequestHandlerInterface::class); + $handler = $this->createStub(RequestHandlerInterface::class); $handler->method('handle')->willReturn($response); $this->expectException(MercureException::class); @@ -156,7 +156,7 @@ public function testMiddlewarePreservesExistingHeaders(): void $response = new Response(); $response = $response->withHeader('X-Custom-Header', 'custom-value'); - $handler = $this->createMock(RequestHandlerInterface::class); + $handler = $this->createStub(RequestHandlerInterface::class); $handler->method('handle')->willReturn($response); $result = $this->middleware->process($request, $handler); @@ -174,7 +174,7 @@ public function testMiddlewareCanCombineWithExistingLinkHeaders(): void $response = new Response(); $response = $response->withAddedHeader('Link', '; rel="other"'); - $handler = $this->createMock(RequestHandlerInterface::class); + $handler = $this->createStub(RequestHandlerInterface::class); $handler->method('handle')->willReturn($response); $result = $this->middleware->process($request, $handler); @@ -193,7 +193,7 @@ public function testMiddlewarePreservesResponseStatusCode(): void $request = new ServerRequest(); $response = new Response(['status' => 201]); - $handler = $this->createMock(RequestHandlerInterface::class); + $handler = $this->createStub(RequestHandlerInterface::class); $handler->method('handle')->willReturn($response); $result = $this->middleware->process($request, $handler); @@ -211,7 +211,7 @@ public function testMiddlewarePreservesResponseBody(): void $response = new Response(); $response->getBody()->write('Test response body'); - $handler = $this->createMock(RequestHandlerInterface::class); + $handler = $this->createStub(RequestHandlerInterface::class); $handler->method('handle')->willReturn($response); $result = $this->middleware->process($request, $handler); @@ -233,7 +233,7 @@ public function testMiddlewareWorksWithDifferentRequestMethods(): void $request = new ServerRequest(['environment' => ['REQUEST_METHOD' => $method]]); $response = new Response(); - $handler = $this->createMock(RequestHandlerInterface::class); + $handler = $this->createStub(RequestHandlerInterface::class); $handler->method('handle')->willReturn($response); $result = $this->middleware->process($request, $handler); @@ -251,9 +251,9 @@ public function testMiddlewareHandlesNonCakePHPResponseGracefully(): void $request = new ServerRequest(); // Create a mock PSR-7 ResponseInterface that is NOT a CakePHP Response - $response = $this->createMock(ResponseInterface::class); + $response = $this->createStub(ResponseInterface::class); - $handler = $this->createMock(RequestHandlerInterface::class); + $handler = $this->createStub(RequestHandlerInterface::class); $handler->method('handle')->willReturn($response); $result = $this->middleware->process($request, $handler); @@ -284,7 +284,7 @@ public function testMiddlewareWithCookiesInResponse(): void $response = new Response(); $response = Authorization::setCookie($response, ['/feeds/123']); - $handler = $this->createMock(RequestHandlerInterface::class); + $handler = $this->createStub(RequestHandlerInterface::class); $handler->method('handle')->willReturn($response); $result = $this->middleware->process($request, $handler); diff --git a/tests/TestCase/Jwt/FirebaseTokenFactoryTest.php b/tests/TestCase/Jwt/FirebaseTokenFactoryTest.php index 5552651..76a1e01 100644 --- a/tests/TestCase/Jwt/FirebaseTokenFactoryTest.php +++ b/tests/TestCase/Jwt/FirebaseTokenFactoryTest.php @@ -6,14 +6,16 @@ use Cake\TestSuite\TestCase; use Firebase\JWT\JWT; use Firebase\JWT\Key; +use InvalidArgumentException; use Mercure\Jwt\FirebaseTokenFactory; +use PHPUnit\Framework\Attributes\DataProvider; /** * FirebaseTokenFactory Test Case */ class FirebaseTokenFactoryTest extends TestCase { - private string $secret = 'test-secret-key-for-jwt-signing'; + private string $secret = 'test-secret-key-with-32-bytes-minimum!!'; /** * Test creating a token with publish claims @@ -122,7 +124,7 @@ public function testCreateWithNullClaims(): void */ public function testCreateWithDifferentAlgorithm(): void { - $secret = 'a-longer-secret-key-for-hs384-algorithm-testing'; + $secret = 'a-longer-secret-key-for-hs384-algorithm-testing!!!!'; $factory = new FirebaseTokenFactory($secret, 'HS384'); $token = $factory->create([], ['*']); @@ -169,4 +171,31 @@ public function testCreateWithMultipleTopics(): void $this->assertSame($publish, $decoded->mercure->publish); $this->assertSame($subscribe, $decoded->mercure->subscribe); } + + /** + * Test creating a token with too short HMAC secret throws an exception + * + * @param string $algorithm HMAC algorithm + * @param string $secret Too short secret + */ + #[DataProvider('tooShortHmacSecretsProvider')] + public function testCreateWithTooShortSecretThrowsException(string $algorithm, string $secret): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('JWT secret is too short for %s', $algorithm)); + + new FirebaseTokenFactory($secret, $algorithm); + } + + /** + * @return array + */ + public static function tooShortHmacSecretsProvider(): array + { + return [ + 'hs256' => ['algorithm' => 'HS256', 'secret' => str_repeat('a', 31)], + 'hs384' => ['algorithm' => 'HS384', 'secret' => str_repeat('a', 47)], + 'hs512' => ['algorithm' => 'HS512', 'secret' => str_repeat('a', 63)], + ]; + } } diff --git a/tests/TestCase/PublisherTest.php b/tests/TestCase/PublisherTest.php index 7e092b4..3c6d29e 100644 --- a/tests/TestCase/PublisherTest.php +++ b/tests/TestCase/PublisherTest.php @@ -47,7 +47,7 @@ public function testGetInstanceCreatesInstanceFromConfig(): void Configure::write('Mercure', [ 'url' => 'http://localhost:3000/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', ], ]); @@ -65,7 +65,7 @@ public function testGetInstanceReturnsSameInstance(): void Configure::write('Mercure', [ 'url' => 'http://localhost:3000/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', ], ]); @@ -96,7 +96,7 @@ public function testClearResetsInstance(): void Configure::write('Mercure', [ 'url' => 'http://localhost:3000/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', ], ]); @@ -146,7 +146,7 @@ public function testSetInstanceOverwritesExisting(): void Configure::write('Mercure', [ 'url' => 'http://localhost:3000/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', ], ]); @@ -169,7 +169,7 @@ public function testServiceProviderReturnsSingletonInstance(): void Configure::write('Mercure', [ 'url' => 'http://localhost:3000/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', ], ]); @@ -208,7 +208,7 @@ public function testGetInstanceWithJwtSecretAndAlgorithm(): void Configure::write('Mercure', [ 'url' => 'http://localhost:3000/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'a-longer-secret-key-for-hs384-algorithm-testing!!!!', 'algorithm' => 'HS384', 'publish' => ['https://example.com/feeds/{id}'], 'subscribe' => ['https://example.com/books/{id}'], @@ -227,7 +227,7 @@ public function testGetInstanceThrowsExceptionWhenHubUrlMissing(): void { Configure::write('Mercure', [ 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', ], ]); @@ -259,7 +259,7 @@ public function testBackwardCompatibilityWithPublisherJwt(): void { Configure::write('Mercure', [ 'url' => 'http://localhost:3000/.well-known/mercure', - 'publisher_jwt' => 'backward-compat-secret', + 'publisher_jwt' => 'backward-compat-secret-with-32-bytes-min!!', ]); $instance = Publisher::create(); @@ -349,7 +349,7 @@ public function testGetInstanceWithCustomTokenFactory(): void Configure::write('Mercure', [ 'url' => 'http://localhost:3000/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', 'factory' => CustomTokenFactory::class, 'algorithm' => 'HS256', ], @@ -368,7 +368,7 @@ public function testGetInstanceThrowsExceptionWithNonExistentFactory(): void Configure::write('Mercure', [ 'url' => 'http://localhost:3000/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', 'factory' => 'NonExistent\TokenFactory', ], ]); @@ -387,7 +387,7 @@ public function testGetInstanceThrowsExceptionWithInvalidFactory(): void Configure::write('Mercure', [ 'url' => 'http://localhost:3000/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', 'factory' => InvalidTokenFactory::class, ], ]); @@ -406,7 +406,7 @@ public function testGetInstanceWithAdditionalJwtClaims(): void Configure::write('Mercure', [ 'url' => 'http://localhost:3000/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', 'algorithm' => 'HS256', 'publish' => ['https://example.com/*'], 'subscribe' => ['https://example.com/feeds/*'], @@ -430,7 +430,7 @@ public function testGetInstanceWithHttpClientConfig(): void Configure::write('Mercure', [ 'url' => 'http://localhost:3000/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', ], 'http_client' => [ 'timeout' => 30, @@ -460,4 +460,23 @@ public function testGetInstanceThrowsExceptionWithEmptyJwtSecret(): void Publisher::create(); } + + /** + * Test getInstance wraps invalid JWT secret length in MercureException + */ + public function testGetInstanceThrowsMercureExceptionWhenJwtSecretTooShort(): void + { + Configure::write('Mercure', [ + 'url' => 'http://localhost:3000/.well-known/mercure', + 'jwt' => [ + 'secret' => 'too-short-secret', + 'algorithm' => 'HS256', + ], + ]); + + $this->expectException(MercureException::class); + $this->expectExceptionMessage('JWT secret is too short for HS256'); + + Publisher::create(); + } } diff --git a/tests/TestCase/Service/AuthorizationServiceTest.php b/tests/TestCase/Service/AuthorizationServiceTest.php index 5552331..4eb209c 100644 --- a/tests/TestCase/Service/AuthorizationServiceTest.php +++ b/tests/TestCase/Service/AuthorizationServiceTest.php @@ -36,7 +36,7 @@ protected function setUp(): void 'public_url' => 'https://public.mercure.example.com/.well-known/mercure', ]); - $this->tokenFactory = new FirebaseTokenFactory('test-secret', 'HS256'); + $this->tokenFactory = new FirebaseTokenFactory('test-secret-key-with-32-bytes-minimum!!', 'HS256'); $this->service = new AuthorizationService($this->tokenFactory, [ 'name' => 'testAuth', 'expires' => null, diff --git a/tests/TestCase/View/Helper/MercureHelperTest.php b/tests/TestCase/View/Helper/MercureHelperTest.php index 1bb0b9d..53e8b84 100644 --- a/tests/TestCase/View/Helper/MercureHelperTest.php +++ b/tests/TestCase/View/Helper/MercureHelperTest.php @@ -36,7 +36,7 @@ protected function setUp(): void Configure::write('Mercure', [ 'url' => 'https://mercure.example.com/.well-known/mercure', 'jwt' => [ - 'secret' => 'test-secret-key', + 'secret' => 'test-secret-key-with-32-bytes-minimum!!', 'algorithm' => 'HS256', ], ]);