Merge branch '1.9' into 1.10

Author: lnjX

src/client/QXmppMovedManager.cpp

@@ -253,41 +253,38 @@ void QXmppMovedManager::onUnregistered(QXmppClient *client)
 /// \endcond
 
 ///
-/// Checks for moved elements in incoming subscription requests and verifies them.
+/// Verifies an old JID in a received presence subscription request and removes it if it is invalid.
 ///
 /// 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.
+/// \param presence presence subscription request to be processed containing a non-empty old JID.
 ///
-std::optional<QXmppTask<bool>> QXmppMovedManager::handleSubscriptionRequest(const QXmppPresence &presence)
+/// \returns the processed presence subscription request
+///
+QXmppTask<QXmppPresence> QXmppMovedManager::processSubscriptionRequest(QXmppPresence presence)
 {
-    // check for moved element
-    if (presence.oldJid().isEmpty()) {
-        return {};
-    }
+    Q_ASSERT(!presence.oldJid().isEmpty());
 
-    // find roster manager
     auto *rosterManager = client()->findExtension<QXmppRosterManager>();
     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;
+        return chain<QXmppPresence>(verifyStatement(presence.oldJid(), QXmppUtils::jidToBareJid(presence.from())), this, [this, presence = presence](Result &&result) mutable {
+            if (std::holds_alternative<QXmppError>(result)) {
+                warning(presence.from() + u" sent a presence subscription request with the invalid old JID "_s + presence.oldJid());
+                presence.setOldJid({});
+            }
+
+            return presence;
+        });
     default:
-        // The subscription state of the old JID needs to be either from or both, else ignore
-        // the moved element
-        return {};
+        presence.setOldJid({});
+        return makeReadyTask(std::move(presence));
     }
-
-    // return verification result
-    return chain<bool>(verifyStatement(presence.oldJid(), QXmppUtils::jidToBareJid(presence.from())), this, [this](Result &&result) mutable {
-        return std::holds_alternative<Success>(result);
-    });
 }
 
 ///

src/client/QXmppMovedManager.h

@@ -41,7 +41,7 @@ class QXMPP_EXPORT QXmppMovedManager : public QXmppClientExtension
     /// \endcond
 
 private:
-    std::optional<QXmppTask<bool>> handleSubscriptionRequest(const QXmppPresence &presence);
+    QXmppTask<QXmppPresence> processSubscriptionRequest(QXmppPresence presence);
     void handleDiscoInfo(const QXmppDiscoveryIq &iq);
     Result movedJidsMatch(const QString &newBareJid, const QString &pepBareJid) const;
 

src/client/QXmppRosterManager.cpp

@@ -74,11 +74,16 @@ static void serializeRosterData(const RosterData &d, QXmlStreamWriter &writer)
 /// The user can either accept the request by calling acceptSubscription() or refuse it
 /// by calling refuseSubscription().
 ///
+/// Since *QXmpp 1.10.2* only verified \xep{0283, Moved} old JIDs are passed in \a presence. If
+/// verification fails or the given old JID is not valid, the attribute is cleared in the
+/// QXmppPresence. See \ref rostermanager_moved "above" for more details.
+///
 /// \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())
+/// \param presence presence stanza, e.g. containing the message (presence.statusText()),
+/// \xep{0283, Moved} old JID or other information
 ///
 /// \since QXmpp 1.5
 ///
@@ -265,42 +270,38 @@ 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<QXmppMovedManager>()) {
-            if (auto verificationTask = movedManager->handleSubscriptionRequest(presence)) {
-                verificationTask->then(this, [this, presence, bareJid](bool valid) {
-                    handleSubscriptionRequest(bareJid, presence, valid);
-                });
-                break;
-            }
-        }
-
-        handleSubscriptionRequest(bareJid, presence, false);
+        handleSubscriptionRequest(bareJid, presence);
         break;
     }
     default:
         break;
     }
 }
 
