diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index 06152e0fbc..88b1c54e2f 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -323,6 +323,25 @@ bool FileSystem::openAndSeekFileSharedRead(QFile *file, QString *errorOrNull, qi #endif } +QString FileSystem::joinPath(const QString& path, const QString& file) +{ + if (path.isEmpty()) { + qCWarning(lcFileSystem).nospace() << "joinPath called with an empty path; returning file=" << file; + return QDir::toNativeSeparators(file); + } + + if (file.isEmpty()) { + qCWarning(lcFileSystem).nospace() << "joinPath called with an empty file; returning path=" << path; + return QDir::toNativeSeparators(path); + } + + if (const auto lastChar = path[path.size() - 1]; lastChar == QLatin1Char{'/'} || lastChar == QLatin1Char{'\\'}) { + return QDir::toNativeSeparators(path + file); + } + + return QDir::toNativeSeparators(path + QDir::separator() + file); +} + #ifdef Q_OS_WIN std::filesystem::perms FileSystem::filePermissionsWinSymlinkSafe(const QString &filename) { @@ -804,7 +823,7 @@ bool FileSystem::setAclPermission(const QString &unsafePath, FolderPermissions p const auto currentFolder = safePathFileInfo.dir(); const auto childFiles = currentFolder.entryList(QDir::Filter::Files); for (const auto &oneEntry : childFiles) { - const auto childFile = QDir::toNativeSeparators(path + QDir::separator() + oneEntry); + const auto childFile = joinPath(path, oneEntry); const auto &childFileStdWString = childFile.toStdWString(); const auto attributes = GetFileAttributes(childFileStdWString.c_str()); diff --git a/src/common/filesystembase.h b/src/common/filesystembase.h index d88bfd6107..9e115b6c9d 100644 --- a/src/common/filesystembase.h +++ b/src/common/filesystembase.h @@ -142,6 +142,16 @@ namespace FileSystem { */ bool OCSYNC_EXPORT openAndSeekFileSharedRead(QFile *file, QString *error, qint64 seek); + /** + * Returns `path + "/" + file` with native directory separators. + * + * If `path` ends in a directory separator this method will not insert another one in-between. + * + * In the case one of the parameters is empty, the other parameter will be returned with native + * directory separators and a warning is logged. + */ + QString OCSYNC_EXPORT joinPath(const QString &path, const QString &file); + #ifdef Q_OS_WIN /** * Returns the file system used at the given path. diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 763dc0fc16..829ed19554 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -1041,18 +1041,21 @@ void Folder::wipeForRemoval() { disconnectFolderWatcher(); + // Delete files that have been partially downloaded. + slotDiscardDownloadProgress(); + // Unregister the socket API so it does not keep the .sync_journal file open FolderMan::instance()->socketApi()->slotUnregisterPath(alias()); - _journal.close(); // close the sync journal + // Close the sync journal. Do NOT call any methods that fetch data from it + // after this point, otherwise the journal is re-opened. On some systems + // (Windows) this prevents the removal of the db file as it's open again... + _journal.close(); if (!QDir(path()).exists()) { qCCritical(lcFolder) << "db files are not going to be deleted, sync folder could not be found at" << path(); return; } - // Delete files that have been partially downloaded. - slotDiscardDownloadProgress(); - // Remove db and temporaries QString stateDbFile = _engine->journal()->databaseFilePath(); @@ -1704,7 +1707,7 @@ void Folder::disconnectFolderWatcher() bool Folder::virtualFilesEnabled() const { - return _definition.virtualFilesMode != Vfs::Off && !isVfsOnOffSwitchPending(); + return _definition.virtualFilesMode != Vfs::Off && !isVfsOnOffSwitchPending() && !_vfs.isNull(); } void Folder::slotAboutToRemoveAllFiles(SyncFileItem::Direction dir, std::function callback) diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 37c9c13cb3..8c1f1122ae 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -1550,7 +1550,7 @@ void FolderMan::slotWipeFolderForAccount(AccountState *accountState) // wipe data QDir userFolder(f->path()); if (userFolder.exists()) { - success = userFolder.removeRecursively(); + success = FileSystem::removeRecursively(f->path()); if (!success) { qCWarning(lcFolderMan) << "Failed to remove existing folder " << f->path(); } else { diff --git a/src/gui/folderman.h b/src/gui/folderman.h index 16e33b0880..bf3775ad11 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -26,6 +26,7 @@ class TestFolderStatusModel; class ShareTestHelper; class EndToEndTestHelper; class TestSyncConflictsModel; +class TestRemoteWipe; namespace OCC { @@ -413,6 +414,7 @@ private: friend class ::ShareTestHelper; friend class ::EndToEndTestHelper; friend class ::TestFolderStatusModel; + friend class ::TestRemoteWipe; }; } // namespace OCC diff --git a/src/gui/remotewipe.cpp b/src/gui/remotewipe.cpp index 327c4f9b86..6613443dec 100644 --- a/src/gui/remotewipe.cpp +++ b/src/gui/remotewipe.cpp @@ -19,26 +19,36 @@ RemoteWipe::RemoteWipe(AccountPtr account, QObject *parent) : QObject(parent), _account(account), _appPassword(QString()), - _networkManager(nullptr) + _networkManager{new QNetworkAccessManager{this}} { QObject::connect(AccountManager::instance(), &AccountManager::accountRemoved, - this, [=, this](AccountState *) { - _accountRemoved = true; + this, [=, this](AccountState *accountState) { + if (_account != accountState->account()) { + return; + } + + notifyServerSuccess(); }); + if (FolderMan::instance()) { + qCDebug(lcRemoteWipe) << "FolderMan instance is present, a remote wipe will clean up local data first"; + _canWipeLocalFiles = true; QObject::connect(this, &RemoteWipe::authorized, FolderMan::instance(), &FolderMan::slotWipeFolderForAccount); QObject::connect(FolderMan::instance(), &FolderMan::wipeDone, this, - &RemoteWipe::notifyServerSuccessJob); + &RemoteWipe::slotWipeDone); } QObject::connect(_account.data(), &Account::appPasswordRetrieved, this, &RemoteWipe::startCheckJobWithAppPassword); } -void RemoteWipe::startCheckJobWithAppPassword(QString pwd){ - if(pwd.isEmpty()) +void RemoteWipe::startCheckJobWithAppPassword(QString pwd) +{ + if (pwd.isEmpty()) { + qCDebug(lcRemoteWipe) << "not checking remote wipe status: app password is empty"; return; + } _appPassword = pwd; QUrl requestUrl = Utility::concatUrlPath(_account->url().toString(), @@ -51,14 +61,14 @@ void RemoteWipe::startCheckJobWithAppPassword(QString pwd){ auto requestBody = new QBuffer; QUrlQuery arguments(QStringLiteral("token=%1").arg(_appPassword)); requestBody->setData(arguments.query(QUrl::FullyEncoded).toLatin1()); - _networkReplyCheck = _networkManager.post(request, requestBody); - QObject::connect(&_networkManager, &QNetworkAccessManager::sslErrors, + _networkReplyCheck = _networkManager->post(request, requestBody); + QObject::connect(_networkManager, &QNetworkAccessManager::sslErrors, _account.data(), &Account::slotHandleSslErrors); QObject::connect(_networkReplyCheck, &QNetworkReply::finished, this, - &RemoteWipe::checkJobSlot); + &RemoteWipe::slotCheckJob); } -void RemoteWipe::checkJobSlot() +void RemoteWipe::slotCheckJob() { auto jsonData = _networkReplyCheck->readAll(); QJsonParseError jsonParseError{}; @@ -83,14 +93,16 @@ void RemoteWipe::checkJobSlot() } // check for wipe request - } else if(!json.value("wipe").isUndefined()){ + } else if (!json.value("wipe").isUndefined()) { wipe = json["wipe"].toBool(); } auto manager = AccountManager::instance(); auto accountState = manager->account(_account->displayName()).data(); - if(wipe){ + if (wipe) { + qCInfo(lcRemoteWipe) << "Starting remote wipe for" << _account->displayName(); + /* IMPORTANT - remove later - FIXME MS@2019-12-07 --> * TODO: For "Log out" & "Remove account": Remove client CA certs and KEY! * @@ -106,9 +118,12 @@ void RemoteWipe::checkJobSlot() // delete data emit authorized(accountState); - // delete account - manager->deleteAccount(accountState); - manager->save(); + if (!_canWipeLocalFiles) { + qCInfo(lcRemoteWipe) << "Deleting account" << _account->displayName(); + // delete account if there was nothing else to wipe + manager->deleteAccount(accountState); + manager->save(); + } } else { // ask user for his credentials again accountState->handleInvalidCredentials(); @@ -117,25 +132,41 @@ void RemoteWipe::checkJobSlot() _networkReplyCheck->deleteLater(); } -void RemoteWipe::notifyServerSuccessJob(AccountState *accountState, bool dataWiped){ - if(_accountRemoved && dataWiped && _account == accountState->account()){ - QUrl requestUrl = Utility::concatUrlPath(_account->url().toString(), - QLatin1String("/index.php/core/wipe/success")); - QNetworkRequest request; - request.setHeader(QNetworkRequest::ContentTypeHeader, - "application/x-www-form-urlencoded"); - request.setUrl(requestUrl); - request.setSslConfiguration(_account->getOrCreateSslConfig()); - auto requestBody = new QBuffer; - QUrlQuery arguments(QStringLiteral("token=%1").arg(_appPassword)); - requestBody->setData(arguments.query(QUrl::FullyEncoded).toLatin1()); - _networkReplySuccess = _networkManager.post(request, requestBody); - QObject::connect(_networkReplySuccess, &QNetworkReply::finished, this, - &RemoteWipe::notifyServerSuccessJobSlot); +void RemoteWipe::slotWipeDone(AccountState *accountState, bool dataWiped) +{ + const bool isCurrentAccount = _account == accountState->account(); + if (!(dataWiped && isCurrentAccount)) { + qCWarning(lcRemoteWipe).nospace() << "will not notify server about wipe success dataWiped=" << dataWiped << " isCurrentAccount=" << isCurrentAccount; + return; } + + // delete account after wiping local data succeeded + // sending the notification to the server will be done after the account got removed + qCInfo(lcRemoteWipe) << "Deleting account" << _account->displayName(); + auto manager = AccountManager::instance(); + manager->deleteAccount(accountState); + manager->save(); } -void RemoteWipe::notifyServerSuccessJobSlot() +void RemoteWipe::notifyServerSuccess() +{ + qCInfo(lcRemoteWipe) << "Notifying server about successful remote wipe for" << _account->displayName(); + QUrl requestUrl = Utility::concatUrlPath(_account->url().toString(), + QLatin1String("/index.php/core/wipe/success")); + QNetworkRequest request; + request.setHeader(QNetworkRequest::ContentTypeHeader, + "application/x-www-form-urlencoded"); + request.setUrl(requestUrl); + request.setSslConfiguration(_account->getOrCreateSslConfig()); + auto requestBody = new QBuffer; + QUrlQuery arguments(QStringLiteral("token=%1").arg(_appPassword)); + requestBody->setData(arguments.query(QUrl::FullyEncoded).toLatin1()); + _networkReplySuccess = _networkManager->post(request, requestBody); + QObject::connect(_networkReplySuccess, &QNetworkReply::finished, this, + &RemoteWipe::slotNotifyServerSuccessFinished); +} + +void RemoteWipe::slotNotifyServerSuccessFinished() { auto jsonData = _networkReplySuccess->readAll(); QJsonParseError jsonParseError{}; diff --git a/src/gui/remotewipe.h b/src/gui/remotewipe.h index 8bfed6c127..fc66ee427a 100644 --- a/src/gui/remotewipe.h +++ b/src/gui/remotewipe.h @@ -43,20 +43,26 @@ private slots: * If wipe is requested, delete account and data, if not continue by asking * the user to login again */ - void checkJobSlot(); + void slotCheckJob(); + + /** + * Local sync folders were wiped, this will now remove the account. + */ + void slotWipeDone(OCC::AccountState *accountState, bool); /** * Once the client has wiped all the required data a POST to * /index.php/core/wipe/success */ - void notifyServerSuccessJob(OCC::AccountState *accountState, bool); - void notifyServerSuccessJobSlot(); + void notifyServerSuccess(); + void slotNotifyServerSuccessFinished(); + private: AccountPtr _account; QString _appPassword; - bool _accountRemoved = false; - QNetworkAccessManager _networkManager; + bool _canWipeLocalFiles = false; + QNetworkAccessManager *_networkManager = nullptr; QNetworkReply *_networkReplyCheck = nullptr; QNetworkReply *_networkReplySuccess = nullptr; diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index d2c007ca90..e54ed50033 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -265,7 +265,7 @@ bool FileSystem::removeRecursively(const QString &path, const std::functionerror() != QNetworkReply::AuthenticationRequiredError; + } void persist() override { } void invalidateToken() override { } void forgetSensitiveData() override { } diff --git a/test/testhelper.h b/test/testhelper.h index e2643a0435..80bb99fe6c 100644 --- a/test/testhelper.h +++ b/test/testhelper.h @@ -36,6 +36,11 @@ public: _state = Connected; } + static OCC::RemoteWipe *remoteWipe(OCC::AccountState *accountState) + { + return accountState->_remoteWipe; + } + public slots: void checkConnectivity() override {}; diff --git a/test/testremotewipe.cpp b/test/testremotewipe.cpp index 13a5d3dae8..9e9b650591 100644 --- a/test/testremotewipe.cpp +++ b/test/testremotewipe.cpp @@ -8,12 +8,11 @@ */ #include -#include #include #include "remotewipe.h" +#include "accountmanager.h" -#include "common/utility.h" #include "folderman.h" #include "account.h" #include "accountstate.h" @@ -22,6 +21,8 @@ #include "testhelper.h" +#include "syncenginetestutils.h" + using namespace OCC; class TestRemoteWipe: public QObject @@ -37,58 +38,110 @@ private slots: QStandardPaths::setTestModeEnabled(true); } - // TODO - void testWipe(){ -// QTemporaryDir dir; -// ConfigFile::setConfDir(dir.path()); // we don't want to pollute the user's config file -// QVERIFY(dir.isValid()); + void testRemoteWipe() + { + auto dir = QTemporaryDir {}; + ConfigFile::setConfDir(dir.path()); // we don't want to pollute the user's config file -// QDir dirToRemove(dir.path()); -// QVERIFY(dirToRemove.mkpath("nextcloud")); + // RemoteWipe needs FolderMan for actually wiping local data + FolderMan fm; + auto folderMan = FolderMan::instance(); + QVERIFY(folderMan); -// QString dirPath = dirToRemove.canonicalPath(); + // RemoteWipe also needs an account present in the AccountManager + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + auto account = fakeFolder.account(); + auto accountState = AccountManager::instance()->addAccount(account); -// AccountPtr account = Account::create(); -// QVERIFY(account); + // retrieve the RemoteWipe instance created from the real AccountState, + // and replace its QNetworkAccessManager with our own one for testing + auto remoteWipe = FakeAccountState::remoteWipe(accountState); + auto fakeQnam = new FakeQNAM({}); + remoteWipe->_networkManager->deleteLater(); + remoteWipe->_networkManager = fakeQnam; -// auto manager = AccountManager::instance(); -// QVERIFY(manager); + // let FolderMan know about our sync folder + FolderMan::instance()->addFolder(accountState, folderDefinition(fakeFolder.localPath())); -// AccountState *newAccountState = manager->addAccount(account); -// manager->save(); -// QVERIFY(newAccountState); + bool revokeAppPassword = false; // whether respond with 401 to requests + bool doWipe = false; // whether a remote wipe should be done -// QUrl url("http://example.de"); -// HttpCredentialsTest *cred = new HttpCredentialsTest("testuser", "secret"); -// account->setCredentials(cred); -// account->setUrl( url ); + const auto fakeQnamOverride = [&](const QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *device) -> QNetworkReply * { + Q_UNUSED(device) + if (!revokeAppPassword) { + qDebug() << "App password not revoked"; + return nullptr; + } -// FolderMan *folderman = FolderMan::instance(); -// folderman->addFolder(newAccountState, folderDefinition(dirPath + "/sub/nextcloud/")); + const auto requestUrl = request.url(); + const auto requestPath = requestUrl.path(); -// // check if account exists -// qDebug() << "Does account exists?!"; -// QVERIFY(!account->id().isEmpty()); + if (op == QNetworkAccessManager::Operation::DeleteOperation && requestPath.endsWith("/ocs/v2.php/core/apppassword")) { + qDebug() << "Responding success for app password deletion"; + // allow deletion of appPassword to succeed + return new FakeJsonReply(op, request, this, 200); + } -// manager->deleteAccount(newAccountState); -// manager->save(); + if (doWipe) { + qDebug() << "Wipe enabled"; + if (requestPath.endsWith("/index.php/core/wipe/check")) { + qDebug() << "Responding with wipe=true"; + return new FakeJsonReply(op, request, this, 200, QJsonDocument::fromJson(R"({"wipe": true})")); + } else if (requestPath.endsWith("/index.php/core/wipe/success")) { + qDebug() << "Responding with successful wipe"; + return new FakeJsonReply(op, request, this, 200, QJsonDocument::fromJson(R"({})")); + } + } -// // check if account exists -// qDebug() << "Does account exists yet?!"; -// QVERIFY(account); + qDebug() << "Responding with unauthorised"; + auto errorReply = new FakeErrorReply(op, request, this, 401); + errorReply->setError(QNetworkReply::AuthenticationRequiredError, QLatin1String("Unauthorised")); -// // check if folder exists -// QVERIFY(dirToRemove.exists()); + return errorReply; + }; + fakeFolder.setServerOverride(fakeQnamOverride); + fakeQnam->setOverride(fakeQnamOverride); -// // remote folders -// qDebug() << "Removing folder for account " << newAccountState->account()->url(); + const auto localFolderExists = [&fakeFolder]() -> bool { + return QDir(fakeFolder.localPath()).exists(); + }; -// folderman->slotWipeFolderForAccount(newAccountState); + // initial sync to ensure we've had a working connection + qDebug() << "Test: Initial sync works"; + QVERIFY(fakeFolder.syncOnce()); -// // check if folders dont exist anymore -// QCOMPARE(dirToRemove.exists(), false); + // just revoking the app password -> no remote wipe should be done + qDebug() << "Test: App password revoked, no remote wipe triggered"; + revokeAppPassword = true; + QVERIFY(!fakeFolder.syncOnce()); + // `Account` will try to retrieve the password from the keychain, + // however during testing the password received from it will be empty. + // An empty password will not perform the wipe check at all. + // Therefore: call the slot which `Account` connects its + // `appPasswordRetrieved` signal directly on the remoteWipe instance + remoteWipe->startCheckJobWithAppPassword("password"); + QTest::qWait(500); // wait a bit to process events + // ensure the account was not removed and the sync folder is still present + QCOMPARE(AccountManager::instance()->accounts().size(), 1); + QVERIFY2(localFolderExists(), "Local sync folder should exist as no wipe was requested"); + + // hack for test: close the journal db of FakeFolder, as FolderMan::addFolder creates its own + // as long as the test DB is open, removing files will break on e.g. Windows + fakeFolder.syncJournal().close(); + + // server tells us to wipe the local data + qDebug() << "Test: Server tells us to remote wipe"; + doWipe = true; + // ensure folder exists before performing the wipe + QVERIFY2(localFolderExists(), "Local sync folder should exist before wiping"); + remoteWipe->startCheckJobWithAppPassword("password"); + QTest::qWait(500); // wait a bit to process events + // account should now be gone + QCOMPARE(AccountManager::instance()->accounts().size(), 0); + // local folder should now be gone + QVERIFY2(!localFolderExists(), "Local sync folder should be removed after wiping"); } }; -QTEST_APPLESS_MAIN(TestRemoteWipe) +QTEST_GUILESS_MAIN(TestRemoteWipe) #include "testremotewipe.moc"