From b5919a534dd8ca368bc113b77310e6ba3d73896f Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Tue, 24 Sep 2019 23:36:20 +0200 Subject: [PATCH] Update MQTT api a bit to be more flexible --- .../mqtt/mqttchannelimplementation.cpp | 4 +-- .../network/mqtt/mqttchannelimplementation.h | 4 +-- .../mqtt/mqttproviderimplementation.cpp | 33 +++++++++++++------ .../network/mqtt/mqttproviderimplementation.h | 4 +-- libnymea/network/mqtt/mqttchannel.h | 2 +- libnymea/network/mqtt/mqttprovider.cpp | 15 +++++++-- libnymea/network/mqtt/mqttprovider.h | 4 +-- 7 files changed, 44 insertions(+), 22 deletions(-) diff --git a/libnymea-core/hardware/network/mqtt/mqttchannelimplementation.cpp b/libnymea-core/hardware/network/mqtt/mqttchannelimplementation.cpp index 0141e340..7a59b5e6 100644 --- a/libnymea-core/hardware/network/mqtt/mqttchannelimplementation.cpp +++ b/libnymea-core/hardware/network/mqtt/mqttchannelimplementation.cpp @@ -52,9 +52,9 @@ quint16 MqttChannelImplementation::serverPort() const return m_serverPort; } -QString MqttChannelImplementation::topicPrefix() const +QStringList MqttChannelImplementation::topicPrefixList() const { - return m_topicPrefix; + return m_topicPrefixList; } void MqttChannelImplementation::publish(const QString &topic, const QByteArray &payload) diff --git a/libnymea-core/hardware/network/mqtt/mqttchannelimplementation.h b/libnymea-core/hardware/network/mqtt/mqttchannelimplementation.h index 51d6ede1..cbdee1bc 100644 --- a/libnymea-core/hardware/network/mqtt/mqttchannelimplementation.h +++ b/libnymea-core/hardware/network/mqtt/mqttchannelimplementation.h @@ -36,7 +36,7 @@ public: QString password() const override; QHostAddress serverAddress() const override; quint16 serverPort() const override; - QString topicPrefix() const override; + QStringList topicPrefixList() const override; void publish(const QString &topic, const QByteArray &payload) override; @@ -49,7 +49,7 @@ private: QString m_password; QHostAddress m_serverAddress; quint16 m_serverPort; - QString m_topicPrefix; + QStringList m_topicPrefixList; friend class MqttProviderImplementation; }; diff --git a/libnymea-core/hardware/network/mqtt/mqttproviderimplementation.cpp b/libnymea-core/hardware/network/mqtt/mqttproviderimplementation.cpp index 3bb9c994..dce64ed7 100644 --- a/libnymea-core/hardware/network/mqtt/mqttproviderimplementation.cpp +++ b/libnymea-core/hardware/network/mqtt/mqttproviderimplementation.cpp @@ -37,18 +37,23 @@ MqttProviderImplementation::MqttProviderImplementation(MqttBroker *broker, QObje connect(broker, &MqttBroker::publishReceived, this, &MqttProviderImplementation::onPublishReceived); } -MqttChannel *MqttProviderImplementation::createChannel(const DeviceId &deviceId, const QHostAddress &clientAddress) +MqttChannel *MqttProviderImplementation::createChannel(const QString &clientId, const QHostAddress &clientAddress, const QStringList &topicPrefixList) { if (m_broker->configurations().isEmpty()) { - qCWarning(dcMqtt) << "MQTT broker not running. Cannot create a channel for device" << deviceId; + qCWarning(dcMqtt) << "MQTT broker not running. Cannot create a channel for device" << clientId; return nullptr; } MqttChannelImplementation* channel = new MqttChannelImplementation(); - channel->m_clientId = deviceId.toString().remove(QRegExp("[{}-]")); + channel->m_clientId = clientId; channel->m_username = QUuid::createUuid().toString().remove(QRegExp("[{}-]")); channel->m_password = QUuid::createUuid().toString().remove(QRegExp("[{}-]")); - channel->m_topicPrefix = channel->m_clientId; + if (!topicPrefixList.isEmpty()) { + channel->m_topicPrefixList = topicPrefixList; + } else { + QString defaultTopicPrefix = channel->m_clientId; + channel->m_topicPrefixList.append(defaultTopicPrefix); + } foreach (const QNetworkInterface &interface, QNetworkInterface::allInterfaces()) { @@ -80,8 +85,10 @@ MqttChannel *MqttProviderImplementation::createChannel(const DeviceId &deviceId, policy.clientId = channel->clientId(); policy.username = channel->username(); policy.password = channel->password(); - policy.allowedPublishTopicFilters.append(QString("%1/#").arg(channel->m_topicPrefix)); - policy.allowedSubscribeTopicFilters.append(QString("%1/#").arg(channel->m_topicPrefix)); + foreach (const QString &topicPrefix, channel->m_topicPrefixList) { + policy.allowedPublishTopicFilters.append(QString("%1/#").arg(topicPrefix)); + policy.allowedSubscribeTopicFilters.append(QString("%1/#").arg(topicPrefix)); + } m_broker->updatePolicy(policy); return channel; @@ -99,7 +106,7 @@ void MqttProviderImplementation::releaseChannel(MqttChannel *channel) delete channel; } -MqttClient *MqttProviderImplementation::createInternalClient(const DeviceId &deviceId) +MqttClient *MqttProviderImplementation::createInternalClient(const QString &clientId) { ServerConfiguration preferredConfig; @@ -117,7 +124,6 @@ MqttClient *MqttProviderImplementation::createInternalClient(const DeviceId &dev return nullptr; } - QString clientId = deviceId.toString().remove(QRegExp("[{}-]")); MqttPolicy policy; policy.clientId = clientId; policy.username = QUuid::createUuid().toString().remove(QRegExp("[{}-]")); @@ -189,9 +195,16 @@ void MqttProviderImplementation::onPublishReceived(const QString &clientId, cons void MqttProviderImplementation::onPluginPublished(const QString &topic, const QByteArray &payload) { MqttChannelImplementation *channel = static_cast(sender()); - if (!topic.startsWith(channel->topicPrefix())) { + bool found = false; + foreach (const QString &topicPrefix, channel->topicPrefixList()) { + if (topicPrefix.startsWith(topicPrefix)) { + found = true; + break; + } + + } + if (!found) { qCWarning(dcMqtt) << "Attempt to publish to MQTT channel for client" << channel->clientId() << "but topic is not within allowed topic prefix. Discarding message."; - return; } m_broker->publish(topic, payload); } diff --git a/libnymea-core/hardware/network/mqtt/mqttproviderimplementation.h b/libnymea-core/hardware/network/mqtt/mqttproviderimplementation.h index 686cfbfa..a18577ae 100644 --- a/libnymea-core/hardware/network/mqtt/mqttproviderimplementation.h +++ b/libnymea-core/hardware/network/mqtt/mqttproviderimplementation.h @@ -34,10 +34,10 @@ class MqttProviderImplementation : public MqttProvider public: explicit MqttProviderImplementation(MqttBroker *broker, QObject *parent = nullptr); - MqttChannel* createChannel(const DeviceId &deviceId, const QHostAddress &clientAddress) override; + MqttChannel* createChannel(const QString &clientId, const QHostAddress &clientAddress, const QStringList &topicPrefixList = QStringList()) override; void releaseChannel(MqttChannel* channel) override; - MqttClient* createInternalClient(const DeviceId &deviceId) override; + MqttClient* createInternalClient(const QString &clientId) override; bool available() const override; bool enabled() const override; diff --git a/libnymea/network/mqtt/mqttchannel.h b/libnymea/network/mqtt/mqttchannel.h index bc1b0857..87c05ab4 100644 --- a/libnymea/network/mqtt/mqttchannel.h +++ b/libnymea/network/mqtt/mqttchannel.h @@ -38,7 +38,7 @@ public: virtual QString password() const = 0; virtual QHostAddress serverAddress() const = 0; virtual quint16 serverPort() const = 0; - virtual QString topicPrefix() const = 0; + virtual QStringList topicPrefixList() const = 0; virtual void publish(const QString &topic, const QByteArray &payload) = 0; diff --git a/libnymea/network/mqtt/mqttprovider.cpp b/libnymea/network/mqtt/mqttprovider.cpp index ffd07bc3..e1d83753 100644 --- a/libnymea/network/mqtt/mqttprovider.cpp +++ b/libnymea/network/mqtt/mqttprovider.cpp @@ -146,11 +146,20 @@ \sa HardwareResource, HardwareManager::mqttProvider() */ -/*! \fn MqttChannel *MqttProvider::createChannel(const DeviceId &deviceId, const QHostAddress &clientAddress); +/*! \fn MqttChannel *MqttProvider::createChannel(const QString &clientId, const QHostAddress &clientAddress, const QString &topicPrefix); Creates a new MQTT channel on the internal broker. The returned channel will have the required details for the - client device to connect to the broker. A temporaray clientId/user/password combination will be created and + client device to connect to the broker. A temporaray user/password combination will be created and clients connecting to the broker with those credentials will have access to subscribe and post to # within the - given topic prefix. + given \a topicPrefixList. + + \a clientId must be unique within the system or the channel creation will fail. If a mqtt client allows to configure the + clientId, using it's deviceId is likely a good idea. + + \a topicPrefixList will be used to generate the policy for this MQTT client by appending "/#" to it. This will allow the + client to publish and subscribe to topics within "/#". See The MQTT specification on topic filters for more details. + It is good practice to isolate clients as much as possible the topics should be as restrictive as possible to avoid devices + snooping in on other things on the MQTT broker. If no topicPrefix is provided, a default of "" Id is generated, + resulting in a policy of "/#". At this point it is not allowed for plugins to publish/subscribe to # or $ topics. \sa releaseChannel(MqttChannel *channel) */ diff --git a/libnymea/network/mqtt/mqttprovider.h b/libnymea/network/mqtt/mqttprovider.h index 187c1d1a..2479b82b 100644 --- a/libnymea/network/mqtt/mqttprovider.h +++ b/libnymea/network/mqtt/mqttprovider.h @@ -38,10 +38,10 @@ class MqttProvider : public HardwareResource public: explicit MqttProvider(QObject *parent = nullptr); - virtual MqttChannel* createChannel(const DeviceId &deviceId, const QHostAddress &clientAddress) = 0; + virtual MqttChannel* createChannel(const QString &clientId, const QHostAddress &clientAddress, const QStringList &topicPrefixList = QStringList()) = 0; virtual void releaseChannel(MqttChannel *channel) = 0; - virtual MqttClient* createInternalClient(const DeviceId &deviceId) = 0; + virtual MqttClient* createInternalClient(const QString &clientId) = 0; }; #endif // MQTTPROVIDER_H