From fdac7b0f61f7352accb727ca47445d86df815132 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Mon, 23 Sep 2024 22:48:26 +0530 Subject: [PATCH 1/9] Added impl. for decoding socketIdObject inside broadcast method, updated tests for the same --- src/AblyBroadcaster.php | 35 ++++++++++++++++++++++++++--------- src/Utils.php | 14 ++++++++++++++ tests/AblyBroadcasterTest.php | 24 +++++++++++++++++++----- 3 files changed, 59 insertions(+), 14 deletions(-) diff --git a/src/AblyBroadcaster.php b/src/AblyBroadcaster.php index 0f5a151..3d2c9f1 100644 --- a/src/AblyBroadcaster.php +++ b/src/AblyBroadcaster.php @@ -2,6 +2,7 @@ namespace Ably\LaravelBroadcaster; +use _PHPStan_cb8f9103f\Nette\Neon\Exception; use Ably\AblyRest; use Ably\Exceptions\AblyException; use Ably\Models\Message as AblyMessage; @@ -177,10 +178,11 @@ public function validAuthenticationResponse($request, $result) */ public function broadcast($channels, $event, $payload = []) { + $socketIdObject = Utils::decodeSocketId(Arr::pull($payload, 'socket')); try { foreach ($this->formatChannels($channels) as $channel) { $this->ably->channels->get($channel)->publish( - $this->buildAblyMessage($event, $payload) + $this->buildAblyMessage($event, $payload, $socketIdObject) ); } } catch (AblyException $e) { @@ -310,19 +312,34 @@ public function formatChannels($channels) /** * Build an Ably message object for broadcasting. * - * @param string $event - * @param array $payload - * @return \Ably\Models\Message + * @param string $event + * @param array $payload + * @param array|null $socketIdObject + * @return AblyMessage + * @throws Exception */ - protected function buildAblyMessage($event, $payload = []) + protected function buildAblyMessage($event, $payload = [], $socketIdObject = null) { - $socket = Arr::pull($payload, 'socket'); - - return tap(new AblyMessage, function ($message) use ($event, $payload, $socket) { + $message = tap(new AblyMessage, function ($message) use ($event, $payload, $socketIdObject) { $message->name = $event; $message->data = $payload; - $message->connectionKey = $socket; }); + + if ($socketIdObject) { + $connectionKey_key = 'connectionKey'; + if (array_key_exists($connectionKey_key, $socketIdObject)) { + $message->connectionKey = $socketIdObject[$connectionKey_key]; + } else { + throw new Exception(Utils::missingKeyErrorForSocketId($connectionKey_key)); + } + $clientId_key = 'clientId'; + if (array_key_exists($clientId_key, $socketIdObject)) { + $message->clientId = $socketIdObject[$clientId_key]; + } else { + throw new Exception(Utils::missingKeyErrorForSocketId($clientId_key)); + } + } + return $message; } /** diff --git a/src/Utils.php b/src/Utils.php index 0729b19..6ce28c3 100644 --- a/src/Utils.php +++ b/src/Utils.php @@ -2,6 +2,8 @@ namespace Ably\LaravelBroadcaster; +use Illuminate\Broadcasting\BroadcastException; + class Utils { // JWT related PHP utility functions @@ -62,4 +64,16 @@ public static function base64urlEncode($str) { return rtrim(strtr(base64_encode($str), '+/', '-_'), '='); } + + public static function decodeSocketId($socketId) { + if ($socketId) { + return json_decode(base64_decode($socketId), true); + } + return null; + } + + public static function missingKeyErrorForSocketId($keyName) { + return $keyName." not present in socketId, please make sure to send base64 encoded json with " + ."connectionKey and clientId as keys. clientId is null if connection is not identified."; + } } diff --git a/tests/AblyBroadcasterTest.php b/tests/AblyBroadcasterTest.php index 6d05097..b772b54 100644 --- a/tests/AblyBroadcasterTest.php +++ b/tests/AblyBroadcasterTest.php @@ -343,23 +343,37 @@ public function testLaravelAblyAgentHeader() public function testPayloadShouldNotIncludeSocketKey() { + +// self::assertArrayNotHasKey('socket', $message->data); + + } + public function testBuildMessageUsingProvidedSocketIdObject() + { $broadcaster = m::mock(AblyBroadcasterExposed::class, [$this->ably, []])->makePartial(); $payload = [ 'foo' => 'bar', - 'socket' => null + 'chat' => 'hello there' + ]; + + $socketIdObject = [ + 'connectionKey' => 'key', + 'clientId' => 'id' ]; - $message = $broadcaster->buildAblyMessage('testEvent', $payload); - self::assertArrayNotHasKey('socket', $message->data); + $message = $broadcaster->buildAblyMessage('testEvent', $payload, $socketIdObject); + self::assertEquals('key', $message->connectionKey); + self::assertEquals('id', $message->clientId); + self::assertEquals('testEvent', $message->name); + self::assertEquals($payload, $message->data); } } class AblyBroadcasterExposed extends AblyBroadcaster { - public function buildAblyMessage($event, $payload = []) + public function buildAblyMessage($event, $payload = [], $socketIdObject = null) { - return parent::buildAblyMessage($event, $payload); + return parent::buildAblyMessage($event, $payload, $socketIdObject); } } From ced106e77fe730ecaf6117d56b9e546f316f8171 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 24 Sep 2024 13:21:41 +0530 Subject: [PATCH 2/9] Refactored AblyBroadcaster and tests with socketIdObject --- src/AblyBroadcaster.php | 28 +++++++++------------------- src/Utils.php | 28 +++++++++++++++++++++------- tests/AblyBroadcasterTest.php | 18 +++++++++++------- 3 files changed, 41 insertions(+), 33 deletions(-) diff --git a/src/AblyBroadcaster.php b/src/AblyBroadcaster.php index 3d2c9f1..56e6d55 100644 --- a/src/AblyBroadcaster.php +++ b/src/AblyBroadcaster.php @@ -2,7 +2,6 @@ namespace Ably\LaravelBroadcaster; -use _PHPStan_cb8f9103f\Nette\Neon\Exception; use Ably\AblyRest; use Ably\Exceptions\AblyException; use Ably\Models\Message as AblyMessage; @@ -169,17 +168,19 @@ public function validAuthenticationResponse($request, $result) /** * Broadcast the given event. * - * @param array $channels - * @param string $event - * @param array $payload + * @param array $channels + * @param string $event + * @param array $payload * @return void * * @throws \Illuminate\Broadcasting\BroadcastException + * @throws \Exception */ public function broadcast($channels, $event, $payload = []) { - $socketIdObject = Utils::decodeSocketId(Arr::pull($payload, 'socket')); + $socketId = Arr::pull($payload, 'socket'); try { + $socketIdObject = Utils::decodeSocketId($socketId); foreach ($this->formatChannels($channels) as $channel) { $this->ably->channels->get($channel)->publish( $this->buildAblyMessage($event, $payload, $socketIdObject) @@ -314,9 +315,8 @@ public function formatChannels($channels) * * @param string $event * @param array $payload - * @param array|null $socketIdObject + * @param object $socketIdObject * @return AblyMessage - * @throws Exception */ protected function buildAblyMessage($event, $payload = [], $socketIdObject = null) { @@ -326,18 +326,8 @@ protected function buildAblyMessage($event, $payload = [], $socketIdObject = nul }); if ($socketIdObject) { - $connectionKey_key = 'connectionKey'; - if (array_key_exists($connectionKey_key, $socketIdObject)) { - $message->connectionKey = $socketIdObject[$connectionKey_key]; - } else { - throw new Exception(Utils::missingKeyErrorForSocketId($connectionKey_key)); - } - $clientId_key = 'clientId'; - if (array_key_exists($clientId_key, $socketIdObject)) { - $message->clientId = $socketIdObject[$clientId_key]; - } else { - throw new Exception(Utils::missingKeyErrorForSocketId($clientId_key)); - } + $message->connectionKey = $socketIdObject->connectionKey; + $message->clientId = $socketIdObject->clientId; } return $message; } diff --git a/src/Utils.php b/src/Utils.php index 6ce28c3..f52d502 100644 --- a/src/Utils.php +++ b/src/Utils.php @@ -2,6 +2,7 @@ namespace Ably\LaravelBroadcaster; +use Ably\Exceptions\AblyException; use Illuminate\Broadcasting\BroadcastException; class Utils @@ -65,15 +66,28 @@ public static function base64urlEncode($str) return rtrim(strtr(base64_encode($str), '+/', '-_'), '='); } + + const SOCKET_ID_ERROR = "please make sure to send base64 encoded json with " + ."'connectionKey' and 'clientId' as keys. 'clientId' is null if connection is not identified"; + + /** + * @throws AblyException + */ public static function decodeSocketId($socketId) { + $socketIdObject = null; if ($socketId) { - return json_decode(base64_decode($socketId), true); + try { + $socketIdObject = json_decode(base64_decode($socketId)); + } catch (\Exception $e) { + throw new AblyException("SocketId decoding failed, ".self::SOCKET_ID_ERROR, 0, $e); + } + if (!isset($socketIdObject->connectionKey)) { + throw new AblyException("ConnectionKey is missing, ".self::SOCKET_ID_ERROR); + } + if (!isset($socketIdObject->clientId)) { + throw new AblyException("ClientId is missing, ".self::SOCKET_ID_ERROR); + } } - return null; - } - - public static function missingKeyErrorForSocketId($keyName) { - return $keyName." not present in socketId, please make sure to send base64 encoded json with " - ."connectionKey and clientId as keys. clientId is null if connection is not identified."; + return $socketIdObject; } } diff --git a/tests/AblyBroadcasterTest.php b/tests/AblyBroadcasterTest.php index b772b54..431c366 100644 --- a/tests/AblyBroadcasterTest.php +++ b/tests/AblyBroadcasterTest.php @@ -347,7 +347,7 @@ public function testPayloadShouldNotIncludeSocketKey() // self::assertArrayNotHasKey('socket', $message->data); } - public function testBuildMessageUsingProvidedSocketIdObject() + public function testBuildMessageBasedOnSocketIdObject() { $broadcaster = m::mock(AblyBroadcasterExposed::class, [$this->ably, []])->makePartial(); @@ -355,17 +355,21 @@ public function testBuildMessageUsingProvidedSocketIdObject() 'foo' => 'bar', 'chat' => 'hello there' ]; + $message = $broadcaster->buildAblyMessage('testEvent', $payload); + self::assertEquals('testEvent', $message->name); + self::assertEquals($payload, $message->data); + self::assertNull($message->connectionKey); + self::assertNull($message->clientId); - $socketIdObject = [ - 'connectionKey' => 'key', - 'clientId' => 'id' - ]; + $socketIdObject = new \stdClass(); + $socketIdObject->connectionKey = 'foo'; + $socketIdObject->clientId = 'sacOO7'; $message = $broadcaster->buildAblyMessage('testEvent', $payload, $socketIdObject); - self::assertEquals('key', $message->connectionKey); - self::assertEquals('id', $message->clientId); self::assertEquals('testEvent', $message->name); self::assertEquals($payload, $message->data); + self::assertEquals('foo', $message->connectionKey); + self::assertEquals('sacOO7', $message->clientId); } } From 50e566883235383846ab15d2d1a3501ab5191cdb Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 24 Sep 2024 13:23:49 +0530 Subject: [PATCH 3/9] Created separate test file for Utils, moved JWT specific test to UtilsTest.php --- tests/AblyBroadcasterTest.php | 21 --------------------- tests/UtilsTest.php | 29 +++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 21 deletions(-) create mode 100644 tests/UtilsTest.php diff --git a/tests/AblyBroadcasterTest.php b/tests/AblyBroadcasterTest.php index 431c366..7ed6342 100644 --- a/tests/AblyBroadcasterTest.php +++ b/tests/AblyBroadcasterTest.php @@ -123,27 +123,6 @@ public function testAuthThrowAccessDeniedHttpExceptionWithPresenceChannelWhenReq ); } - public function testGenerateAndValidateToken() - { - $headers = ['alg' => 'HS256', 'typ' => 'JWT']; - $payload = ['sub' => '1234567890', 'name' => 'John Doe', 'admin' => true, 'exp' => (time() + 60)]; - $jwtToken = Utils::generateJwt($headers, $payload, 'efgh'); - - $parsedJwt = Utils::parseJwt($jwtToken); - self::assertEquals('HS256', $parsedJwt['header']['alg']); - self::assertEquals('JWT', $parsedJwt['header']['typ']); - - self::assertEquals('1234567890', $parsedJwt['payload']['sub']); - self::assertEquals('John Doe', $parsedJwt['payload']['name']); - self::assertEquals(true, $parsedJwt['payload']['admin']); - - $timeFn = function () { - return time(); - }; - $jwtIsValid = Utils::isJwtValid($jwtToken, $timeFn, 'efgh'); - self::assertTrue($jwtIsValid); - } - public function testShouldGetSignedToken() { $token = $this->broadcaster->getSignedToken(null, null, 'user123', $this->guardedChannelCapability); diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php new file mode 100644 index 0000000..975462b --- /dev/null +++ b/tests/UtilsTest.php @@ -0,0 +1,29 @@ + 'HS256', 'typ' => 'JWT']; + $payload = ['sub' => '1234567890', 'name' => 'John Doe', 'admin' => true, 'exp' => (time() + 60)]; + $jwtToken = Utils::generateJwt($headers, $payload, 'efgh'); + + $parsedJwt = Utils::parseJwt($jwtToken); + self::assertEquals('HS256', $parsedJwt['header']['alg']); + self::assertEquals('JWT', $parsedJwt['header']['typ']); + + self::assertEquals('1234567890', $parsedJwt['payload']['sub']); + self::assertEquals('John Doe', $parsedJwt['payload']['name']); + self::assertEquals(true, $parsedJwt['payload']['admin']); + + $timeFn = function () { + return time(); + }; + $jwtIsValid = Utils::isJwtValid($jwtToken, $timeFn, 'efgh'); + self::assertTrue($jwtIsValid); + } +} From 35a56bea08ad6f3ad20c70a9a653c5f5065fec9d Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 24 Sep 2024 17:28:47 +0530 Subject: [PATCH 4/9] Added base64 url decode helper method, replaced all normal base64 decodes with the same --- src/Utils.php | 29 +++++++++++++++++------------ tests/UtilsTest.php | 9 +++++++++ 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/Utils.php b/src/Utils.php index f52d502..ef76657 100644 --- a/src/Utils.php +++ b/src/Utils.php @@ -15,8 +15,8 @@ class Utils public static function parseJwt($jwt) { $tokenParts = explode('.', $jwt); - $header = json_decode(base64_decode($tokenParts[0]), true); - $payload = json_decode(base64_decode($tokenParts[1]), true); + $header = json_decode(self::base64url_decode($tokenParts[0]), true); + $payload = json_decode(self::base64url_decode($tokenParts[1]), true); return ['header' => $header, 'payload' => $payload]; } @@ -28,11 +28,11 @@ public static function parseJwt($jwt) */ public static function generateJwt($headers, $payload, $key) { - $encodedHeaders = self::base64urlEncode(json_encode($headers)); - $encodedPayload = self::base64urlEncode(json_encode($payload)); + $encodedHeaders = self::base64url_encode(json_encode($headers)); + $encodedPayload = self::base64url_encode(json_encode($payload)); $signature = hash_hmac('SHA256', "$encodedHeaders.$encodedPayload", $key, true); - $encodedSignature = self::base64urlEncode($signature); + $encodedSignature = self::base64url_encode($signature); return "$encodedHeaders.$encodedPayload.$encodedSignature"; } @@ -51,21 +51,27 @@ public static function isJwtValid($jwt, $timeFn, $key) $tokenSignature = $tokenParts[2]; // check the expiration time - note this will cause an error if there is no 'exp' claim in the jwt - $expiration = json_decode(base64_decode($payload))->exp; + $expiration = json_decode(self::base64url_decode($payload))->exp; $isTokenExpired = $expiration <= $timeFn(); // build a signature based on the header and payload using the secret $signature = hash_hmac('SHA256', $header.'.'.$payload, $key, true); - $isSignatureValid = self::base64urlEncode($signature) === $tokenSignature; + $isSignatureValid = self::base64url_encode($signature) === $tokenSignature; return $isSignatureValid && ! $isTokenExpired; } - public static function base64urlEncode($str) + public static function base64url_encode($str): string { return rtrim(strtr(base64_encode($str), '+/', '-_'), '='); } + /** + * https://www.php.net/manual/en/function.base64-encode.php#127544 + */ + public static function base64url_decode($data) { + return base64_decode(strtr($data, '-_', '+/'), true); + } const SOCKET_ID_ERROR = "please make sure to send base64 encoded json with " ."'connectionKey' and 'clientId' as keys. 'clientId' is null if connection is not identified"; @@ -76,10 +82,9 @@ public static function base64urlEncode($str) public static function decodeSocketId($socketId) { $socketIdObject = null; if ($socketId) { - try { - $socketIdObject = json_decode(base64_decode($socketId)); - } catch (\Exception $e) { - throw new AblyException("SocketId decoding failed, ".self::SOCKET_ID_ERROR, 0, $e); + $socketIdObject = json_decode(self::base64url_decode($socketId)); + if (!$socketIdObject) { + throw new AblyException("SocketId decoding failed, ".self::SOCKET_ID_ERROR); } if (!isset($socketIdObject->connectionKey)) { throw new AblyException("ConnectionKey is missing, ".self::SOCKET_ID_ERROR); diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php index 975462b..bdb6ad1 100644 --- a/tests/UtilsTest.php +++ b/tests/UtilsTest.php @@ -26,4 +26,13 @@ public function testGenerateAndValidateToken() $jwtIsValid = Utils::isJwtValid($jwtToken, $timeFn, 'efgh'); self::assertTrue($jwtIsValid); } + + public function testValidateDecodingSocketId() { + $socketId = new \stdClass(); + $socketId->connectionKey = 'key'; + $socketId->clientId = 'clientId'; + $socketIdObject = Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketId))); + self::assertEquals('key', $socketIdObject->connectionKey); + self::assertEquals('clientId', $socketIdObject->clientId); + } } From 9e94625111f3d41ae22b109b355afbab3f65b9c4 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 24 Sep 2024 19:00:11 +0530 Subject: [PATCH 5/9] Added unit tests to check for decoding invalid socket id --- src/AblyBroadcaster.php | 2 +- src/Utils.php | 7 +++---- tests/UtilsTest.php | 44 ++++++++++++++++++++++++++++++++++++++--- 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/AblyBroadcaster.php b/src/AblyBroadcaster.php index 56e6d55..2afcad1 100644 --- a/src/AblyBroadcaster.php +++ b/src/AblyBroadcaster.php @@ -320,7 +320,7 @@ public function formatChannels($channels) */ protected function buildAblyMessage($event, $payload = [], $socketIdObject = null) { - $message = tap(new AblyMessage, function ($message) use ($event, $payload, $socketIdObject) { + $message = tap(new AblyMessage, function ($message) use ($event, $payload) { $message->name = $event; $message->data = $payload; }); diff --git a/src/Utils.php b/src/Utils.php index ef76657..d77b369 100644 --- a/src/Utils.php +++ b/src/Utils.php @@ -3,7 +3,6 @@ namespace Ably\LaravelBroadcaster; use Ably\Exceptions\AblyException; -use Illuminate\Broadcasting\BroadcastException; class Utils { @@ -73,7 +72,7 @@ public static function base64url_decode($data) { return base64_decode(strtr($data, '-_', '+/'), true); } - const SOCKET_ID_ERROR = "please make sure to send base64 encoded json with " + const SOCKET_ID_ERROR = "please make sure to send base64 url encoded json with " ."'connectionKey' and 'clientId' as keys. 'clientId' is null if connection is not identified"; /** @@ -87,9 +86,9 @@ public static function decodeSocketId($socketId) { throw new AblyException("SocketId decoding failed, ".self::SOCKET_ID_ERROR); } if (!isset($socketIdObject->connectionKey)) { - throw new AblyException("ConnectionKey is missing, ".self::SOCKET_ID_ERROR); + throw new AblyException("ConnectionKey is not set, ".self::SOCKET_ID_ERROR); } - if (!isset($socketIdObject->clientId)) { + if (!property_exists($socketIdObject, 'clientId')) { throw new AblyException("ClientId is missing, ".self::SOCKET_ID_ERROR); } } diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php index bdb6ad1..5f79723 100644 --- a/tests/UtilsTest.php +++ b/tests/UtilsTest.php @@ -2,6 +2,7 @@ namespace Ably\LaravelBroadcaster\Tests; +use Ably\Exceptions\AblyException; use Ably\LaravelBroadcaster\Utils; class UtilsTest extends TestCase @@ -27,12 +28,49 @@ public function testGenerateAndValidateToken() self::assertTrue($jwtIsValid); } - public function testValidateDecodingSocketId() { + /** + * @throws AblyException + */ + public function testDecodingSocketId() { $socketId = new \stdClass(); $socketId->connectionKey = 'key'; - $socketId->clientId = 'clientId'; + $socketId->clientId = null; $socketIdObject = Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketId))); self::assertEquals('key', $socketIdObject->connectionKey); - self::assertEquals('clientId', $socketIdObject->clientId); + self::assertNull($socketIdObject->clientId); + + $socketId = new \stdClass(); + $socketId->connectionKey = 'key'; + $socketId->clientId = 'id'; + $socketIdObject = Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketId))); + self::assertEquals('key', $socketIdObject->connectionKey); + self::assertEquals('id', $socketIdObject->clientId); + } + + public function testExceptionOnDecodingInvalidSocketId() + { + self::expectException(AblyException::class); + self::expectExceptionMessage("SocketId decoding failed, ".Utils::SOCKET_ID_ERROR); + Utils::decodeSocketId("invalid_socket_id"); + } + + public function testExceptionOnMissingClientIdInSocketId() + { + $socketId = new \stdClass(); + $socketId->connectionKey = 'key'; + + self::expectException(AblyException::class); + self::expectExceptionMessage("ClientId is missing, ".Utils::SOCKET_ID_ERROR); + Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketId))); + } + + public function testExceptionOnMissingConnectionKeyInSocketId() + { + $socketId = new \stdClass(); + $socketId->clientId = 'id'; + + self::expectException(AblyException::class); + self::expectExceptionMessage("ConnectionKey is not set, ".Utils::SOCKET_ID_ERROR); + Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketId))); } } From 3b271cf946d193133f78fa94f2a8635f6389468c Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 24 Sep 2024 20:00:51 +0530 Subject: [PATCH 6/9] Refactored UtilsTest.php, renamed test names and variables --- tests/UtilsTest.php | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php index 5f79723..3fb3663 100644 --- a/tests/UtilsTest.php +++ b/tests/UtilsTest.php @@ -31,18 +31,21 @@ public function testGenerateAndValidateToken() /** * @throws AblyException */ - public function testDecodingSocketId() { - $socketId = new \stdClass(); - $socketId->connectionKey = 'key'; - $socketId->clientId = null; - $socketIdObject = Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketId))); + public function testDecodeSocketId() { + $socketIdObject = Utils::decodeSocketId(null); + self::assertNull($socketIdObject); + + $originalSocketIdObj = new \stdClass(); + $originalSocketIdObj->connectionKey = 'key'; + $originalSocketIdObj->clientId = null; + $socketIdObject = Utils::decodeSocketId(Utils::base64url_encode(json_encode($originalSocketIdObj))); self::assertEquals('key', $socketIdObject->connectionKey); self::assertNull($socketIdObject->clientId); - $socketId = new \stdClass(); - $socketId->connectionKey = 'key'; - $socketId->clientId = 'id'; - $socketIdObject = Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketId))); + $originalSocketIdObj = new \stdClass(); + $originalSocketIdObj->connectionKey = 'key'; + $originalSocketIdObj->clientId = 'id'; + $socketIdObject = Utils::decodeSocketId(Utils::base64url_encode(json_encode($originalSocketIdObj))); self::assertEquals('key', $socketIdObject->connectionKey); self::assertEquals('id', $socketIdObject->clientId); } @@ -56,21 +59,21 @@ public function testExceptionOnDecodingInvalidSocketId() public function testExceptionOnMissingClientIdInSocketId() { - $socketId = new \stdClass(); - $socketId->connectionKey = 'key'; + $socketIdObject = new \stdClass(); + $socketIdObject->connectionKey = 'key'; self::expectException(AblyException::class); self::expectExceptionMessage("ClientId is missing, ".Utils::SOCKET_ID_ERROR); - Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketId))); + Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketIdObject))); } public function testExceptionOnMissingConnectionKeyInSocketId() { - $socketId = new \stdClass(); - $socketId->clientId = 'id'; + $socketIdObject = new \stdClass(); + $socketIdObject->clientId = 'id'; self::expectException(AblyException::class); self::expectExceptionMessage("ConnectionKey is not set, ".Utils::SOCKET_ID_ERROR); - Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketId))); + Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketIdObject))); } } From 43a6e8906c6f28e6fad66711bca6050a0103bd38 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 24 Sep 2024 20:02:21 +0530 Subject: [PATCH 7/9] Added missing test to make sure publish payload doesn't have socket key --- src/Utils.php | 3 +++ tests/AblyBroadcasterTest.php | 25 ++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/Utils.php b/src/Utils.php index d77b369..1a506d6 100644 --- a/src/Utils.php +++ b/src/Utils.php @@ -60,6 +60,9 @@ public static function isJwtValid($jwt, $timeFn, $key) return $isSignatureValid && ! $isTokenExpired; } + /** + * https://www.php.net/manual/en/function.base64-encode.php#127544 + */ public static function base64url_encode($str): string { return rtrim(strtr(base64_encode($str), '+/', '-_'), '='); diff --git a/tests/AblyBroadcasterTest.php b/tests/AblyBroadcasterTest.php index 7ed6342..354a693 100644 --- a/tests/AblyBroadcasterTest.php +++ b/tests/AblyBroadcasterTest.php @@ -320,16 +320,33 @@ public function testLaravelAblyAgentHeader() $this->assertcontains( 'Ably-Agent: '.$expectedLaravelHeader, $ably->http->lastHeaders, 'Expected Laravel broadcaster header in HTTP request '.json_encode($ably->http->lastHeaders)); } - public function testPayloadShouldNotIncludeSocketKey() + public function testPublishPayloadShouldNotIncludeSocketKey() { + $ably = (new AblyFactory())->make([ + 'key' => 'abcd:efgh', + 'httpClass' => 'Ably\LaravelBroadcaster\Tests\HttpMock', + ]); + $broadcaster = m::mock(AblyBroadcasterExposed::class, [$ably, []])->makePartial(); -// self::assertArrayNotHasKey('socket', $message->data); + $socketIdObject = new \stdClass(); + $socketIdObject->connectionKey = 'foo'; + $socketIdObject->clientId = 'sacOO7'; + $payload = [ + 'foo' => 'bar', + 'socket' => Utils::base64url_encode(json_encode($socketIdObject)) + ]; + $broadcaster->broadcast(["channel1", "channel2"], 'testEvent', $payload); + + self::assertCount(2, $broadcaster->payloads); + foreach ($broadcaster->payloads as $payload) { + self::assertArrayNotHasKey('socket', $payload); + } } + public function testBuildMessageBasedOnSocketIdObject() { $broadcaster = m::mock(AblyBroadcasterExposed::class, [$this->ably, []])->makePartial(); - $payload = [ 'foo' => 'bar', 'chat' => 'hello there' @@ -354,8 +371,10 @@ public function testBuildMessageBasedOnSocketIdObject() class AblyBroadcasterExposed extends AblyBroadcaster { + public $payloads = []; public function buildAblyMessage($event, $payload = [], $socketIdObject = null) { + $this->payloads[] = $payload; return parent::buildAblyMessage($event, $payload, $socketIdObject); } } From baf6820f581c87cf7526297de14b48b610d108cf Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 24 Sep 2024 22:31:25 +0530 Subject: [PATCH 8/9] Added JSON error check while decoding socketId as per review comment --- src/Utils.php | 14 ++++++++++---- tests/UtilsTest.php | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Utils.php b/src/Utils.php index 1a506d6..8c4d607 100644 --- a/src/Utils.php +++ b/src/Utils.php @@ -79,14 +79,20 @@ public static function base64url_decode($data) { ."'connectionKey' and 'clientId' as keys. 'clientId' is null if connection is not identified"; /** + * @return object * @throws AblyException */ - public static function decodeSocketId($socketId) { + public static function decodeSocketId($socketId): ?object + { $socketIdObject = null; if ($socketId) { - $socketIdObject = json_decode(self::base64url_decode($socketId)); - if (!$socketIdObject) { - throw new AblyException("SocketId decoding failed, ".self::SOCKET_ID_ERROR); + $socketIdJsonString = self::base64url_decode($socketId); + if (!$socketIdJsonString) { + throw new AblyException("Base64 decoding failed, ".self::SOCKET_ID_ERROR); + } + $socketIdObject = json_decode($socketIdJsonString); + if (json_last_error() !== JSON_ERROR_NONE) { + throw new AblyException("JSON decoding failed: " . json_last_error_msg() . ", " . self::SOCKET_ID_ERROR); } if (!isset($socketIdObject->connectionKey)) { throw new AblyException("ConnectionKey is not set, ".self::SOCKET_ID_ERROR); diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php index 3fb3663..a4362fd 100644 --- a/tests/UtilsTest.php +++ b/tests/UtilsTest.php @@ -53,7 +53,7 @@ public function testDecodeSocketId() { public function testExceptionOnDecodingInvalidSocketId() { self::expectException(AblyException::class); - self::expectExceptionMessage("SocketId decoding failed, ".Utils::SOCKET_ID_ERROR); + self::expectExceptionMessage("Base64 decoding failed, ".Utils::SOCKET_ID_ERROR); Utils::decodeSocketId("invalid_socket_id"); } From f4e2361d544bd560bf08c6eb3cbdeed94e4cccaa Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 25 Sep 2024 11:14:15 +0530 Subject: [PATCH 9/9] Renamed base64encode and decode util helpers to camelCase, added return types for all helper methods --- src/Utils.php | 27 ++++++++++++++------------- tests/AblyBroadcasterTest.php | 2 +- tests/UtilsTest.php | 8 ++++---- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/Utils.php b/src/Utils.php index 8c4d607..b3c8c9f 100644 --- a/src/Utils.php +++ b/src/Utils.php @@ -11,11 +11,11 @@ class Utils * @param string $jwt * @return array */ - public static function parseJwt($jwt) + public static function parseJwt($jwt): array { $tokenParts = explode('.', $jwt); - $header = json_decode(self::base64url_decode($tokenParts[0]), true); - $payload = json_decode(self::base64url_decode($tokenParts[1]), true); + $header = json_decode(self::base64urlDecode($tokenParts[0]), true); + $payload = json_decode(self::base64urlDecode($tokenParts[1]), true); return ['header' => $header, 'payload' => $payload]; } @@ -25,13 +25,13 @@ public static function parseJwt($jwt) * @param array $payload * @return string */ - public static function generateJwt($headers, $payload, $key) + public static function generateJwt($headers, $payload, $key): string { - $encodedHeaders = self::base64url_encode(json_encode($headers)); - $encodedPayload = self::base64url_encode(json_encode($payload)); + $encodedHeaders = self::base64urlEncode(json_encode($headers)); + $encodedPayload = self::base64urlEncode(json_encode($payload)); $signature = hash_hmac('SHA256', "$encodedHeaders.$encodedPayload", $key, true); - $encodedSignature = self::base64url_encode($signature); + $encodedSignature = self::base64urlEncode($signature); return "$encodedHeaders.$encodedPayload.$encodedSignature"; } @@ -41,7 +41,7 @@ public static function generateJwt($headers, $payload, $key) * @param mixed $timeFn * @return bool */ - public static function isJwtValid($jwt, $timeFn, $key) + public static function isJwtValid($jwt, $timeFn, $key): bool { // split the jwt $tokenParts = explode('.', $jwt); @@ -50,12 +50,12 @@ public static function isJwtValid($jwt, $timeFn, $key) $tokenSignature = $tokenParts[2]; // check the expiration time - note this will cause an error if there is no 'exp' claim in the jwt - $expiration = json_decode(self::base64url_decode($payload))->exp; + $expiration = json_decode(self::base64urlDecode($payload))->exp; $isTokenExpired = $expiration <= $timeFn(); // build a signature based on the header and payload using the secret $signature = hash_hmac('SHA256', $header.'.'.$payload, $key, true); - $isSignatureValid = self::base64url_encode($signature) === $tokenSignature; + $isSignatureValid = self::base64urlEncode($signature) === $tokenSignature; return $isSignatureValid && ! $isTokenExpired; } @@ -63,7 +63,7 @@ public static function isJwtValid($jwt, $timeFn, $key) /** * https://www.php.net/manual/en/function.base64-encode.php#127544 */ - public static function base64url_encode($str): string + public static function base64urlEncode($str): string { return rtrim(strtr(base64_encode($str), '+/', '-_'), '='); } @@ -71,7 +71,8 @@ public static function base64url_encode($str): string /** * https://www.php.net/manual/en/function.base64-encode.php#127544 */ - public static function base64url_decode($data) { + public static function base64urlDecode($data): string + { return base64_decode(strtr($data, '-_', '+/'), true); } @@ -86,7 +87,7 @@ public static function decodeSocketId($socketId): ?object { $socketIdObject = null; if ($socketId) { - $socketIdJsonString = self::base64url_decode($socketId); + $socketIdJsonString = self::base64urlDecode($socketId); if (!$socketIdJsonString) { throw new AblyException("Base64 decoding failed, ".self::SOCKET_ID_ERROR); } diff --git a/tests/AblyBroadcasterTest.php b/tests/AblyBroadcasterTest.php index 354a693..b06d403 100644 --- a/tests/AblyBroadcasterTest.php +++ b/tests/AblyBroadcasterTest.php @@ -333,7 +333,7 @@ public function testPublishPayloadShouldNotIncludeSocketKey() $socketIdObject->clientId = 'sacOO7'; $payload = [ 'foo' => 'bar', - 'socket' => Utils::base64url_encode(json_encode($socketIdObject)) + 'socket' => Utils::base64urlEncode(json_encode($socketIdObject)) ]; $broadcaster->broadcast(["channel1", "channel2"], 'testEvent', $payload); diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php index a4362fd..27f5ae4 100644 --- a/tests/UtilsTest.php +++ b/tests/UtilsTest.php @@ -38,14 +38,14 @@ public function testDecodeSocketId() { $originalSocketIdObj = new \stdClass(); $originalSocketIdObj->connectionKey = 'key'; $originalSocketIdObj->clientId = null; - $socketIdObject = Utils::decodeSocketId(Utils::base64url_encode(json_encode($originalSocketIdObj))); + $socketIdObject = Utils::decodeSocketId(Utils::base64urlEncode(json_encode($originalSocketIdObj))); self::assertEquals('key', $socketIdObject->connectionKey); self::assertNull($socketIdObject->clientId); $originalSocketIdObj = new \stdClass(); $originalSocketIdObj->connectionKey = 'key'; $originalSocketIdObj->clientId = 'id'; - $socketIdObject = Utils::decodeSocketId(Utils::base64url_encode(json_encode($originalSocketIdObj))); + $socketIdObject = Utils::decodeSocketId(Utils::base64urlEncode(json_encode($originalSocketIdObj))); self::assertEquals('key', $socketIdObject->connectionKey); self::assertEquals('id', $socketIdObject->clientId); } @@ -64,7 +64,7 @@ public function testExceptionOnMissingClientIdInSocketId() self::expectException(AblyException::class); self::expectExceptionMessage("ClientId is missing, ".Utils::SOCKET_ID_ERROR); - Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketIdObject))); + Utils::decodeSocketId(Utils::base64urlEncode(json_encode($socketIdObject))); } public function testExceptionOnMissingConnectionKeyInSocketId() @@ -74,6 +74,6 @@ public function testExceptionOnMissingConnectionKeyInSocketId() self::expectException(AblyException::class); self::expectExceptionMessage("ConnectionKey is not set, ".Utils::SOCKET_ID_ERROR); - Utils::decodeSocketId(Utils::base64url_encode(json_encode($socketIdObject))); + Utils::decodeSocketId(Utils::base64urlEncode(json_encode($socketIdObject))); } }