-
Notifications
You must be signed in to change notification settings - Fork 915
gh-9345 Fix E2E encrypted folders removed and unchecked on restart (v3.15.3→4.0.4 regression) - option 2 #9379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
d204214
0a9e82e
b64a9ed
30ea5ff
4e8b46e
3b3847c
66cca74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1497,7 +1497,10 @@ void AccountSettings::slotSelectiveSyncChanged(const QModelIndex &topLeft, | |||||
|
|
||||||
| void AccountSettings::slotPossiblyUnblacklistE2EeFoldersAndRestartSync() | ||||||
| { | ||||||
| qCInfo(lcAccountSettings) << "E2E restoration triggered"; | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I do not think we need info level (always present) but rather may want this in debug mode only |
||||||
|
|
||||||
| if (!_accountState->account()->e2e()->isInitialized()) { | ||||||
| qCInfo(lcAccountSettings) << "E2E not initialized, skipping restoration"; | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -1512,21 +1515,27 @@ void AccountSettings::slotPossiblyUnblacklistE2EeFoldersAndRestartSync() | |||||
| if (foldersToRemoveFromBlacklist.isEmpty()) { | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| qCInfo(lcAccountSettings) << "Found E2E folders to restore:" << foldersToRemoveFromBlacklist; | ||||||
|
|
||||||
| auto blackList = folder->journalDb()->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &ok); | ||||||
| const auto blackListSize = blackList.size(); | ||||||
| if (blackListSize == 0) { | ||||||
| continue; | ||||||
| } | ||||||
| qCInfo(lcAccountSettings) << "Current blacklist:" << blackList; | ||||||
|
|
||||||
| // Remove E2E folders from blacklist | ||||||
| for (const auto &pathToRemoveFromBlackList : foldersToRemoveFromBlacklist) { | ||||||
| blackList.removeAll(pathToRemoveFromBlackList); | ||||||
| } | ||||||
| if (blackList.size() != blackListSize) { | ||||||
| if (folder->isSyncRunning()) { | ||||||
| folderTerminateSyncAndUpdateBlackList(blackList, folder, foldersToRemoveFromBlacklist); | ||||||
| return; | ||||||
| } | ||||||
| updateBlackListAndScheduleFolderSync(blackList, folder, foldersToRemoveFromBlacklist); | ||||||
|
|
||||||
| qCInfo(lcAccountSettings) << "New blacklist after removal:" << blackList; | ||||||
|
|
||||||
| // Always update even if blacklist becomes empty - we need to trigger restoration | ||||||
| if (folder->isSyncRunning()) { | ||||||
| qCInfo(lcAccountSettings) << "Folder is syncing, will terminate and update blacklist"; | ||||||
| folderTerminateSyncAndUpdateBlackList(blackList, folder, foldersToRemoveFromBlacklist); | ||||||
| return; | ||||||
| } | ||||||
| qCInfo(lcAccountSettings) << "Updating blacklist and scheduling sync"; | ||||||
| updateBlackListAndScheduleFolderSync(blackList, folder, foldersToRemoveFromBlacklist); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -1681,11 +1690,11 @@ void AccountSettings::customizeStyle() | |||||
|
|
||||||
| void AccountSettings::setupE2eEncryption() | ||||||
| { | ||||||
| connect(_accountState->account()->e2e(), &ClientSideEncryption::initializationFinished, this, &AccountSettings::slotPossiblyUnblacklistE2EeFoldersAndRestartSync); | ||||||
|
|
||||||
| if (_accountState->account()->e2e()->isInitialized()) { | ||||||
| slotE2eEncryptionMnemonicReady(); | ||||||
| } else { | ||||||
| // Connect signal to restore E2E folders when initialization completes | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this comment is probably not that useful |
||||||
| connect(_accountState->account()->e2e(), &ClientSideEncryption::initializationFinished, this, &AccountSettings::slotPossiblyUnblacklistE2EeFoldersAndRestartSync); | ||||||
| setupE2eEncryptionMessage(); | ||||||
|
|
||||||
| connect(_accountState->account()->e2e(), &ClientSideEncryption::initializationFinished, this, [this] { | ||||||
|
|
@@ -1714,6 +1723,17 @@ void AccountSettings::forgetE2eEncryption() | |||||
| const auto account = _accountState->account(); | ||||||
| if (!account->e2e()->isInitialized()) { | ||||||
| FolderMan::instance()->removeE2eFiles(account); | ||||||
|
|
||||||
| // Clear E2E restoration tracking list for all folders | ||||||
| for (const auto folder : FolderMan::instance()->map()) { | ||||||
| if (folder->accountState()->account() == account) { | ||||||
| folder->journalDb()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncE2eFoldersToRemoveFromBlacklist, {}); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Reset E2E initialization state to allow re-setup | ||||||
| account->setE2eEncryptionKeysGenerationAllowed(false); | ||||||
| account->setAskUserForMnemonic(false); | ||||||
|
Comment on lines
+1734
to
+1736
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I fail to see the point of this change |
||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
| #include "configfile.h" | ||
| #include "connectionvalidator.h" | ||
| #include "creds/abstractcredentials.h" | ||
| #include "e2efoldermanager.h" | ||
| #include "editlocallymanager.h" | ||
| #include "folder.h" | ||
| #include "folderman.h" | ||
|
|
@@ -504,6 +505,9 @@ void Application::setupAccountsAndFolders() | |
| const auto foldersListSize = FolderMan::instance()->setupFolders(); | ||
| FolderMan::instance()->setSyncEnabled(true); | ||
|
|
||
| // Initialize E2E folder restoration manager | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please remove the comment |
||
| E2EFolderManager::instance()->initialize(); | ||
|
|
||
| const auto prettyNamesList = [](const QList<AccountStatePtr> &accounts) { | ||
| QStringList list; | ||
| for (const auto &account : accounts) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,164 @@ | ||||||
| // SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have 2026 now:
Suggested change
|
||||||
| // SPDX-License-Identifier: GPL-2.0-or-later | ||||||
|
|
||||||
| #include "e2efoldermanager.h" | ||||||
| #include "accountmanager.h" | ||||||
| #include "clientsideencryption.h" | ||||||
| #include "folderman.h" | ||||||
| #include "folder.h" | ||||||
|
|
||||||
| #include <QLoggingCategory> | ||||||
|
|
||||||
| namespace OCC { | ||||||
|
|
||||||
| Q_LOGGING_CATEGORY(lcE2eFolderManager, "nextcloud.gui.e2efoldermanager", QtInfoMsg) | ||||||
|
|
||||||
| E2EFolderManager *E2EFolderManager::_instance = nullptr; | ||||||
|
|
||||||
| E2EFolderManager *E2EFolderManager::instance() | ||||||
| { | ||||||
| if (!_instance) { | ||||||
| _instance = new E2EFolderManager(); | ||||||
| } | ||||||
| return _instance; | ||||||
| } | ||||||
|
|
||||||
| E2EFolderManager::E2EFolderManager(QObject *parent) | ||||||
| : QObject(parent) | ||||||
| { | ||||||
| qCInfo(lcE2eFolderManager) << "E2EFolderManager created"; | ||||||
| } | ||||||
|
|
||||||
| E2EFolderManager::~E2EFolderManager() | ||||||
| { | ||||||
| _instance = nullptr; | ||||||
| } | ||||||
|
|
||||||
| void E2EFolderManager::initialize() | ||||||
| { | ||||||
| qCInfo(lcE2eFolderManager) << "Initializing E2EFolderManager"; | ||||||
|
|
||||||
| // Connect to existing accounts | ||||||
| const auto accounts = AccountManager::instance()->accounts(); | ||||||
| for (const auto &accountState : accounts) { | ||||||
| if (accountState && accountState->account() && accountState->account()->e2e()) { | ||||||
| connectE2eSignals(accountState->account()); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Connect to new accounts being added | ||||||
| connect(AccountManager::instance(), &AccountManager::accountAdded, | ||||||
| this, &E2EFolderManager::slotAccountAdded); | ||||||
|
|
||||||
| qCInfo(lcE2eFolderManager) << "E2EFolderManager initialized for" << accounts.size() << "accounts"; | ||||||
| } | ||||||
|
Comment on lines
+37
to
+54
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since E2EFolderManager is a singleton anyway, couldn't this initialisation step be done in its constructor already? The AccountManager instance is already present (or will be created) when |
||||||
|
|
||||||
| void E2EFolderManager::slotAccountAdded(AccountState *accountState) | ||||||
| { | ||||||
| if (accountState && accountState->account() && accountState->account()->e2e()) { | ||||||
| qCInfo(lcE2eFolderManager) << "New account added, connecting E2E signals:" << accountState->account()->displayName(); | ||||||
| connectE2eSignals(accountState->account()); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| void E2EFolderManager::connectE2eSignals(const AccountPtr &account) | ||||||
| { | ||||||
| if (!account || !account->e2e()) { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| qCInfo(lcE2eFolderManager) << "Connecting E2E initialization signal for account:" << account->displayName(); | ||||||
|
|
||||||
| connect(account->e2e(), &ClientSideEncryption::initializationFinished, | ||||||
| this, &E2EFolderManager::slotE2eInitializationFinished, Qt::UniqueConnection); | ||||||
|
|
||||||
| // If E2E is already initialized, restore folders immediately | ||||||
| if (account->e2e()->isInitialized()) { | ||||||
| qCInfo(lcE2eFolderManager) << "E2E already initialized for account:" << account->displayName() | ||||||
| << ", restoring folders immediately"; | ||||||
| restoreE2eFoldersForAccount(account); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| void E2EFolderManager::slotE2eInitializationFinished() | ||||||
| { | ||||||
| qCInfo(lcE2eFolderManager) << "E2E initialization finished, restoring blacklisted E2E folders"; | ||||||
|
|
||||||
| auto *e2e = qobject_cast<ClientSideEncryption *>(sender()); | ||||||
| if (!e2e) { | ||||||
| qCWarning(lcE2eFolderManager) << "slotE2eInitializationFinished called but sender is not ClientSideEncryption"; | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| // Find the account this E2E belongs to | ||||||
| const auto accounts = AccountManager::instance()->accounts(); | ||||||
| for (const auto &accountState : accounts) { | ||||||
| if (accountState->account()->e2e() == e2e) { | ||||||
| restoreE2eFoldersForAccount(accountState->account()); | ||||||
| break; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| void E2EFolderManager::restoreE2eFoldersForAccount(const AccountPtr &account) | ||||||
| { | ||||||
| if (!account || !account->e2e() || !account->e2e()->isInitialized()) { | ||||||
| qCDebug(lcE2eFolderManager) << "Cannot restore folders - account or E2E not ready"; | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| qCInfo(lcE2eFolderManager) << "Restoring E2E folders for account:" << account->displayName(); | ||||||
|
|
||||||
| auto *folderMan = FolderMan::instance(); | ||||||
| const auto folders = folderMan->map(); | ||||||
|
|
||||||
| int foldersProcessed = 0; | ||||||
| for (const auto &folder : folders) { | ||||||
| if (folder->accountState()->account() != account) { | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| bool ok = false; | ||||||
| const auto foldersToRemoveFromBlacklist = folder->journalDb()->getSelectiveSyncList( | ||||||
| SyncJournalDb::SelectiveSyncE2eFoldersToRemoveFromBlacklist, &ok); | ||||||
|
|
||||||
| if (foldersToRemoveFromBlacklist.isEmpty()) { | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| qCInfo(lcE2eFolderManager) << "Found E2E folders to restore for" << folder->alias() | ||||||
| << ":" << foldersToRemoveFromBlacklist; | ||||||
|
|
||||||
| auto blackList = folder->journalDb()->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &ok); | ||||||
| qCDebug(lcE2eFolderManager) << "Current blacklist:" << blackList; | ||||||
|
|
||||||
| // Remove E2E folders from blacklist | ||||||
| for (const auto &pathToRemoveFromBlackList : foldersToRemoveFromBlacklist) { | ||||||
| blackList.removeAll(pathToRemoveFromBlackList); | ||||||
| } | ||||||
|
|
||||||
| qCInfo(lcE2eFolderManager) << "New blacklist after E2E folder removal:" << blackList; | ||||||
|
|
||||||
| // Update database | ||||||
| folder->journalDb()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, blackList); | ||||||
| folder->journalDb()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncE2eFoldersToRemoveFromBlacklist, {}); | ||||||
|
|
||||||
| // Schedule remote discovery for restored folders | ||||||
| for (const auto &pathToRemoteDiscover : foldersToRemoveFromBlacklist) { | ||||||
| folder->journalDb()->schedulePathForRemoteDiscovery(pathToRemoteDiscover); | ||||||
| qCDebug(lcE2eFolderManager) << "Scheduled remote discovery for:" << pathToRemoteDiscover; | ||||||
| } | ||||||
|
|
||||||
| // Schedule folder sync | ||||||
| folderMan->scheduleFolder(folder); | ||||||
| foldersProcessed++; | ||||||
| } | ||||||
|
|
||||||
| if (foldersProcessed > 0) { | ||||||
| qCInfo(lcE2eFolderManager) << "Restored E2E folders for" << foldersProcessed << "sync folders"; | ||||||
| } else { | ||||||
| qCDebug(lcE2eFolderManager) << "No E2E folders needed restoration"; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| } // namespace OCC | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,68 @@ | ||||||
| // SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have 2026 now:
Suggested change
|
||||||
| // SPDX-License-Identifier: GPL-2.0-or-later | ||||||
|
|
||||||
| #pragma once | ||||||
|
|
||||||
| #include <QObject> | ||||||
| #include "account.h" | ||||||
| #include "accountmanager.h" | ||||||
|
|
||||||
| namespace OCC { | ||||||
|
|
||||||
| /** | ||||||
| * @brief Stateless bridge between E2E encryption and folder management | ||||||
| * | ||||||
| * This class acts as a mediator that: | ||||||
| * - Listens to E2E initialization signals from all accounts | ||||||
| * - Coordinates folder restoration when E2E becomes ready | ||||||
| * - Keeps E2E concerns separate from FolderMan's core responsibilities | ||||||
| * | ||||||
| * @ingroup gui | ||||||
| */ | ||||||
| class E2EFolderManager : public QObject | ||||||
| { | ||||||
| Q_OBJECT | ||||||
|
|
||||||
| public: | ||||||
| static E2EFolderManager *instance(); | ||||||
| ~E2EFolderManager() override; | ||||||
|
|
||||||
| /** | ||||||
| * Initialize the manager and connect to existing accounts | ||||||
| * Should be called once during application startup | ||||||
| */ | ||||||
| void initialize(); | ||||||
|
|
||||||
| private slots: | ||||||
| /** | ||||||
| * Called when E2E initialization completes for any account | ||||||
| * Triggers restoration of blacklisted E2E folders for that account | ||||||
| */ | ||||||
| void slotE2eInitializationFinished(); | ||||||
|
|
||||||
| /** | ||||||
| * Called when a new account is added | ||||||
| * Connects E2E signals for the new account | ||||||
| */ | ||||||
| void slotAccountAdded(AccountState *accountState); | ||||||
|
|
||||||
| private: | ||||||
| E2EFolderManager(QObject *parent = nullptr); | ||||||
|
|
||||||
| /** | ||||||
| * Connect E2E initialization signals for an account | ||||||
| * @param account The account to connect signals for | ||||||
| */ | ||||||
| void connectE2eSignals(const AccountPtr &account); | ||||||
|
|
||||||
| /** | ||||||
| * Restore E2E folders for a specific account | ||||||
| * Removes E2E folders from blacklist and schedules sync | ||||||
| * @param account The account to restore folders for | ||||||
| */ | ||||||
| void restoreE2eFoldersForAccount(const AccountPtr &account); | ||||||
|
|
||||||
| static E2EFolderManager *_instance; | ||||||
| }; | ||||||
|
|
||||||
| } // namespace OCC | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -113,8 +113,7 @@ void FolderWatcher::performSetPermissionsTest(const QString &path) | |
|
|
||
| if (!QFile::exists(path)) { | ||
| QFile f(path); | ||
| f.open(QIODevice::WriteOnly); | ||
| if (!f.isOpen()) { | ||
| if (!f.open(QIODevice::WriteOnly)) { | ||
| qCWarning(lcFolderWatcher()) << "Failed to create test file: " << path; | ||
| return; | ||
| } | ||
|
|
@@ -158,7 +157,7 @@ void FolderWatcher::startNotificationTestWhenReady() | |
| FileSystem::setModTime(path, mtime + 1); | ||
| } else { | ||
| QFile f(path); | ||
| f.open(QIODevice::WriteOnly | QIODevice::Append); | ||
| [[maybe_unused]] bool opened = f.open(QIODevice::WriteOnly | QIODevice::Append); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if the |
||
| } | ||
| FileSystem::setFileHidden(path, true); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you are to use an abbreviation, please use
e2eethat stand for end-to-end encryption (hence the twoeat the end)