diff --git a/.gitignore b/.gitignore index 1bb8cf8..eb3dd24 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ build vendor +composer.lock *.swp *.swo .idea diff --git a/composer.json b/composer.json index b5e76bb..8e1fc17 100644 --- a/composer.json +++ b/composer.json @@ -20,7 +20,7 @@ "php": ">=7.4", "ext-curl": "*", "ext-json": "*", - "firebase/php-jwt": "^6.0" + "firebase/php-jwt": "^6.0 || ^7.0" }, "require-dev": { "phpunit/phpunit": "^9.0", diff --git a/src/Client.php b/src/Client.php index efe8743..406e0b0 100644 --- a/src/Client.php +++ b/src/Client.php @@ -34,6 +34,7 @@ class Client const DEFAULT_STATE_LENGTH = 36; const CLIENT_ID_LENGTH = 20; const CLIENT_SECRET_LENGTH = 40; + const HS512_MIN_KEY_LENGTH = 64; const JWT_EXPIRATION = 300; const JWT_LEEWAY = 60; const SUCCESS_STATUS_CODE = 200; @@ -121,6 +122,18 @@ protected function makeHttpsCall(string $endpoint, array $request, ?string $user return json_decode($result, true); } + /** + * Pads the client secret to meet minimum key length requirements for HS512. + * HMAC-SHA512 in php-jwt v7+ requires 64-byte keys. Padding with null bytes + * doesn't affect HMAC output because HMAC internally pads to block size. + * + * @return string The padded client secret + */ + private function getPaddedSecret(): string + { + return str_pad($this->client_secret, self::HS512_MIN_KEY_LENGTH, "\0"); + } + private function createJwtPayload(string $audience): string { $date = new \DateTime(); @@ -132,7 +145,7 @@ private function createJwtPayload(string $audience): string "iat" => $current_date, "exp" => $current_date + self::JWT_EXPIRATION ]; - return JWT::encode($payload, $this->client_secret, self::SIG_ALGORITHM); + return JWT::encode($payload, $this->getPaddedSecret(), self::SIG_ALGORITHM); } /** @@ -283,7 +296,7 @@ public function createAuthUrl(string $username, string $state): string 'use_duo_code_attribute' => $this->use_duo_code_attribute ]; - $jwt = JWT::encode($payload, $this->client_secret, self::SIG_ALGORITHM); + $jwt = JWT::encode($payload, $this->getPaddedSecret(), self::SIG_ALGORITHM); $allArgs = [ 'response_type' => 'code', 'client_id' => $this->client_id, @@ -334,7 +347,7 @@ public function exchangeAuthorizationCodeFor2FAResult(string $duoCode, string $u try { JWT::$leeway = self::JWT_LEEWAY; - $jwt_key = new Key($this->client_secret, self::SIG_ALGORITHM); + $jwt_key = new Key($this->getPaddedSecret(), self::SIG_ALGORITHM); $token_obj = JWT::decode($result['id_token'], @$jwt_key); /* JWT::decode returns a PHP object, this will turn the object into a multidimensional array */ $token = json_decode(json_encode($token_obj), true); diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 82095c9..906ffc9 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -39,6 +39,17 @@ final class ClientTest extends TestCase public $expected_good_http_request = array("response" => array("timestamp" => 1607009339), "stat" => "OK"); + /** + * Pads the client secret to meet minimum key length requirements for HS512. + * + * Note: This reimplements the logic from Client::getPaddedSecret() for testing purposes. + * The real implementation is tested through integration tests like testTokenExchangeSuccess(). + * testPaddedSecretMatchesImplementation() verifies this reimplementation stays in sync. + */ + private function getPaddedSecret(): string + { + return str_pad($this->client_secret, Client::HS512_MIN_KEY_LENGTH, "\0"); + } protected function setUp(): void { @@ -102,7 +113,7 @@ public function createIdToken(?string $remove_index = null, array $change_val = if ($change_val) { $payload[key($change_val)] = $change_val[key($change_val)]; } - return JWT::encode($payload, $this->client_secret, Client::SIG_ALGORITHM); + return JWT::encode($payload, $this->getPaddedSecret(), Client::SIG_ALGORITHM); } /** @@ -123,6 +134,31 @@ public function createTokenResult(string $id_token = ''): array "token_type" => "Bearer"]; } + /** + * Test that the test's getPaddedSecret() reimplementation matches the real Client implementation. + * This ensures the test helper stays in sync with the actual implementation. + */ + public function testPaddedSecretMatchesImplementation(): void + { + $client = $this->createGoodClient(); + + // Create a JWT using the Client's internal getPaddedSecret() (via createAuthUrl) + $auth_url = $client->createAuthUrl($this->username, $this->good_state); + + // Extract the JWT from the URL + $query_str = parse_url($auth_url, PHP_URL_QUERY); + parse_str($query_str, $query_params); + $token = $query_params["request"]; + + // Try to decode it using the test's getPaddedSecret() + // If the signatures don't match, this will throw SignatureInvalidException + $jwt_key = new Key($this->getPaddedSecret(), Client::SIG_ALGORITHM); + $decoded = JWT::decode($token, $jwt_key); + + // Verify the decoded token contains expected data + $this->assertEquals($this->username, $decoded->duo_uname); + } + /** * Test that creating a client with proper inputs does not throw an error. */ @@ -422,7 +458,7 @@ public function testTokenExchangeSuccess(): void { $id_token = $this->createIdToken(); $result = $this->createTokenResult($id_token); - $jwt_key = new Key($this->client_secret, Client::SIG_ALGORITHM); + $jwt_key = new Key($this->getPaddedSecret(), Client::SIG_ALGORITHM); $expected_result_obj = JWT::decode($id_token, $jwt_key); $expected_result = json_decode(json_encode($expected_result_obj), true); $client = $this->createClientMockHttp($result); @@ -494,7 +530,7 @@ public function decodeJWTFromURL(string $url): array $query_str = parse_url($url, PHP_URL_QUERY); parse_str($query_str, $query_params); $token = $query_params["request"]; - $jwt_key = new Key($this->client_secret, Client::SIG_ALGORITHM); + $jwt_key = new Key($this->getPaddedSecret(), Client::SIG_ALGORITHM); $result_obj = JWT::decode($token, $jwt_key); return json_decode(json_encode($result_obj), true); }