diff --git a/lib/SessionManager.php b/lib/SessionManager.php index 131d67e3..b8677ec0 100644 --- a/lib/SessionManager.php +++ b/lib/SessionManager.php @@ -361,8 +361,9 @@ private function getCachedJwks(string $clientId, bool $forceRefresh = false): ar * Decode and validate an access token JWT. * * Verifies the JWS signature against the JWKS published for `$clientId`, - * enforces an algorithm allow-list, and rejects expired tokens. This is - * the only path used by {@see authenticate()}; callers must not bypass it. + * enforces an algorithm allow-list, and requires a numeric, unexpired exp + * claim. This is the only path used by {@see authenticate()}; callers must + * not bypass it. * * @param string $accessToken The JWT access token. * @param string $clientId The WorkOS client ID (used to fetch JWKS). @@ -436,8 +437,12 @@ private function decodeAccessToken( throw new \InvalidArgumentException('JWT signature verification failed'); } - // Expiration check (after signature verification). - if (isset($decoded['exp']) && is_numeric($decoded['exp']) && (int) $decoded['exp'] < time()) { + // Require expiration after signature verification; missing or malformed + // claims must not bypass the expiry check. + if (!isset($decoded['exp']) || !is_numeric($decoded['exp'])) { + throw new \InvalidArgumentException('JWT exp claim is missing or invalid'); + } + if ((int) $decoded['exp'] <= time()) { throw new \InvalidArgumentException('JWT has expired'); } diff --git a/tests/Fixtures/session_expiration_clock.php b/tests/Fixtures/session_expiration_clock.php new file mode 100644 index 00000000..1dbd54ae --- /dev/null +++ b/tests/Fixtures/session_expiration_clock.php @@ -0,0 +1,11 @@ +assertSame('org_test', $result['organization_id']); } + /** + * @return array, 1: bool}> + */ + public static function expirationClaimsProvider(): array + { + // The isolated test clock is fixed at 1700000000. + return [ + 'missing' => [[], false], + 'null' => [['exp' => null], false], + 'non-numeric string' => [['exp' => 'never'], false], + 'empty string' => [['exp' => ''], false], + 'true' => [['exp' => true], false], + 'false' => [['exp' => false], false], + 'array' => [['exp' => [1700003600]], false], + 'object' => [['exp' => (object) ['value' => 1700003600]], false], + 'expired' => [['exp' => 1699999999], false], + 'expired numeric string' => [['exp' => '1699999999'], false], + 'exactly now' => [['exp' => 1700000000], false], + 'exactly now numeric string' => [['exp' => '1700000000'], false], + 'future' => [['exp' => 1700000001], true], + 'future numeric string' => [['exp' => '1700000001'], true], + 'future float' => [['exp' => 1700000001.5], true], + ]; + } + + #[DataProvider('expirationClaimsProvider')] + #[RunInSeparateProcess] + #[PreserveGlobalState(false)] + public function testAuthenticateRequiresUnexpiredNumericExp(array $claims, bool $authenticated): void + { + // Keep the exact expiry boundary deterministic without changing the + // production clock or leaking the clock override into other tests. + require __DIR__ . '/Fixtures/session_expiration_clock.php'; + + [$jwks, $jwt] = $this->buildSignedJwt(['sid' => 'session_test'] + $claims); + $sealed = SessionManager::sealSessionFromAuthResponse( + accessToken: $jwt, + refreshToken: 'ref_test', + cookiePassword: $this->cookiePassword, + ); + + $client = $this->createMockClient([['status' => 200, 'body' => $jwks]]); + $result = $client->sessionManager()->authenticate( + sessionData: $sealed, + cookiePassword: $this->cookiePassword, + clientId: 'client_123', + ); + + $this->assertSame($authenticated, $result['authenticated']); + if ($authenticated) { + $this->assertSame('session_test', $result['session_id']); + } else { + $this->assertSame('invalid_jwt', $result['reason']); + } + } + public function testAuthenticateRejectsTamperedSignature(): void { [$jwks, $jwt] = $this->buildSignedJwt([