Changed PostOffice to verify read access also on shared subscriptions

moquette-io · Dec 8, 2023 · e6ef6f7 · e6ef6f7
1 parent ad5577d
commit e6ef6f7
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 8 deletions.
diff --git a/broker/src/main/java/io/moquette/broker/Authorizator.java b/broker/src/main/java/io/moquette/broker/Authorizator.java
@@ -25,6 +25,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.function.Function;
 
 import static io.moquette.broker.Utils.messageId;
 import static io.netty.handler.codec.mqtt.MqttQoS.FAILURE;
@@ -39,6 +40,15 @@ final class Authorizator {
         this.policy = policy;
     }
 
+
+    List<MqttTopicSubscription> verifyAlsoSharedTopicsReadAccess(String clientID, String username, MqttSubscribeMessage msg) {
+        return verifyTopicsReadAccessWithTopicExtractor(clientID, username, msg, Authorizator::extractShareTopic);
+    }
+
+    private static Topic extractShareTopic(String s) {
+        return Topic.asTopic(PostOffice.extractFilterFromShared(s));
+    }
+
     /**
      * @param clientID
      *            the clientID
@@ -49,11 +59,16 @@ final class Authorizator {
      * @return the list of verified topics for the given subscribe message.
      */
     List<MqttTopicSubscription> verifyTopicsReadAccess(String clientID, String username, MqttSubscribeMessage msg) {
+        return verifyTopicsReadAccessWithTopicExtractor(clientID, username, msg, Topic::asTopic);
+    }
+
+    private List<MqttTopicSubscription> verifyTopicsReadAccessWithTopicExtractor(String clientID, String username,
+                                                 MqttSubscribeMessage msg, Function<String, Topic> topicExtractor) {
         List<MqttTopicSubscription> ackTopics = new ArrayList<>();
 
         final int messageId = messageId(msg);
         for (MqttTopicSubscription req : msg.payload().topicSubscriptions()) {
-            Topic topic = new Topic(req.topicName());
+            Topic topic = topicExtractor.apply(req.topicName());
             if (!policy.canRead(topic, username, clientID)) {
                 // send SUBACK with 0x80, the user hasn't credentials to read the topic
                 LOG.warn("Client does not have read permissions on the topic username: {}, messageId: {}, " +

diff --git a/broker/src/main/java/io/moquette/broker/PostOffice.java b/broker/src/main/java/io/moquette/broker/PostOffice.java
@@ -315,8 +315,13 @@ public void subscribeClientToTopics(MqttSubscribeMessage msg, String clientID, S
         } else {
             sharedSubscriptions = Collections.emptyList();
         }
-        // TODO verify read access also for shared subscriptions
-        List<MqttTopicSubscription> ackTopics = authorizator.verifyTopicsReadAccess(clientID, username, msg);
+
+        List<MqttTopicSubscription> ackTopics;
+        if (mqttConnection.isProtocolVersion5()) {
+            ackTopics = authorizator.verifyAlsoSharedTopicsReadAccess(clientID, username, msg);
+        } else {
+            ackTopics = authorizator.verifyTopicsReadAccess(clientID, username, msg);
+        }
         MqttSubAckMessage ackMessage = doAckMessageFromValidateFilters(ackTopics, messageID);
 
         // store topics subscriptions in session

diff --git a/broker/src/main/java/io/moquette/broker/security/DeclarativeAuthorizatorPolicy.java b/broker/src/main/java/io/moquette/broker/security/DeclarativeAuthorizatorPolicy.java
@@ -11,26 +11,26 @@ public static final class Builder {
         private DeclarativeAuthorizatorPolicy instance;
 
         public Builder readFrom(Topic topic, String user, String client) {
-            final DeclarativeAuthorizatorPolicy policy = lazyGet();
+            final DeclarativeAuthorizatorPolicy policy = createOrGet();
             policy.addReadFrom(topic, user, client);
             return this;
         }
 
         public Builder writeTo(Topic topic, String user, String client) {
-            final DeclarativeAuthorizatorPolicy policy = lazyGet();
+            final DeclarativeAuthorizatorPolicy policy = createOrGet();
             policy.addWriteTo(topic, user, client);
             return this;
         }
 
-        private DeclarativeAuthorizatorPolicy lazyGet() {
+        private DeclarativeAuthorizatorPolicy createOrGet() {
             if (instance == null) {
                 instance = new DeclarativeAuthorizatorPolicy();
             }
             return instance;
         }
 
         public IAuthorizatorPolicy build() {
-            return instance;
+            return createOrGet();
         }
     }
 
@@ -41,7 +41,7 @@ static final class TopicUserClient {
 
         public TopicUserClient(Topic topic, String user, String client) {
             Objects.requireNonNull(topic);
-            Objects.requireNonNull(user);
+            user = user == null ? "<>" : user;
             Objects.requireNonNull(client);
             this.topic = topic;
             this.user = user;