Allow publication of $SYS topics to be seen by plugin

The plugin can't deny it though.

Author: halfgaar

FlashMQTests/tst_maintests.cpp

@@ -481,6 +481,30 @@ void MainTests::test_acl_patterns_clientid()
     QCOMPARE(aclTree.findPermission(splitToVector("d/clientid_one/f/A/B", '/'), AclGrant::Read, "foo", "clientid_one"), AuthResult::success);
 }
 
+/**
+ * @brief MainTests::test_loading_acl_file was created because assertions in it failed when publishing $SYS topics were passed through the ACL
+ * layer. That's why it seemingly doesn't do anything.
+ */
+void MainTests::test_loading_acl_file()
+{
+    ConfFileTemp aclFile;
+    aclFile.writeLine("topic readwrite one/two");
+    aclFile.closeFile();
+
+    ConfFileTemp confFile;
+    confFile.writeLine("mosquitto_acl_file " + aclFile.getFilePath());
+    confFile.closeFile();
+
+    std::vector<std::string> args {"--config-file", confFile.getFilePath()};
+
+    cleanup();
+    init(args);
+
+    usleep(1000000);
+
+    QVERIFY(true);
+}
+
 #ifndef FMQ_NO_SSE
 void MainTests::test_sse_split()
 {

FlashMQTests/tst_maintests.h

@@ -92,6 +92,7 @@ private slots:
     void test_acl_tree2();
     void test_acl_patterns_username();
     void test_acl_patterns_clientid();
+    void test_loading_acl_file();
 
     void test_validUtf8Generic();
 #ifndef FMQ_NO_SSE

acltree.cpp

@@ -236,7 +236,9 @@ void AclTree::findPermissionRecursive(std::vector<std::string>::const_iterator c
 AuthResult AclTree::findPermission(const std::vector<std::string> &subtopicsPublish, AclGrant access, const std::string &username, const std::string &clientid)
 {
     assert(access == AclGrant::Read || access == AclGrant::Write);
-    assert(!clientid.empty());
+
+    // Empty clientid is when FlashMQ itself publishes, and that is fine for 'write'. on 'read', it should still never happen.
+    assert(!(clientid.empty() && access == AclGrant::Read ));
 
     collectedPermissions.clear();
 

plugin.cpp

@@ -268,15 +268,20 @@ void Authentication::securityCleanup(bool reloading)
  * @brief Authentication::aclCheck performs a write ACL check on the incoming publish.
  * @param publishData
  * @return
+ *
+ * Internal publishes write (publish) access is always allowed (it makes little sense that a plugin would have to explicitly allow
+ * those), but they are passed through the plugin, so you can inspect them. The read access can still be rejected by a plugin.
  */
 AuthResult Authentication::aclCheck(Publish &publishData, std::string_view payload, AclAccess access)
 {
+    AuthResult result = aclCheck(publishData.client_id, publishData.username, publishData.topic, publishData.getSubtopics(), payload, access, publishData.qos,
+                                 publishData.retain, publishData.getUserProperties());
+
     // Anonymous publishes come from FlashMQ internally, like SYS topics. We need to allow them.
-    if (publishData.client_id.empty())
-        return AuthResult::success;
+    if (access == AclAccess::write && publishData.client_id.empty())
+        result = AuthResult::success;
 
-    return aclCheck(publishData.client_id, publishData.username, publishData.topic, publishData.getSubtopics(), payload, access, publishData.qos,
-                    publishData.retain, publishData.getUserProperties());
+    return result;
 }
 
 AuthResult Authentication::aclCheck(const std::string &clientid, const std::string &username, const std::string &topic, const std::vector<std::string> &subtopics,

threaddata.cpp

@@ -478,20 +478,15 @@ void ThreadData::publishStatsOnDollarTopic(std::vector<std::shared_ptr<ThreadDat
     for (auto &pair : globalStats->getExtras())
     {
         Publish p(pair.first, pair.second, 0);
-        PublishCopyFactory factory(&p);
-        std::shared_ptr<SubscriptionStore> subscriptionStore = MainApp::getMainApp()->getSubscriptionStore();
-        subscriptionStore->queuePacketAtSubscribers(factory, "", true);
+        publishWithAcl(p);
     }
 }
 
 void ThreadData::publishStat(const std::string &topic, uint64_t n)
 {
     const std::string payload = std::to_string(n);
     Publish p(topic, payload, 0);
-    PublishCopyFactory factory(&p);
-    std::shared_ptr<SubscriptionStore> subscriptionStore = MainApp::getMainApp()->getSubscriptionStore();
-    subscriptionStore->queuePacketAtSubscribers(factory, "", true);
-    subscriptionStore->setRetainedMessage(p, factory.getSubtopics());
+    publishWithAcl(p, true);
 }
 
 void ThreadData::publishBridgeState(std::shared_ptr<BridgeState> bridge, bool connected)
@@ -509,10 +504,19 @@ void ThreadData::publishBridgeState(std::shared_ptr<BridgeState> bridge, bool co
     globalStats->setExtra(topic, payload);
 
     Publish p(topic, payload, 0);
-    PublishCopyFactory factory(&p);
+    publishWithAcl(p, true);
+}
+
+void ThreadData::publishWithAcl(Publish &pub, bool setRetain)
+{
+    authentication.aclCheck(pub, pub.payload, AclAccess::write);
+
+    PublishCopyFactory factory(&pub);
     std::shared_ptr<SubscriptionStore> subscriptionStore = MainApp::getMainApp()->getSubscriptionStore();
     subscriptionStore->queuePacketAtSubscribers(factory, "", true);
-    subscriptionStore->setRetainedMessage(p, factory.getSubtopics());
+
+    if (setRetain)
+        subscriptionStore->setRetainedMessage(pub, factory.getSubtopics());
 }
 
 /**

threaddata.h

@@ -89,6 +89,7 @@ class ThreadData
     void bridgeReconnect();
 
     void removeQueuedClients();
+    void publishWithAcl(Publish &pub, bool setRetain=false);
 
 public:
     Settings settingsLocalCopy; // Is updated on reload, within the thread loop.