Merge pull request #5640 from nextcloud/bugfix/folder-migration-from-old

Fix account migration from legacy desktop clients (again)

src/gui/accountmanager.cpp

@@ -36,6 +36,7 @@ constexpr auto userC = "user";
 constexpr auto displayNameC = "displayName";
 constexpr auto httpUserC = "http_user";
 constexpr auto davUserC = "dav_user";
+constexpr auto webflowUserC = "webflow_user";
 constexpr auto shibbolethUserC = "shibboleth_shib_user";
 constexpr auto caCertsKeyC = "CaCertificates";
 constexpr auto accountsC = "Accounts";
@@ -70,7 +71,7 @@ AccountManager *AccountManager::instance()
     return &instance;
 }
 
-bool AccountManager::restore(bool alsoRestoreLegacySettings)
+AccountManager::AccountsRestoreResult AccountManager::restore(const bool alsoRestoreLegacySettings)
 {
     QStringList skipSettingsKeys;
     backwardMigrationSettingsKeys(&skipSettingsKeys, &skipSettingsKeys);
@@ -79,21 +80,22 @@ bool AccountManager::restore(bool alsoRestoreLegacySettings)
     if (settings->status() != QSettings::NoError || !settings->isWritable()) {
         qCWarning(lcAccountManager) << "Could not read settings from" << settings->fileName()
                                     << settings->status();
-        return false;
+        return AccountsRestoreFailure;
     }
 
     if (skipSettingsKeys.contains(settings->group())) {
         // Should not happen: bad container keys should have been deleted
         qCWarning(lcAccountManager) << "Accounts structure is too new, ignoring";
-        return true;
+        return AccountsRestoreSuccessWithSkipped;
     }
 
     // If there are no accounts, check the old format.
     if (settings->childGroups().isEmpty() && !settings->contains(QLatin1String(versionC)) && alsoRestoreLegacySettings) {
         restoreFromLegacySettings();
-        return true;
+        return AccountsRestoreSuccessFromLegacyVersion;
     }
 
+    auto result = AccountsRestoreSuccess;
     const auto settingsChildGroups = settings->childGroups();
     for (const auto &accountId : settingsChildGroups) {
         settings->beginGroup(accountId);
@@ -111,11 +113,12 @@ bool AccountManager::restore(bool alsoRestoreLegacySettings)
         } else {
             qCInfo(lcAccountManager) << "Account" << accountId << "is too new, ignoring";
             _additionalBlockedAccountIds.insert(accountId);
+            result = AccountsRestoreSuccessWithSkipped;
         }
         settings->endGroup();
     }
 
-    return true;
+    return result;
 }
 
 void AccountManager::backwardMigrationSettingsKeys(QStringList *deleteKeys, QStringList *ignoreKeys)
@@ -220,6 +223,7 @@ bool AccountManager::restoreFromLegacySettings()
                     settings = std::move(oCSettings);
                 }
 
+                ConfigFile::setDiscoveredLegacyConfigPath(configFileInfo.canonicalPath());
                 break;
             } else {
                 qCInfo(lcAccountManager) << "Migrate: could not read old config " << configFile;
@@ -359,6 +363,8 @@ AccountPtr AccountManager::loadAccountHelper(QSettings &settings)
             authType = httpAuthTypeC;
         } else if (settings.contains(QLatin1String(shibbolethUserC))) {
             authType = shibbolethAuthTypeC;
+        } else if (settings.contains(webflowUserC)) {
+            authType = webflowAuthTypeC;
         }
     }
 

src/gui/accountmanager.h

