Merge pull request #6558 from nextcloud/backport/6529/stable-3.12

[stable-3.12] E2EE. Fix root metadata fetching path for non-root remote sync folder. Refactoring. Stabilizing paths.
nextcloud · Mar 20, 2024 · de4a41f · de4a41f
2 parents 151be9c + 428bfb2
commit de4a41f
Show file tree

Hide file tree

Showing 28 changed files with 160 additions and 146 deletions.
diff --git a/src/common/utility.cpp b/src/common/utility.cpp
@@ -681,6 +681,12 @@ bool Utility::isCaseClashConflictFile(const QString &name)
     return bname.contains(QStringLiteral("(case clash from"));
 }
 
+QString Utility::leadingSlashPath(const QString &path)
+{
+    static const auto slash = QLatin1Char('/');
+    return !path.startsWith(slash) ? QString(slash + path) : path;
+}
+
 QString Utility::trailingSlashPath(const QString &path)
 {
     static const auto slash = QLatin1Char('/');
@@ -690,13 +696,13 @@ QString Utility::trailingSlashPath(const QString &path)
 QString Utility::noLeadingSlashPath(const QString &path)
 {
     static const auto slash = QLatin1Char('/');
-    return path.startsWith(slash) ? path.mid(1) : path;
+    return path.size() > 1 && path.startsWith(slash) ? path.mid(1) : path;
 }
 
 QString Utility::noTrailingSlashPath(const QString &path)
 {
     static const auto slash = QLatin1Char('/');
-    return path.endsWith(slash) ? path.chopped(1) : path;
+    return path.size() > 1 && path.endsWith(slash) ? path.chopped(1) : path;
 }
 
 QString Utility::fullRemotePathToRemoteSyncRootRelative(const QString &fullRemotePath, const QString &remoteSyncRoot)

diff --git a/src/common/utility.h b/src/common/utility.h
@@ -266,7 +266,8 @@ namespace Utility {
      * @brief Registers the desktop app as a handler for a custom URI to enable local editing
      */
     OCSYNC_EXPORT void registerUriHandlerForLocalEditing();
-
+
+    OCSYNC_EXPORT QString leadingSlashPath(const QString &path);
     OCSYNC_EXPORT QString trailingSlashPath(const QString &path);
     OCSYNC_EXPORT QString noLeadingSlashPath(const QString &path);
     OCSYNC_EXPORT QString noTrailingSlashPath(const QString &path);

diff --git a/src/gui/filedetails/sharemodel.cpp b/src/gui/filedetails/sharemodel.cpp
@@ -857,6 +857,9 @@ void ShareModel::slotDeleteE2EeShare(const SharePtr &share) const
         return;
     }
 
+    Q_ASSERT(folder->remotePath() == QStringLiteral("/")
+             || Utility::noLeadingSlashPath(share->path()).startsWith(Utility::noLeadingSlashPath(Utility::noTrailingSlashPath(folder->remotePath()))));
+
     const auto removeE2eeShareJob = new UpdateE2eeFolderUsersMetadataJob(account,
                                                                          folder->journalDb(),
                                                                          folder->remotePath(),

diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp
@@ -1537,7 +1537,7 @@ void FolderMan::leaveShare(const QString &localFile)
 {
     const auto localFileNoTrailingSlash = localFile.endsWith('/') ? localFile.chopped(1) : localFile;
     if (const auto folder = FolderMan::instance()->folderForPath(localFileNoTrailingSlash)) {
-        const auto filePathRelative = QString(localFileNoTrailingSlash).remove(folder->path());
+        const auto filePathRelative = Utility::noLeadingSlashPath(QString(localFileNoTrailingSlash).remove(folder->path()));
 
         SyncJournalFileRecord rec;
         if (folder->journalDb()->getFileRecord(filePathRelative, &rec)
@@ -1551,8 +1551,7 @@ void FolderMan::leaveShare(const QString &localFile)
                                                                                  folder->journalDb(),
                                                                                  folder->remotePath(),
                                                                                  UpdateE2eeFolderUsersMetadataJob::Remove,
-                //TODO: Might need to add a slash to "filePathRelative" once the server is working
-                                                                                 filePathRelative,
+                                                                                 folder->remotePathTrailingSlash() + filePathRelative,
                                                                                  folder->accountState()->account()->davUser());
             _removeE2eeShareJob->setParent(this);
             _removeE2eeShareJob->start(true);

diff --git a/src/gui/sharemanager.cpp b/src/gui/sharemanager.cpp
@@ -488,7 +488,7 @@ void ShareManager::createShare(const QString &path,
     job->getSharedWithMe();
 }
 
-void ShareManager::createE2EeShareJob(const QString &path,
+void ShareManager::createE2EeShareJob(const QString &fullRemotePath,
                                       const ShareePtr sharee,
                                       const Share::Permissions permissions,
                                       const QString &password)
@@ -506,11 +506,14 @@ void ShareManager::createE2EeShareJob(const QString &path,
         return;
     }
 
+    Q_ASSERT(folder->remotePath() == QStringLiteral("/") ||
+        Utility::noLeadingSlashPath(fullRemotePath).startsWith(Utility::noLeadingSlashPath(Utility::noTrailingSlashPath(folder->remotePath()))));
+
     const auto createE2eeShareJob = new UpdateE2eeFolderUsersMetadataJob(_account,
                                                                          folder->journalDb(),
                                                                          folder->remotePath(),
                                                                          UpdateE2eeFolderUsersMetadataJob::Add,
-                                                                         path,
+                                                                         fullRemotePath,
                                                                          sharee->shareWith(),
                                                                          QSslCertificate{},
                                                                          this);

diff --git a/src/gui/sharemanager.h b/src/gui/sharemanager.h
@@ -416,7 +416,7 @@ class ShareManager : public QObject
         /**
      * Tell the manager to create and start new UpdateE2eeShareMetadataJob job
      *
-     * @param path The path of the share relative to the user folder on the server
+     * @param fullRemotePath The path of the share relative to the user folder on the server
      * @param shareType The type of share (TypeUser, TypeGroup, TypeRemote)
      * @param Permissions The share permissions
      * @param folderId The id for an E2EE folder
@@ -425,7 +425,7 @@ class ShareManager : public QObject
      * On success the signal shareCreated is emitted
      * In case of a server error the serverError signal is emitted
      */
-    void createE2EeShareJob(const QString &path,
+    void createE2EeShareJob(const QString &fullRemotePath,
                             const ShareePtr sharee,
                             const Share::Permissions permissions,
                             const QString &password = "");

diff --git a/src/gui/socketapi/socketapi.cpp b/src/gui/socketapi/socketapi.cpp
@@ -534,13 +534,7 @@ void SocketApi::processEncryptRequest(const QString &localFile)
     auto path = rec._path;
     // Folder records have directory paths in Foo/Bar/ convention...
     // But EncryptFolderJob expects directory path Foo/Bar convention
-    auto choppedPath = path;
-    if (choppedPath.endsWith('/') && choppedPath != QStringLiteral("/")) {
-        choppedPath.chop(1);
-    }
-    if (choppedPath.startsWith('/') && choppedPath != QStringLiteral("/")) {
-        choppedPath = choppedPath.mid(1);
-    }
+    const auto choppedPath = Utility::noTrailingSlashPath(Utility::noLeadingSlashPath(path));
 
     auto job = new OCC::EncryptFolderJob(account, folder->journalDb(), choppedPath, choppedPath, folder->remotePath(), rec.numericFileId());
     job->setParent(this);

diff --git a/src/libsync/basepropagateremotedeleteencrypted.cpp b/src/libsync/basepropagateremotedeleteencrypted.cpp
@@ -57,17 +57,17 @@ void BasePropagateRemoteDeleteEncrypted::storeFirstErrorString(const QString &er
 
 void BasePropagateRemoteDeleteEncrypted::fetchMetadataForPath(const QString &path)
 {
-    qCDebug(ABSTRACT_PROPAGATE_REMOVE_ENCRYPTED) << "Folder is encrypted, let's its metadata.";
-    _fullFolderRemotePath = _propagator->fullRemotePath(path);
-
+    qCDebug(ABSTRACT_PROPAGATE_REMOVE_ENCRYPTED) << "Folder is encrypted, let's fetch its metadata.";
+
     SyncJournalFileRecord rec;
-    if (!_propagator->_journal->getRootE2eFolderRecord(Utility::fullRemotePathToRemoteSyncRootRelative(_fullFolderRemotePath, _propagator->remotePath()), &rec) || !rec.isValid()) {
+    if (!_propagator->_journal->getRootE2eFolderRecord(Utility::noLeadingSlashPath(path), &rec) || !rec.isValid()) {
         taskFailed();
         return;
     }
 
     _encryptedFolderMetadataHandler.reset(new EncryptedFolderMetadataHandler(_propagator->account(),
-                                                                                       _fullFolderRemotePath,
+                                                                                       _propagator->fullRemotePath(path),
+                                                                                       _propagator->remotePath(),
                                                                                        _propagator->_journal,
                                                                                        rec.path()));
 

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
@@ -2024,10 +2024,11 @@ void ProcessDirectoryJob::chopVirtualFileSuffix(QString &str) const
 DiscoverySingleDirectoryJob *ProcessDirectoryJob::startAsyncServerQuery()
 {
     if (_dirItem && _dirItem->isEncrypted() && _dirItem->_encryptedFileName.isEmpty()) {
-        _discoveryData->_topLevelE2eeFolderPaths.insert(QLatin1Char('/') + _dirItem->_file);
+        _discoveryData->_topLevelE2eeFolderPaths.insert(_discoveryData->_remoteFolder + _dirItem->_file);
     }
     auto serverJob = new DiscoverySingleDirectoryJob(_discoveryData->_account,
-                                                     _discoveryData->_remoteFolder + _currentFolder._server,
+                                                     _currentFolder._server,
+                                                     _discoveryData->_remoteFolder,
                                                      _discoveryData->_topLevelE2eeFolderPaths,
                                                      this);
     if (!_dirItem) {

diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp
@@ -371,13 +371,16 @@ void DiscoverySingleLocalDirectoryJob::run() {
 
 DiscoverySingleDirectoryJob::DiscoverySingleDirectoryJob(const AccountPtr &account,
                                                          const QString &path,
+                                                         const QString &remoteRootFolderPath,
                                                          const QSet<QString> &topLevelE2eeFolderPaths,
                                                          QObject *parent)
     : QObject(parent)
-    , _subPath(path)
+    , _subPath(remoteRootFolderPath + path)
+    , _remoteRootFolderPath(remoteRootFolderPath)
     , _account(account)
     , _topLevelE2eeFolderPaths(topLevelE2eeFolderPaths)
 {
+    Q_ASSERT(!_remoteRootFolderPath.isEmpty());
 }
 
 void DiscoverySingleDirectoryJob::start()
@@ -692,8 +695,9 @@ void DiscoverySingleDirectoryJob::metadataReceived(const QJsonDocument &json, in
     }
 
     const auto e2EeFolderMetadata = new FolderMetadata(_account,
+                                                 _remoteRootFolderPath,
                                                  statusCode == 404 ? QByteArray{} : json.toJson(QJsonDocument::Compact),
-                                                 RootEncryptedFolderInfo(topLevelFolderPath),
+                                                 RootEncryptedFolderInfo(Utility::fullRemotePathToRemoteSyncRootRelative(topLevelFolderPath, _remoteRootFolderPath)),
                                                  job->signature());
     connect(e2EeFolderMetadata, &FolderMetadata::setupComplete, this, [this, e2EeFolderMetadata] {
         e2EeFolderMetadata->deleteLater();

diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h
@@ -145,6 +145,7 @@ class DiscoverySingleDirectoryJob : public QObject
 public:
     explicit DiscoverySingleDirectoryJob(const AccountPtr &account,
                                          const QString &path,
+                                         const QString &remoteRootFolderPath,
         /* TODO for topLevelE2eeFolderPaths, from review: I still do not get why giving the whole QSet instead of just the parent of the folder we are in
         sounds to me like it would be much more efficient to just have the e2ee parent folder that we are
         inside*/
@@ -179,6 +180,7 @@ private slots:
 
     QVector<RemoteInfo> _results;
     QString _subPath;
+    QString _remoteRootFolderPath;
     QByteArray _firstEtag;
     QByteArray _fileId;
     QByteArray _localFileId;

diff --git a/src/libsync/encryptedfoldermetadatahandler.cpp b/src/libsync/encryptedfoldermetadatahandler.cpp
@@ -32,17 +32,22 @@ Q_LOGGING_CATEGORY(lcFetchAndUploadE2eeFolderMetadataJob, "nextcloud.sync.propag
 namespace OCC {
 
 EncryptedFolderMetadataHandler::EncryptedFolderMetadataHandler(const AccountPtr &account,
-                                                                         const QString &folderPath,
+                                                                         const QString &folderFullRemotePath,
+                                                                         const QString &remoteFolderRoot,
                                                                          SyncJournalDb *const journalDb,
                                                                          const QString &pathForTopLevelFolder,
                                                                          QObject *parent)
     : QObject(parent)
     , _account(account)
-    , _folderPath(folderPath)
     , _journalDb(journalDb)
+    , _folderFullRemotePath(Utility::noLeadingSlashPath(Utility::noTrailingSlashPath(folderFullRemotePath)))
+    , _remoteFolderRoot(Utility::noLeadingSlashPath(Utility::noTrailingSlashPath(remoteFolderRoot)))
 {
-    _rootEncryptedFolderInfo = RootEncryptedFolderInfo(
-        RootEncryptedFolderInfo::createRootPath(folderPath, pathForTopLevelFolder));
+    Q_ASSERT(!_remoteFolderRoot.isEmpty());
+    Q_ASSERT(_remoteFolderRoot == QStringLiteral("/") || _folderFullRemotePath.startsWith(_remoteFolderRoot));
+
+    const auto folderRelativePath = Utility::fullRemotePathToRemoteSyncRootRelative(_folderFullRemotePath, _remoteFolderRoot);
+    _rootEncryptedFolderInfo = RootEncryptedFolderInfo(RootEncryptedFolderInfo::createRootPath(folderRelativePath, pathForTopLevelFolder));
 }
 
 void EncryptedFolderMetadataHandler::fetchMetadata(const FetchMode fetchMode)
@@ -53,9 +58,22 @@ void EncryptedFolderMetadataHandler::fetchMetadata(const FetchMode fetchMode)
 
 void EncryptedFolderMetadataHandler::fetchMetadata(const RootEncryptedFolderInfo &rootEncryptedFolderInfo, const FetchMode fetchMode)
 {
+    Q_ASSERT(!rootEncryptedFolderInfo.path.isEmpty());
+    if (rootEncryptedFolderInfo.path.isEmpty()) {
+        qCWarning(lcFetchAndUploadE2eeFolderMetadataJob) << "Error fetching metadata for" << _folderFullRemotePath << ". Invalid rootEncryptedFolderInfo!";
+        emit fetchFinished(-1, tr("Error fetching metadata."));
+        return;
+    }
+
     _rootEncryptedFolderInfo = rootEncryptedFolderInfo;
     if (_rootEncryptedFolderInfo.path.isEmpty()) {
-        qCWarning(lcFetchAndUploadE2eeFolderMetadataJob) << "Error fetching metadata for" << _folderPath << ". Invalid _rootEncryptedFolderInfo!";
+        qCWarning(lcFetchAndUploadE2eeFolderMetadataJob) << "Error fetching metadata for" << _folderFullRemotePath << ". Invalid _rootEncryptedFolderInfo!";
+        emit fetchFinished(-1, tr("Error fetching metadata."));
+        return;
+    }
+    if (_remoteFolderRoot != QStringLiteral("/") && !_folderFullRemotePath.startsWith(_remoteFolderRoot)) {
+        qCWarning(lcFetchAndUploadE2eeFolderMetadataJob) << "Error fetching metadata for" << _folderFullRemotePath
+            << " and remote root" << _remoteFolderRoot << ". Invalid _remoteFolderRoot or _folderFullRemotePath!";
         emit fetchFinished(-1, tr("Error fetching metadata."));
         return;
     }
@@ -99,7 +117,7 @@ void EncryptedFolderMetadataHandler::startFetchMetadata()
 void EncryptedFolderMetadataHandler::fetchFolderEncryptedId()
 {
     qCDebug(lcFetchAndUploadE2eeFolderMetadataJob) << "Folder is encrypted, let's get the Id from it.";
-    const auto job = new LsColJob(_account, _folderPath);
+    const auto job = new LsColJob(_account, _folderFullRemotePath);
     job->setProperties({"resourcetype", "http://owncloud.org/ns:fileid"});
     connect(job, &LsColJob::directoryListingSubfolders, this, &EncryptedFolderMetadataHandler::slotFolderEncryptedIdReceived);
     connect(job, &LsColJob::finishedWithError, this, &EncryptedFolderMetadataHandler::slotFolderEncryptedIdError);
@@ -167,17 +185,17 @@ void EncryptedFolderMetadataHandler::slotMetadataReceived(const QJsonDocument &j
 
     if (statusCode != 200 && statusCode != 404) {
         // neither successfully fetched, nor a folder without a metadata, fail further logic
-        qCDebug(lcFetchAndUploadE2eeFolderMetadataJob) << "Error fetching metadata for folder" << _folderPath;
+        qCDebug(lcFetchAndUploadE2eeFolderMetadataJob) << "Error fetching metadata for folder" << _folderFullRemotePath;
         emit fetchFinished(statusCode, tr("Error fetching metadata."));
         return;
     }
 
     const auto rawMetadata = statusCode == 404
         ? QByteArray{} : json.toJson(QJsonDocument::Compact);
-    const auto metadata(QSharedPointer<FolderMetadata>::create(_account, rawMetadata, _rootEncryptedFolderInfo, job->signature()));
+    const auto metadata(QSharedPointer<FolderMetadata>::create(_account, _remoteFolderRoot, rawMetadata, _rootEncryptedFolderInfo, job->signature()));
     connect(metadata.data(), &FolderMetadata::setupComplete, this, [this, metadata] {
         if (!metadata->isValid()) {
-            qCDebug(lcFetchAndUploadE2eeFolderMetadataJob) << "Error parsing or decrypting metadata for folder" << _folderPath;
+            qCDebug(lcFetchAndUploadE2eeFolderMetadataJob) << "Error parsing or decrypting metadata for folder" << _folderFullRemotePath;
             emit fetchFinished(-1, tr("Error parsing or decrypting metadata."));
             return;
         }

diff --git a/src/libsync/encryptedfoldermetadatahandler.h b/src/libsync/encryptedfoldermetadatahandler.h
@@ -50,7 +50,7 @@ class OWNCLOUDSYNC_EXPORT EncryptedFolderMetadataHandler
     };
     Q_ENUM(UnlockFolderWithResult);
 
-    explicit EncryptedFolderMetadataHandler(const AccountPtr &account, const QString &folderPath, SyncJournalDb *const journalDb, const QString &pathForTopLevelFolder, QObject *parent = nullptr);
+    explicit EncryptedFolderMetadataHandler(const AccountPtr &account, const QString &folderPath, const QString &remoteFolderRoot, SyncJournalDb *const journalDb, const QString &pathForTopLevelFolder, QObject *parent = nullptr);
 
     [[nodiscard]] QSharedPointer<FolderMetadata> folderMetadata() const;
 
@@ -101,8 +101,9 @@ public: signals:
 
 private:
     AccountPtr _account;
-    QString _folderPath;
     QPointer<SyncJournalDb> _journalDb;
+    QString _folderFullRemotePath;
+    QString _remoteFolderRoot;
     QByteArray _folderId;
     QByteArray _folderToken;