-void QXmppRosterManager::handleSubscriptionRequest(const QString &bareJid, const QXmppPresence &presence, bool accept)
+void QXmppRosterManager::handleSubscriptionRequest(const QString &bareJid, const QXmppPresence &presence)
 {
-    if (accept) {
-        // accept subscription request
-        acceptSubscription(bareJid);
+    auto notifyOnSubscriptionRequest = [this, bareJid](const QXmppPresence &presence) {
+        Q_EMIT subscriptionReceived(bareJid);
+        Q_EMIT subscriptionRequestReceived(bareJid, presence);
+    };
 
-        // ask for reciprocal subscription
+    // Automatically accept all incoming subscription requests if enabled.
+    if (client()->configuration().autoAcceptSubscriptions()) {
+        acceptSubscription(bareJid);
         subscribe(bareJid);
+        return;
+    }
+
+    // check for XEP-0283: Moved subscription requests and verify them
+    if (auto *movedManager = client()->findExtension<QXmppMovedManager>(); movedManager && !presence.oldJid().isEmpty()) {
+        movedManager->processSubscriptionRequest(presence).then(this, [this, notifyOnSubscriptionRequest](QXmppPresence &&presence) mutable {
+            notifyOnSubscriptionRequest(presence);
+        });
     } else {
-        // let user decide whether to accept the subscription request
-        Q_EMIT subscriptionReceived(bareJid);
-        Q_EMIT subscriptionRequestReceived(bareJid, presence);
+        auto safePresence = presence;
+        safePresence.setOldJid({});
+
+        notifyOnSubscriptionRequest(safePresence);
     }
 }
 

src/client/QXmppRosterManager.h

@@ -51,6 +51,33 @@ class QXmppRosterManagerPrivate;
 /// The \c presenceChanged() signal is emitted whenever the presence for a
 /// roster item changes.
 ///
+/// \anchor rostermanager_moved
+/// ## XEP-0283: Moved
+///
+/// \xep{0283, Moved} provides a way to inform other users that an account has moved. This allows a
+/// presence subscription request from a new account to be automatically linked to the old account.
+/// However, this requires verification via a corresponding PubSub node on the old account.
+///
+/// The roster manager automatically checks entries using the QXmppMovedManager. For this to work,
+/// the moved manager must be added to the QXmppClient. If the moved manager is not found or the
+/// specified old JID is incorrect, the entry in the presence subscription request will be removed.
+///
+/// The received and verified presence subscription request is emitted via
+/// subscriptionRequestReceived() and the old JID can be found in QXmppPresence::oldJid().
+/// It is safe to assume that this old JID is valid since QXmpp 1.10.2.
+///
+/// Accepting moved subscription requests automatically is discouraged in
+/// [section 4.3](https://xmpp.org/extensions/xep-0283.html#receive-notification-client) of the
+/// XEP.
+///
+/// ### Behaviour in previous versions
+///
+/// Support in the roster manager and in QXmppPresence was initially added in 1.9.0.
+///
+/// In QXmpp 1.9.0 until 1.9.4 and 1.10.0 until 1.10.1 subscription requests with a Moved element
+/// are verified and automatically accepted without emitting subscriptionRequestReceived(). If
+/// verification fails, the **invalid** old JID is passed in subscriptionRequestReceived()!
+///
 /// \ingroup Managers
 ///
 class QXMPP_EXPORT QXmppRosterManager : public QXmppClientExtension
@@ -144,7 +171,7 @@ private Q_SLOTS:
 private:
     using RosterResult = std::variant<QXmppRosterIq, QXmppError>;
 
-    void handleSubscriptionRequest(const QString &bareJid, const QXmppPresence &presence, bool accept);
+    void handleSubscriptionRequest(const QString &bareJid, const QXmppPresence &presence);
     QXmppTask<RosterResult> requestRoster();
 
     const std::unique_ptr<QXmppRosterManagerPrivate> d;

tests/qxmpprostermanager/tst_qxmpprostermanager.cpp

@@ -5,6 +5,8 @@
 
 #include "QXmppClient.h"
 #include "QXmppDiscoveryManager.h"
+#include "QXmppMovedManager.h"
+#include "QXmppPubSubManager.h"
 #include "QXmppRosterManager.h"
 
 #include "TestClient.h"
@@ -18,7 +20,9 @@ class tst_QXmppRosterManager : public QObject
 
     Q_SLOT void testDiscoFeatures();
     Q_SLOT void testRenameItem();
-    Q_SLOT void subscriptionRequestReceived();
+    Q_SLOT void testSubscriptionRequestReceived();
+    Q_SLOT void testMovedSubscriptionRequestReceived_data();
+    Q_SLOT void testMovedSubscriptionRequestReceived();
     Q_SLOT void testAddItem();
     Q_SLOT void testRemoveItem();
 
@@ -90,7 +94,7 @@ void tst_QXmppRosterManager::testRenameItem()
     QVERIFY(requestSent);
 }
 