@@ -27,6 +27,14 @@ class AccountManager : public QObject
 {
     Q_OBJECT
 public:
+    enum AccountsRestoreResult {
+        AccountsRestoreFailure = 0,
+        AccountsRestoreSuccess,
+        AccountsRestoreSuccessFromLegacyVersion,
+        AccountsRestoreSuccessWithSkipped
+    };
+    Q_ENUM (AccountsRestoreResult);
+
     static AccountManager *instance();
     ~AccountManager() override = default;
 
@@ -41,7 +49,7 @@ public:
      * Returns false if there was an error reading the settings,
      * but note that settings not existing is not an error.
      */
-    bool restore(bool alsoRestoreLegacySettings = true);
+    AccountsRestoreResult restore(const bool alsoRestoreLegacySettings = true);
 
     /**
      * Add this account in the list of saved accounts.

src/gui/application.cpp

@@ -377,12 +377,16 @@ Application::Application(int &argc, char **argv)
 
     connect(this, &SharedTools::QtSingleApplication::messageReceived, this, &Application::slotParseMessage);
 
-    if (!AccountManager::instance()->restore(cfg.overrideServerUrl().isEmpty())) {
+    const auto tryMigrate = cfg.overrideServerUrl().isEmpty();
+    auto accountsRestoreResult = AccountManager::AccountsRestoreFailure;
+    if (accountsRestoreResult = AccountManager::instance()->restore(tryMigrate);
+            accountsRestoreResult == AccountManager::AccountsRestoreFailure) {
         // If there is an error reading the account settings, try again
         // after a couple of seconds, if that fails, give up.
         // (non-existence is not an error)
         Utility::sleep(5);
-        if (!AccountManager::instance()->restore(cfg.overrideServerUrl().isEmpty())) {
+        if (accountsRestoreResult = AccountManager::instance()->restore(tryMigrate);
+                accountsRestoreResult == AccountManager::AccountsRestoreFailure) {
             qCCritical(lcApplication) << "Could not read the account settings, quitting";
             QMessageBox::critical(
                 nullptr,

src/gui/folderman.cpp

@@ -178,11 +178,11 @@ int FolderMan::setupFolders()
     auto settings = ConfigFile::settingsWithGroup(QLatin1String("Accounts"));
     const auto accountsWithSettings = settings->childGroups();
     if (accountsWithSettings.isEmpty()) {
-        int r = setupFoldersMigration();
+        const auto migratedFoldersCount = setupFoldersMigration();
-        if (r > 0) {
+        if (migratedFoldersCount > 0) {
             AccountManager::instance()->save(false); // don't save credentials, they had not been loaded from keychain
         }
-        return r;
+        return migratedFoldersCount;
     }
 
     qCInfo(lcFolderMan) << "Setup folders from settings file";
@@ -197,7 +197,7 @@ int FolderMan::setupFolders()
 
         // The "backwardsCompatible" flag here is related to migrating old
         // database locations
-        auto process = [&](const QString &groupName, bool backwardsCompatible, bool foldersWithPlaceholders) {
+        auto process = [&](const QString &groupName, const bool backwardsCompatible, const bool foldersWithPlaceholders) {
             settings->beginGroup(groupName);
             if (skipSettingsKeys.contains(settings->group())) {
                 // Should not happen: bad container keys should have been deleted
@@ -284,8 +284,8 @@ void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account,
                     qCWarning(lcFolderMan) << "Could not load plugin for mode" << folderDefinition.virtualFilesMode;
                 }
 
-                Folder *f = addFolderInternal(folderDefinition, account.data(), std::move(vfs));
+                const auto folder = addFolderInternal(folderDefinition, account.data(), std::move(vfs));
-                f->saveToSettings();
+                folder->saveToSettings();
 
                 continue;
             }
@@ -316,26 +316,25 @@ void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account,
                 qFatal("Could not load plugin");
             }
 
-            Folder *f = addFolderInternal(std::move(folderDefinition), account.data(), std::move(vfs));
+            if (const auto folder = addFolderInternal(std::move(folderDefinition), account.data(), std::move(vfs))) {
-            if (f) {
                 if (switchToVfs) {
-                    f->switchToVirtualFiles();
+                    folder->switchToVirtualFiles();
                 }
                 // Migrate the old "usePlaceholders" setting to the root folder pin state
                 if (settings.value(QLatin1String(settingsVersionC), 1).toInt() == 1
                     && settings.value(QLatin1String("usePlaceholders"), false).toBool()) {
                     qCInfo(lcFolderMan) << "Migrate: From usePlaceholders to PinState::OnlineOnly";
-                    f->setRootPinState(PinState::OnlineOnly);
+                    folder->setRootPinState(PinState::OnlineOnly);
                 }
 
                 // Migration: Mark folders that shall be saved in a backwards-compatible way
                 if (backwardsCompatible)
-                    f->setSaveBackwardsCompatible(true);
+                    folder->setSaveBackwardsCompatible(true);
                 if (foldersWithPlaceholders)
-                    f->setSaveInFoldersWithPlaceholders();
+                    folder->setSaveInFoldersWithPlaceholders();
 
-                scheduleFolder(f);
+                scheduleFolder(folder);
-                emit folderSyncStateChange(f);
+                emit folderSyncStateChange(folder);
             }
         }
         settings.endGroup();
@@ -348,20 +347,24 @@ int FolderMan::setupFoldersMigration()
     QDir storageDir(cfg.configPath());
     _folderConfigPath = cfg.configPath();
 
-    qCInfo(lcFolderMan) << "Setup folders from " << _folderConfigPath << "(migration)";
+    const auto legacyConfigPath = ConfigFile::discoveredLegacyConfigPath();
+    const auto configPath = legacyConfigPath.isEmpty() ? _folderConfigPath : legacyConfigPath;
 
-    QDir dir(_folderConfigPath);
+    qCInfo(lcFolderMan) << "Setup folders from " << configPath << "(migration)";
+
+    QDir dir(configPath);
     //We need to include hidden files just in case the alias starts with '.'
     dir.setFilter(QDir::Files | QDir::Hidden);
-    const auto list = dir.entryList();
+    const auto dirFiles = dir.entryList();
 
-    // Normally there should be only one account when migrating.
+    // Normally there should be only one account when migrating. TODO: Change
-    AccountState *accountState = AccountManager::instance()->accounts().value(0).data();
+    const auto accountState = AccountManager::instance()->accounts().value(0).data();
-    for (const auto &alias : list) {
+    for (const auto &fileName : dirFiles) {
-        Folder *f = setupFolderFromOldConfigFile(alias, accountState);
+        const auto fullFilePath = dir.filePath(fileName);
-        if (f) {
+        const auto folder = setupFolderFromOldConfigFile(fullFilePath, accountState);
-            scheduleFolder(f);
+        if (folder) {
-            emit folderSyncStateChange(f);
+            scheduleFolder(folder);
+            emit folderSyncStateChange(folder);
         }
     }
 
@@ -377,11 +380,11 @@ void FolderMan::backwardMigrationSettingsKeys(QStringList *deleteKeys, QStringLi
 
     auto processSubgroup = [&](const QString &name) {
         settings->beginGroup(name);
-        const int foldersVersion = settings->value(QLatin1String(settingsVersionC), 1).toInt();
+        const auto foldersVersion = settings->value(QLatin1String(settingsVersionC), 1).toInt();
         if (foldersVersion <= maxFoldersVersion) {
-            foreach (const auto &folderAlias, settings->childGroups()) {
+            for (const auto &folderAlias : settings->childGroups()) {
                 settings->beginGroup(folderAlias);
-                const int folderVersion = settings->value(QLatin1String(settingsVersionC), 1).toInt();
+                const auto folderVersion = settings->value(QLatin1String(settingsVersionC), 1).toInt();
                 if (folderVersion > FolderDefinition::maxSettingsVersion()) {
                     ignoreKeys->append(settings->group());
                 }
@@ -478,31 +481,27 @@ QString FolderMan::unescapeAlias(const QString &alias)
     return a;
 }
 
-// filename is the name of the file only, it does not include
-// the configuration directory path
 // WARNING: Do not remove this code, it is used for predefined/automated deployments (2016)
-Folder *FolderMan::setupFolderFromOldConfigFile(const QString &file, AccountState *accountState)
+Folder *FolderMan::setupFolderFromOldConfigFile(const QString &fileNamePath, AccountState *accountState)
 {
-    Folder *folder = nullptr;
+    qCInfo(lcFolderMan) << "  ` -> setting up:" << fileNamePath;
-
+    QString escapedFileNamePath(fileNamePath);
-    qCInfo(lcFolderMan) << "  ` -> setting up:" << file;
-    QString escapedAlias(file);
     // check the unescaped variant (for the case when the filename comes out
     // of the directory listing). If the file does not exist, escape the
     // file and try again.
-    QFileInfo cfgFile(_folderConfigPath, file);
+    QFileInfo cfgFile(fileNamePath);
 
     if (!cfgFile.exists()) {
         // try the escaped variant.
-        escapedAlias = escapeAlias(file);
+        escapedFileNamePath = escapeAlias(fileNamePath);
-        cfgFile.setFile(_folderConfigPath, escapedAlias);
+        cfgFile.setFile(_folderConfigPath, escapedFileNamePath);
     }
     if (!cfgFile.isReadable()) {
         qCWarning(lcFolderMan) << "Cannot read folder definition for alias " << cfgFile.filePath();
-        return folder;
+        return nullptr;
     }
 
-    QSettings settings(_folderConfigPath + QLatin1Char('/') + escapedAlias, QSettings::IniFormat);
+    QSettings settings(escapedFileNamePath, QSettings::IniFormat);
     qCInfo(lcFolderMan) << "    -> file path: " << settings.fileName();
 
     // Check if the filename is equal to the group setting. If not, use the group
@@ -510,7 +509,7 @@ Folder *FolderMan::setupFolderFromOldConfigFile(const QString &file, AccountStat
     const auto groups = settings.childGroups();
     if (groups.isEmpty()) {
         qCWarning(lcFolderMan) << "empty file:" << cfgFile.filePath();
-        return folder;
+        return nullptr;
     }
 
     if (!accountState) {
@@ -566,8 +565,7 @@ Folder *FolderMan::setupFolderFromOldConfigFile(const QString &file, AccountStat
             folderDefinition.paused = paused;
             folderDefinition.ignoreHiddenFiles = ignoreHiddenFiles;
 
-            folder = addFolderInternal(folderDefinition, accountState, std::make_unique<VfsOff>());
+            if (const auto folder = addFolderInternal(folderDefinition, accountState, std::make_unique<VfsOff>())) {
-            if (folder) {
                 const auto blackList = settings.value(QLatin1String("blackList")).toStringList();
                 if (!blackList.empty()) {
                     //migrate settings
@@ -578,11 +576,10 @@ Folder *FolderMan::setupFolderFromOldConfigFile(const QString &file, AccountStat
                 }
 
                 folder->saveToSettings();
-            }
-            qCInfo(lcFolderMan) << "Migrated!" << folder;
-            settings.sync();
 
-            if (folder) {
+                qCInfo(lcFolderMan) << "Migrated!" << folder;
+                settings.sync();
+
                 return folder;
             }
 
@@ -592,7 +589,8 @@ Folder *FolderMan::setupFolderFromOldConfigFile(const QString &file, AccountStat
         settings.endGroup();
         settings.endGroup();
     }
-    return folder;
+
+    return nullptr;
 }
 
 void FolderMan::slotFolderSyncPaused(Folder *f, bool paused)

src/libsync/configfile.cpp

@@ -114,8 +114,8 @@ namespace chrono = std::chrono;
 
 Q_LOGGING_CATEGORY(lcConfigFile, "nextcloud.sync.configfile", QtInfoMsg)
 
-QString ConfigFile::_confDir = QString();
+QString ConfigFile::_confDir = {};
-bool ConfigFile::_askedUser = false;
+QString ConfigFile::_discoveredLegacyConfigPath = {};
 
 static chrono::milliseconds millisecondsValue(const QSettings &setting, const char *key,
     chrono::milliseconds defaultValue)
@@ -1156,4 +1156,19 @@ void ConfigFile::setupDefaultExcludeFilePaths(ExcludedFiles &excludedFiles)
         excludedFiles.addExcludeFilePath(userList);
     }
 }
+
+QString ConfigFile::discoveredLegacyConfigPath()
+{
+    return _discoveredLegacyConfigPath;
+}
+
+void ConfigFile::setDiscoveredLegacyConfigPath(const QString &discoveredLegacyConfigPath)
+{
+    if (_discoveredLegacyConfigPath == discoveredLegacyConfigPath) {
+        return;
+    }
+
+    _discoveredLegacyConfigPath = discoveredLegacyConfigPath;
+}
+
 }

src/libsync/configfile.h

@@ -219,6 +219,10 @@ public:
     /// Add the system and user exclude file path to the ExcludedFiles instance.
     static void setupDefaultExcludeFilePaths(ExcludedFiles &excludedFiles);
 
+    /// Set during first time migration of legacy accounts in AccountManager
+    [[nodiscard]] static QString discoveredLegacyConfigPath();
+    static void setDiscoveredLegacyConfigPath(const QString &discoveredLegacyConfigPath);
+
 protected:
     [[nodiscard]] QVariant getPolicySetting(const QString &policy, const QVariant &defaultValue = QVariant()) const;
     void storeData(const QString &group, const QString &key, const QVariant &value);
@@ -236,9 +240,8 @@ private:
 private:
     using SharedCreds = QSharedPointer<AbstractCredentials>;
 
-    static bool _askedUser;
-    static QString _oCVersion;
     static QString _confDir;
+    static QString _discoveredLegacyConfigPath;
 };
 }
 #endif // CONFIGFILE_H