Fixed secret tokens being logged when connecting to nextcloud

PhilInTheGaps · Nov 3, 2023 · e39711f · e39711f
1 parent 9c279a4
commit e39711f
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 11 deletions.
diff --git a/src/common/utils/stringutils.cpp b/src/common/utils/stringutils.cpp
@@ -5,6 +5,7 @@
 using namespace Qt::Literals::StringLiterals;
 
 constexpr int ROT13_PLACES = 13;
+constexpr int CENSOR_LENGTH = 4;
 
 auto StringUtils::imageFromString(const QByteArray &string) -> QImage
 {
@@ -66,3 +67,12 @@ auto StringUtils::hasWildcardMatch(const QString &string, const QString &wildcar
 
     return hasMatch;
 }
+
+auto StringUtils::censor(QAnyStringView string) -> QString
+{
+    if (string.isEmpty()) return ""_L1;
+
+    if (string.length() <= CENSOR_LENGTH) return string.toString();
+
+    return string.first(CENSOR_LENGTH).toString() + u"..."_s;
+}
diff --git a/src/common/utils/stringutils.h b/src/common/utils/stringutils.h
@@ -43,6 +43,8 @@ class StringUtils
     static auto rot13(const QString &input) -> QString;
     static auto hasWildcardMatch(const QString &string, const QString &wildcard) -> bool;
 
+    static auto censor(QAnyStringView string) -> QString;
+
 private:
     constexpr static auto JPG_BASE64_PREFIX = QLatin1StringView("data:image/jpg;base64,");
 };
diff --git a/src/services/nextcloud/nextcloud.cpp b/src/services/nextcloud/nextcloud.cpp
@@ -1,6 +1,7 @@
 #include "nextcloud.h"
 #include "settings/settingsmanager.h"
 #include "utils/networkutils.h"
+#include "utils/stringutils.h"
 #include <QDesktopServices>
 #include <QJsonDocument>
 #include <QJsonObject>
@@ -171,8 +172,6 @@ void NextCloud::startLoginFlow()
     connect(reply, &QNetworkReply::finished, this, [this, reply]() {
         auto data = QJsonDocument::fromJson(reply->readAll());
 
-        qCDebug(gmNextCloud()) << "Auth reply:" << data.toJson();
-
         if (reply->error() != QNetworkReply::NoError)
         {
             qCWarning(gmNextCloud()) << "Error:" << reply->error() << reply->errorString();
@@ -207,7 +206,7 @@ void NextCloud::pollAuthPoint(const QUrl &url, const QString &token)
     m_authPolls++;
 
     qCDebug(gmNextCloud()) << "URL:" << url.toString();
-    qCDebug(gmNextCloud()) << "Token:" << token;
+    qCDebug(gmNextCloud()) << "Token:" << StringUtils::censor(token);
 
     connect(reply, &QNetworkReply::finished, this,
             [this, url, token, reply]() { handleAuthPointReply(reply, url, token); });
@@ -255,7 +254,7 @@ void NextCloud::handleAuthPointSuccess(QNetworkReply &reply)
 
     qCDebug(gmNextCloud()) << "Logged in successfully!";
     qCDebug(gmNextCloud()) << "LoginName:" << loginName();
-    qCDebug(gmNextCloud()) << "AppPassword:" << m_appPassword;
+    qCDebug(gmNextCloud()) << "AppPassword:" << StringUtils::censor(m_appPassword);
 
     SettingsManager::instance()->set("loginName"_L1, loginName(), serviceName());
     SettingsManager::setPassword(loginName(), m_appPassword, serviceName());

diff --git a/src/services/rest/restserviceconnectorlocal.cpp b/src/services/rest/restserviceconnectorlocal.cpp
@@ -1,5 +1,6 @@
 #include "restserviceconnectorlocal.h"
 #include "settings/settingsmanager.h"
+#include "utils/stringutils.h"
 #include <QDesktopServices>
 #include <QJsonArray>
 #include <QJsonDocument>
@@ -66,8 +67,8 @@ void RESTServiceConnectorLocal::grantAccess()
     m_o2->setClientSecret(secret);
 
     qCDebug(m_loggingCategory) << "Found client id and secret. Trying to link now ...";
-    qCDebug(m_loggingCategory) << m_o2->token();
-    qCDebug(m_loggingCategory) << m_o2->refreshToken();
+    qCDebug(m_loggingCategory) << StringUtils::censor(m_o2->token());
+    qCDebug(m_loggingCategory) << StringUtils::censor(m_o2->refreshToken());
 
     if (m_o2->linked())
     {
@@ -225,7 +226,7 @@ void RESTServiceConnectorLocal::onRefreshFinished(QNetworkReply::NetworkError er
         return;
     }
 
-    qCDebug(m_loggingCategory) << "Refresh Token:" << m_o2->refreshToken();
+    qCDebug(m_loggingCategory) << "Refresh Token:" << StringUtils::censor(m_o2->refreshToken());
 
     updateTokenExpireTime(std::chrono::seconds(m_o2->expires()));
 }

diff --git a/tests/common/utils/teststringutils.cpp b/tests/common/utils/teststringutils.cpp
@@ -3,6 +3,8 @@
 #include <QPixmap>
 #include <gtest/gtest.h>
 
+using namespace Qt::Literals::StringLiterals;
+
 struct Rot13Test
 {
     const char *description;
@@ -34,10 +36,10 @@ struct WildcardTest
 
 TEST(StringUtilsTest, HasWildcardMatch)
 {
-    std::vector<WildcardTest> tests = {{"mp3 file match", "file.mp3", "*.mp3", true},
-                                       {"mp3 file no match", "file.mp3_", "*.mp3", false},
-                                       {"doc file match", "file.doc", "*.doc", true},
-                                       {"doc file no match", "file.not_a_doc", "*.doc", false}};
+    std::vector<WildcardTest> tests = {{"mp3 file match", u"file.mp3"_s, u"*.mp3"_s, true},
+                                       {"mp3 file no match", u"file.mp3_"_s, u"*.mp3"_s, false},
+                                       {"doc file match", u"file.doc"_s, u"*.doc"_s, true},
+                                       {"doc file no match", u"file.not_a_doc"_s, u"*.doc"_s, false}};
 
     for (const auto &test : tests)
     {
@@ -65,3 +67,13 @@ TEST(StringUtilsTest, CanConvertJpgToStringAndBack)
     auto image3 = pixmap.toImage();
     EXPECT_EQ(image3.pixelColor(0, 0), image.pixelColor(0, 0));
 }
+
+TEST(StringUtilsTest, CanCensorStrings)
+{
+    EXPECT_EQ(StringUtils::censor("").toStdString(), "");
+    EXPECT_EQ(StringUtils::censor("1234").toStdString(), "1234");
+    EXPECT_EQ(StringUtils::censor("12345").toStdString(), "1234...");
+    EXPECT_EQ(StringUtils::censor("123456789abcdefghijklmnopqrstuvwxyz").toStdString(), "1234...");
+    EXPECT_EQ(StringUtils::censor(R"(Yf)jH?BaGY>>N-{'!qF.y{^BkKt\~aqB:LpoBzXNffY#y>/n*@>N<Q&%n|FNs{S")").toStdString(),
+              "Yf)j...");
+}