Hotfix/2657 prevent share overwrite (#2746)

* Fix problem with export from newly saved database

Newly created/saved databases (or used with DatabaseWidget::saveAs)
were not exported/imported correctly.
Fixed the problem by reinitializing the ShareObserver on
DatabaseWidget::saveAs.

* Introduce warnings and prevent conflicting shares

Introduced several warnings and errors to indicate improper settings.
Prevent export when a path is used multiple times (only the file path is
checked - may ignore multiple similar ways to reference a share).

* Improve KeeShare integration in DatabaseWidget

Moved initial KeeShare association to constructor.
Introduced Q_UNUSED to indicate need for assignment statement.

src/gui/DatabaseWidget.cpp

@@ -208,6 +208,12 @@ DatabaseWidget::DatabaseWidget(QSharedPointer<Database> db, QWidget* parent)
     }
 #endif
 
+#ifdef WITH_XC_KEESHARE
+    // We need to reregister the database to allow exports
+    // from a newly created database
+    KeeShare::instance()->connectDatabase(m_db, {});
+#endif
+
     switchToMainView();
 }
 
@@ -391,6 +397,9 @@ void DatabaseWidget::replaceDatabase(QSharedPointer<Database> db)
     processAutoOpen();
 #if defined(WITH_XC_KEESHARE)
     KeeShare::instance()->connectDatabase(m_db, oldDb);
+#else
+    // Keep the instance active till the end of this function
+    Q_UNUSED(oldDb);
 #endif
 }
 

src/gui/group/EditGroupWidget.cpp

@@ -36,9 +36,9 @@ public:
     {
     }
 
-    void set(Group* temporaryGroup) const
+    void set(Group* temporaryGroup, QSharedPointer<Database> database) const
     {
-        editPage->set(widget, temporaryGroup);
+        editPage->set(widget, temporaryGroup, database);
     }
 
     void assign() const
@@ -133,7 +133,7 @@ void EditGroupWidget::loadGroup(Group* group, bool create, const QSharedPointer<
     m_editWidgetProperties->setCustomData(m_temporaryGroup->customData());
 
     for (const ExtraPage& page : asConst(m_extraPages)) {
-        page.set(m_temporaryGroup.data());
+        page.set(m_temporaryGroup.data(), m_db);
     }
 
     setCurrentPage(0);

src/gui/group/EditGroupWidget.h

@@ -43,7 +43,7 @@ public:
     virtual QString name() = 0;
     virtual QIcon icon() = 0;
     virtual QWidget* createWidget() = 0;
-    virtual void set(QWidget* widget, Group* tempoaryGroup) = 0;
+    virtual void set(QWidget* widget, Group* tempoaryGroup, QSharedPointer<Database> database) = 0;
     virtual void assign(QWidget* widget) = 0;
 };
 

src/keeshare/ShareObserver.cpp

@@ -25,6 +25,7 @@
 #include "core/Entry.h"
 #include "core/FilePath.h"
 #include "core/FileWatcher.h"
+#include "core/Global.h"
 #include "core/Group.h"
 #include "core/Merger.h"
 #include "core/Metadata.h"
@@ -191,7 +192,7 @@ void ShareObserver::reinitialize()
 
     const auto active = KeeShare::active();
     QList<Update> updated;
