Fix re-encrypting tabs loaded with other plugins

src/common/keychainaccess.cpp

@@ -114,9 +114,8 @@ bool writePassword(const QString &, const QString &, const QByteArray &)
     return false;
 }
 
-bool deletePassword(const QString &, const QString &)
+void deletePassword(const QString &, const QString &)
 {
-    return false;
 }
 
 } // namespace KeychainAccess

src/gui/encryptionpassword.cpp

@@ -14,6 +14,7 @@
 #include "item/itemstore.h"
 #include "item/itemwidget.h"
 #include "item/serialize.h"
+#include "gui/clipboardbrowsershared.h"
 
 #include <QCoreApplication>
 #include <QInputDialog>
@@ -325,9 +326,9 @@ Encryption::EncryptionKey promptForEncryptionPasswordChange(QWidget *parent)
 
 bool reencryptTabs(
     const QStringList &tabNames,
-    ItemFactory *itemFactory,
+    ClipboardBrowserShared *sharedData,
     const Encryption::EncryptionKey &oldKey,
-    Encryption::EncryptionKey &newKey,
+    const Encryption::EncryptionKey &newKey,
     int maxItems,
     QWidget *parent)
 {
@@ -342,13 +343,20 @@ bool reencryptTabs(
     progress.setMinimumDuration(500);  // Show after 500ms if not done
     progress.setValue(0);
 
-    const Encryption::EncryptionKey newKeyBackup = newKey;
+    // Skip plugins that do not support encryption
+    ItemLoaderList skipLoaders;
+    for (auto &loader : sharedData->itemFactory->loaders()) {
+        if (loader->isEnabled() && !loader->supportsEncryption()) {
+            skipLoaders.append(loader);
+            loader->setEnabled(false);
+        }
+    }
 
     QStringList failedTabs;
+
     for (int i = 0; i < tabNames.size(); ++i) {
         if (progress.wasCanceled()) {
             log("Tab re-encryption cancelled by user", LogWarning);
-            failedTabs.append(QObject::tr("(Cancelled by user)"));
             break;
         }
 
@@ -369,14 +377,12 @@ bool reencryptTabs(
         ClipboardModel model;
 
         // Set old encryption key temporarily and load items
-        newKey = oldKey;
-        ItemSaverPtr saver = loadItems(tabName, model, itemFactory, maxItems);
-        newKey = newKeyBackup;
+        sharedData->encryptionKey = oldKey;
+        ItemSaverPtr saver = loadItems(tabName, model, sharedData->itemFactory, maxItems);
+        sharedData->encryptionKey = newKey;
 
         if (!saver) {
-            const QString error = QStringLiteral("Failed to load tab: %1").arg(tabName);
-            log(error, LogError);
-            failedTabs.append(tabName);
+            COPYQ_LOG(QStringLiteral("Skipping encryption on unsupported tab: %1").arg(tabName));
             continue;
         }
 
@@ -387,15 +393,17 @@ bool reencryptTabs(
         COPYQ_LOG(QStringLiteral("Loaded %1 items from tab: %2").arg(itemCount).arg(tabName));
 
         if (!saveItems(tabName, model, saver)) {
-            const QString error = QStringLiteral("Failed to save tab: %1").arg(tabName);
-            log(error, LogError);
+            log(QStringLiteral("Failed to re-encrypt tab: %1").arg(tabName), LogError);
             failedTabs.append(tabName);
             continue;
         }
 
         COPYQ_LOG(QStringLiteral("Successfully re-encrypted tab: %1").arg(tabName));
     }
 
+    for (auto &loader : skipLoaders)
+        loader->setEnabled(true);
+
     progress.setValue(tabNames.size());
 
     if (!failedTabs.isEmpty()) {
@@ -411,11 +419,7 @@ bool reencryptTabs(
         return false;
     }
 
-    log(QStringLiteral("Successfully re-encrypted all %1 tabs").arg(tabNames.size()), LogNote);
-
-    if (!newKey.isValid())
-        removePasswordFromKeychain();
-
+    log("Successfully re-encrypted tabs");
     return true;
 }
 
@@ -444,9 +448,9 @@ Encryption::EncryptionKey promptForEncryptionPasswordChange(QWidget *)
 
 bool reencryptTabs(
     const QStringList &,
-    ItemFactory *,
+    ClipboardBrowserShared *,
+    const Encryption::EncryptionKey &,
     const Encryption::EncryptionKey &,
-    Encryption::EncryptionKey &,
     int,
     QWidget *)
 {

src/gui/encryptionpassword.h

@@ -4,6 +4,7 @@
 
 #include "common/encryption.h"
 
+class ClipboardBrowserShared;
 class QWidget;
 class QAbstractItemModel;
 class QIODevice;
@@ -46,9 +47,9 @@ Encryption::EncryptionKey promptForEncryptionPasswordChange(QWidget *parent);
  */
 bool reencryptTabs(
     const QStringList &tabNames,
-    ItemFactory *itemFactory,
+    ClipboardBrowserShared *sharedData,
     const Encryption::EncryptionKey &oldKey,
-    Encryption::EncryptionKey &newKey,
+    const Encryption::EncryptionKey &newKey,
     int maxItems,
     QWidget *parent
 );

src/gui/mainwindow.cpp

@@ -597,6 +597,7 @@ Encryption::EncryptionKey promptForImportPassword(QWidget *parent)
     return Encryption::EncryptionKey(password);
 }
 
+#ifdef WITH_QCA_ENCRYPTION
 Encryption::EncryptionKey promptForExportPassword(QWidget *parent, bool *ok)
 {
     if (!Encryption::initialize()) {
@@ -623,6 +624,7 @@ Encryption::EncryptionKey promptForExportPassword(QWidget *parent, bool *ok)
     *ok = false;
     return {};
 }
+#endif
 
 } // namespace
 
@@ -3718,11 +3720,23 @@ void MainWindow::promptForEncryptionPasswordIfNeeded(AppConfig *appConfig)
 }
 
 void MainWindow::reencryptTabsIfNeeded(const QStringList &tabNames, AppConfig *appConfig)
+{
+    if (m_reencrypting)
+        return;
+
+    m_reencrypting = true;
+    reencryptTabsIfNeededHelper(tabNames, appConfig);
+    m_reencrypting = false;
+}
+
+void MainWindow::reencryptTabsIfNeededHelper(const QStringList &tabNames, AppConfig *appConfig)
 {
     const bool isEncrypted = appConfig->option<Config::tab_encryption_enabled>();
     if (m_wasEncrypted == isEncrypted)
         return;
 
+    const Encryption::EncryptionKey newEncryptionKey = m_sharedData->encryptionKey;
+
     // Always ask for the previous password when re-encrypting.
     Encryption::EncryptionKey oldEncryptionKey;
     if (m_wasEncrypted) {
@@ -3731,33 +3745,35 @@ void MainWindow::reencryptTabsIfNeeded(const QStringList &tabNames, AppConfig *a
     }
 
     // Revert encryption option if password was not provided.
-    if (!oldEncryptionKey.isValid() && !m_sharedData->encryptionKey.isValid()) {
+    if (!oldEncryptionKey.isValid() && !newEncryptionKey.isValid()) {
         appConfig->setOption(Config::tab_encryption_enabled::name(), m_wasEncrypted);
         return;
     }
 
-    m_wasEncrypted = isEncrypted;
-
-    const bool ok = reencryptTabs(
+    const bool allEncrypted = reencryptTabs(
         tabNames,
-        m_sharedData->itemFactory,
+        m_sharedData.get(),
         oldEncryptionKey,
-        m_sharedData->encryptionKey,
+        newEncryptionKey,
         Config::maxItems,
         this
     );
 
-    if (!ok) {
-        // If saving some tabs failed, keep the keys and the encryption enabled
-        if ( oldEncryptionKey.isValid() ) {
+    m_wasEncrypted = isEncrypted;
+    m_sharedData->encryptionKey = newEncryptionKey;
+
+    // If saving some tabs failed, keep the keys and the encryption enabled,
+    // otherwise remove unneeded key files.
+    if (oldEncryptionKey.isValid()) {
+        if (allEncrypted) {
+            Encryption::removeEncryptionKeys();
+            if (appConfig->option<Config::use_key_store>())
+                removePasswordFromKeychain();
+        } else {
             appConfig->setOption(Config::tab_encryption_enabled::name(), true);
+            m_wasEncrypted = true;
             m_sharedData->encryptionKey = oldEncryptionKey;
         }
-        return;
-    }
-
-    if ( !m_sharedData->encryptionKey.isValid() ) {
-        Encryption::removeEncryptionKeys();
     }
 }
 

src/gui/mainwindow.h

@@ -658,6 +658,7 @@ class MainWindow final : public QMainWindow
 
     void promptForEncryptionPasswordIfNeeded(AppConfig *appConfig);
     void reencryptTabsIfNeeded(const QStringList &tabNames, AppConfig *appConfig);
+    void reencryptTabsIfNeededHelper(const QStringList &tabNames, AppConfig *appConfig);
 
     /**
      * Update tab name in placeholder and configuration.
@@ -686,6 +687,7 @@ class MainWindow final : public QMainWindow
 
     ClipboardBrowserSharedPtr m_sharedData;
     bool m_wasEncrypted = false;
+    bool m_reencrypting = false;
 
     QVector<Command> m_automaticCommands;
     QVector<Command> m_displayCommands;

src/item/itemfactory.cpp

@@ -270,6 +270,8 @@ class DummyLoader final : public ItemLoaderInterface
         return filter.matches(text) || filter.matches(accentsRemoved(text));
     }
 
+    bool supportsEncryption() const override { return true; }
+
 private:
     int m_itemDataThreshold = -1;
     ClipboardBrowserSharedPtr m_sharedData;
@@ -437,11 +439,13 @@ void ItemFactory::setPluginPriority(const QStringList &pluginNames)
 ItemSaverPtr ItemFactory::loadItems(const QString &tabName, QAbstractItemModel *model, QIODevice *file, int maxItems)
 {
     for (auto &loader : m_loaders) {
-        if ( !loader->isEnabled() )
-            continue;
-
         file->seek(0);
         if ( loader->canLoadItems(file) ) {
+            if ( !loader->isEnabled() ) {
+                log(QStringLiteral("Cannot load tab %1, plugin %2 must be enabled")
+                    .arg(quoteString(tabName), loader->name()));
+                return nullptr;
+            }
             file->seek(0);
             auto saver = loader->loadItems(tabName, model, file, maxItems);
             if (!saver)

src/item/itemstore.cpp

@@ -103,8 +103,8 @@ ItemSaverPtr loadItems(const QString &tabName, QAbstractItemModel &model, ItemFa
         return saver;
     }
 
-    log( QStringLiteral("Tab \"%1\": Failed to load tab file: %2")
-            .arg(tabName, tabFileName), LogError );
+    log( QStringLiteral("Tab \"%1\": Cannot load tab file: %2")
+            .arg(tabName, tabFileName) );
     model.removeRows(0, model.rowCount());
     return nullptr;
 }

src/item/itemwidget.h

@@ -382,6 +382,8 @@ class ItemLoaderInterface
      */
     virtual QObject *createExternalEditor(const QModelIndex &index, const QVariantMap &data, QWidget *parent) const;
 
+    virtual bool supportsEncryption() const { return false; }
+
     ItemLoaderInterface(const ItemLoaderInterface &) = delete;
     ItemLoaderInterface &operator=(const ItemLoaderInterface &) = delete;
 

src/item/serialize.cpp

@@ -128,31 +128,30 @@ QDataStream &operator>>(QDataStream &in, DataFile &value)
 
     bool hasKey = false;
     in >> hasKey;
-#ifdef WITH_QCA_ENCRYPTION
     if (hasKey) {
-        QByteArray dekBytes;
-        in >> dekBytes;
-
-        // Initialize QCA if not already done
+#ifdef WITH_QCA_ENCRYPTION
         if (!Encryption::initialize()) {
             qCCritical(serializeCategory) << "Failed to initialize encryption for DataFile deserialization";
             in.setStatus(QDataStream::ReadCorruptData);
             return in;
         }
 
+        QByteArray dekBytes;
+        in >> dekBytes;
+
         Encryption::EncryptionKey key;
-        if (key.importDEK(dekBytes)) {
-            value.setEncryptionKey(key);
-        } else {
+        if (!key.importDEK(dekBytes)) {
             qCCritical(serializeCategory) << "Failed to import encryption key for DataFile";
+            in.setStatus(QDataStream::ReadCorruptData);
+            return in;
         }
-    }
+
+        value.setEncryptionKey(key);
 #else
-    if (hasKey) {
         qCCritical(serializeCategory) << "Cannot deserialize encrypted DataFile: encryption support not compiled";
         in.setStatus(QDataStream::ReadCorruptData);
-    }
 #endif
+    }
     return in;
 }
 

src/tests/itemsynctests.cpp

@@ -1030,3 +1030,57 @@ void ItemSyncTests::copyFiles()
     RUN("tab" << tab3 << getFirstItemFormats, "text/plain\n");
     QCOMPARE(dir3.files().join(" "), "test-0001.txt test.txt");
 }
+
+void ItemSyncTests::encryptionShouldNotAffectFiles()
+{
+    TestDir dir1(1);
+    const QString tab1 = testTab(1);
+    RUN(Args() << "show" << tab1, "");
+
+    const Args args = Args() << "tab" << tab1;
+    const QString inspect = R"(
+        const sel = ItemSelection().selectAll();
+        const items = sel.items();
+        for (const i in items) {
+            if (i > 0) { print(' ;; '); }
+            print(i + ':');
+            const item = items[i];
+            for (const format in item) {
+                if (!format.includes('itemsync') || format.includes('itemsync-basename')) {
+                    const fmt = format.replace('application/x-copyq-', '');
+                    print(' ' + fmt + ':' + item[format]);
+                }
+            }
+        }
+    )";
+
+    const QString file1 = "test1.txt";
+    TEST(createFile(dir1, file1, "TEXT1"));
+    QCOMPARE(dir1.files().join(sep), file1);
+
+    WAIT_ON_OUTPUT(args << "size", "1\n");
+    RUN(args << inspect, "0: itemsync-basename:test1 text/plain:TEXT1");
+
+    const QString file2_yyy = "test2.yyy";
+    const QString file2_txt = "test2.txt";
+    TEST(createFile(dir1, file2_yyy, "YYY"));
+    TEST(createFile(dir1, file2_txt, "TEXT2"));
+
+    WAIT_ON_OUTPUT(
+        args << inspect, "0: itemsync-basename:test1 text/plain:TEXT1 ;; 1: itemsync-basename:test2 test-zzz:YYY text/plain:TEXT2");
+
+    const QStringList files({file1, file2_txt, file2_yyy});
+    QCOMPARE(dir1.files().join(sep), files.join(sep));
+
+    RUN(Args() << "show" << "CLIPBOARD", "");
+    RUN(Args() << "unload" << tab1, tab1 + "\n");
+
+    QCOMPARE(dir1.files().join(sep), files.join(sep));
+
+    RUN("config" << "tab_encryption_enabled" << "true", "true\n");
+
+    RUN(args << inspect,
+        "0: itemsync-basename:test1 text/plain:TEXT1 ;; "
+        "1: itemsync-basename:test2 test-zzz:YYY text/plain:TEXT2");
+    QCOMPARE(dir1.files().join(sep), files.join(sep));
+}

src/tests/itemsynctests.h

@@ -59,6 +59,8 @@ private slots:
 
     void copyFiles();
 
+    void encryptionShouldNotAffectFiles();
+
 private:
     TestInterfacePtr m_test;
 };