Skip to content

Commit

Permalink
Merge pull request #7436 from nextcloud/bugfix/accounts2
Browse files Browse the repository at this point in the history
Fix adding account and skipping folder configuration crash.
  • Loading branch information
mgallien authored Nov 22, 2024
2 parents 14581d2 + a7be4d7 commit 408955e
Show file tree
Hide file tree
Showing 12 changed files with 143 additions and 144 deletions.
88 changes: 44 additions & 44 deletions src/gui/accountmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ AccountManager::AccountsRestoreResult AccountManager::restore(const bool alsoRes
if (const auto acc = loadAccountHelper(*settings)) {
acc->_id = accountId;
const auto accState = new AccountState(acc);
const auto jar = qobject_cast<CookieJar*>(acc->_am->cookieJar());
const auto jar = qobject_cast<CookieJar*>(acc->_networkAccessManager->cookieJar());
Q_ASSERT(jar);
if (jar) {
jar->restore(acc->cookieJarPath());
Expand Down Expand Up @@ -305,12 +305,12 @@ void AccountManager::save(bool saveCredentials)
qCInfo(lcAccountManager) << "Saved all account settings, status:" << settings->status();
}

void AccountManager::saveAccount(Account *a)
void AccountManager::saveAccount(Account *newAccountData)
{
qCDebug(lcAccountManager) << "Saving account" << a->url().toString();
qCDebug(lcAccountManager) << "Saving account" << newAccountData->url().toString();
const auto settings = ConfigFile::settingsWithGroup(QLatin1String(accountsC));
settings->beginGroup(a->id());
saveAccountHelper(a, *settings, false); // don't save credentials they might not have been loaded yet
settings->beginGroup(newAccountData->id());
saveAccountHelper(newAccountData, *settings, false); // don't save credentials they might not have been loaded yet
settings->endGroup();

settings->sync();
Expand All @@ -328,36 +328,36 @@ void AccountManager::saveAccountState(AccountState *a)
qCDebug(lcAccountManager) << "Saved account state settings, status:" << settings->status();
}

void AccountManager::saveAccountHelper(Account *acc, QSettings &settings, bool saveCredentials)
void AccountManager::saveAccountHelper(Account *account, QSettings &settings, bool saveCredentials)
{
qCDebug(lcAccountManager) << "Saving settings to" << settings.fileName();
settings.setValue(QLatin1String(versionC), maxAccountVersion);
settings.setValue(QLatin1String(urlC), acc->_url.toString());
settings.setValue(QLatin1String(davUserC), acc->_davUser);
settings.setValue(QLatin1String(displayNameC), acc->_displayName);
settings.setValue(QLatin1String(serverVersionC), acc->_serverVersion);
settings.setValue(QLatin1String(serverColorC), acc->_serverColor);
settings.setValue(QLatin1String(serverTextColorC), acc->_serverTextColor);
settings.setValue(QLatin1String(serverHasValidSubscriptionC), acc->serverHasValidSubscription());

if (!acc->_skipE2eeMetadataChecksumValidation) {
settings.setValue(QLatin1String(urlC), account->_url.toString());
settings.setValue(QLatin1String(davUserC), account->_davUser);
settings.setValue(QLatin1String(displayNameC), account->davDisplayName());
settings.setValue(QLatin1String(serverVersionC), account->_serverVersion);
settings.setValue(QLatin1String(serverColorC), account->_serverColor);
settings.setValue(QLatin1String(serverTextColorC), account->_serverTextColor);
settings.setValue(QLatin1String(serverHasValidSubscriptionC), account->serverHasValidSubscription());

if (!account->_skipE2eeMetadataChecksumValidation) {
settings.remove(QLatin1String(skipE2eeMetadataChecksumValidationC));
} else {
settings.setValue(QLatin1String(skipE2eeMetadataChecksumValidationC), acc->_skipE2eeMetadataChecksumValidation);
settings.setValue(QLatin1String(skipE2eeMetadataChecksumValidationC), account->_skipE2eeMetadataChecksumValidation);
}
settings.setValue(networkProxySettingC, static_cast<std::underlying_type_t<Account::AccountNetworkProxySetting>>(acc->networkProxySetting()));
settings.setValue(networkProxyTypeC, acc->proxyType());
settings.setValue(networkProxyHostNameC, acc->proxyHostName());
settings.setValue(networkProxyPortC, acc->proxyPort());
settings.setValue(networkProxyNeedsAuthC, acc->proxyNeedsAuth());
settings.setValue(networkProxyUserC, acc->proxyUser());
settings.setValue(networkUploadLimitSettingC, static_cast<std::underlying_type_t<Account::AccountNetworkTransferLimitSetting>>(acc->uploadLimitSetting()));
settings.setValue(networkDownloadLimitSettingC, static_cast<std::underlying_type_t<Account::AccountNetworkTransferLimitSetting>>(acc->downloadLimitSetting()));
settings.setValue(networkUploadLimitC, acc->uploadLimit());
settings.setValue(networkDownloadLimitC, acc->downloadLimit());

const auto proxyPasswordKey = QString(acc->userIdAtHostWithPort() + networkProxyPasswordKeychainKeySuffixC);
if (const auto proxyPassword = acc->proxyPassword(); proxyPassword.isEmpty()) {
settings.setValue(networkProxySettingC, static_cast<std::underlying_type_t<Account::AccountNetworkProxySetting>>(account->networkProxySetting()));
settings.setValue(networkProxyTypeC, account->proxyType());
settings.setValue(networkProxyHostNameC, account->proxyHostName());
settings.setValue(networkProxyPortC, account->proxyPort());
settings.setValue(networkProxyNeedsAuthC, account->proxyNeedsAuth());
settings.setValue(networkProxyUserC, account->proxyUser());
settings.setValue(networkUploadLimitSettingC, static_cast<std::underlying_type_t<Account::AccountNetworkTransferLimitSetting>>(account->uploadLimitSetting()));
settings.setValue(networkDownloadLimitSettingC, static_cast<std::underlying_type_t<Account::AccountNetworkTransferLimitSetting>>(account->downloadLimitSetting()));
settings.setValue(networkUploadLimitC, account->uploadLimit());
settings.setValue(networkDownloadLimitC, account->downloadLimit());

const auto proxyPasswordKey = QString(account->userIdAtHostWithPort() + networkProxyPasswordKeychainKeySuffixC);
if (const auto proxyPassword = account->proxyPassword(); proxyPassword.isEmpty()) {
const auto job = new QKeychain::DeletePasswordJob(Theme::instance()->appName(), this);
job->setKey(proxyPasswordKey);
connect(job, &QKeychain::Job::finished, this, [](const QKeychain::Job *const incomingJob) {
Expand All @@ -384,37 +384,37 @@ void AccountManager::saveAccountHelper(Account *acc, QSettings &settings, bool s
job->start();
}

if (acc->_credentials) {
if (account->_credentials) {
if (saveCredentials) {
// Only persist the credentials if the parameter is set, on migration from 1.8.x
// we want to save the accounts but not overwrite the credentials
// (This is easier than asynchronously fetching the credentials from keychain and then
// re-persisting them)
acc->_credentials->persist();
account->_credentials->persist();
}

const auto settingsMapKeys = acc->_settingsMap.keys();
const auto settingsMapKeys = account->_settingsMap.keys();
for (const auto &key : settingsMapKeys) {
if (!acc->_settingsMap.value(key).isValid()) {
if (!account->_settingsMap.value(key).isValid()) {
continue;
}

settings.setValue(key, acc->_settingsMap.value(key));
settings.setValue(key, account->_settingsMap.value(key));
}
settings.setValue(QLatin1String(authTypeC), acc->_credentials->authType());
settings.setValue(QLatin1String(authTypeC), account->_credentials->authType());

// HACK: Save http_user also as user
const auto settingsMap = acc->_settingsMap;
const auto settingsMap = account->_settingsMap;
if (settingsMap.contains(httpUserC) && settingsMap.value(httpUserC).isValid()) {
settings.setValue(userC, settingsMap.value(httpUserC));
}
}

// Save accepted certificates.
settings.beginGroup(QLatin1String(generalC));
qCInfo(lcAccountManager) << "Saving " << acc->approvedCerts().count() << " unknown certs.";
qCInfo(lcAccountManager) << "Saving " << account->approvedCerts().count() << " unknown certs.";
QByteArray certs;
const auto approvedCerts = acc->approvedCerts();
const auto approvedCerts = account->approvedCerts();
for (const auto &cert : approvedCerts) {
certs += cert.toPem() + '\n';
}
Expand All @@ -424,13 +424,13 @@ void AccountManager::saveAccountHelper(Account *acc, QSettings &settings, bool s
settings.endGroup();

// Save cookies.
if (acc->_am) {
auto *jar = qobject_cast<CookieJar *>(acc->_am->cookieJar());
if (account->_networkAccessManager) {
const auto jar = qobject_cast<CookieJar *>(account->_networkAccessManager->cookieJar());
if (jar) {
qCInfo(lcAccountManager) << "Saving cookies." << acc->cookieJarPath();
if (!jar->save(acc->cookieJarPath()))
qCInfo(lcAccountManager) << "Saving cookies." << account->cookieJarPath();
if (!jar->save(account->cookieJarPath()))
{
qCWarning(lcAccountManager) << "Failed to save cookies to" << acc->cookieJarPath();
qCWarning(lcAccountManager) << "Failed to save cookies to" << account->cookieJarPath();
}
}
}
Expand Down Expand Up @@ -498,7 +498,7 @@ AccountPtr AccountManager::loadAccountHelper(QSettings &settings)
acc->_davUser = settings.value(QLatin1String(davUserC)).toString();

acc->_settingsMap.insert(QLatin1String(userC), settings.value(userC));
acc->_displayName = settings.value(QLatin1String(displayNameC), "").toString();
acc->setDavDisplayName(settings.value(QLatin1String(displayNameC), "").toString());
const QString authTypePrefix = authType + "_";
const auto settingsChildKeys = settings.childKeys();
for (const auto &key : settingsChildKeys) {
Expand Down
4 changes: 2 additions & 2 deletions src/gui/accountmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ class AccountManager : public QObject
static void backwardMigrationSettingsKeys(QStringList *deleteKeys, QStringList *ignoreKeys);

public slots:
/// Saves account data, not including the credentials
void saveAccount(OCC::Account *a);
/// Saves account data when adding user, when updating e.g. dav user, not including the credentials
void saveAccount(OCC::Account *newAccountData);

/// Saves account state data, not including the account
void saveAccountState(OCC::AccountState *a);
Expand Down
4 changes: 3 additions & 1 deletion src/gui/accountstate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,10 @@ void AccountState::signOutByUi()

void AccountState::freshConnectionAttempt()
{
if (isConnected())
if (isConnected()) {
setState(Disconnected);
}

checkConnectivity();
}

Expand Down
11 changes: 3 additions & 8 deletions src/gui/application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ Application::Application(int &argc, char **argv)
}

_theme->setSystrayUseMonoIcons(ConfigFile().monoIcons());
connect(_theme, &Theme::systrayUseMonoIconsChanged, this, &Application::slotUseMonoIconsChanged);
connect(_theme, &Theme::systrayUseMonoIconsChanged, _gui, &ownCloudGui::slotComputeOverallSyncStatus);
connect(this, &Application::systemPaletteChanged,
_theme, &Theme::systemPaletteHasChanged);

Expand Down Expand Up @@ -589,7 +589,7 @@ void Application::slotAccountStateRemoved(AccountState *accountState)
{
if (_gui) {
disconnect(accountState, &AccountState::stateChanged,
_gui.data(), &ownCloudGui::slotAccountStateChanged);
_gui.data(), &ownCloudGui::slotComputeOverallSyncStatus);
disconnect(accountState->account().data(), &Account::serverVersionChanged,
_gui.data(), &ownCloudGui::slotTrayMessageIfServerUnsupported);
}
Expand All @@ -611,7 +611,7 @@ void Application::slotAccountStateRemoved(AccountState *accountState)
void Application::slotAccountStateAdded(AccountState *accountState)
{
connect(accountState, &AccountState::stateChanged,
_gui.data(), &ownCloudGui::slotAccountStateChanged);
_gui.data(), &ownCloudGui::slotComputeOverallSyncStatus);
connect(accountState->account().data(), &Account::serverVersionChanged,
_gui.data(), &ownCloudGui::slotTrayMessageIfServerUnsupported);
connect(accountState, &AccountState::termsOfServiceChanged,
Expand Down Expand Up @@ -716,11 +716,6 @@ void Application::setupLogging()
qCInfo(lcApplication) << "Arguments:" << qApp->arguments();
}

void Application::slotUseMonoIconsChanged(bool)
{
_gui->slotComputeOverallSyncStatus();
}

void Application::slotParseMessage(const QString &msg, QObject *)
{
if (msg.startsWith(QLatin1String("MSG_PARSEOPTIONS:"))) {
Expand Down
1 change: 0 additions & 1 deletion src/gui/application.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ public slots:
protected slots:
void slotParseMessage(const QString &, QObject *);
void slotCheckConnection();
void slotUseMonoIconsChanged(bool);
void slotCleanup();
void slotAccountStateAdded(OCC::AccountState *accountState);
void slotAccountStateRemoved(OCC::AccountState *accountState);
Expand Down
16 changes: 3 additions & 13 deletions src/gui/owncloudgui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ ownCloudGui::ownCloudGui(Application *parent)

FolderMan *folderMan = FolderMan::instance();
connect(folderMan, &FolderMan::folderSyncStateChange, this, &ownCloudGui::slotSyncStateChange);
connect(folderMan, &FolderMan::folderSyncStateChange, this, &ownCloudGui::slotComputeOverallSyncStatus);


#ifdef BUILD_FILE_PROVIDER_MODULE
connect(Mac::FileProvider::instance()->socketServer(), &Mac::FileProviderSocketServer::syncStateChanged, this, &ownCloudGui::slotComputeOverallSyncStatus);
Expand Down Expand Up @@ -240,8 +242,6 @@ void ownCloudGui::slotTrayClicked(QSystemTrayIcon::ActivationReason reason)

void ownCloudGui::slotSyncStateChange(Folder *folder)
{
slotComputeOverallSyncStatus();

if (!folder) {
return; // Valid, just a general GUI redraw was needed.
}
Expand All @@ -258,21 +258,11 @@ void ownCloudGui::slotSyncStateChange(Folder *folder)
}
}

void ownCloudGui::slotFoldersChanged()
{
slotComputeOverallSyncStatus();
}

void ownCloudGui::slotOpenPath(const QString &path)
{
showInFileManager(path);
}

void ownCloudGui::slotAccountStateChanged()
{
slotComputeOverallSyncStatus();
}

void ownCloudGui::slotTrayMessageIfServerUnsupported(Account *account)
{
if (account->serverVersionUnsupported()) {
Expand Down Expand Up @@ -371,7 +361,7 @@ void ownCloudGui::slotComputeOverallSyncStatus()
#else
QStringList messages;
messages.append(tr("Disconnected from accounts:"));
for (const auto &accountState : problemAccounts) {
for (const auto &accountState : qAsConst(problemAccounts)) {
QString message = tr("Account %1: %2").arg(accountState->account()->displayName(), accountState->stateString(accountState->state()));
if (!accountState->connectionErrors().empty()) {
message += QLatin1String("\n");
Expand Down
2 changes: 0 additions & 2 deletions src/gui/owncloudgui.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ public slots:
void slotFolderOpenAction(const QString &alias);
void slotUpdateProgress(const QString &folder, const OCC::ProgressInfo &progress);
void slotShowGuiMessage(const QString &title, const QString &message);
void slotFoldersChanged();
void slotShowSettings();
void slotShowSyncProtocol();
void slotShutdown();
Expand All @@ -92,7 +91,6 @@ public slots:
void slotSettingsDialogActivated();
void slotHelp();
void slotOpenPath(const QString &path);
void slotAccountStateChanged();
void slotTrayMessageIfServerUnsupported(OCC::Account *account);
void slotNeedToAcceptTermsOfService(OCC::AccountPtr account,
OCC::AccountState::State state);
Expand Down
26 changes: 15 additions & 11 deletions src/gui/owncloudsetupwizard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ OwncloudSetupWizard::~OwncloudSetupWizard()
_ocWizard->deleteLater();
}

static QPointer<OwncloudSetupWizard> wiz = nullptr;
static QPointer<OwncloudSetupWizard> owncloudSetupWizard = nullptr;

void OwncloudSetupWizard::runWizard(QObject *obj, const char *amember, QWidget *parent)
{
Expand All @@ -78,25 +78,26 @@ void OwncloudSetupWizard::runWizard(QObject *obj, const char *amember, QWidget *

Theme::instance()->setStartLoginFlowAutomatically(true);
}
if (!wiz.isNull()) {
if (!owncloudSetupWizard.isNull()) {
bringWizardToFrontIfVisible();
return;
}

wiz = new OwncloudSetupWizard(parent);
connect(wiz, SIGNAL(ownCloudWizardDone(int)), obj, amember);
connect(wiz->_ocWizard, &OwncloudWizard::wizardClosed, obj, [] { wiz.clear(); });
owncloudSetupWizard = new OwncloudSetupWizard(parent);
connect(owncloudSetupWizard, SIGNAL(ownCloudWizardDone(int)), obj, amember);
connect(owncloudSetupWizard->_ocWizard, &OwncloudWizard::wizardClosed, obj, [] { owncloudSetupWizard.clear(); });

FolderMan::instance()->setSyncEnabled(false);
wiz->startWizard();
owncloudSetupWizard->startWizard();
}

bool OwncloudSetupWizard::bringWizardToFrontIfVisible()
{
if (wiz.isNull()) {
if (owncloudSetupWizard.isNull()) {
return false;
}

ownCloudGui::raiseDialog(wiz->_ocWizard);
ownCloudGui::raiseDialog(owncloudSetupWizard->_ocWizard);
return true;
}

Expand Down Expand Up @@ -700,7 +701,7 @@ void OwncloudSetupWizard::slotAssistantFinished(int result)
}

// notify others.
_ocWizard->done(QWizard::Accepted);
_ocWizard->done(result);
emit ownCloudWizardDone(result);
}

Expand All @@ -710,7 +711,10 @@ void OwncloudSetupWizard::slotSkipFolderConfiguration()

disconnect(_ocWizard, &OwncloudWizard::basicSetupFinished,
this, &OwncloudSetupWizard::slotAssistantFinished);
_ocWizard->close();

_ocWizard->done(QDialog::Rejected);

// Accept to check connectivity, only skip folder setup
emit ownCloudWizardDone(QDialog::Accepted);
}

Expand All @@ -727,7 +731,7 @@ AccountState *OwncloudSetupWizard::applyAccountChanges()
auto manager = AccountManager::instance();

auto newState = manager->addAccount(newAccount);
manager->save();
manager->saveAccount(newAccount.data());
return newState;
}

Expand Down
2 changes: 1 addition & 1 deletion src/gui/settingsdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ void SettingsDialog::accountAdded(AccountState *s)
_actionForAccount.insert(s->account().data(), accountAction);
accountAction->trigger();

connect(accountSettings, &AccountSettings::folderChanged, _gui, &ownCloudGui::slotFoldersChanged);
connect(accountSettings, &AccountSettings::folderChanged, _gui, &ownCloudGui::slotComputeOverallSyncStatus);
connect(accountSettings, &AccountSettings::openFolderAlias,
_gui, &ownCloudGui::slotFolderOpenAction);
connect(accountSettings, &AccountSettings::showIssuesList, this, &SettingsDialog::showIssuesList);
Expand Down
1 change: 1 addition & 0 deletions src/gui/tray/TrayFoldersMenuButton.qml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* for more details.
*/
pragma NativeMethodBehavior: AcceptThisObject
import QtQuick
import QtQuick.Controls
import QtQuick.Layouts
Expand Down
Loading

0 comments on commit 408955e

Please sign in to comment.