-    QList<Group*> groups = m_db->rootGroup()->groupsRecursive(true);
+    const QList<Group*> groups = m_db->rootGroup()->groupsRecursive(true);
     for (Group* group : groups) {
         Update couple{group, m_groupToReference.value(group), KeeShare::referenceOf(group)};
         if (couple.oldReference == couple.newReference) {
@@ -214,7 +215,9 @@ void ShareObserver::reinitialize()
     QStringList success;
     QStringList warning;
     QStringList error;
-    for (const auto& update : updated) {
+    QMap<QString, QStringList> imported;
+    QMap<QString, QStringList> exported;
+    for (const auto& update : asConst(updated)) {
         if (!update.oldReference.path.isEmpty()) {
             m_fileWatcher->removePath(update.oldReference.path);
         }
@@ -222,8 +225,12 @@ void ShareObserver::reinitialize()
         if (!update.newReference.path.isEmpty() && update.newReference.type != KeeShareSettings::Inactive) {
             m_fileWatcher->addPath(update.newReference.path);
         }
+        if (update.newReference.isExporting()) {
+            exported[update.newReference.path] << update.group->name();
+        }
 
         if (update.newReference.isImporting()) {
+            imported[update.newReference.path] << update.group->name();
             const auto result = this->importFromReferenceContainer(update.newReference.path);
             if (!result.isValid()) {
                 // tolerable result - blocked import or missing source
@@ -241,6 +248,16 @@ void ShareObserver::reinitialize()
             }
         }
     }
+    for (auto it = imported.cbegin(); it != imported.cend(); ++it) {
+        if (it.value().count() > 1) {
+            warning << tr("Multiple import source path to %1 in %2").arg(it.key(), it.value().join(", "));
+        }
+    }
+    for (auto it = exported.cbegin(); it != exported.cend(); ++it) {
+        if (it.value().count() > 1) {
+            error << tr("Conflicting export target path %1 in %2").arg(it.key(), it.value().join(", "));
+        }
+    }
 
     notifyAbout(success, warning, error);
 }
@@ -733,28 +750,55 @@ ShareObserver::Result ShareObserver::exportIntoReferenceUnsignedContainer(const
 QList<ShareObserver::Result> ShareObserver::exportIntoReferenceContainers()
 {
     QList<Result> results;
+    struct Reference
+    {
+        KeeShareSettings::Reference config;
+        const Group* group;
+    };
+
+    QMap<QString, QList<Reference>> references;
     const auto groups = m_db->rootGroup()->groupsRecursive(true);
     for (const auto* group : groups) {
         const auto reference = KeeShare::referenceOf(group);
         if (!reference.isExporting()) {
             continue;
         }
+        references[reference.path] << Reference{reference, group};
+    }
 
-        m_fileWatcher->ignoreFileChanges(reference.path);
-        QScopedPointer<Database> targetDb(exportIntoContainer(reference, group));
-        QFileInfo info(reference.path);
+    for (auto it = references.cbegin(); it != references.cend(); ++it) {
+        if (it.value().count() != 1) {
+            const auto path = it.value().first().config.path;
+            QStringList groups;
+            for (const auto& reference : it.value()) {
+                groups << reference.group->name();
+            }
+            results << Result{
+                path, Result::Error, tr("Conflicting export target path %1 in %2").arg(path, groups.join(", "))};
+        }
+    }
+    if (!results.isEmpty()) {
+        // We need to block export due to config
+        return results;
+    }
+
+    for (auto it = references.cbegin(); it != references.cend(); ++it) {
+        const auto& reference = it.value().first();
+        m_fileWatcher->ignoreFileChanges(reference.config.path);
+        QScopedPointer<Database> targetDb(exportIntoContainer(reference.config, reference.group));
+        QFileInfo info(reference.config.path);
         if (isOfExportType(info, KeeShare::signedContainerFileType())) {
-            results << exportIntoReferenceSignedContainer(reference, targetDb.data());
+            results << exportIntoReferenceSignedContainer(reference.config, targetDb.data());
             m_fileWatcher->observeFileChanges(true);
             continue;
         }
         if (isOfExportType(info, KeeShare::unsignedContainerFileType())) {
-            results << exportIntoReferenceUnsignedContainer(reference, targetDb.data());
+            results << exportIntoReferenceUnsignedContainer(reference.config, targetDb.data());
             m_fileWatcher->observeFileChanges(true);
             continue;
         }
         Q_ASSERT(false);
-        results << Result{reference.path, Result::Error, tr("Unexpected export error occurred")};
+        results << Result{reference.config.path, Result::Error, tr("Unexpected export error occurred")};
     }
     return results;
 }
@@ -767,6 +811,7 @@ void ShareObserver::handleDatabaseSaved()
     QStringList error;
     QStringList warning;
     QStringList success;
+
     const auto results = exportIntoReferenceContainers();
     for (const Result& result : results) {
         if (!result.isValid()) {

src/keeshare/group/EditGroupPageKeeShare.cpp

@@ -42,10 +42,10 @@ QWidget* EditGroupPageKeeShare::createWidget()
     return new EditGroupWidgetKeeShare();
 }
 
-void EditGroupPageKeeShare::set(QWidget* widget, Group* temporaryGroup)
+void EditGroupPageKeeShare::set(QWidget* widget, Group* temporaryGroup, QSharedPointer<Database> database)
 {
     EditGroupWidgetKeeShare* settingsWidget = reinterpret_cast<EditGroupWidgetKeeShare*>(widget);
-    settingsWidget->setGroup(temporaryGroup);
+    settingsWidget->setGroup(temporaryGroup, database);
 }
 
 void EditGroupPageKeeShare::assign(QWidget* widget)

src/keeshare/group/EditGroupPageKeeShare.h

@@ -30,7 +30,7 @@ public:
     QString name() override;
     QIcon icon() override;
     QWidget* createWidget() override;
-    void set(QWidget* widget, Group* temporaryGroup) override;
+    void set(QWidget* widget, Group* temporaryGroup, QSharedPointer<Database> database) override;
     void assign(QWidget* widget) override;
 };
 

src/keeshare/group/EditGroupWidgetKeeShare.cpp

@@ -85,12 +85,13 @@ EditGroupWidgetKeeShare::~EditGroupWidgetKeeShare()
 {
 }
 
-void EditGroupWidgetKeeShare::setGroup(Group* temporaryGroup)
+void EditGroupWidgetKeeShare::setGroup(Group* temporaryGroup, QSharedPointer<Database> database)
 {
     if (m_temporaryGroup) {
         m_temporaryGroup->disconnect(this);
     }
 
+    m_database = database;
     m_temporaryGroup = temporaryGroup;
 
     if (m_temporaryGroup) {
@@ -128,9 +129,43 @@ void EditGroupWidgetKeeShare::showSharingState()
                     .arg(supportedExtensions.join(", ")),
                 MessageWidget::Warning);
             return;
-        } else {
-            m_ui->messageWidget->hide();
         }
+
+        const auto groups = m_database->rootGroup()->groupsRecursive(true);
+        bool conflictExport = false;
+        bool multipleImport = false;
+        bool cycleImportExport = false;
+        for (const auto* group : groups) {
+            if (group->uuid() == m_temporaryGroup->uuid()) {
+                continue;
+            }
+            const auto other = KeeShare::referenceOf(group);
+            if (other.path != reference.path) {
+                continue;
+            }
+            multipleImport |= other.isImporting() && reference.isImporting();
+            conflictExport |= other.isExporting() && reference.isExporting();
+            cycleImportExport |=
+                (other.isImporting() && reference.isExporting()) || (other.isExporting() && reference.isImporting());
+        }
+        if (conflictExport) {
+            m_ui->messageWidget->showMessage(tr("The export container %1 is already referenced.").arg(reference.path),
+                                             MessageWidget::Error);
+            return;
+        }
+        if (multipleImport) {
+            m_ui->messageWidget->showMessage(tr("The import container %1 is already imported.").arg(reference.path),
+                                             MessageWidget::Warning);
+            return;
+        }
+        if (cycleImportExport) {
+            m_ui->messageWidget->showMessage(
+                tr("The container %1 imported and export by different groups.").arg(reference.path),
+                MessageWidget::Warning);
+            return;
+        }
+
+        m_ui->messageWidget->hide();
     }
     const auto active = KeeShare::active();
     if (!active.in && !active.out) {

src/keeshare/group/EditGroupWidgetKeeShare.h

@@ -37,7 +37,7 @@ public:
     explicit EditGroupWidgetKeeShare(QWidget* parent = nullptr);
     ~EditGroupWidgetKeeShare();
 
-    void setGroup(Group* temporaryGroup);
+    void setGroup(Group* temporaryGroup, QSharedPointer<Database> database);
 
 private slots:
     void showSharingState();
@@ -55,6 +55,7 @@ private slots:
 private:
     QScopedPointer<Ui::EditGroupWidgetKeeShare> m_ui;
     QPointer<Group> m_temporaryGroup;
+    QSharedPointer<Database> m_database;
 };
 
 #endif // KEEPASSXC_EDITGROUPWIDGETKEESHARE_H