From 1af6427b951fe32090e5baac7840ea4cc62c423a Mon Sep 17 00:00:00 2001 From: Filipe Azevedo Date: Tue, 10 Sep 2024 15:57:08 +0200 Subject: [PATCH 1/5] QXmppRosterManager: Fix import/export of roster items This fix sending the item ask attribute while it should not be sent. --- src/client/QXmppRosterManager.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/client/QXmppRosterManager.cpp b/src/client/QXmppRosterManager.cpp index 5671a4114..0083acf73 100644 --- a/src/client/QXmppRosterManager.cpp +++ b/src/client/QXmppRosterManager.cpp @@ -557,7 +557,15 @@ void QXmppRosterManager::onRegistered(QXmppClient *client) auto exportData = [this]() { return chainMapSuccess(requestRoster(), this, [](QXmppRosterIq &&iq) -> RosterData { - return { iq.items() }; + auto items = iq.items(); + + // We don't want this to be sent while importing. + // See https://datatracker.ietf.org/doc/html/rfc6121#section-2.1.2.2 + for (auto &item: items) { + item.setSubscriptionStatus({}); + } + + return { items }; }); }; From fb1e7591c2f8095a250066c931ed462d842c08d6 Mon Sep 17 00:00:00 2001 From: Filipe Azevedo Date: Thu, 25 Jul 2024 18:15:33 +0200 Subject: [PATCH 2/5] XEP 0283: Moved - Add support in QXmppPresence This allow to track if the request is being issued as part of a moved user. --- src/base/QXmppPresence.cpp | 35 +++++++++++++++++++++++++++++++++++ src/base/QXmppPresence.h | 5 +++++ 2 files changed, 40 insertions(+) diff --git a/src/base/QXmppPresence.cpp b/src/base/QXmppPresence.cpp index 727ec1f37..e8fd041e8 100644 --- a/src/base/QXmppPresence.cpp +++ b/src/base/QXmppPresence.cpp @@ -1,5 +1,6 @@ // SPDX-FileCopyrightText: 2009 Manjeet Dahiya // SPDX-FileCopyrightText: 2022 Melvin Keskin +// SPDX-FileCopyrightText: 2024 Filipe Azevedo // // SPDX-License-Identifier: LGPL-2.1-or-later @@ -76,6 +77,9 @@ class QXmppPresencePrivate : public QSharedData // XEP-0405: Mediated Information eXchange (MIX): Participant Server Requirements QString mixUserJid; QString mixUserNick; + + // XEP-0283: Moved + QString oldJid; }; QXmppPresencePrivate::QXmppPresencePrivate() @@ -348,6 +352,26 @@ void QXmppPresence::setMucSupported(bool supported) d->mucSupported = supported; } +/// +/// Returns the \xep{0283, Moved} user's old jid. +/// +/// \since QXmpp 1.9 +/// +QString QXmppPresence::oldJid() const +{ + return d->oldJid; +} + +/// +/// Sets the \xep{0283, Moved} user's old jid. +/// +/// \since QXmpp 1.9 +/// +void QXmppPresence::setOldJid(const QString &oldJid) +{ + d->oldJid = oldJid; +} + /// /// Returns when the last user interaction with the client took place. See /// \xep{0319}: Last User Interaction in Presence for details. @@ -483,6 +507,9 @@ void QXmppPresence::parseExtension(const QDomElement &element, QXmppElementList content.parse(contentElement); d->mujiContents.append(content); } + // XEP-0283: Moved + } else if (element.tagName() == u"moved" && element.namespaceURI() == ns_moved) { + d->oldJid = element.firstChildElement(u"old-jid"_s).text(); // XEP-0319: Last User Interaction in Presence } else if (element.tagName() == u"idle" && element.namespaceURI() == ns_idle) { if (element.hasAttribute(u"since"_s)) { @@ -584,6 +611,14 @@ void QXmppPresence::toXml(QXmlStreamWriter *xmlWriter) const xmlWriter->writeEndElement(); } + // XEP-0283: Moved + if (!d->oldJid.isEmpty()) { + xmlWriter->writeStartElement(QSL65("moved")); + xmlWriter->writeDefaultNamespace(ns_moved.toString()); + writeXmlTextElement(xmlWriter, u"old-jid", d->oldJid); + xmlWriter->writeEndElement(); + } + // XEP-0319: Last User Interaction in Presence if (!d->lastUserInteraction.isNull() && d->lastUserInteraction.isValid()) { xmlWriter->writeStartElement(QSL65("idle")); diff --git a/src/base/QXmppPresence.h b/src/base/QXmppPresence.h index 0086c26a9..a4619acad 100644 --- a/src/base/QXmppPresence.h +++ b/src/base/QXmppPresence.h @@ -1,5 +1,6 @@ // SPDX-FileCopyrightText: 2009 Manjeet Dahiya // SPDX-FileCopyrightText: 2022 Melvin Keskin +// SPDX-FileCopyrightText: 2024 Filipe Azevedo // // SPDX-License-Identifier: LGPL-2.1-or-later @@ -115,6 +116,10 @@ class QXMPP_EXPORT QXmppPresence : public QXmppStanza QVector mujiContents() const; void setMujiContents(const QVector &mujiContents); + // XEP-0283: Moved + QString oldJid() const; + void setOldJid(const QString &oldJid); + // XEP-0319: Last User Interaction in Presence QDateTime lastUserInteraction() const; void setLastUserInteraction(const QDateTime &); From 9c498213901fe49ef396c6a6a2fcf9d76ef008de Mon Sep 17 00:00:00 2001 From: Filipe Azevedo Date: Fri, 6 Sep 2024 16:56:50 +0200 Subject: [PATCH 3/5] XEP 0283: Moved - Introduce QXmppMovedItem and QXmppMovedManager This will be used to handle publishing moved statement later and allow to handle moved users. --- doc/doap.xml | 8 + src/CMakeLists.txt | 2 + src/base/QXmppConstants_p.h | 2 + src/client/QXmppMovedItem_p.h | 30 ++ src/client/QXmppMovedManager.cpp | 300 +++++++++++++++++ src/client/QXmppMovedManager.h | 52 +++ src/client/QXmppRosterManager.cpp | 80 ++++- src/client/QXmppRosterManager.h | 6 +- tests/CMakeLists.txt | 1 + .../tst_qxmppmovedmanager.cpp | 307 ++++++++++++++++++ 10 files changed, 775 insertions(+), 13 deletions(-) create mode 100644 src/client/QXmppMovedItem_p.h create mode 100644 src/client/QXmppMovedManager.cpp create mode 100644 src/client/QXmppMovedManager.h create mode 100644 tests/qxmppmovedmanager/tst_qxmppmovedmanager.cpp diff --git a/doc/doap.xml b/doc/doap.xml index 7302b9f65..4494eb326 100644 --- a/doc/doap.xml +++ b/doc/doap.xml @@ -432,6 +432,14 @@ SPDX-License-Identifier: CC0-1.0 1.0 + + + + complete + 0.2.0 + 1.8 + + diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 82e645634..6617e8b96 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -130,6 +130,7 @@ set(INSTALL_HEADER_FILES client/QXmppMessageHandler.h client/QXmppMessageReceiptManager.h client/QXmppMixManager.h + client/QXmppMovedManager.h client/QXmppMucManager.h client/QXmppOutgoingClient.h client/QXmppRegistrationManager.h @@ -267,6 +268,7 @@ set(SOURCE_FILES client/QXmppMamManager.cpp client/QXmppMessageReceiptManager.cpp client/QXmppMixManager.cpp + client/QXmppMovedManager.cpp client/QXmppMucManager.cpp client/QXmppOutgoingClient.cpp client/QXmppRosterManager.cpp diff --git a/src/base/QXmppConstants_p.h b/src/base/QXmppConstants_p.h index 276f61b49..2cf0c9f3b 100644 --- a/src/base/QXmppConstants_p.h +++ b/src/base/QXmppConstants_p.h @@ -171,6 +171,8 @@ inline constexpr QStringView ns_thumbs = u"urn:xmpp:thumbs:1"; inline constexpr QStringView ns_muji = u"urn:xmpp:jingle:muji:0"; // XEP-0280: Message Carbons inline constexpr QStringView ns_carbons = u"urn:xmpp:carbons:2"; +// XEP-0283: Moved +inline constexpr QStringView ns_moved = u"urn:xmpp:moved:1"; // XEP-0293: Jingle RTP Feedback Negotiation inline constexpr QStringView ns_jingle_rtp_feedback_negotiation = u"urn:xmpp:jingle:apps:rtp:rtcp-fb:0"; // XEP-0294: Jingle RTP Header Extensions Negotiation diff --git a/src/client/QXmppMovedItem_p.h b/src/client/QXmppMovedItem_p.h new file mode 100644 index 000000000..c53e16144 --- /dev/null +++ b/src/client/QXmppMovedItem_p.h @@ -0,0 +1,30 @@ +// SPDX-FileCopyrightText: 2024 Filipe Azevedo +// +// SPDX-License-Identifier: LGPL-2.1-or-later + +#ifndef QXMPPMOVEDITEM_P_H +#define QXMPPMOVEDITEM_P_H + +#include + +class QXmppMovedItem : public QXmppPubSubBaseItem +{ +public: + QXmppMovedItem(const QString &newJid = {}); + + QString newJid() const { return m_newJid; } + void setNewJid(const QString &newJid) { m_newJid = newJid; } + + static bool isItem(const QDomElement &itemElement); + +protected: + /// \cond + void parsePayload(const QDomElement &payloadElement) override; + void serializePayload(QXmlStreamWriter *writer) const override; + /// \endcond + +private: + QString m_newJid; +}; + +#endif // QXMPPMOVEDITEM_P_H diff --git a/src/client/QXmppMovedManager.cpp b/src/client/QXmppMovedManager.cpp new file mode 100644 index 000000000..a20f12fc9 --- /dev/null +++ b/src/client/QXmppMovedManager.cpp @@ -0,0 +1,300 @@ +// SPDX-FileCopyrightText: 2024 Filipe Azevedo +// +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "QXmppMovedManager.h" + +#include "QXmppConstants_p.h" +#include "QXmppDiscoveryIq.h" +#include "QXmppDiscoveryManager.h" +#include "QXmppPubSubManager.h" +#include "QXmppTask.h" +#include "QXmppUri.h" +#include "QXmppUtils_p.h" +#include "QXmppUtils.h" +#include "QXmppMovedItem_p.h" + +#include "StringLiterals.h" + +#include + +using namespace QXmpp; +using namespace QXmpp::Private; + +QXmppMovedItem::QXmppMovedItem(const QString &newJid) + : m_newJid(newJid) +{ + setId(QXmppPubSubManager::standardItemIdToString(QXmppPubSubManager::Current)); +} + +/// +/// Returns true if the given DOM element is a valid \xep{0283, Moved} item. +/// +bool QXmppMovedItem::isItem(const QDomElement &itemElement) +{ + return QXmppPubSubBaseItem::isItem(itemElement, [](const QDomElement &payload) { + if (payload.tagName() != u"moved" || payload.namespaceURI() != ns_moved) { + return false; + } + return payload.firstChildElement().tagName() == u"new-jid"; + }); +} + +void QXmppMovedItem::parsePayload(const QDomElement &payloadElement) +{ + m_newJid = payloadElement.firstChildElement(u"new-jid"_s).text(); +} + +void QXmppMovedItem::serializePayload(QXmlStreamWriter *writer) const +{ + if (m_newJid.isEmpty()) { + return; + } + + writer->writeStartElement(QSL65("moved")); + writer->writeDefaultNamespace(ns_moved.toString()); + writer->writeTextElement(QSL65("new-jid"), m_newJid); + writer->writeEndElement(); +} + +class QXmppMovedManagerPrivate +{ +public: + QXmppDiscoveryManager *discoveryManager = nullptr; + bool supportedByServer = false; +}; + +/// +/// \class QXmppMovedManager +/// +/// This class manages user account moving as specified in \xep{0283, Moved} +/// +/// In order to use this manager, make sure to add all managers needed by this manager: +/// \code +/// client->addNewExtension(); +/// client->addNewExtension(); +/// \endcode +/// +/// Afterwards, you need to add this manager to the client: +/// \code +/// auto *manager = client->addNewExtension(); +/// \endcode +/// +/// If you want to publish a moved statement use the publishStatement call with the old account: +/// \code +/// manager->publishStatement("new@example.org"); +/// \endcode +/// +/// Once you published your statement, you then need to subscribe to your old contacts with the new account: +/// \code +/// manager->notifyContact("contact@xmpp.example", "old@example.org", "Hey, I moved my account, please accept me."); +/// \endcode +/// +/// When a contact receive a subscription request from a moved user he needs to verify the authenticity of the request. +/// The QXmppRosterManager handle it on its own if the client has the QXmppMovedManager extension available. +/// The request will be ignored entirely if the old jid incoming subscription is not part of the roster with a 'from' or 'both' type. +/// In case of the authenticity can't be established the moved element is ignored entirely. Alternatively, if the client +/// does not has QXmppMovedManager support the request message will be changed to introduce a warning message before emitting +/// the subscription{Request}Received signal. +/// +/// \ingroup Managers +/// +/// \since QXmpp 1.9 +/// + +/// +/// Constructs a \xep{0283, Moved} manager. +/// +QXmppMovedManager::QXmppMovedManager() + : d(new QXmppMovedManagerPrivate()) +{ +} + +QXmppMovedManager::~QXmppMovedManager() = default; + +QStringList QXmppMovedManager::discoveryFeatures() const +{ + return { ns_moved.toString() }; +} + +/// +/// \property QXmppMovedManager::supportedByServer +/// +/// \see QXmppMovedManager::supportedByServer() +/// + +/// +/// Returns whether the own server supports \xep{0283, Moved} feature. +/// +/// \return whether \xep{0283, Moved} feature is supported +/// +bool QXmppMovedManager::supportedByServer() const +{ + return d->supportedByServer; +} + +/// +/// \fn QXmppMovedManager::supportedByServerChanged() +/// +/// Emitted when the server enabled or disabled support for \xep{0283, Moved}. +/// + +/// +/// Publish a moved statement. +/// +/// \param newBareJid JID of the new account +/// +/// \return the result of the action +/// +QXmppTask QXmppMovedManager::publishStatement(const QString &newBareJid) +{ + return chainSuccess(client()->findExtension()->publishOwnPepItem(ns_moved.toString(), QXmppMovedItem { newBareJid }), this); +} + +/// +/// Verify a user moved statement. +/// +/// \param oldBareJid JID of the old account to check statement +/// \param newBareJid JID of the new account that send the subscription request +/// +/// \return the result of the action +/// +QXmppTask QXmppMovedManager::verifyStatement(const QString &oldBareJid, const QString &newBareJid) +{ + return chain( + client()->findExtension()->requestItem(oldBareJid, ns_moved.toString(), u"current"_s), + this, + [=, this](QXmppPubSubManager::ItemResult &&result) { + return std::visit( + overloaded { + [newBareJid, this](QXmppMovedItem item) -> Result { + return movedJidsMatch(newBareJid, item.newJid()); + }, + [newBareJid, this](QXmppError err) -> Result { + // As a special case, if the attempt to retrieve the moved statement results in an error with the condition + // as defined in RFC 6120, and that element contains a valid XMPP URI (e.g. xmpp:user@example.com), then the + // error response MUST be handled equivalent to a statement containing a element with the JID + // provided in the URI (e.g. user@example.com). + if (auto e = err.value()) { + const auto newJid = [&e]() -> QString { + if (e->condition() != QXmppStanza::Error::Gone) { + return {}; + } + + const auto result = QXmppUri::fromString(e->redirectionUri()); + + if (std::holds_alternative(result)) { + return std::get(result).jid(); + } + + return {}; + }(); + + if (!newJid.isEmpty()) { + return movedJidsMatch(newBareJid, newJid); + } + } + + return err; + }, + }, + std::move(result)); + }); +} + +/// +/// Notifies a contact that the user has moved to another account. +/// +/// \param contactBareJid JID of the contact to send the subscription request +/// \param oldBareJid JID of the old account we moved from +/// \param sensitive If true the notification is sent sensitively +/// \param reason The reason of the move +/// +/// \return the result of the action +/// +QXmppTask QXmppMovedManager::notifyContact(const QString &contactBareJid, const QString &oldBareJid, bool sensitive, const QString &reason) +{ + QXmppPresence packet; + packet.setTo(QXmppUtils::jidToBareJid(contactBareJid)); + packet.setType(QXmppPresence::Subscribe); + packet.setStatusText(reason); + packet.setOldJid(oldBareJid); + return sensitive ? client()->sendSensitive(std::move(packet)) : client()->send(std::move(packet)); +} + +/// \cond +void QXmppMovedManager::onRegistered(QXmppClient *client) +{ + connect(client, &QXmppClient::connected, this, [this, client]() { + if (client->streamManagementState() == QXmppClient::NewStream) { + resetCachedData(); + } + }); + + d->discoveryManager = client->findExtension(); + Q_ASSERT_X(d->discoveryManager, "QXmppMovedManager", "QXmppDiscoveryManager is missing"); + + connect(d->discoveryManager, &QXmppDiscoveryManager::infoReceived, this, &QXmppMovedManager::handleDiscoInfo); + + Q_ASSERT_X(client->findExtension(), "QXmppMovedManager", "QXmppPubSubManager is missing"); +} + +void QXmppMovedManager::onUnregistered(QXmppClient *client) +{ + disconnect(d->discoveryManager, &QXmppDiscoveryManager::infoReceived, this, &QXmppMovedManager::handleDiscoInfo); + resetCachedData(); + disconnect(client, &QXmppClient::connected, this, nullptr); +} +/// \endcond + +/// +/// Handles incoming service infos specified by \xep{0030, Service Discovery}. +/// +/// \param iq received Service Discovery IQ stanza +/// +void QXmppMovedManager::handleDiscoInfo(const QXmppDiscoveryIq &iq) +{ + // Check the server's functionality to support MOVED feature. + if (iq.from().isEmpty() || iq.from() == client()->configuration().domain()) { + // Check whether MOVED is supported. + setSupportedByServer(iq.features().contains(ns_moved)); + } +} + +/// +/// Ensures that both JIDs match. +/// +/// \param newBareJid JID of the contact that sent the subscription request +/// \param pepBareJid JID of the new account as fetched from the old account statement +/// +/// \return the result of the action +/// +QXmppMovedManager::Result QXmppMovedManager::movedJidsMatch(const QString &newBareJid, const QString &pepBareJid) const +{ + if (newBareJid == pepBareJid) { + return Success(); + } + + return QXmppError { u"The JID does not match the user's statement."_s, {} }; +} + +/// +/// Sets whether the own server supports \xep{0283, Moved}. +/// +/// \param supportedByServer whether \xep{0283, Moved} is supported by the server +/// +void QXmppMovedManager::setSupportedByServer(bool supportedByServer) +{ + if (d->supportedByServer != supportedByServer) { + d->supportedByServer = supportedByServer; + Q_EMIT supportedByServerChanged(); + } +} + +/// +/// Resets the cached data. +/// +void QXmppMovedManager::resetCachedData() +{ + setSupportedByServer(false); +} diff --git a/src/client/QXmppMovedManager.h b/src/client/QXmppMovedManager.h new file mode 100644 index 000000000..e7390a3b5 --- /dev/null +++ b/src/client/QXmppMovedManager.h @@ -0,0 +1,52 @@ +// SPDX-FileCopyrightText: 2024 Filipe Azevedo +// +// SPDX-License-Identifier: LGPL-2.1-or-later + +#ifndef QXMPPMOVEDMANAGER_H +#define QXMPPMOVEDMANAGER_H + +#include "QXmppClient.h" +#include "QXmppClientExtension.h" + +class QXmppMovedManagerPrivate; + +class QXMPP_EXPORT QXmppMovedManager : public QXmppClientExtension +{ + Q_OBJECT + Q_PROPERTY(bool supportedByServer READ supportedByServer NOTIFY supportedByServerChanged) + +public: + using Result = std::variant; + + explicit QXmppMovedManager(); + ~QXmppMovedManager() override; + + QStringList discoveryFeatures() const override; + + bool supportedByServer() const; + Q_SIGNAL void supportedByServerChanged(); + + QXmppTask publishStatement(const QString &newBareJid); + QXmppTask verifyStatement(const QString &oldBareJid, const QString &newBareJid); + + QXmppTask notifyContact(const QString &contactBareJid, const QString &oldBareJid, bool sensitive = true, const QString &reason = {}); + +protected: + /// \cond + void onRegistered(QXmppClient *client) override; + void onUnregistered(QXmppClient *client) override; + /// \endcond + +private: + void handleDiscoInfo(const QXmppDiscoveryIq &iq); + QXmppClient::EmptyResult movedJidsMatch(const QString &newBareJid, const QString &pepBareJid) const; + + void setSupportedByServer(bool supportedByServer); + void resetCachedData(); + + const std::unique_ptr d; + + friend class tst_QXmppMovedManager; +}; + +#endif // QXMPPMOVEDMANAGER_H diff --git a/src/client/QXmppRosterManager.cpp b/src/client/QXmppRosterManager.cpp index 0083acf73..c5bbca917 100644 --- a/src/client/QXmppRosterManager.cpp +++ b/src/client/QXmppRosterManager.cpp @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: 2010 Manjeet Dahiya // SPDX-FileCopyrightText: 2010 Jeremy Lainé // SPDX-FileCopyrightText: 2020 Melvin Keskin +// SPDX-FileCopyrightText: 2024 Filipe Azevedo // // SPDX-License-Identifier: LGPL-2.1-or-later @@ -10,6 +11,7 @@ #include "QXmppClient.h" #include "QXmppConstants_p.h" #include "QXmppFutureUtils_p.h" +#include "QXmppMovedManager.h" #include "QXmppPresence.h" #include "QXmppRosterIq.h" #include "QXmppUtils.h" @@ -70,7 +72,9 @@ static void serializeRosterData(const RosterData &d, QXmlStreamWriter &writer) /// by calling refuseSubscription(). /// /// \note If QXmppConfiguration::autoAcceptSubscriptions() is set to true, this -/// signal will not be emitted. +/// signal will not be emitted. This is only valid for non moved or verified moved subscription. +/// If the subscription is a moved one and the roster's old-jid's subscription is not either from +/// or both then QXmppConfiguration::autoAcceptSubscriptions() is ignored. /// /// \param subscriberBareJid bare JID that wants to subscribe to the user's presence /// \param presence presence stanza containing the reason / message (presence.statusText()) @@ -240,8 +244,10 @@ bool QXmppRosterManager::handleStanza(const QDomElement &element) } /// \endcond -void QXmppRosterManager::_q_presenceReceived(const QXmppPresence &presence) +void QXmppRosterManager::_q_presenceReceived(const QXmppPresence &presence_) { + auto presence = presence_; + const auto jid = presence.from(); const auto bareJid = QXmppUtils::jidToBareJid(jid); const auto resource = QXmppUtils::jidToResource(jid); @@ -259,23 +265,73 @@ void QXmppRosterManager::_q_presenceReceived(const QXmppPresence &presence) d->presences[bareJid].remove(resource); Q_EMIT presenceChanged(bareJid, resource); break; - case QXmppPresence::Subscribe: - if (client()->configuration().autoAcceptSubscriptions()) { - // accept subscription request - acceptSubscription(bareJid); - - // ask for reciprocal subscription - subscribe(bareJid); - } else { - Q_EMIT subscriptionReceived(bareJid); - Q_EMIT subscriptionRequestReceived(bareJid, presence); + case QXmppPresence::Subscribe: { + bool autoAccept = client()->configuration().autoAcceptSubscriptions(); + + // XEP-0283: Moved + if (!presence.oldJid().isEmpty()) { + const auto entry = getRosterEntry(presence.oldJid()); + + switch (entry.subscriptionType()) { + case QXmppRosterIq::Item::From: + case QXmppRosterIq::Item::Both: + break; + case QXmppRosterIq::Item::None: + case QXmppRosterIq::Item::To: + case QXmppRosterIq::Item::Remove: + case QXmppRosterIq::Item::NotSet: + // We need to be either from or both, else ignore the moved element + presence.setOldJid({}); + autoAccept = false; + break; + } + + if (auto movedManager = client()->findExtension(); movedManager) { + if (!presence.oldJid().isEmpty()) { + movedManager->verifyStatement(presence.oldJid(), QXmppUtils::jidToBareJid(presence.from())).then(this, [=, this](QXmppClient::EmptyResult &&result) mutable { + if (!std::holds_alternative(result)) { + // Ignore the moved element entirely if we can't verify authenticity + presence.setOldJid({}); + autoAccept = false; + } + + handlePresenceReceived(bareJid, presence, autoAccept); + }); + + return; + } + } else { + if (!presence.oldJid().isEmpty()) { + // Fix up status text presented to user and disable auto accept mode + presence.setStatusText(tr("You received a contact *moved* subscription from %1, but your client does not support it, thus verification for legitimity can not be done. Please carefully review it before accepting the request.\\n").arg(presence.oldJid()) + presence.statusText()); + presence.setOldJid({}); + autoAccept = false; + } + } } + + handlePresenceReceived(bareJid, presence, autoAccept); break; + } default: break; } } +void QXmppRosterManager::handlePresenceReceived(const QString &bareJid, const QXmppPresence &presence, bool autoAccept) +{ + if (autoAccept) { + // accept subscription request + acceptSubscription(bareJid); + + // ask for reciprocal subscription + subscribe(bareJid); + } else { + Q_EMIT subscriptionReceived(bareJid); + Q_EMIT subscriptionRequestReceived(bareJid, presence); + } +} + QXmppTask QXmppRosterManager::requestRoster() { QXmppRosterIq iq; diff --git a/src/client/QXmppRosterManager.h b/src/client/QXmppRosterManager.h index 3eedca292..e9c675664 100644 --- a/src/client/QXmppRosterManager.h +++ b/src/client/QXmppRosterManager.h @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: 2010 Manjeet Dahiya // SPDX-FileCopyrightText: 2010 Jeremy Lainé // SPDX-FileCopyrightText: 2021 Melvin Keskin +// SPDX-FileCopyrightText: 2024 Filipe Azevedo // // SPDX-License-Identifier: LGPL-2.1-or-later @@ -112,7 +113,9 @@ public Q_SLOTS: /// by calling refuseSubscription(). /// /// \note If you set QXmppConfiguration::autoAcceptSubscriptions() to true, this - /// signal will not be emitted. + /// signal will not be emitted. This is only valid for non moved or verified moved subscription. + /// If the subscription is a moved one and the roster's old-jid's subscription is not either from + /// or both then QXmppConfiguration::autoAcceptSubscriptions() is ignored. void subscriptionReceived(const QString &bareJid); void subscriptionRequestReceived(const QString &subscriberBareJid, const QXmppPresence &presence); @@ -140,6 +143,7 @@ private Q_SLOTS: private: using RosterResult = std::variant; + void handlePresenceReceived(const QString &bareJid, const QXmppPresence &presence, bool autoAccept); QXmppTask requestRoster(); const std::unique_ptr d; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ab646f6cf..0ba66a7c2 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -57,6 +57,7 @@ add_simple_test(qxmppmessage) add_simple_test(qxmppmessagereaction) add_simple_test(qxmppmessagereceiptmanager) add_simple_test(qxmppmixiq) +add_simple_test(qxmppmovedmanager TestClient.h) add_simple_test(qxmppnonsaslauthiq) add_simple_test(qxmpppushenableiq) add_simple_test(qxmpppresence) diff --git a/tests/qxmppmovedmanager/tst_qxmppmovedmanager.cpp b/tests/qxmppmovedmanager/tst_qxmppmovedmanager.cpp new file mode 100644 index 000000000..a8775dedc --- /dev/null +++ b/tests/qxmppmovedmanager/tst_qxmppmovedmanager.cpp @@ -0,0 +1,307 @@ +// SPDX-FileCopyrightText: 2024 Filipe Azevedo +// +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "QXmppDiscoveryManager.h" +#include "QXmppMovedManager.h" +#include "QXmppConstants_p.h" +#include "QXmppPubSubManager.h" +#ifdef BUILD_INTERNAL_TESTS +#include "QXmppMovedItem_p.h" +#endif + +#include "TestClient.h" + +struct Tester { + Tester() + { + client.addNewExtension(); + client.addNewExtension(); + manager = client.addNewExtension(); + } + + Tester(const QString &jid) + : Tester() + { + client.configuration().setJid(jid); + } + + TestClient client; + QXmppMovedManager *manager; +}; + +class tst_QXmppMovedManager : public QObject +{ + Q_OBJECT + +private: +#ifdef BUILD_INTERNAL_TESTS + Q_SLOT void testMovedItem(); +#endif + Q_SLOT void testMovedPresence(); + Q_SLOT void testDiscoveryFeatures(); + Q_SLOT void testSupportedByServer(); + Q_SLOT void testResetCachedData(); + Q_SLOT void testHandleDiscoInfo(); + Q_SLOT void testOnRegistered(); + Q_SLOT void testOnUnregistered(); + Q_SLOT void testPublishMoved(); + Q_SLOT void testVerifyMoved(); + Q_SLOT void testNotify(); + + template + void testError(QXmppTask &task, TestClient &client, const QString &id, const QString &from); +}; + +#ifdef BUILD_INTERNAL_TESTS +void tst_QXmppMovedManager::testMovedItem() +{ + const auto expected = u"new@shakespeare.example"_s; + const QDomElement expectedElement = xmlToDom(expected); + + { + QXmppMovedItem packet; + packet.setNewJid(u"new@shakespeare.example"_s); + + QCOMPARE(packetToXml(packet), expected); + } + + { + QXmppMovedItem packet; + packet.parse(expectedElement); + + QVERIFY(!packet.newJid().isEmpty()); + } +} +#endif + +void tst_QXmppMovedManager::testMovedPresence() +{ + const auto expected = + u"" + "old@shakespeare.example" + ""_s; + const QDomElement expectedElement = xmlToDom(expected); + + { + QXmppPresence packet; + packet.setTo(u"contact@shakespeare.example"_s); + packet.setType(QXmppPresence::Subscribe); + packet.setOldJid(u"old@shakespeare.example"_s); + + QCOMPARE(packetToXml(packet), expected); + } + + { + QXmppPresence packet; + packet.parse(expectedElement); + + QVERIFY(!packet.oldJid().isEmpty()); + } +} + +void tst_QXmppMovedManager::testDiscoveryFeatures() +{ + QXmppMovedManager manager; + + QCOMPARE(manager.discoveryFeatures(), QStringList { ns_moved.toString() }); +} + +void tst_QXmppMovedManager::testSupportedByServer() +{ + QXmppMovedManager manager; + QSignalSpy spy(&manager, &QXmppMovedManager::supportedByServerChanged); + + QVERIFY(!manager.supportedByServer()); + + manager.setSupportedByServer(true); + + QVERIFY(manager.supportedByServer()); + QCOMPARE(spy.size(), 1); +} + +void tst_QXmppMovedManager::testResetCachedData() +{ + QXmppMovedManager manager; + + manager.setSupportedByServer(true); + manager.resetCachedData(); + + QVERIFY(!manager.supportedByServer()); +} + +void tst_QXmppMovedManager::testHandleDiscoInfo() +{ + auto [client, manager] = Tester(u"hag66@shakespeare.example"_s); + + QXmppDiscoveryIq iq; + iq.setFeatures({ ns_moved.toString() }); + + manager->handleDiscoInfo(iq); + + QVERIFY(manager->supportedByServer()); + + iq.setFeatures({}); + + manager->handleDiscoInfo(iq); + + QVERIFY(!manager->supportedByServer()); +} + +void tst_QXmppMovedManager::testOnRegistered() +{ + TestClient client; + QXmppMovedManager manager; + + client.addNewExtension(); + client.addNewExtension(); + client.configuration().setJid(u"hag66@shakespeare.example"_s); + client.addExtension(&manager); + + manager.setSupportedByServer(true); + + client.setStreamManagementState(QXmppClient::NewStream); + Q_EMIT client.connected(); + + QVERIFY(!manager.supportedByServer()); + + QXmppDiscoveryIq iq; + iq.setFeatures({ ns_moved.toString() }); + Q_EMIT manager.client()->findExtension()->infoReceived(iq); + + QVERIFY(manager.supportedByServer()); +} + +void tst_QXmppMovedManager::testOnUnregistered() +{ + QXmppClient client; + QXmppMovedManager manager; + + client.addNewExtension(); + client.addNewExtension(); + client.configuration().setJid(u"hag66@shakespeare.example"_s); + client.addExtension(&manager); + + manager.setSupportedByServer(true); + manager.onUnregistered(&client); + + QXmppDiscoveryIq iq; + iq.setFeatures({ ns_moved.toString() }); + Q_EMIT manager.client()->findExtension()->infoReceived(iq); + + QVERIFY(!manager.supportedByServer()); + + manager.setSupportedByServer(true); + Q_EMIT client.connected(); + + QVERIFY(manager.supportedByServer()); +} + +void tst_QXmppMovedManager::testPublishMoved() +{ + auto tester = Tester(u"old@shakespeare.example"_s); + auto &client = tester.client; + auto manager = tester.manager; + + auto call = [manager]() { + return manager->publishStatement(u"moved@shakespeare.example"_s); + }; + + auto task = call(); + + client.expect(u"" + "" + "" + "" + "" + "moved@shakespeare.example" + "" + "" + "" + "" + ""_s); + client.inject(u"" + "" + "" + "" + "" + "" + ""_s); + + expectFutureVariant(task); + + testError(task = call(), client, u"qxmpp1"_s, u"old@shakespeare.example"_s); +} + +void tst_QXmppMovedManager::testVerifyMoved() +{ + auto tester = Tester(u"contact@shakespeare.example"_s); + auto &client = tester.client; + auto manager = tester.manager; + + auto call = [manager]() { + return manager->verifyStatement(u"old@shakespeare.example"_s, u"moved@shakespeare.example"_s); + }; + + auto task = call(); + + client.expect(u"" + "" + "" + "" + "" + "" + ""_s); + client.inject(u"" + "" + "" + "" + "" + "moved@shakespeare.example" + "" + "" + "" + "" + ""_s); + + expectFutureVariant(task); + + testError(task = call(), client, u"qxmpp1"_s, u"old@shakespeare.example"_s); +} + +void tst_QXmppMovedManager::testNotify() +{ + auto tester = Tester(u"moved@shakespeare.example"_s); + auto &client = tester.client; + auto manager = tester.manager; + + auto call = [manager]() { + return manager->notifyContact(u"contact@shakespeare.example"_s, u"old@shakespeare.example"_s, true, u"I moved."_s); + }; + + auto task = call(); + + client.expect(u"" + "I moved." + "" + "old@shakespeare.example" + "" + ""_s); +} + +template +void tst_QXmppMovedManager::testError(QXmppTask &task, TestClient &client, const QString &id, const QString &from) +{ + client.ignore(); + client.inject(u"" + "" + "" + "" + ""_s + .arg(id, from)); + + expectFutureVariant(task); +} + +QTEST_MAIN(tst_QXmppMovedManager) +#include "tst_qxmppmovedmanager.moc" From 567568f8adb56e60c82b6e67035417cdb7b2ef8a Mon Sep 17 00:00:00 2001 From: Linus Jahn Date: Sun, 8 Sep 2024 17:00:04 +0200 Subject: [PATCH 4/5] RosterManager: Restructure verification of moved elements in sub reqs --- src/client/QXmppMovedManager.cpp | 43 +++++++++++++++++++- src/client/QXmppMovedManager.h | 2 + src/client/QXmppRosterManager.cpp | 67 +++++++++---------------------- src/client/QXmppRosterManager.h | 3 +- 4 files changed, 63 insertions(+), 52 deletions(-) diff --git a/src/client/QXmppMovedManager.cpp b/src/client/QXmppMovedManager.cpp index a20f12fc9..bd3516011 100644 --- a/src/client/QXmppMovedManager.cpp +++ b/src/client/QXmppMovedManager.cpp @@ -7,12 +7,13 @@ #include "QXmppConstants_p.h" #include "QXmppDiscoveryIq.h" #include "QXmppDiscoveryManager.h" +#include "QXmppMovedItem_p.h" #include "QXmppPubSubManager.h" +#include "QXmppRosterManager.h" #include "QXmppTask.h" #include "QXmppUri.h" -#include "QXmppUtils_p.h" #include "QXmppUtils.h" -#include "QXmppMovedItem_p.h" +#include "QXmppUtils_p.h" #include "StringLiterals.h" @@ -247,6 +248,44 @@ void QXmppMovedManager::onUnregistered(QXmppClient *client) } /// \endcond +/// +/// Checks for moved elements in incoming subscription requests and verifies them. +/// +/// This requires the QXmppRosterManager to be registered with the client. +/// +/// \returns a task for the verification result if the subscription request contains a moved +/// element with an 'old-jid' that is already in the account's roster. +/// +std::optional> QXmppMovedManager::handleSubscriptionRequest(const QXmppPresence &presence) +{ + // check for moved element + if (presence.oldJid().isEmpty()) { + return {}; + } + + // find roster manager + auto *rosterManager = client()->findExtension(); + Q_ASSERT(rosterManager); + + // check subscription state of old-jid + const auto entry = rosterManager->getRosterEntry(presence.oldJid()); + + switch (entry.subscriptionType()) { + case QXmppRosterIq::Item::From: + case QXmppRosterIq::Item::Both: + break; + default: + // The subscription state of the old JID needs to be either from or both, else ignore + // the moved element + return {}; + } + + // return verification result + return chain(verifyStatement(presence.oldJid(), QXmppUtils::jidToBareJid(presence.from())), this, [this](Result &&result) mutable { + return std::holds_alternative(result); + }); +} + /// /// Handles incoming service infos specified by \xep{0030, Service Discovery}. /// diff --git a/src/client/QXmppMovedManager.h b/src/client/QXmppMovedManager.h index e7390a3b5..3734ca032 100644 --- a/src/client/QXmppMovedManager.h +++ b/src/client/QXmppMovedManager.h @@ -38,6 +38,7 @@ class QXMPP_EXPORT QXmppMovedManager : public QXmppClientExtension /// \endcond private: + std::optional> handleSubscriptionRequest(const QXmppPresence &presence); void handleDiscoInfo(const QXmppDiscoveryIq &iq); QXmppClient::EmptyResult movedJidsMatch(const QString &newBareJid, const QString &pepBareJid) const; @@ -46,6 +47,7 @@ class QXMPP_EXPORT QXmppMovedManager : public QXmppClientExtension const std::unique_ptr d; + friend class QXmppRosterManager; friend class tst_QXmppMovedManager; }; diff --git a/src/client/QXmppRosterManager.cpp b/src/client/QXmppRosterManager.cpp index c5bbca917..2b0c3cb67 100644 --- a/src/client/QXmppRosterManager.cpp +++ b/src/client/QXmppRosterManager.cpp @@ -71,10 +71,8 @@ static void serializeRosterData(const RosterData &d, QXmlStreamWriter &writer) /// The user can either accept the request by calling acceptSubscription() or refuse it /// by calling refuseSubscription(). /// -/// \note If QXmppConfiguration::autoAcceptSubscriptions() is set to true, this -/// signal will not be emitted. This is only valid for non moved or verified moved subscription. -/// If the subscription is a moved one and the roster's old-jid's subscription is not either from -/// or both then QXmppConfiguration::autoAcceptSubscriptions() is ignored. +/// \note If QXmppConfiguration::autoAcceptSubscriptions() is set to true or the subscription +/// request is automatically accepted by the QXmppMovedManager, this signal will not be emitted. /// /// \param subscriberBareJid bare JID that wants to subscribe to the user's presence /// \param presence presence stanza containing the reason / message (presence.statusText()) @@ -244,10 +242,8 @@ bool QXmppRosterManager::handleStanza(const QDomElement &element) } /// \endcond -void QXmppRosterManager::_q_presenceReceived(const QXmppPresence &presence_) +void QXmppRosterManager::_q_presenceReceived(const QXmppPresence &presence) { - auto presence = presence_; - const auto jid = presence.from(); const auto bareJid = QXmppUtils::jidToBareJid(jid); const auto resource = QXmppUtils::jidToResource(jid); @@ -266,51 +262,23 @@ void QXmppRosterManager::_q_presenceReceived(const QXmppPresence &presence_) Q_EMIT presenceChanged(bareJid, resource); break; case QXmppPresence::Subscribe: { - bool autoAccept = client()->configuration().autoAcceptSubscriptions(); - - // XEP-0283: Moved - if (!presence.oldJid().isEmpty()) { - const auto entry = getRosterEntry(presence.oldJid()); + // accept all incoming subscription requests if enabled + if (client()->configuration().autoAcceptSubscriptions()) { + handleSubscriptionRequest(bareJid, presence, true); + break; + } - switch (entry.subscriptionType()) { - case QXmppRosterIq::Item::From: - case QXmppRosterIq::Item::Both: - break; - case QXmppRosterIq::Item::None: - case QXmppRosterIq::Item::To: - case QXmppRosterIq::Item::Remove: - case QXmppRosterIq::Item::NotSet: - // We need to be either from or both, else ignore the moved element - presence.setOldJid({}); - autoAccept = false; + // check for XEP-0283: Moved subscription requests and verify them + if (auto *movedManager = client()->findExtension()) { + if (auto verificationTask = movedManager->handleSubscriptionRequest(presence)) { + verificationTask->then(this, [this, presence, bareJid](bool valid) { + handleSubscriptionRequest(bareJid, presence, valid); + }); break; } - - if (auto movedManager = client()->findExtension(); movedManager) { - if (!presence.oldJid().isEmpty()) { - movedManager->verifyStatement(presence.oldJid(), QXmppUtils::jidToBareJid(presence.from())).then(this, [=, this](QXmppClient::EmptyResult &&result) mutable { - if (!std::holds_alternative(result)) { - // Ignore the moved element entirely if we can't verify authenticity - presence.setOldJid({}); - autoAccept = false; - } - - handlePresenceReceived(bareJid, presence, autoAccept); - }); - - return; - } - } else { - if (!presence.oldJid().isEmpty()) { - // Fix up status text presented to user and disable auto accept mode - presence.setStatusText(tr("You received a contact *moved* subscription from %1, but your client does not support it, thus verification for legitimity can not be done. Please carefully review it before accepting the request.\\n").arg(presence.oldJid()) + presence.statusText()); - presence.setOldJid({}); - autoAccept = false; - } - } } - handlePresenceReceived(bareJid, presence, autoAccept); + handleSubscriptionRequest(bareJid, presence, false); break; } default: @@ -318,15 +286,16 @@ void QXmppRosterManager::_q_presenceReceived(const QXmppPresence &presence_) } } -void QXmppRosterManager::handlePresenceReceived(const QString &bareJid, const QXmppPresence &presence, bool autoAccept) +void QXmppRosterManager::handleSubscriptionRequest(const QString &bareJid, const QXmppPresence &presence, bool accept) { - if (autoAccept) { + if (accept) { // accept subscription request acceptSubscription(bareJid); // ask for reciprocal subscription subscribe(bareJid); } else { + // let user decide whether to accept the subscription request Q_EMIT subscriptionReceived(bareJid); Q_EMIT subscriptionRequestReceived(bareJid, presence); } diff --git a/src/client/QXmppRosterManager.h b/src/client/QXmppRosterManager.h index e9c675664..2edce2cd4 100644 --- a/src/client/QXmppRosterManager.h +++ b/src/client/QXmppRosterManager.h @@ -143,7 +143,8 @@ private Q_SLOTS: private: using RosterResult = std::variant; - void handlePresenceReceived(const QString &bareJid, const QXmppPresence &presence, bool autoAccept); + + void handleSubscriptionRequest(const QString &bareJid, const QXmppPresence &presence, bool accept); QXmppTask requestRoster(); const std::unique_ptr d; From 5c718a276f38e6784306d4cb794f8734ebda33a4 Mon Sep 17 00:00:00 2001 From: Filipe Azevedo Date: Tue, 10 Sep 2024 15:54:32 +0200 Subject: [PATCH 5/5] Suggested changes --- src/client/QXmppRosterManager.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/client/QXmppRosterManager.cpp b/src/client/QXmppRosterManager.cpp index 2b0c3cb67..68542b05d 100644 --- a/src/client/QXmppRosterManager.cpp +++ b/src/client/QXmppRosterManager.cpp @@ -262,12 +262,6 @@ void QXmppRosterManager::_q_presenceReceived(const QXmppPresence &presence) Q_EMIT presenceChanged(bareJid, resource); break; case QXmppPresence::Subscribe: { - // accept all incoming subscription requests if enabled - if (client()->configuration().autoAcceptSubscriptions()) { - handleSubscriptionRequest(bareJid, presence, true); - break; - } - // check for XEP-0283: Moved subscription requests and verify them if (auto *movedManager = client()->findExtension()) { if (auto verificationTask = movedManager->handleSubscriptionRequest(presence)) { @@ -278,7 +272,9 @@ void QXmppRosterManager::_q_presenceReceived(const QXmppPresence &presence) } } - handleSubscriptionRequest(bareJid, presence, false); + // accept all incoming subscription requests if enabled + const bool autoAccept = client()->configuration().autoAcceptSubscriptions(); + handleSubscriptionRequest(bareJid, presence, autoAccept); break; } default: