From d8ae8ade706d0cbcc330ebce918a2213dd46fb8d Mon Sep 17 00:00:00 2001 From: Christopher Vagnetoft Date: Thu, 14 Mar 2024 14:03:27 +0100 Subject: [PATCH] Refactored out claim check logic to its own class --- src/Broker/Security/ClaimChecker.php | 46 +++++++++++++++++++ src/Broker/Subscriber/SseSubscriber.php | 5 ++ src/Broker/Subscriber/SubscriberInterface.php | 7 +++ src/Broker/Subscriber/WsSubscriber.php | 10 ++-- src/Http/Middleware/MercureHandler.php | 28 +++-------- tests/Broker/TopicTest.php | 1 + 6 files changed, 71 insertions(+), 26 deletions(-) create mode 100644 src/Broker/Security/ClaimChecker.php diff --git a/src/Broker/Security/ClaimChecker.php b/src/Broker/Security/ClaimChecker.php new file mode 100644 index 0000000..2753436 --- /dev/null +++ b/src/Broker/Security/ClaimChecker.php @@ -0,0 +1,46 @@ +uriTemplate = new UriTemplate(); + } + + public function matchAll(array $topics, array $claims): bool + { + $matched = 0; + foreach ((array)$topics as $match) { + foreach ($claims as $claim) { + if (($claim === "*") + || ($claim === $match) + || ($this->uriTemplate->extract($claim, $match, true))) { + $matched++; + break; + } + } + } + return ($matched == count($topics)); + } + + public function matchOne(array $topics, array $claims): bool + { + foreach ((array)$topics as $match) { + foreach ($claims as $claim) { + if (($claim === "*") + || ($claim === $match) + || ($this->uriTemplate->extract($claim, $match, true))) { + return true; + } + } + } + return false; + } +} \ No newline at end of file diff --git a/src/Broker/Subscriber/SseSubscriber.php b/src/Broker/Subscriber/SseSubscriber.php index 66b99fc..7c2fcaf 100644 --- a/src/Broker/Subscriber/SseSubscriber.php +++ b/src/Broker/Subscriber/SseSubscriber.php @@ -30,6 +30,11 @@ class SseSubscriber implements SubscriberInterface return $this->request->getAttribute('authorized'); } + public function getMercureClaims(): ?array + { + return $this->request->getAttribute('mercure.claims'); + } + public function getPayload(): array { return $this->request->getAttribute('mercure.payload')??[]; diff --git a/src/Broker/Subscriber/SubscriberInterface.php b/src/Broker/Subscriber/SubscriberInterface.php index 9530aa1..4883703 100644 --- a/src/Broker/Subscriber/SubscriberInterface.php +++ b/src/Broker/Subscriber/SubscriberInterface.php @@ -21,6 +21,13 @@ interface SubscriberInterface */ public function isAuthenticated(): bool; + /** + * Returns the content of the JWT mercure claim if present. + * + * @return array|null + */ + public function getMercureClaims(): ?array; + /** * Returns the content of the JWT mercure.payload claim if present. * diff --git a/src/Broker/Subscriber/WsSubscriber.php b/src/Broker/Subscriber/WsSubscriber.php index ea96ccd..3825cf5 100644 --- a/src/Broker/Subscriber/WsSubscriber.php +++ b/src/Broker/Subscriber/WsSubscriber.php @@ -21,13 +21,8 @@ class WsSubscriber implements SubscriberInterface, EventEmitterInterface const EVENT_UNSUBSCRIBE = 'unsubscribe'; const EVENT_ERROR = 'error'; - const STATE_UNAUTHORIZED = 0; - const STATE_AUTHORIZED = 1; - private string $id; - private int $state = self::STATE_UNAUTHORIZED; - public function __construct( private WebSocketConnection $stream, private ServerRequestInterface $request, @@ -73,6 +68,11 @@ class WsSubscriber implements SubscriberInterface, EventEmitterInterface return $this->token && $this->token->isValid(); } + public function getMercureClaims(): ?array + { + return $this->request->getAttribute('mercure.claims'); + } + public function getPayload(): array { return $this->request->getAttribute('mercure.payload')??[]; diff --git a/src/Http/Middleware/MercureHandler.php b/src/Http/Middleware/MercureHandler.php index a07fca7..2dbf91c 100644 --- a/src/Http/Middleware/MercureHandler.php +++ b/src/Http/Middleware/MercureHandler.php @@ -3,6 +3,7 @@ namespace NoccyLabs\Mercureact\Http\Middleware; use NoccyLabs\Mercureact\Broker\Message; +use NoccyLabs\Mercureact\Broker\Security\ClaimChecker; use NoccyLabs\Mercureact\Broker\Subscriber\SseSubscriber; use NoccyLabs\Mercureact\Broker\TopicManager; use NoccyLabs\Mercureact\Configuration; @@ -27,6 +28,8 @@ class MercureHandler private int $seenIdHistorySize = 100; + private ClaimChecker $claimChecker; + public function __construct( private Configuration $config, private TopicManager $topicManager, @@ -35,6 +38,7 @@ class MercureHandler { $this->loop = $loop ?? Loop::get(); $this->seenIdHistorySize = $this->config->getDuplicateIdHistorySize(); + $this->claimChecker = new ClaimChecker(); } /** @@ -94,7 +98,7 @@ class MercureHandler // Grab the JWT token from the requests authorization attribute if ($request->getAttribute('authorized')) { $claims = $request->getAttribute('mercure.subscribe'); - if (!$this->checkTopicClaims($topics, $claims)) { + if (!$this->claimChecker->matchAll($topics, $claims)) { throw new SecurityException( message: "Insufficient permissions for subscribe", code: SecurityException::ERR_NO_PERMISSION @@ -112,6 +116,7 @@ class MercureHandler } $this->topicManager->subscribe($subscriber, $topics); + $responseStream->on('close', function () use ($subscriber, $topics) { $this->topicManager->unsubscribe($subscriber, $topics); }); @@ -156,7 +161,7 @@ class MercureHandler if ($request->getAttribute('authorized')) { $claims = $request->getAttribute('mercure.publish'); // check topic against publishClaims - if (!$this->checkTopicClaims($data['topic']??[], $claims)) { + if (!$this->claimChecker->matchAll($data['topic']??[], $claims)) { throw new SecurityException( message: "Insufficient permissions for publish", code: SecurityException::ERR_NO_PERMISSION @@ -195,23 +200,4 @@ class MercureHandler return Response::plaintext($message->id."\n"); } - private function checkTopicClaims(string|array $topic, array $claims): bool - { - $matched = 0; - foreach ((array)$topic as $match) { - foreach ($claims as $claim) { - if (($claim === "*") || ($claim === $match)) { - $matched++; - break; - } - // TODO make sure that UriTemplate parsing works - if ((new UriTemplate())->extract($claim, $match, true)) { - $matched++; - break; - } - } - } - return ($matched == count($topic)); - } - } \ No newline at end of file diff --git a/tests/Broker/TopicTest.php b/tests/Broker/TopicTest.php index 7327f86..ec00a50 100644 --- a/tests/Broker/TopicTest.php +++ b/tests/Broker/TopicTest.php @@ -56,6 +56,7 @@ abstract class _Subscriber implements SubscriberInterface { public array $messages = []; public function isAuthenticated():bool { return false; } public function deliver(Message $message):void { $this->messages[] = $message; } + public function getMercureClaims(): ?array { return []; } public function getPayload(): ?array { return null; } public function getId(): string { return ""; } }; \ No newline at end of file