-void tst_QXmppRosterManager::subscriptionRequestReceived()
+void tst_QXmppRosterManager::testSubscriptionRequestReceived()
 {
     QXmppPresence presence;
     presence.setType(QXmppPresence::Subscribe);
@@ -110,6 +114,112 @@ void tst_QXmppRosterManager::subscriptionRequestReceived()
     QVERIFY(subscriptionRequestReceived);
 }
 
+void tst_QXmppRosterManager::testMovedSubscriptionRequestReceived_data()
+{
+    QTest::addColumn<bool>("movedManagerAdded");
+    QTest::addColumn<QString>("oldJid");
+    QTest::addColumn<QString>("oldJidResponse");
+    QTest::addColumn<bool>("valid");
+
+    QTest::newRow("noMovedManagerNoJid")
+        << false
+        << QString()
+        << QString()
+        << false;
+    QTest::newRow("noMovedManagerJid")
+        << false
+        << u"[email protected]"_s
+        << QString()
+        << false;
+    QTest::newRow("oldJidEmpty")
+        << true
+        << QString()
+        << QString()
+        << false;
+    QTest::newRow("oldJidNotInRoster")
+        << true
+        << u"[email protected]"_s
+        << QString()
+        << false;
+    QTest::newRow("oldJidRespondingWithError")
+        << true
+        << u"[email protected]"_s
+        << u"<iq id='qxmpp1' from='[email protected]' type='error'>"
+           u"<error type='cancel'>"
+           u"<not-allowed xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>"
+           u"</error>"
+           u"</iq>"_s
+        << false;
+    QTest::newRow("oldJidValid")
+        << true
+        << u"[email protected]"_s
+        << u"<iq id='qxmpp1' from='[email protected]' type='result'>"
+           "<pubsub xmlns='http://jabber.org/protocol/pubsub'>"
+           "<items node='urn:xmpp:moved:1'>"
+           "<item id='current'>"
+           "<moved xmlns='urn:xmpp:moved:1'>"
+           "<new-jid>[email protected]</new-jid>"
+           "</moved>"
+           "</item>"
+           "</items>"
+           "</pubsub>"
+           "</iq>"_s
+        << true;
+}
+
+void tst_QXmppRosterManager::testMovedSubscriptionRequestReceived()
+{
+    TestClient client;
+    client.configuration().setJid(u"[email protected]"_s);
+    auto *rosterManager = client.addNewExtension<QXmppRosterManager>(&client);
+
+    QFETCH(bool, movedManagerAdded);
+    QFETCH(QString, oldJid);
+    QFETCH(QString, oldJidResponse);
+    QFETCH(bool, valid);
+
+    if (movedManagerAdded) {
+        client.addNewExtension<QXmppDiscoveryManager>();
+        client.addNewExtension<QXmppPubSubManager>();
+        client.addNewExtension<QXmppMovedManager>();
+
+        QXmppRosterIq::Item rosterItem;
+        rosterItem.setBareJid(u"[email protected]"_s);
+        rosterItem.setSubscriptionType(QXmppRosterIq::Item::SubscriptionType::Both);
+
+        QXmppRosterIq rosterIq;
+        rosterIq.setType(QXmppIq::Set);
+        rosterIq.setItems({ rosterItem });
+        rosterManager->handleStanza(writePacketToDom(rosterIq));
+    }
+
+    QXmppPresence presence;
+    presence.setType(QXmppPresence::Subscribe);
+    presence.setFrom(u"[email protected]/notebook"_s);
+    presence.setOldJid(oldJid);
+
+    bool subscriptionRequestReceived = false;
+    client.resetIdCount();
+
+    connect(rosterManager, &QXmppRosterManager::subscriptionRequestReceived, this, [&](const QString &subscriberBareJid, const QXmppPresence &presence) {
+        subscriptionRequestReceived = true;
+        QCOMPARE(subscriberBareJid, u"[email protected]"_s);
+        if (valid && movedManagerAdded) {
+            QCOMPARE(oldJid, presence.oldJid());
+        } else {
+            QVERIFY(presence.oldJid().isEmpty());
+        }
+    });
+
+    Q_EMIT client.presenceReceived(presence);
+
+    if (!oldJidResponse.isEmpty()) {
+        client.inject(oldJidResponse);
+    }
+
+    QVERIFY(subscriptionRequestReceived);
+}
+
 void tst_QXmppRosterManager::testAddItem()
 {
     TestClient test;