Tuned fine-grained topic matching

FraunhoferIOSB · Aug 6, 2024 · a1763f0 · a1763f0
1 parent 2d00092
commit a1763f0
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 11 deletions.
diff --git a/....Moquette/src/main/java/de/fraunhofer/iosb/ilt/frostserver/mqtt/moquette/AuthWrapper.java b/....Moquette/src/main/java/de/fraunhofer/iosb/ilt/frostserver/mqtt/moquette/AuthWrapper.java
@@ -189,24 +189,31 @@ public boolean canRead(Topic mqttTopic, String user, String clientId) {
             return true;
         }
         if (topicAllowPattern != null) {
-            if (!topicAllowPattern.matcher(topic).matches()) {
-                LOGGER.debug("Denied access to {}, not matching allow pattern.", topic);
+            Version version;
+            try {
+                version = MqttManager.getVersionFromTopic(coreSettings, topic);
+            } catch (UnknownVersionException ex) {
+                LOGGER.debug("Denied access to {}, unknown version.", topic);
                 return false;
             }
+            String internalTopic = topic.substring(version.urlPart.length() + 1);
+            internalTopic = SubscriptionFactory.getPathFromTopic(internalTopic);
 
-            return validatePath(topic, userPrincipal);
+            if (!topicAllowPattern.matcher(internalTopic).matches()) {
+                LOGGER.debug("Denied access to {}, not matching allow pattern.", internalTopic);
+                return false;
+            }
+
+            return validatePath(version, internalTopic, userPrincipal);
         }
 
         return anonymousRead;
     }
 
-    private boolean validatePath(String topic, PrincipalExtended userPrincipal) {
+    private boolean validatePath(Version version, String topic, PrincipalExtended userPrincipal) {
         PrincipalExtended lp = PrincipalExtended.getLocalPrincipal();
         try {
-            Version version = MqttManager.getVersionFromTopic(coreSettings, topic);
-            String internalTopic = topic.substring(version.urlPart.length() + 1);
-            internalTopic = SubscriptionFactory.getPathFromTopic(internalTopic);
-            internalTopic = URLDecoder.decode(internalTopic, StringHelper.UTF8.name());
+            String internalTopic = URLDecoder.decode(topic, StringHelper.UTF8.name());
             ResourcePath path = PathParser.parsePath(
                     coreSettings.getModelRegistry(),
                     coreSettings.getQueryDefaults().getServiceRootUrl(),
@@ -221,7 +228,7 @@ private boolean validatePath(String topic, PrincipalExtended userPrincipal) {
                 LOGGER.debug(" Denying access for user {} to {}.", topic, userPrincipal);
             }
             return validPath;
-        } catch (RuntimeException | UnknownVersionException | UnsupportedEncodingException ex) {
+        } catch (RuntimeException | UnsupportedEncodingException ex) {
             LOGGER.warn("Exception trying to validate access for user {} to topic {}.", userPrincipal, topic, ex);
             return false;
         } finally {

diff --git a/...te/src/main/java/de/fraunhofer/iosb/ilt/frostserver/mqtt/moquette/MoquetteMqttServer.java b/...te/src/main/java/de/fraunhofer/iosb/ilt/frostserver/mqtt/moquette/MoquetteMqttServer.java
@@ -85,7 +85,7 @@ public class MoquetteMqttServer implements MqttServer, ConfigDefaults {
     @DefaultValue("memory")
     public static final String TAG_PERSISTENT_STORE_TYPE = "persistentStoreType";
     @DefaultValue("")
-    public static final String TAG_MQTT_TOPIC_ALLOWLIST = "topicAllowList";
+    public static final String TAG_MQTT_TOPIC_ALLOWLIST = "mqtt.topicAllowList";
 
     private static final String VALUE_STORE_TYPE_H2 = "h2";
 

diff --git a/...ver.Tests/src/test/java/de/fraunhofer/iosb/ilt/statests/f01auth/FineGrainedAuthTests.java b/...ver.Tests/src/test/java/de/fraunhofer/iosb/ilt/statests/f01auth/FineGrainedAuthTests.java
@@ -171,7 +171,7 @@ protected static void addCommonProperties(Map<String, String> properties) {
         properties.put("plugins.modelLoader.idType.User", "STRING");
         properties.put("persistence.idGenerationMode.Role", "ClientGeneratedOnly");
         properties.put("persistence.idGenerationMode.User", "ClientGeneratedOnly");
-        properties.put("auth.topicAllowList", "v[0-9].[0-9]/[a-zA-Z0-9_-]+\\((('[^']+')|([0-9]+))\\)/[a-zA-Z0-9_-]+");
+        properties.put("auth.mqtt.topicAllowList", "^/[a-zA-Z0-9_-]+\\((('[^']+')|([0-9]+))\\)/[a-zA-Z0-9_-]+$");
     }
 
     public FineGrainedAuthTests(ServerVersion version, Map<String, String> properties) {

diff --git a/docs/settings/auth.md b/docs/settings/auth.md
@@ -76,6 +76,10 @@ These are generic settings for authentication/authorisation.
   The maximum accepted length of usernames. Longer usernames are not checked and directly rejected. Default value: `128`
 * **auth.maxPasswordLength:** Since 2.4.0  
   The maximum accepted length of passwords. Longer passwords are not checked and directly rejected. Default value: `128`
+* **auth.mqtt.topicAllowList:** Since 2.4.0  
+  A regular expression that MQTT topics must match for users that do not have global admin or read rights.
+  The topic matched starts with the first slash after the version number and ends before the `?` of the query (if present).
+  Suggested value: `^/[a-zA-Z0-9_-]+\((('[^']+')|([0-9]+))\)/[a-zA-Z0-9_-]+$`. Default value: empty
 
 
 ### Settings for the auth provider class `BasicAuthProvider`