Merge pull request #10351 from nextcloud/bugfix/notifyWss

fix: do not allow scheme change for push notifications

Author: Rello

src/libsync/account.cpp

@@ -60,6 +60,17 @@ constexpr int pushNotificationsReconnectInterval = 1000 * 60 * 2;
 constexpr int usernamePrefillServerVersionMinSupportedMajor = 24;
 constexpr int checksumRecalculateRequestServerVersionMinSupportedMajor = 24;
 constexpr auto isSkipE2eeMetadataChecksumValidationAllowedInClientVersion = MIRALL_VERSION_MAJOR == 3 && MIRALL_VERSION_MINOR == 8;
+
+bool isPushNotificationsWebSocketUrlAllowed(const QUrl &accountUrl, const QUrl &webSocketUrl)
+{
+    const auto webSocketScheme = webSocketUrl.scheme();
+    if (webSocketScheme.compare(QLatin1String("wss"), Qt::CaseInsensitive) == 0) {
+        return true;
+    }
+
+    return webSocketScheme.compare(QLatin1String("ws"), Qt::CaseInsensitive) == 0
+        && accountUrl.scheme().compare(QLatin1String("http"), Qt::CaseInsensitive) == 0;
+}
 }
 
 namespace OCC {
@@ -344,6 +355,17 @@ void Account::trySetupPushNotifications()
     _pushNotificationsReconnectTimer.stop();
 
     if (_capabilities.availablePushNotifications() != PushNotificationType::None) {
+        const auto webSocketUrl = _capabilities.pushNotificationsWebSocketUrl();
+        if (!isPushNotificationsWebSocketUrlAllowed(url(), webSocketUrl)) {
+            qCWarning(lcAccount) << "Reject insecure push notifications websocket endpoint" << webSocketUrl << "for account" << url();
+            if (_pushNotifications) {
+                delete _pushNotifications;
+                _pushNotifications = nullptr;
+            }
+            emit pushNotificationsDisabled(sharedFromThis());
+            return;
+        }
+
         qCInfo(lcAccount) << "Try to setup push notifications";
 
         if (!_pushNotifications) {

test/pushnotificationstestutils.cpp

@@ -144,19 +144,21 @@ void FakeWebSocketServer::clearTextMessages()
     _processTextMessageSpy->clear();
 }
 
-OCC::AccountPtr FakeWebSocketServer::createAccount(const QString &username, const QString &password)
+OCC::AccountPtr FakeWebSocketServer::createAccount(const QString &username,
+    const QString &password,
+    const QUrl &accountUrl,
+    const QUrl &webSocketUrl)
 {
     auto account = OCC::Account::create();
+    account->setUrl(accountUrl);
 
     QStringList typeList;
     typeList.append("files");
     typeList.append("activities");
     typeList.append("notifications");
 
-    QString websocketUrl("ws://localhost:12345");
-
     QVariantMap endpointsMap;
-    endpointsMap["websocket"] = websocketUrl;
+    endpointsMap["websocket"] = webSocketUrl;
 
     QVariantMap notifyPushMap;
     notifyPushMap["type"] = typeList;

test/pushnotificationstestutils.h

@@ -37,7 +37,10 @@ class FakeWebSocketServer : public QObject
 
     void clearTextMessages();
 
-    static OCC::AccountPtr createAccount(const QString &username = "user", const QString &password = "password");
+    static OCC::AccountPtr createAccount(const QString &username = "user",
+        const QString &password = "password",
+        const QUrl &accountUrl = QUrl(QStringLiteral("http://localhost")),
+        const QUrl &webSocketUrl = QUrl(QStringLiteral("ws://localhost:12345")));
 
 signals:
     void closed();

test/testpushnotifications.cpp

@@ -106,6 +106,24 @@ private slots:
             }));
     }
 
+    void testSetup_httpsAccountWithPlaintextWebSocket_doesNotSendCredentials()
+    {
+        FakeWebSocketServer fakeServer;
+        const auto account = FakeWebSocketServer::createAccount(
+            QStringLiteral("user"),
+            QStringLiteral("app-password"),
+            QUrl(QStringLiteral("https://cloud.example.test")),
+            QUrl(QStringLiteral("ws://localhost:12345")));
+
+        QVERIFY(!account->pushNotifications());
+        QCOMPARE(fakeServer.textMessagesCount(), 0);
+
+        account->trySetupPushNotifications();
+
+        QVERIFY(!account->pushNotifications());
+        QCOMPARE(fakeServer.textMessagesCount(), 0);
+    }
+
     void testOnWebSocketTextMessageReceived_notifyFileMessage_emitFilesChanged()
     {
         FakeWebSocketServer fakeServer;