Skip to content

Commit a97167a

Browse files
pbekpbek
committed
#3597 secrets: improve simplecrypt fallback when keychain access fails
Signed-off-by: Patrizio Bekerle <patrizio@bekerle.com>
1 parent c13da10 commit a97167a

10 files changed

Lines changed: 82 additions & 12 deletions

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
- Fixed the missing internal `bookmarks.svg` icon resource used by the note
66
bookmark menu, so QOwnNotes no longer logs a startup warning for that icon
77
(for [#3649](https://github.com/pbek/QOwnNotes/issues/3649))
8+
- Improved secret storage fallback handling by keeping legacy SimpleCrypt
9+
encryption compatible with pre-qtkeychain secrets and migrating old plaintext
10+
secret settings to encrypted storage even when the desktop keychain is
11+
unavailable (for [#3597](https://github.com/pbek/QOwnNotes/issues/3597))
812

913
## 26.6.8
1014

‎src/dialogs/websockettokendialog.cpp‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <QShowEvent>
66
#include <QtGui/QClipboard>
77

8+
#include "services/cryptoservice.h"
89
#include "services/settingsservice.h"
910
#include "ui_websockettokendialog.h"
1011

@@ -20,7 +21,8 @@ WebSocketTokenDialog::~WebSocketTokenDialog() { delete ui; }
2021

2122
void WebSocketTokenDialog::loadTokenFromSettings() {
2223
SettingsService settings;
23-
_initialToken = settings.value(QStringLiteral("webSocketServerService/token")).toString();
24+
_initialToken = CryptoService::instance()->decryptToStringWithPlaintextFallback(
25+
settings.value(QStringLiteral("webSocketServerService/token")).toString());
2426

2527
if (_initialToken.isEmpty()) {
2628
ui->tokenLineEdit->setText(generateToken());
@@ -44,12 +46,17 @@ void WebSocketTokenDialog::on_generateButton_clicked() {
4446

4547
void WebSocketTokenDialog::on_buttonBox_accepted() {
4648
SettingsService settings;
47-
settings.setValue(QStringLiteral("webSocketServerService/token"), ui->tokenLineEdit->text());
49+
settings.setValue(
50+
QStringLiteral("webSocketServerService/token"),
51+
CryptoService::instance()->encryptToString(
52+
ui->tokenLineEdit->text(), QStringLiteral("settings/webSocketServerService/token")));
4853
}
4954

5055
void WebSocketTokenDialog::reject() {
5156
SettingsService settings;
52-
settings.setValue(QStringLiteral("webSocketServerService/token"), _initialToken);
57+
settings.setValue(QStringLiteral("webSocketServerService/token"),
58+
CryptoService::instance()->encryptToString(
59+
_initialToken, QStringLiteral("settings/webSocketServerService/token")));
5360
ui->tokenLineEdit->setText(_initialToken.isEmpty() ? generateToken() : _initialToken);
5461

5562
MasterDialog::reject();

‎src/services/cryptoservice.cpp‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,16 @@ QString CryptoService::decryptToString(const QString &text) {
155155
return legacyDecryptToString(text);
156156
}
157157

158+
QString CryptoService::decryptToStringWithPlaintextFallback(const QString &text) {
159+
const QString plainText = decryptToString(text);
160+
161+
if (plainText.isEmpty() && !text.isEmpty() && !isKeychainReference(text)) {
162+
return text;
163+
}
164+
165+
return plainText;
166+
}
167+
158168
bool CryptoService::isKeychainReference(const QString &text) {
159169
return text.startsWith(KeychainMarkerPrefix);
160170
}
@@ -247,10 +257,12 @@ QStringList CryptoService::keychainReferencesFromDiskDatabase() {
247257
}
248258

249259
QString CryptoService::legacyEncryptToString(const QString &text) {
260+
// Keep this compatible with the pre-qtkeychain SimpleCrypt storage format.
250261
return _simpleCrypt->encryptToString(text);
251262
}
252263

253264
QString CryptoService::legacyDecryptToString(const QString &text) {
265+
// Keep this compatible with the pre-qtkeychain SimpleCrypt storage format.
254266
return _simpleCrypt->decryptToString(text);
255267
}
256268

@@ -321,7 +333,16 @@ bool CryptoService::migrateSecret(QString *storedValue, const QString &key,
321333
plainText = *storedValue;
322334
}
323335

324-
if (plainText.isEmpty() || !writeSecret(key, plainText)) {
336+
if (plainText.isEmpty()) {
337+
return false;
338+
}
339+
340+
if (!writeSecret(key, plainText)) {
341+
if (allowPlaintextFallback && *storedValue == plainText) {
342+
*storedValue = legacyEncryptToString(plainText);
343+
return true;
344+
}
345+
325346
return false;
326347
}
327348

@@ -350,7 +371,10 @@ void CryptoService::migrateSettingsSecrets() {
350371
};
351372
const QStringList plaintextFallbackSettingsKeys = {
352373
QStringLiteral("webSocketServerService/bookmarkSuggestionApiToken"),
374+
QStringLiteral("webSocketServerService/token"),
375+
QStringLiteral("webAppClientService/token"),
353376
QStringLiteral("ai/mcpServerToken"),
377+
QStringLiteral("languageToolApiKey"),
354378
};
355379

356380
SettingsService settings;

‎src/services/cryptoservice.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ class CryptoService : public QObject {
1919
QString encryptToString(const QString &text);
2020
QString encryptToString(const QString &text, const QString &key);
2121
QString decryptToString(const QString &text);
22+
QString decryptToStringWithPlaintextFallback(const QString &text);
2223
static bool isKeychainReference(const QString &text);
2324
bool deleteSecret(const QString &storedValueOrKey) const;
2425
void deleteSecrets(const QStringList &storedValuesOrKeys) const;

‎src/services/languagetoolchecker.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "helpers/qownspellchecker.h"
1212
#include "libraries/qmarkdowntextedit/markdownhighlighter.h"
1313
#include "mainwindow.h"
14+
#include "services/cryptoservice.h"
1415
#include "services/languagetoolclient.h"
1516
#include "services/settingsservice.h"
1617

@@ -335,7 +336,10 @@ void LanguageToolChecker::readSettings() {
335336
.trimmed();
336337
_language =
337338
settings.value(QStringLiteral("languageToolLanguage"), QStringLiteral("auto")).toString();
338-
_apiKey = settings.value(QStringLiteral("languageToolApiKey")).toString().trimmed();
339+
_apiKey = CryptoService::instance()
340+
->decryptToStringWithPlaintextFallback(
341+
settings.value(QStringLiteral("languageToolApiKey")).toString())
342+
.trimmed();
339343
_checkDelayMs = settings.value(QStringLiteral("languageToolCheckDelay"), 1500).toInt();
340344
_timeoutMs = settings.value(QStringLiteral("languageToolTimeout"), 5000).toInt();
341345
_enabledCategories =

‎src/services/webappclientservice.cpp‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include <memory>
3535

3636
#include "metricsservice.h"
37+
#include "services/cryptoservice.h"
3738
#include "services/settingsservice.h"
3839

3940
using namespace std;
@@ -208,12 +209,21 @@ QString WebAppClientService::getServerUrl() {
208209
}
209210

210211
QString WebAppClientService::getOrGenerateToken() {
211-
QString token = SettingsService().value(QStringLiteral("webAppClientService/token")).toString();
212+
SettingsService settings;
213+
const QString key = QStringLiteral("webAppClientService/token");
214+
const QString stored = settings.value(key).toString();
215+
QString token = CryptoService::instance()->decryptToStringWithPlaintextFallback(stored);
216+
217+
if (!token.isEmpty() && token == stored && !CryptoService::isKeychainReference(stored)) {
218+
settings.setValue(key, CryptoService::instance()->encryptToString(
219+
token, QStringLiteral("settings/") + key));
220+
}
212221

213222
// if not token was set
214223
if (token.isEmpty()) {
215224
token = Utils::Misc::generateRandomString(32);
216-
SettingsService().setValue(QStringLiteral("webAppClientService/token"), token);
225+
settings.setValue(key, CryptoService::instance()->encryptToString(
226+
token, QStringLiteral("settings/") + key));
217227
}
218228

219229
return token;

‎src/services/websocketserverservice.cpp‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,18 @@ void WebSocketServerService::processMessage(const QString &message) {
476476
MetricsService::instance()->sendVisitIfEnabled("websocket/message/" + type);
477477
const QString token = jsonObject.value(QStringLiteral("token")).toString();
478478
SettingsService settings;
479-
QString storedToken = settings.value(QStringLiteral("webSocketServerService/token")).toString();
479+
const QString storedTokenValue =
480+
settings.value(QStringLiteral("webSocketServerService/token")).toString();
481+
QString storedToken =
482+
CryptoService::instance()->decryptToStringWithPlaintextFallback(storedTokenValue);
483+
484+
if (!storedToken.isEmpty() && storedToken == storedTokenValue &&
485+
!CryptoService::isKeychainReference(storedTokenValue)) {
486+
settings.setValue(
487+
QStringLiteral("webSocketServerService/token"),
488+
CryptoService::instance()->encryptToString(
489+
storedToken, QStringLiteral("settings/webSocketServerService/token")));
490+
}
480491

481492
// request the token if not set
482493
if (token.isEmpty() || storedToken.isEmpty() || token != storedToken) {

‎src/utils/misc.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2357,6 +2357,7 @@ QString Utils::Misc::generateDebugInformation(bool withGitHubLineBreaks, bool an
23572357
"networking/proxyPassword",
23582358
"ai/groq/apiKey",
23592359
"ai/openai/apiKey",
2360+
"languageToolApiKey",
23602361
"webSocketServerService/token",
23612362
"webSocketServerService/bookmarkSuggestionApiToken",
23622363
"webAppClientService/token"};

‎src/widgets/settings/languagetoolsettingswidget.cpp‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include <QMessageBox>
1818

19+
#include "services/cryptoservice.h"
1920
#include "services/settingsservice.h"
2021
#include "ui_languagetoolsettingswidget.h"
2122

@@ -61,7 +62,8 @@ void LanguageToolSettingsWidget::readSettings() {
6162
.value(QStringLiteral("languageToolServerUrl"), QStringLiteral("http://localhost:8081"))
6263
.toString());
6364
ui->languageToolApiKeyLineEdit->setText(
64-
settings.value(QStringLiteral("languageToolApiKey")).toString());
65+
CryptoService::instance()->decryptToStringWithPlaintextFallback(
66+
settings.value(QStringLiteral("languageToolApiKey")).toString()));
6567
ui->languageToolCheckDelaySpinBox->setValue(
6668
settings.value(QStringLiteral("languageToolCheckDelay"), 1500).toInt());
6769

@@ -111,8 +113,10 @@ void LanguageToolSettingsWidget::storeSettings() {
111113
ui->languageToolLanguageComboBox->currentData().toString().isEmpty()
112114
? ui->languageToolLanguageComboBox->currentText().trimmed()
113115
: ui->languageToolLanguageComboBox->currentData().toString());
114-
settings.setValue(QStringLiteral("languageToolApiKey"),
115-
ui->languageToolApiKeyLineEdit->text().trimmed());
116+
settings.setValue(
117+
QStringLiteral("languageToolApiKey"),
118+
CryptoService::instance()->encryptToString(ui->languageToolApiKeyLineEdit->text().trimmed(),
119+
QStringLiteral("settings/languageToolApiKey")));
116120
settings.setValue(QStringLiteral("languageToolCheckDelay"),
117121
ui->languageToolCheckDelaySpinBox->value());
118122
settings.setValue(QStringLiteral("languageToolEnabledCategories"),

‎src/widgets/settings/webapplicationsettingswidget.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <QMessageBox>
2121
#include <QSysInfo>
2222

23+
#include "services/cryptoservice.h"
2324
#include "services/settingsservice.h"
2425
#include "services/webappclientservice.h"
2526
#include "ui_webapplicationsettingswidget.h"
@@ -59,7 +60,10 @@ void WebApplicationSettingsWidget::storeSettings() {
5960
ui->enableWebApplicationCheckBox->isChecked());
6061
settings.setValue(QStringLiteral("webAppClientService/serverUrl"),
6162
ui->webAppServerUrlLineEdit->text());
62-
settings.setValue(QStringLiteral("webAppClientService/token"), ui->webAppTokenLineEdit->text());
63+
settings.setValue(
64+
QStringLiteral("webAppClientService/token"),
65+
CryptoService::instance()->encryptToString(
66+
ui->webAppTokenLineEdit->text(), QStringLiteral("settings/webAppClientService/token")));
6367
settings.setValue(QStringLiteral("webAppClientService/connectionName"),
6468
ui->webAppConnectionNameLineEdit->text());
6569
}

0 commit comments

Comments
 (0)