[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-4-0' - wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/Storage.cpp wsd/Storage.hpp

Ashod Nakashian (via logerrit) logerrit at kemper.freedesktop.org
Mon Jan 20 11:01:32 UTC 2020


 wsd/ClientSession.cpp  |   11 +++++++
 wsd/ClientSession.hpp  |    5 +++
 wsd/DocumentBroker.cpp |   10 ++++--
 wsd/Storage.cpp        |   74 +++++++++++++++++++++----------------------------
 wsd/Storage.hpp        |   39 +++++++++++++++++++------
 5 files changed, 86 insertions(+), 53 deletions(-)

New commits:
commit ffa11008a2d025cfcc1ff760e72b4422c72790de
Author:     Ashod Nakashian <ashod.nakashian at collabora.co.uk>
AuthorDate: Sat Jan 18 16:04:26 2020 -0500
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Mon Jan 20 12:01:11 2020 +0100

    wsd: per-user cookies
    
    Cookies may be passed from the client to the storage,
    in which case each user may have its own unique set
    of cookies. These cookies are now preserved in the
    ClientSession, which is per connection, and are then
    passed to the storage to use when communicating with
    the WOPI-like backend.
    
    Change-Id: Ic2e13fa541a5ee01b7383939bbbf7d46ea75684b
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/87033
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
    Tested-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index c83d3abb2..42b8a5f73 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -56,6 +56,17 @@ ClientSession::ClientSession(const std::string& id,
 {
     const size_t curConnections = ++LOOLWSD::NumConnections;
     LOG_INF("ClientSession ctor [" << getName() << "], current number of connections: " << curConnections);
+
+    for (const auto& param : _uriPublic.getQueryParameters())
+    {
+        if (param.first == "reuse_cookies")
+        {
+            // Cache the cookies to avoid re-parsing the URI again.
+            _cookies = param.second;
+            break;
+        }
+    }
+
 }
 
 ClientSession::~ClientSession()
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index b701e5568..5b3d98ace 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -93,6 +93,8 @@ public:
     /// The access token of this session.
     Authorization getAuthorization() const;
 
+    const std::string& getCookies() const { return _cookies; }
+
     /// Set WOPI fileinfo object
     void setWopiFileInfo(std::unique_ptr<WopiStorage::WOPIFileInfo>& wopiFileInfo) { _wopiFileInfo = std::move(wopiFileInfo); }
 
@@ -179,6 +181,9 @@ private:
     /// URI with which client made request to us
     const Poco::URI _uriPublic;
 
+    /// The cookies we should pass on to the storage on saving.
+    std::string _cookies;
+
     /// Whether this session is the owner of currently opened document
     bool _isDocumentOwner;
 
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 30d6d5e8d..6cebd7d25 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -519,7 +519,8 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
     WopiStorage* wopiStorage = dynamic_cast<WopiStorage*>(_storage.get());
     if (wopiStorage != nullptr)
     {
-        std::unique_ptr<WopiStorage::WOPIFileInfo> wopifileinfo = wopiStorage->getWOPIFileInfo(session->getAuthorization());
+        std::unique_ptr<WopiStorage::WOPIFileInfo> wopifileinfo
+            = wopiStorage->getWOPIFileInfo(session->getAuthorization(), session->getCookies());
         userId = wopifileinfo->getUserId();
         username = wopifileinfo->getUsername();
         userExtraInfo = wopifileinfo->getUserExtraInfo();
@@ -677,7 +678,8 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
     // Let's load the document now, if not loaded.
     if (!_storage->isLoaded())
     {
-        std::string localPath = _storage->loadStorageFileToLocal(session->getAuthorization(), templateSource);
+        std::string localPath = _storage->loadStorageFileToLocal(
+            session->getAuthorization(), session->getCookies(), templateSource);
 
 #ifndef MOBILEAPP
         // Check if we have a prefilter "plugin" for this document format
@@ -885,7 +887,9 @@ bool DocumentBroker::saveToStorageInternal(const std::string& sessionId, bool su
     LOG_DBG("Persisting [" << _docKey << "] after saving to URI [" << uriAnonym << "].");
 
     assert(_storage && _tileCache);
-    StorageBase::SaveResult storageSaveResult = _storage->saveLocalFileToStorage(auth, saveAsPath, saveAsFilename, isRename);
+    const std::string cookies = it->second->getCookies();
+    const StorageBase::SaveResult storageSaveResult
+        = _storage->saveLocalFileToStorage(auth, cookies, saveAsPath, saveAsFilename, isRename);
     if (storageSaveResult.getResult() == StorageBase::SaveResult::OK)
     {
         if (!isSaveAs && !isRename)
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 14e797e66..04d5298a1 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -272,7 +272,9 @@ std::unique_ptr<LocalStorage::LocalFileInfo> LocalStorage::getLocalFileInfo()
     return std::unique_ptr<LocalStorage::LocalFileInfo>(new LocalFileInfo({"localhost" + std::to_string(LastLocalStorageId), "LocalHost#" + std::to_string(LastLocalStorageId++)}));
 }
 
-std::string LocalStorage::loadStorageFileToLocal(const Authorization& /*auth*/, const std::string& /*templateUri*/)
+std::string LocalStorage::loadStorageFileToLocal(const Authorization& /*auth*/,
+                                                 const std::string& /*cookies*/,
+                                                 const std::string& /*templateUri*/)
 {
 #ifndef MOBILEAPP
     // /chroot/jailId/user/doc/childId/file.ext
@@ -336,7 +338,11 @@ std::string LocalStorage::loadStorageFileToLocal(const Authorization& /*auth*/,
 
 }
 
-StorageBase::SaveResult LocalStorage::saveLocalFileToStorage(const Authorization& /*auth*/, const std::string& /*saveAsPath*/, const std::string& /*saveAsFilename*/, bool /*isRename*/)
+StorageBase::SaveResult LocalStorage::saveLocalFileToStorage(const Authorization& /*auth*/,
+                                                             const std::string& /*cookies*/,
+                                                             const std::string& /*saveAsPath*/,
+                                                             const std::string& /*saveAsFilename*/,
+                                                             bool /*isRename*/)
 {
     try
     {
@@ -402,6 +408,8 @@ static void addStorageReuseCookie(Poco::Net::HTTPRequest& request, const std::st
     if (!reuseStorageCookies.empty())
     {
         Poco::Net::NameValueCollection nvcCookies;
+        request.getCookies(nvcCookies); // Preserve existing cookies.
+
         std::vector<std::string> cookies = LOOLProtocol::tokenize(reuseStorageCookies, ':');
         for (auto cookie : cookies)
         {
@@ -409,9 +417,10 @@ static void addStorageReuseCookie(Poco::Net::HTTPRequest& request, const std::st
             if (cookieTokens.size() == 2)
             {
                 nvcCookies.add(cookieTokens[0], cookieTokens[1]);
-                LOG_DBG("Added storage reuse cookie [" << cookieTokens[0] << "=" << cookieTokens[1] << "].");
+                LOG_DBG("Added storage reuse cookie [" << cookieTokens[0] << '=' << cookieTokens[1] << "].");
             }
         }
+
         request.setCookies(nvcCookies);
     }
 }
@@ -440,23 +449,14 @@ static Poco::Timestamp iso8601ToTimestamp(const std::string& iso8601Time, const
 
 } // anonymous namespace
 
-std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Authorization& auth)
+std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Authorization& auth,
+                                                                        const std::string& cookies)
 {
     // update the access_token to the one matching to the session
     Poco::URI uriObject(getUri());
     auth.authorizeURI(uriObject);
     const std::string uriAnonym = LOOLWSD::anonymizeUrl(uriObject.toString());
 
-    std::string reuseStorageCookies;
-    for (const auto& param : uriObject.getQueryParameters())
-    {
-        if (param.first == "reuse_cookies")
-        {
-            reuseStorageCookies = param.second;
-            break;
-        }
-    }
-
     LOG_DBG("Getting info for wopi uri [" << uriAnonym << "].");
 
     std::string wopiResponse;
@@ -468,7 +468,7 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Au
         auth.authorizeRequest(request);
         addStorageDebugCookie(request);
         if (_reuseCookies)
-            addStorageReuseCookie(request, reuseStorageCookies);
+            addStorageReuseCookie(request, cookies);
         const auto startTime = std::chrono::steady_clock::now();
 
         std::unique_ptr<Poco::Net::HTTPClientSession> psession(getHTTPClientSession(uriObject));
@@ -659,7 +659,9 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Au
 }
 
 /// uri format: http://server/<...>/wopi*/files/<id>/content
-std::string WopiStorage::loadStorageFileToLocal(const Authorization& auth, const std::string& templateUri)
+std::string WopiStorage::loadStorageFileToLocal(const Authorization& auth,
+                                                const std::string& cookies,
+                                                const std::string& templateUri)
 {
     // WOPI URI to download files ends in '/contents'.
     // Add it here to get the payload instead of file info.
@@ -667,16 +669,6 @@ std::string WopiStorage::loadStorageFileToLocal(const Authorization& auth, const
     uriObject.setPath(uriObject.getPath() + "/contents");
     auth.authorizeURI(uriObject);
 
-    std::string reuseStorageCookies;
-    for (const auto& param : uriObject.getQueryParameters())
-    {
-        if (param.first == "reuse_cookies")
-        {
-            reuseStorageCookies = param.second;
-            break;
-        }
-    }
-
     Poco::URI uriObjectAnonym(getUri());
     uriObjectAnonym.setPath(LOOLWSD::anonymizeUrl(uriObjectAnonym.getPath()) + "/contents");
     const std::string uriAnonym = uriObjectAnonym.toString();
@@ -704,7 +696,7 @@ std::string WopiStorage::loadStorageFileToLocal(const Authorization& auth, const
         auth.authorizeRequest(request);
         addStorageDebugCookie(request);
         if (_reuseCookies)
-            addStorageReuseCookie(request, reuseStorageCookies);
+            addStorageReuseCookie(request, cookies);
         psession->sendRequest(request);
 
         Poco::Net::HTTPResponse response;
@@ -756,7 +748,11 @@ std::string WopiStorage::loadStorageFileToLocal(const Authorization& auth, const
     return "";
 }
 
-StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Authorization& auth, const std::string& saveAsPath, const std::string& saveAsFilename, const bool isRename)
+StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Authorization& auth,
+                                                            const std::string& cookies,
+                                                            const std::string& saveAsPath,
+                                                            const std::string& saveAsFilename,
+                                                            const bool isRename)
 {
     // TODO: Check if this URI has write permission (canWrite = true)
 
@@ -770,16 +766,6 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Authorization&
     uriObject.setPath(isSaveAs || isRename? uriObject.getPath(): uriObject.getPath() + "/contents");
     auth.authorizeURI(uriObject);
 
-    std::string reuseStorageCookies;
-    for (const auto& param : uriObject.getQueryParameters())
-    {
-        if (param.first == "reuse_cookies")
-        {
-            reuseStorageCookies = param.second;
-            break;
-        }
-    }
-
     const std::string uriAnonym = LOOLWSD::anonymizeUrl(uriObject.toString());
 
     LOG_INF("Uploading URI via WOPI [" << uriAnonym << "] from [" << filePathAnonym + "].");
@@ -859,7 +845,7 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Authorization&
         request.setContentLength(size);
         addStorageDebugCookie(request);
         if (_reuseCookies)
-            addStorageReuseCookie(request, reuseStorageCookies);
+            addStorageReuseCookie(request, cookies);
         std::ostream& os = psession->sendRequest(request);
 
         std::ifstream ifs(filePath);
@@ -981,14 +967,20 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Authorization&
     return saveResult;
 }
 
-std::string WebDAVStorage::loadStorageFileToLocal(const Authorization& /*auth*/, const std::string& /*templateUri*/)
+std::string WebDAVStorage::loadStorageFileToLocal(const Authorization& /*auth*/,
+                                                  const std::string& /*cookies*/,
+                                                  const std::string& /*templateUri*/)
 {
     // TODO: implement webdav GET.
     setLoaded(true);
     return getUri().toString();
 }
 
-StorageBase::SaveResult WebDAVStorage::saveLocalFileToStorage(const Authorization& /*auth*/, const std::string& /*saveAsPath*/, const std::string& /*saveAsFilename*/, bool /*isRename*/)
+StorageBase::SaveResult WebDAVStorage::saveLocalFileToStorage(const Authorization& /*auth*/,
+                                                              const std::string& /*cookies*/,
+                                                              const std::string& /*saveAsPath*/,
+                                                              const std::string& /*saveAsFilename*/,
+                                                              bool /*isRename*/)
 {
     // TODO: implement webdav PUT.
     return StorageBase::SaveResult(StorageBase::SaveResult::OK);
diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp
index d2049d488..8508d142d 100644
--- a/wsd/Storage.hpp
+++ b/wsd/Storage.hpp
@@ -190,11 +190,19 @@ public:
 
     /// Returns a local file path for the given URI.
     /// If necessary copies the file locally first.
-    virtual std::string loadStorageFileToLocal(const Authorization& auth, const std::string& templateUri) = 0;
+    virtual std::string loadStorageFileToLocal(const Authorization& auth,
+                                               const std::string& cookies,
+                                               const std::string& templateUri)
+        = 0;
 
     /// Writes the contents of the file back to the source.
+    /// @param cookies A string representing key=value pairs that are set as cookies.
     /// @param savedFile When the operation was saveAs, this is the path to the file that was saved.
-    virtual SaveResult saveLocalFileToStorage(const Authorization& auth, const std::string& saveAsPath, const std::string& saveAsFilename, const bool isRename) = 0;
+    virtual SaveResult saveLocalFileToStorage(const Authorization& auth, const std::string& cookies,
+                                              const std::string& saveAsPath,
+                                              const std::string& saveAsFilename,
+                                              const bool isRename)
+        = 0;
 
     static size_t getFileSize(const std::string& filename);
 
@@ -278,9 +286,13 @@ public:
     /// obtained using getFileInfo method
     std::unique_ptr<LocalFileInfo> getLocalFileInfo();
 
-    std::string loadStorageFileToLocal(const Authorization& auth, const std::string& templateUri) override;
+    std::string loadStorageFileToLocal(const Authorization& auth, const std::string& cookies,
+                                       const std::string& templateUri) override;
 
-    SaveResult saveLocalFileToStorage(const Authorization& auth, const std::string& saveAsPath, const std::string& saveAsFilename, const bool isRename) override;
+    SaveResult saveLocalFileToStorage(const Authorization& auth, const std::string& cookies,
+                                      const std::string& saveAsPath,
+                                      const std::string& saveAsFilename,
+                                      const bool isRename) override;
 
 private:
     /// True if the jailed file is not linked but copied.
@@ -494,12 +506,17 @@ public:
     /// provided during the initial creation of the WOPI storage.
     /// Also extracts the basic file information from the response
     /// which can then be obtained using getFileInfo()
-    std::unique_ptr<WOPIFileInfo> getWOPIFileInfo(const Authorization& auth);
+    std::unique_ptr<WOPIFileInfo> getWOPIFileInfo(const Authorization& auth,
+                                                  const std::string& cookies);
 
     /// uri format: http://server/<...>/wopi*/files/<id>/content
-    std::string loadStorageFileToLocal(const Authorization& auth, const std::string& templateUri) override;
+    std::string loadStorageFileToLocal(const Authorization& auth, const std::string& cookies,
+                                       const std::string& templateUri) override;
 
-    SaveResult saveLocalFileToStorage(const Authorization& auth, const std::string& saveAsPath, const std::string& saveAsFilename, const bool isRename) override;
+    SaveResult saveLocalFileToStorage(const Authorization& auth, const std::string& cookies,
+                                      const std::string& saveAsPath,
+                                      const std::string& saveAsFilename,
+                                      const bool isRename) override;
 
     /// Total time taken for making WOPI calls during load
     std::chrono::duration<double> getWopiLoadDuration() const { return _wopiLoadDuration; }
@@ -529,9 +546,13 @@ public:
     // Implement me
     // WebDAVFileInfo getWebDAVFileInfo(const Poco::URI& uriPublic);
 
-    std::string loadStorageFileToLocal(const Authorization& auth, const std::string& templateUri) override;
+    std::string loadStorageFileToLocal(const Authorization& auth, const std::string& cookies,
+                                       const std::string& templateUri) override;
 
-    SaveResult saveLocalFileToStorage(const Authorization& auth, const std::string& saveAsPath, const std::string& saveAsFilename, const bool isRename) override;
+    SaveResult saveLocalFileToStorage(const Authorization& auth, const std::string& cookies,
+                                      const std::string& saveAsPath,
+                                      const std::string& saveAsFilename,
+                                      const bool isRename) override;
 
 private:
     std::unique_ptr<AuthBase> _authAgent;


More information about the Libreoffice-commits mailing list