mirrors/nextcloud-desktop
3
0
Fork 0
mirror of https://github.com/nextcloud/desktop.git synced 2025-10-26 11:17:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #8465 from nextcloud/bugfix/remote-wipe

Browse Source 
fix(remotewipe): do not reopen sync db before wiping
This commit is contained in:
Jyrki Gadinger 2025-07-29 11:53:46 +02:00 committed by GitHub
commit c139f91145
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 215 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/common/filesystembase.cpp
View File

@ -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());

10
src/common/filesystembase.h
View File

@ -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.

13
src/gui/folder.cpp
View File

@ -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<void(bool)> callback)

2
src/gui/folderman.cpp
View File

@ -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 {

2
src/gui/folderman.h
View File

@ -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

93
src/gui/remotewipe.cpp
View File

@ -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{};

16
src/gui/remotewipe.h
View File

@ -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
* <server>/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;

2
src/libsync/filesystem.cpp
View File

@ -265,7 +265,7 @@ bool FileSystem::removeRecursively(const QString &path, const std::function<void
// we never want to go into this branch for .lnk files
bool isDir = FileSystem::isDir(fi.absoluteFilePath()) && !FileSystem::isSymLink(fi.absoluteFilePath()) && !FileSystem::isJunction(fi.absoluteFilePath());
if (isDir) {
removeOk = removeRecursively(path + QLatin1Char('/') + di.fileName(), onDeleted, errors, onError); // recursive
removeOk = removeRecursively(joinPath(path, di.fileName()), onDeleted, errors, onError); // recursive
} else {
QString removeError;

4
test/syncenginetestutils.h
View File

@ -550,7 +550,9 @@ public:
[[nodiscard]] bool ready() const override { return true; }
void fetchFromKeychain() override { }
void askFromUser() override { }
bool stillValid(QNetworkReply *) override { return true; }
bool stillValid(QNetworkReply *reply) override {
return reply->error() != QNetworkReply::AuthenticationRequiredError;
}
void persist() override { }
void invalidateToken() override { }
void forgetSensitiveData() override { }

5
test/testhelper.h
View File

@ -36,6 +36,11 @@ public:
_state = Connected;
}
static OCC::RemoteWipe *remoteWipe(OCC::AccountState *accountState)
{
return accountState->_remoteWipe;
}
public slots:
void checkConnectivity() override {};

131
test/testremotewipe.cpp
View File

@ -8,12 +8,11 @@
*/
#include <qglobal.h>
#include <QTemporaryDir>
#include <QtTest>
#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"