From b10b3e5eeb41163c54fd039f75bf156667df8d29 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Tue, 16 Oct 2018 14:55:39 +0200 Subject: [PATCH] Discovery: move checkMovePermissions to its own function --- src/libsync/discovery.cpp | 81 ++++++++++++++++++++++----------------- src/libsync/discovery.h | 6 +++ 2 files changed, 51 insertions(+), 36 deletions(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 79a53cde3..5bb07f990 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -818,41 +818,9 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( isMove = false; //Check local permission if we are allowed to put move the file here - if (isMove) { - auto destPerms = !_rootPermissions.isNull() ? _rootPermissions - : _dirItem ? _dirItem->_remotePerm : _rootPermissions; - auto filePerms = base._remotePerm; // Technicly we should use the one from the server, but we'll assume it is the same - //true when it is just a rename in the same directory. (not a move) - bool isRename = originalPath.startsWith(_currentFolder._original) - && originalPath.lastIndexOf('/') == _currentFolder._original.size(); - // Check if we are allowed to move to the destination. - bool destinationOK = true; - if (isRename || destPerms.isNull()) { - // no need to check for the destination dir permission - destinationOK = true; - } else if (item->isDirectory() && !destPerms.hasPermission(RemotePermissions::CanAddSubDirectories)) { - destinationOK = false; - } else if (!item->isDirectory() && !destPerms.hasPermission(RemotePermissions::CanAddFile)) { - destinationOK = false; - } - - // check if we are allowed to move from the source - bool sourceOK = true; - if (!filePerms.isNull() - && ((isRename && !filePerms.hasPermission(RemotePermissions::CanRename)) - || (!isRename && !filePerms.hasPermission(RemotePermissions::CanMove)))) { - // We are not allowed to move or rename this file - sourceOK = false; - } - if (!sourceOK || !destinationOK) { - qCInfo(lcDisco) << "Not a move because permission does not allow it." << sourceOK << destinationOK; - if (!sourceOK) { - // This is the behavior that we had in the client <= 2.5. - _discoveryData->_statedb->avoidRenamesOnNextSync(base._path); - } - isMove = false; - } - } + // Technically we should use the one from the server, but we'll assume it is the same + if (isMove && !checkMovePermissions(base._remotePerm, originalPath, item->isDirectory())) + isMove = false; if (isMove) { auto wasDeletedOnClient = _discoveryData->findAndCancelDeletedJob(originalPath); @@ -933,7 +901,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( } } else if (_queryLocal == ParentNotChanged && dbEntry.isValid()) { if (noServerEntry) { - // Not modified locally (ParentNotChanged), bit not on the server: Removed on the server. + // Not modified locally (ParentNotChanged), but not on the server: Removed on the server. item->_instruction = CSYNC_INSTRUCTION_REMOVE; item->_direction = SyncFileItem::Down; } @@ -1114,6 +1082,47 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item) return true; } + +bool ProcessDirectoryJob::checkMovePermissions(RemotePermissions srcPerm, const QString &srcPath, + bool isDirectory) +{ + auto destPerms = !_rootPermissions.isNull() ? _rootPermissions + : _dirItem ? _dirItem->_remotePerm : _rootPermissions; + auto filePerms = srcPerm; + //true when it is just a rename in the same directory. (not a move) + bool isRename = srcPath.startsWith(_currentFolder._original) + && srcPath.lastIndexOf('/') == _currentFolder._original.size(); + // Check if we are allowed to move to the destination. + bool destinationOK = true; + if (isRename || destPerms.isNull()) { + // no need to check for the destination dir permission + destinationOK = true; + } else if (isDirectory && !destPerms.hasPermission(RemotePermissions::CanAddSubDirectories)) { + destinationOK = false; + } else if (!isDirectory && !destPerms.hasPermission(RemotePermissions::CanAddFile)) { + destinationOK = false; + } + + // check if we are allowed to move from the source + bool sourceOK = true; + if (!filePerms.isNull() + && ((isRename && !filePerms.hasPermission(RemotePermissions::CanRename)) + || (!isRename && !filePerms.hasPermission(RemotePermissions::CanMove)))) { + // We are not allowed to move or rename this file + sourceOK = false; + } + if (!sourceOK || !destinationOK) { + qCInfo(lcDisco) << "Not a move because permission does not allow it." << sourceOK << destinationOK; + if (!sourceOK) { + // This is the behavior that we had in the client <= 2.5. + // but that might not be needed anymore + _discoveryData->_statedb->avoidRenamesOnNextSync(srcPath); + } + return false; + } + return true; +} + void ProcessDirectoryJob::subJobFinished() { auto job = qobject_cast(sender()); diff --git a/src/libsync/discovery.h b/src/libsync/discovery.h index a1402e3bb..3f66754d6 100644 --- a/src/libsync/discovery.h +++ b/src/libsync/discovery.h @@ -111,6 +111,12 @@ private: */ bool checkPermissions(const SyncFileItemPtr &item); + /** + * Check if the move is of a specified file within this directory is allowed. + * Return true if it is allowed, false otherwise + */ + bool checkMovePermissions(RemotePermissions srcPerm, const QString &srcPath, bool isDirectory); + void processBlacklisted(const PathTuple &, const LocalInfo &, const SyncJournalFileRecord &dbEntry); void subJobFinished(); -- 2.30.2