From c7ed24cc681a4018e07c636006455514581e7ae6 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Mon, 25 Mar 2024 22:06:11 +0100 Subject: [PATCH] properly compute if a folder is top level or child extern mounted asks new permission to server to be able to know if a folder is a top level mounted folder should allow detecting the top level folders from external storages or group folders should also make the client reliably detect that it is handling a child folder inside a group folder and be allowed to rename such folders Signed-off-by: Matthieu Gallien --- VERSION.cmake | 4 +++ src/common/remotepermissions.cpp | 39 ++++++++++++++++++++++++- src/common/remotepermissions.h | 21 ++++++++++++- src/gui/editlocallyjob.cpp | 7 +++-- src/gui/folderstatusmodel.cpp | 1 + src/gui/invalidfilenamedialog.cpp | 2 +- src/gui/shellextensionsserver.cpp | 2 +- src/libsync/account.cpp | 11 +++++++ src/libsync/account.h | 2 ++ src/libsync/caseclashconflictsolver.cpp | 2 +- src/libsync/discovery.cpp | 8 +++-- src/libsync/discoveryphase.cpp | 23 +++++++-------- src/libsync/propagateremotemkdir.cpp | 8 +++-- src/libsync/syncfileitem.cpp | 4 +-- src/libsync/syncfileitem.h | 2 +- version.h.in | 4 +++ 16 files changed, 114 insertions(+), 26 deletions(-) diff --git a/VERSION.cmake b/VERSION.cmake index 05cd7b6c00735..b2ae913d35a84 100644 --- a/VERSION.cmake +++ b/VERSION.cmake @@ -13,6 +13,10 @@ set(NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_MAJOR 26) set(NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_MINOR 0) set(NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_PATCH 0) +set(NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MAJOR 28) +set(NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MINOR 0) +set(NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_PATCH 3) + if ( NOT DEFINED MIRALL_VERSION_SUFFIX ) set( MIRALL_VERSION_SUFFIX "git") #e.g. beta1, beta2, rc1 endif( NOT DEFINED MIRALL_VERSION_SUFFIX ) diff --git a/src/common/remotepermissions.cpp b/src/common/remotepermissions.cpp index 00827f8f13f75..bfe93922a679c 100644 --- a/src/common/remotepermissions.cpp +++ b/src/common/remotepermissions.cpp @@ -17,10 +17,15 @@ */ #include "remotepermissions.h" + +#include + #include namespace OCC { +Q_LOGGING_CATEGORY(lcRemotePermissions, "nextcloud.sync.remotepermissions", QtInfoMsg) + static const char letters[] = " WDNVCKRSMm"; @@ -68,11 +73,43 @@ RemotePermissions RemotePermissions::fromDbValue(const QByteArray &value) return perm; } -RemotePermissions RemotePermissions::fromServerString(const QString &value) +template +RemotePermissions RemotePermissions::internalFromServerString(const QString &value, + const T&otherProperties, + MountedPermissionAlgorithm algorithm) { RemotePermissions perm; perm.fromArray(value.utf16()); + + if (algorithm == MountedPermissionAlgorithm::WildGuessMountedSubProperty) { + return perm; + } + + if ((otherProperties.contains(QStringLiteral("is-mount-root")) && otherProperties.value(QStringLiteral("is-mount-root")) == QStringLiteral("false") && perm.hasPermission(RemotePermissions::IsMounted)) || + (!otherProperties.contains(QStringLiteral("is-mount-root")) && perm.hasPermission(RemotePermissions::IsMounted))) { + /* All the entries in a external storage have 'M' in their permission. However, for all + purposes in the desktop client, we only need to know about the mount points. + So replace the 'M' by a 'm' for every sub entries in an external storage */ + perm.unsetPermission(RemotePermissions::IsMounted); + perm.setPermission(RemotePermissions::IsMountedSub); + qCInfo(lcRemotePermissions()) << otherProperties.value(QStringLiteral("permissions")) << "replacing M permissions by m for subfolders inside a group folder"; + } + return perm; } +RemotePermissions RemotePermissions::fromServerString(const QString &value, + MountedPermissionAlgorithm algorithm, + const QMap &otherProperties) +{ + return internalFromServerString(value, otherProperties, algorithm); +} + +RemotePermissions RemotePermissions::fromServerString(const QString &value, + MountedPermissionAlgorithm algorithm, + const QVariantMap &otherProperties) +{ + return internalFromServerString(value, otherProperties, algorithm); +} + } // namespace OCC diff --git a/src/common/remotepermissions.h b/src/common/remotepermissions.h index 6d7c5663324c6..a84c359d3b446 100644 --- a/src/common/remotepermissions.h +++ b/src/common/remotepermissions.h @@ -59,6 +59,11 @@ class OCSYNC_EXPORT RemotePermissions PermissionsCount = IsMountedSub }; + enum class MountedPermissionAlgorithm { + UseMountRootProperty, + WildGuessMountedSubProperty, + }; + /// null permissions RemotePermissions() = default; @@ -72,7 +77,14 @@ class OCSYNC_EXPORT RemotePermissions static RemotePermissions fromDbValue(const QByteArray &); /// read a permissions string received from the server, never null - static RemotePermissions fromServerString(const QString &); + static RemotePermissions fromServerString(const QString &value, + MountedPermissionAlgorithm algorithm = MountedPermissionAlgorithm::WildGuessMountedSubProperty, + const QMap &otherProperties = {}); + + /// read a permissions string received from the server, never null + static RemotePermissions fromServerString(const QString &value, + MountedPermissionAlgorithm algorithm, + const QVariantMap &otherProperties = {}); [[nodiscard]] bool hasPermission(Permissions p) const { @@ -101,6 +113,13 @@ class OCSYNC_EXPORT RemotePermissions { return dbg << p.toString(); } + +private: + + template + static RemotePermissions internalFromServerString(const QString &value, + const T&otherProperties, + MountedPermissionAlgorithm algorithm); }; diff --git a/src/gui/editlocallyjob.cpp b/src/gui/editlocallyjob.cpp index 167cf99c33cb9..f4109a970d8c4 100644 --- a/src/gui/editlocallyjob.cpp +++ b/src/gui/editlocallyjob.cpp @@ -213,7 +213,8 @@ void EditLocallyJob::fetchRemoteFileParentInfo() QByteArrayLiteral("http://owncloud.org/ns:size"), QByteArrayLiteral("http://owncloud.org/ns:id"), QByteArrayLiteral("http://owncloud.org/ns:permissions"), - QByteArrayLiteral("http://owncloud.org/ns:checksums")}; + QByteArrayLiteral("http://owncloud.org/ns:checksums"), + QByteArrayLiteral("http://nextcloud.org/ns:is-mount-root")}; job->setProperties(props); connect(job, &LsColJob::directoryListingIterated, this, &EditLocallyJob::slotDirectoryListingIterated); @@ -545,7 +546,9 @@ void EditLocallyJob::slotDirectoryListingIterated(const QString &name, const QMa const auto cleanName = nameWithoutDavPath.startsWith(remoteFolderPathWithoutLeadingSlash) ? nameWithoutDavPath.mid(remoteFolderPathWithoutLeadingSlash.size()) : nameWithoutDavPath; disconnect(job, &LsColJob::directoryListingIterated, this, &EditLocallyJob::slotDirectoryListingIterated); - _fileParentItem = SyncFileItem::fromProperties(cleanName, properties); + _fileParentItem = SyncFileItem::fromProperties(cleanName, + properties, + _accountState->account()->serverHasMountRootProperty() ? RemotePermissions::MountedPermissionAlgorithm::UseMountRootProperty : RemotePermissions::MountedPermissionAlgorithm::WildGuessMountedSubProperty); } } diff --git a/src/gui/folderstatusmodel.cpp b/src/gui/folderstatusmodel.cpp index cc3aede361715..e21644aecd8f7 100644 --- a/src/gui/folderstatusmodel.cpp +++ b/src/gui/folderstatusmodel.cpp @@ -616,6 +616,7 @@ void FolderStatusModel::fetchMore(const QModelIndex &parent) auto props = QList() << "resourcetype" << "http://owncloud.org/ns:size" << "http://owncloud.org/ns:permissions" + << "http://nextcloud.org/ns:is-mount-root" << "http://owncloud.org/ns:fileid"; if (_accountState->account()->capabilities().clientSideEncryptionAvailable()) { props << "http://nextcloud.org/ns:is-encrypted"; diff --git a/src/gui/invalidfilenamedialog.cpp b/src/gui/invalidfilenamedialog.cpp index 235f29650e9dd..844f5b5e4bdf2 100644 --- a/src/gui/invalidfilenamedialog.cpp +++ b/src/gui/invalidfilenamedialog.cpp @@ -117,7 +117,7 @@ InvalidFilenameDialog::~InvalidFilenameDialog() = default; void InvalidFilenameDialog::checkIfAllowedToRename() { const auto propfindJob = new PropfindJob(_account, QDir::cleanPath(_folder->remotePath() + _originalFileName)); - propfindJob->setProperties({ "http://owncloud.org/ns:permissions" }); + propfindJob->setProperties({"http://owncloud.org/ns:permissions", "http://nextcloud.org/ns:is-mount-root"}); connect(propfindJob, &PropfindJob::result, this, &InvalidFilenameDialog::onPropfindPermissionSuccess); connect(propfindJob, &PropfindJob::finishedWithError, this, &InvalidFilenameDialog::onPropfindPermissionError); propfindJob->start(); diff --git a/src/gui/shellextensionsserver.cpp b/src/gui/shellextensionsserver.cpp index 88c5571b8b128..1150a3107b7a0 100644 --- a/src/gui/shellextensionsserver.cpp +++ b/src/gui/shellextensionsserver.cpp @@ -169,7 +169,7 @@ void ShellExtensionsServer::processCustomStateRequest(QLocalSocket *socket, cons })); auto *const lsColJob = new LsColJob(folder->accountState()->account(), QDir::cleanPath(folder->remotePath() + lsColJobPath)); - lsColJob->setProperties({QByteArrayLiteral("http://owncloud.org/ns:share-types"), QByteArrayLiteral("http://owncloud.org/ns:permissions")}); + lsColJob->setProperties({QByteArrayLiteral("http://owncloud.org/ns:share-types"), QByteArrayLiteral("http://owncloud.org/ns:permissions"), QByteArrayLiteral("http://nextcloud.org/ns:is-mount-root")}); const auto folderAlias = customStateRequestInfo.folderAlias; diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp index 15712b11e165f..9be583b49c8aa 100644 --- a/src/libsync/account.cpp +++ b/src/libsync/account.cpp @@ -731,6 +731,17 @@ int Account::serverVersionInt() const components.value(2).toInt()); } +bool Account::serverHasMountRootProperty() const +{ + if (serverVersionInt() == 0) { + return false; + } + + return serverVersionInt() >= Account::makeServerVersion(NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MAJOR, + NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MINOR, + NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_PATCH); +} + bool Account::serverVersionUnsupported() const { if (serverVersionInt() == 0) { diff --git a/src/libsync/account.h b/src/libsync/account.h index d21748f872a1a..b7702e525fdf8 100644 --- a/src/libsync/account.h +++ b/src/libsync/account.h @@ -247,6 +247,8 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject */ [[nodiscard]] int serverVersionInt() const; + [[nodiscard]] bool serverHasMountRootProperty() const; + static constexpr int makeServerVersion(const int majorVersion, const int minorVersion, const int patchVersion) { return (majorVersion << 16) + (minorVersion << 8) + patchVersion; }; diff --git a/src/libsync/caseclashconflictsolver.cpp b/src/libsync/caseclashconflictsolver.cpp index b93c06d6ba37b..753c2a7c0fc68 100644 --- a/src/libsync/caseclashconflictsolver.cpp +++ b/src/libsync/caseclashconflictsolver.cpp @@ -220,7 +220,7 @@ void CaseClashConflictSolver::processLeadingOrTrailingSpacesError(const QString void CaseClashConflictSolver::checkIfAllowedToRename() { const auto propfindJob = new PropfindJob(_account, QDir::cleanPath(remoteTargetFilePath())); - propfindJob->setProperties({ "http://owncloud.org/ns:permissions" }); + propfindJob->setProperties({"http://owncloud.org/ns:permissions", "http://nextcloud.org/ns:is-mount-root"}); connect(propfindJob, &PropfindJob::result, this, &CaseClashConflictSolver::onPropfindPermissionSuccess); connect(propfindJob, &PropfindJob::finishedWithError, this, &CaseClashConflictSolver::onPropfindPermissionError); propfindJob->start(); diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 2fc2b5bf26d8c..ef0644ca14260 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1438,14 +1438,18 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( // Check local permission if we are allowed to put move the file here // Technically we should use the permissions from the server, but we'll assume it is the same + const auto serverHasMountRootProperty = _discoveryData->_account->serverHasMountRootProperty(); const auto isExternalStorage = base._remotePerm.hasPermission(RemotePermissions::IsMounted); const auto movePerms = checkMovePermissions(base._remotePerm, originalPath, item->isDirectory()); - if (!movePerms.sourceOk || !movePerms.destinationOk || isExternalStorage || isE2eeMoveOnlineOnlyItemWithCfApi) { + if (!movePerms.sourceOk || !movePerms.destinationOk || (serverHasMountRootProperty && isExternalStorage) || isE2eeMoveOnlineOnlyItemWithCfApi) { qCInfo(lcDisco) << "Move without permission to rename base file, " << "source:" << movePerms.sourceOk << ", target:" << movePerms.destinationOk << ", targetNew:" << movePerms.destinationNewOk - << ", isExternalStorage:" << isExternalStorage; + << ", isExternalStorage:" << isExternalStorage + << ", serverHasMountRootProperty:" << serverHasMountRootProperty + << ", base._remotePerm:" << base._remotePerm.toString() + << ", base.path():" << base.path(); // If we can create the destination, do that. // Permission errors on the destination will be handled by checkPermissions later. diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index fdf66e4058b02..33516635d9384 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -419,6 +419,7 @@ void DiscoverySingleDirectoryJob::start() << "http://nextcloud.org/ns:lock-time" << "http://nextcloud.org/ns:lock-timeout"; } + props << "http://nextcloud.org/ns:is-mount-root"; lsColJob->setProperties(props); @@ -458,7 +459,7 @@ SyncFileItem::EncryptionStatus DiscoverySingleDirectoryJob::requiredEncryptionSt return _encryptionStatusRequired; } -static void propertyMapToRemoteInfo(const QMap &map, RemoteInfo &result) +static void propertyMapToRemoteInfo(const QMap &map, RemotePermissions::MountedPermissionAlgorithm algorithm, RemoteInfo &result) { for (auto it = map.constBegin(); it != map.constEnd(); ++it) { QString property = it.key(); @@ -490,7 +491,7 @@ static void propertyMapToRemoteInfo(const QMap &map, RemoteInf } else if (property == "dDC") { result.directDownloadCookies = value; } else if (property == "permissions") { - result.remotePerm = RemotePermissions::fromServerString(value); + result.remotePerm = RemotePermissions::fromServerString(value, algorithm, map); } else if (property == "checksums") { result.checksumHeader = findBestChecksum(value.toUtf8()); } else if (property == "share-types" && !value.isEmpty()) { @@ -560,7 +561,10 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(const QString &fi // The first entry is for the folder itself, we should process it differently. _ignoredFirst = true; if (map.contains("permissions")) { - auto perm = RemotePermissions::fromServerString(map.value("permissions")); + auto perm = RemotePermissions::fromServerString(map.value("permissions"), + _account->serverHasMountRootProperty() ? RemotePermissions::MountedPermissionAlgorithm::UseMountRootProperty : RemotePermissions::MountedPermissionAlgorithm::WildGuessMountedSubProperty, + map); + qCInfo(lcDiscovery()) << file << map.value("permissions") << map; emit firstDirectoryPermissions(perm); _isExternalStorage = perm.hasPermission(RemotePermissions::IsMounted); } @@ -585,22 +589,17 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(const QString &fi _size = map.value("size").toInt(); } } else { - RemoteInfo result; int slash = file.lastIndexOf('/'); result.name = file.mid(slash + 1); result.size = -1; - propertyMapToRemoteInfo(map, result); + propertyMapToRemoteInfo(map, + _account->serverHasMountRootProperty() ? RemotePermissions::MountedPermissionAlgorithm::UseMountRootProperty : RemotePermissions::MountedPermissionAlgorithm::WildGuessMountedSubProperty, + result); if (result.isDirectory) result.size = 0; - if (_isExternalStorage && result.remotePerm.hasPermission(RemotePermissions::IsMounted)) { - /* All the entries in a external storage have 'M' in their permission. However, for all - purposes in the desktop client, we only need to know about the mount points. - So replace the 'M' by a 'm' for every sub entries in an external storage */ - result.remotePerm.unsetPermission(RemotePermissions::IsMounted); - result.remotePerm.setPermission(RemotePermissions::IsMountedSub); - } + qCInfo(lcDiscovery()) << file << map.value("permissions") << result.remotePerm.toString() << map; _results.push_back(std::move(result)); } diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp index cb7a6587888ea..e301beed5a723 100644 --- a/src/libsync/propagateremotemkdir.cpp +++ b/src/libsync/propagateremotemkdir.cpp @@ -139,10 +139,13 @@ void PropagateRemoteMkdir::finalizeMkColJob(QNetworkReply::NetworkError err, con propagator()->_activeJobList.append(this); auto propfindJob = new PropfindJob(propagator()->account(), jobPath, this); - propfindJob->setProperties({QByteArrayLiteral("http://owncloud.org/ns:share-types"), QByteArrayLiteral("http://owncloud.org/ns:permissions")}); + propfindJob->setProperties({QByteArrayLiteral("http://owncloud.org/ns:share-types"), QByteArrayLiteral("http://owncloud.org/ns:permissions"), QByteArrayLiteral("http://nextcloud.org/ns:is-mount-root")}); connect(propfindJob, &PropfindJob::result, this, [this, jobPath](const QVariantMap &result){ propagator()->_activeJobList.removeOne(this); - _item->_remotePerm = RemotePermissions::fromServerString(result.value(QStringLiteral("permissions")).toString()); + _item->_remotePerm = RemotePermissions::fromServerString(result.value(QStringLiteral("permissions")).toString(), + propagator()->account()->serverHasMountRootProperty() ? RemotePermissions::MountedPermissionAlgorithm::UseMountRootProperty : RemotePermissions::MountedPermissionAlgorithm::WildGuessMountedSubProperty, + result); + _item->_sharedByMe = !result.value(QStringLiteral("share-types")).toString().isEmpty(); _item->_isShared = _item->_remotePerm.hasPermission(RemotePermissions::IsShared) || _item->_sharedByMe; _item->_lastShareStateFetchedTimestamp = QDateTime::currentMSecsSinceEpoch(); @@ -231,6 +234,7 @@ void PropagateRemoteMkdir::slotMkcolJobFinished() _item->_fileId = _job->reply()->rawHeader("OC-FileId"); + qCInfo(lcPropagateRemoteMkdir()) << "mkcol job error string:" << _item->_errorString << _job->errorString(); _item->_errorString = _job->errorString(); const auto jobHttpReasonPhraseString = _job->reply()->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString(); diff --git a/src/libsync/syncfileitem.cpp b/src/libsync/syncfileitem.cpp index 4a169d9cbefbb..6bf10cd7af1b0 100644 --- a/src/libsync/syncfileitem.cpp +++ b/src/libsync/syncfileitem.cpp @@ -169,7 +169,7 @@ SyncFileItemPtr SyncFileItem::fromSyncJournalFileRecord(const SyncJournalFileRec return item; } -SyncFileItemPtr SyncFileItem::fromProperties(const QString &filePath, const QMap &properties) +SyncFileItemPtr SyncFileItem::fromProperties(const QString &filePath, const QMap &properties, RemotePermissions::MountedPermissionAlgorithm algorithm) { SyncFileItemPtr item(new SyncFileItem); item->_file = filePath; @@ -182,7 +182,7 @@ SyncFileItemPtr SyncFileItem::fromProperties(const QString &filePath, const QMap item->_fileId = properties.value(QStringLiteral("id")).toUtf8(); if (properties.contains(QStringLiteral("permissions"))) { - item->_remotePerm = RemotePermissions::fromServerString(properties.value("permissions")); + item->_remotePerm = RemotePermissions::fromServerString(properties.value("permissions"), algorithm, properties); } if (!properties.value(QStringLiteral("share-types")).isEmpty()) { diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index 89e68ca99b209..d06e6f6d93b23 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -133,7 +133,7 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem /** Creates a basic SyncFileItem from remote properties */ - [[nodiscard]] static SyncFileItemPtr fromProperties(const QString &filePath, const QMap &properties); + [[nodiscard]] static SyncFileItemPtr fromProperties(const QString &filePath, const QMap &properties, RemotePermissions::MountedPermissionAlgorithm algorithm); SyncFileItem() diff --git a/version.h.in b/version.h.in index 9e508d937b811..b8d97bb785746 100644 --- a/version.h.in +++ b/version.h.in @@ -45,4 +45,8 @@ constexpr int NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_MAJOR = @NE constexpr int NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_MINOR = @NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_MINOR@; constexpr int NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_PATCH = @NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_PATCH@; +constexpr int NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MAJOR = @NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MAJOR@; +constexpr int NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MINOR = @NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MINOR@; +constexpr int NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_PATCH = @NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_PATCH@; + #endif // VERSION_H