From 0513ab09993e73f94c0cfca2717ebb4889abb7df Mon Sep 17 00:00:00 2001 From: Christopher Vagnetoft Date: Mon, 11 Mar 2024 22:29:17 +0100 Subject: [PATCH] Improved jwt logic * No longer stores full token, but only payload. --- README.md | 10 +++--- src/Broker/SseSubscriber.php | 7 +++- src/Broker/TopicManager.php | 2 +- src/Http/Middleware/MercureHandler.php | 38 +++++++++------------- src/Http/Middleware/SecurityMiddleware.php | 8 +++-- 5 files changed, 33 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 67b7dba..742aa88 100644 --- a/README.md +++ b/README.md @@ -34,24 +34,24 @@ $ ./mercureact.phar -c mercureact.conf ## ToDos -* [ ] Read config from file +* [x] Read config from file * [ ] Security Security Security * [x] Check JWTs on connect * [x] Check claims on subscribe and publish - * [ ] WebSocket authentication - * [ ] Extract JWT claims to request attributes, instead of JWTToken + * [x] Extract JWT claims to request attributes, instead of JWTToken * [x] Subscription/Topic manager * [x] Unify distribution - * [ ] Enumerate subscriptions and topics + * [x] Enumerate subscriptions and topics * [x] Publish events * [x] Server-Side Events distributor * [x] Distribute events over SSE * [ ] WebSocket distributor + * [ ] WebSocket authentication * [ ] Setup subscriptions * [ ] Dynamic subscriptions * [x] Distribute events over WS * [x] Break out HTTP middleware into classes * [ ] HTTP middleware unittests * [ ] Replay missed events based on event id -* [ ] Figure out how to determine last event IDs +* [x] Figure out how to determine last event IDs * [ ] Metrics endpoint diff --git a/src/Broker/SseSubscriber.php b/src/Broker/SseSubscriber.php index 68745df..2c532e3 100644 --- a/src/Broker/SseSubscriber.php +++ b/src/Broker/SseSubscriber.php @@ -26,7 +26,12 @@ class SseSubscriber implements SubscriberInterface public function isAuthorized(): bool { - return $this->request->getAttribute('authorization') instanceof JWTToken; + return $this->request->getAttribute('authorized'); + } + + public function getPayload(): array + { + return $this->request->getAttribute('mercure.payload')??[]; } public function getId(): string diff --git a/src/Broker/TopicManager.php b/src/Broker/TopicManager.php index ffc6ead..fefd29d 100644 --- a/src/Broker/TopicManager.php +++ b/src/Broker/TopicManager.php @@ -69,7 +69,7 @@ class TopicManager 'topic' => $topic->getTopic(), 'subscriber' => $sub->getId(), 'active' => true, - 'payload' => null, // TODO populate from mercure.payload in JWT + 'payload' => $sub->getPayload(), ]; } } diff --git a/src/Http/Middleware/MercureHandler.php b/src/Http/Middleware/MercureHandler.php index d68e3b1..684d3bd 100644 --- a/src/Http/Middleware/MercureHandler.php +++ b/src/Http/Middleware/MercureHandler.php @@ -88,17 +88,13 @@ class MercureHandler } // Grab the JWT token from the requests authorization attribute - $tok = $request->getAttribute('authorization'); - if ($tok instanceof JWTToken) { - $claims = $tok->claims->getAll(); - if (isset($claims['mercure']['subscribe'])) { - $subscribeClaims = $claims['mercure']['subscribe']; - if (!$this->checkTopicClaims($topics, $subscribeClaims)) { - throw new SecurityException( - message: "Insufficient permissions for subscribe", - code: SecurityException::ERR_NO_PERMISSION - ); - } + if ($request->getAttribute('authorized')) { + $claims = $request->getAttribute('mercure.subscribe'); + if (!$this->checkTopicClaims($topics, $claims)) { + throw new SecurityException( + message: "Insufficient permissions for subscribe", + code: SecurityException::ERR_NO_PERMISSION + ); } } else { // Disallow if we don't allow anonymous subscribers. Note that anonymous @@ -153,18 +149,14 @@ class MercureHandler } // Grab the JWT token from the requests authorization attribute - $tok = $request->getAttribute('authorization'); - if ($tok instanceof JWTToken) { - $claims = $tok->claims->getAll(); - if (isset($claims['mercure']['publish'])) { - $publishClaims = $claims['mercure']['publish']; - // check topic against publishClaims - if (!$this->checkTopicClaims($data['topic']??[], $publishClaims)) { - throw new SecurityException( - message: "Insufficient permissions for publish", - code: SecurityException::ERR_NO_PERMISSION - ); - } + if ($request->getAttribute('authorized')) { + $claims = $request->getAttribute('mercure.publish'); + // check topic against publishClaims + if (!$this->checkTopicClaims($data['topic']??[], $claims)) { + throw new SecurityException( + message: "Insufficient permissions for publish", + code: SecurityException::ERR_NO_PERMISSION + ); } } else { // reject if access denied diff --git a/src/Http/Middleware/SecurityMiddleware.php b/src/Http/Middleware/SecurityMiddleware.php index 1fe1cb5..148b07a 100644 --- a/src/Http/Middleware/SecurityMiddleware.php +++ b/src/Http/Middleware/SecurityMiddleware.php @@ -61,11 +61,15 @@ class SecurityMiddleware code: SecurityException::ERR_ACCESS_DENIED ); } + $claims = $tok->claims->getAll()['mercure']??[]; return $request - ->withAttribute('authorization', $tok); + ->withAttribute('mercure.publish', $claims['publish']??[]) + ->withAttribute('mercure.subscribe', $claims['subscribe']??[]) + ->withAttribute('mercure.payload', $claims['payload']??[]) + ->withAttribute('authorized', true); } else { return $request - ->withAttribute('authorization', null); + ->withAttribute('authorized', false); } }