wsd: When connecting new sessions, always use the original URI...

...but in combination with the appropriate session's access_token to always
authenticate against the same instance of the WOPI host.

Change-Id: Ic94dfa8fcb226a2d134272b22edc1f8f76c24e34
Reviewed-on: https://gerrit.libreoffice.org/37994
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>

5 files changed, 86 insertions, 45 deletions

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index a6e94b9e8..c3fcc188f 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -500,4 +500,17 @@ void ClientSession::senderThread()
     LOG_DBG(getName() + " SenderThread finished");
 }
 
+std::string ClientSession::getAccessToken() const
+{
+    std::string accessToken;
+    Poco::URI::QueryParameters queryParams = _uriPublic.getQueryParameters();
+    for (auto& param: queryParams)
+    {
+        if (param.first == "access_token")
+            return param.second;
+    }
+
+    return std::string();
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 36a1f857c..f2f5208c1 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -92,8 +92,15 @@ public:
 
     /// Exact URI (including query params - access tokens etc.) with which
     /// client made the request to us
+    ///
+    /// Note: This URI is unsafe - when connecting to existing sessions, we must
+    /// ignore everything but the access_token, and use the access_token with
+    /// the URI of the initial request.
     const Poco::URI& getPublicUri() const { return _uriPublic; }
 
+    /// The access token of this session.
+    std::string getAccessToken() const;
+
     /// Set WOPI fileinfo object
     void setWopiFileInfo(std::unique_ptr<WopiStorage::WOPIFileInfo>& wopiFileInfo) { _wopiFileInfo = std::move(wopiFileInfo); }
 
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 423474e19..96ac6273c 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -210,9 +210,6 @@ bool DocumentBroker::load(std::shared_ptr<ClientSession>& session, const std::st
         return false;
     }
 
-    const Poco::URI& uriPublic = session->getPublicUri();
-    LOG_DBG("Loading from URI: " << uriPublic.toString());
-
     _jailId = jailId;
 
     // The URL is the publicly visible one, not visible in the chroot jail.
@@ -230,10 +227,11 @@ bool DocumentBroker::load(std::shared_ptr<ClientSession>& session, const std::st
 
     if (_storage == nullptr)
     {
-        // TODO: Maybe better to pass docKey to storage here instead of uriPublic here because
-        // uriPublic would be different for each view of the document (due to
-        // different query params like access token etc.)
-        LOG_DBG("Creating new storage instance for URI [" << uriPublic.toString() << "].");
+        // Pass the public URI to storage as it needs to load using the token
+        // and other storage-specific data provided in the URI.
+        const Poco::URI& uriPublic = session->getPublicUri();
+        LOG_DBG("Loading, and creating new storage instance for URI [" << uriPublic.toString() << "].");
+
         _storage = StorageBase::create(uriPublic, jailRoot, jailPath.toString());
         if (_storage == nullptr)
         {
@@ -250,7 +248,7 @@ bool DocumentBroker::load(std::shared_ptr<ClientSession>& session, const std::st
     std::chrono::duration<double> getInfoCallDuration(0);
     if (dynamic_cast<WopiStorage*>(_storage.get()) != nullptr)
     {
-        std::unique_ptr<WopiStorage::WOPIFileInfo> wopifileinfo = static_cast<WopiStorage*>(_storage.get())->getWOPIFileInfo(uriPublic);
+        std::unique_ptr<WopiStorage::WOPIFileInfo> wopifileinfo = static_cast<WopiStorage*>(_storage.get())->getWOPIFileInfo(session->getAccessToken());
         userid = wopifileinfo->_userid;
         username = wopifileinfo->_username;
 
@@ -298,7 +296,7 @@ bool DocumentBroker::load(std::shared_ptr<ClientSession>& session, const std::st
     }
     else if (dynamic_cast<LocalStorage*>(_storage.get()) != nullptr)
     {
-        std::unique_ptr<LocalStorage::LocalFileInfo> localfileinfo = static_cast<LocalStorage*>(_storage.get())->getLocalFileInfo(uriPublic);
+        std::unique_ptr<LocalStorage::LocalFileInfo> localfileinfo = static_cast<LocalStorage*>(_storage.get())->getLocalFileInfo();
         userid = localfileinfo->_userid;
         username = localfileinfo->_username;
     }
@@ -311,7 +309,7 @@ bool DocumentBroker::load(std::shared_ptr<ClientSession>& session, const std::st
     const auto fileInfo = _storage->getFileInfo();
     if (!fileInfo.isValid())
     {
-        LOG_ERR("Invalid fileinfo for URI [" << uriPublic.toString() << "].");
+        LOG_ERR("Invalid fileinfo for URI [" << session->getPublicUri().toString() << "].");
         return false;
     }
 
@@ -319,13 +317,13 @@ bool DocumentBroker::load(std::shared_ptr<ClientSession>& session, const std::st
     const bool loaded = _storage->isLoaded();
     if (!loaded)
     {
-        const auto localPath = _storage->loadStorageFileToLocal();
+        const auto localPath = _storage->loadStorageFileToLocal(session->getAccessToken());
         _uriJailed = Poco::URI(Poco::URI("file://"), localPath);
         _filename = fileInfo._filename;
 
         // Use the local temp file's timestamp.
         _lastFileModifiedTime = Poco::File(_storage->getRootFilePath()).getLastModified();
-        _tileCache.reset(new TileCache(_uriPublic.toString(), _lastFileModifiedTime, _cacheRoot));
+        _tileCache.reset(new TileCache(_storage->getUri(), _lastFileModifiedTime, _cacheRoot));
     }
 
     // Since document has been loaded, send the stats if its WOPI
@@ -354,8 +352,8 @@ bool DocumentBroker::save(const std::string& sessionId, bool success, const std:
         return false;
     }
 
-    const Poco::URI& uriPublic = it->second->getPublicUri();
-    const auto uri = uriPublic.toString();
+    const std::string accessToken = it->second->getAccessToken();
+    const auto uri = it->second->getPublicUri().toString();
 
     // If save requested, but core didn't save because document was unmodified
     // notify the waiting thread, if any.
@@ -382,7 +380,7 @@ bool DocumentBroker::save(const std::string& sessionId, bool success, const std:
     LOG_DBG("Saving to URI [" << uri << "].");
 
     assert(_storage && _tileCache);
-    StorageBase::SaveResult storageSaveResult = _storage->saveLocalFileToStorage(uriPublic);
+    StorageBase::SaveResult storageSaveResult = _storage->saveLocalFileToStorage(accessToken);
     if (storageSaveResult == StorageBase::SaveResult::OK)
     {
         _isModified = false;
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index d4426d721..0d4ef1297 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -186,10 +186,10 @@ std::unique_ptr<StorageBase> StorageBase::create(const Poco::URI& uri, const std
 
 std::atomic<unsigned> LocalStorage::LastLocalStorageId;
 
-std::unique_ptr<LocalStorage::LocalFileInfo> LocalStorage::getLocalFileInfo(const Poco::URI& uriPublic)
+std::unique_ptr<LocalStorage::LocalFileInfo> LocalStorage::getLocalFileInfo()
 {
-    const auto path = Poco::Path(uriPublic.getPath());
-    Log::debug("Getting info for local uri [" + uriPublic.toString() + "], path [" + path.toString() + "].");
+    const auto path = Poco::Path(_uri.getPath());
+    LOG_DBG("Getting info for local uri [" << _uri.toString() << "], path [" << path.toString() << "].");
 
     if (!_fileInfo.isValid())
     {
@@ -205,7 +205,7 @@ std::unique_ptr<LocalStorage::LocalFileInfo> LocalStorage::getLocalFileInfo(cons
     return std::unique_ptr<LocalStorage::LocalFileInfo>(new LocalFileInfo({"localhost", std::string("Local Host #") + std::to_string(LastLocalStorageId++)}));
 }
 
-std::string LocalStorage::loadStorageFileToLocal()
+std::string LocalStorage::loadStorageFileToLocal(const std::string& /*accessToken*/)
 {
     // /chroot/jailId/user/doc/childId/file.ext
     const auto filename = Poco::Path(_uri.getPath()).getFileName();
@@ -249,20 +249,20 @@ std::string LocalStorage::loadStorageFileToLocal()
     return Poco::Path(_jailPath, filename).toString();
 }
 
-StorageBase::SaveResult LocalStorage::saveLocalFileToStorage(const Poco::URI& uriPublic)
+StorageBase::SaveResult LocalStorage::saveLocalFileToStorage(const std::string& /*accessToken*/)
 {
     try
     {
         // Copy the file back.
         if (_isCopy && Poco::File(_jailedFilePath).exists())
         {
-            LOG_INF("Copying " << _jailedFilePath << " to " << uriPublic.getPath());
-            Poco::File(_jailedFilePath).copyTo(uriPublic.getPath());
+            LOG_INF("Copying " << _jailedFilePath << " to " << _uri.getPath());
+            Poco::File(_jailedFilePath).copyTo(_uri.getPath());
         }
     }
     catch (const Poco::Exception& exc)
     {
-        LOG_ERR("copyTo(\"" << _jailedFilePath << "\", \"" << uriPublic.getPath() <<
+        LOG_ERR("copyTo(\"" << _jailedFilePath << "\", \"" << _uri.getPath() <<
                 "\") failed: " << exc.displayText());
         throw;
     }
@@ -351,16 +351,37 @@ void getWOPIValue(const Poco::JSON::Object::Ptr &object, const std::string& key,
     LOG_WRN("Missing JSON property [" << key << "]");
 }
 
+void setQueryParameter(Poco::URI& uriObject, const std::string& key, const std::string& value)
+{
+    Poco::URI::QueryParameters queryParams = uriObject.getQueryParameters();
+    for (auto& param: queryParams)
+    {
+        if (param.first == key)
+        {
+            param.second = value;
+            uriObject.setQueryParameters(queryParams);
+            return;
+        }
+    }
+
+    // it did not exist yet
+    uriObject.addQueryParameter(key, value);
+}
+
 } // anonymous namespace
 
-std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Poco::URI& uriPublic)
+std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const std::string& accessToken)
 {
-    Log::debug("Getting info for wopi uri [" + uriPublic.toString() + "].");
+    // update the access_token to the one matching to the session
+    Poco::URI uriObject(_uri);
+    setQueryParameter(uriObject, "access_token", accessToken);
+
+    Log::debug("Getting info for wopi uri [" + uriObject.toString() + "].");
 
     const auto startTime = std::chrono::steady_clock::now();
-    std::unique_ptr<Poco::Net::HTTPClientSession> psession(getHTTPClientSession(uriPublic));
+    std::unique_ptr<Poco::Net::HTTPClientSession> psession(getHTTPClientSession(uriObject));
 
-    Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, uriPublic.getPathAndQuery(), Poco::Net::HTTPMessage::HTTP_1_1);
+    Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, uriObject.getPathAndQuery(), Poco::Net::HTTPMessage::HTTP_1_1);
     request.set("User-Agent", "LOOLWSD WOPI Agent");
     psession->sendRequest(request);
 
@@ -368,7 +389,7 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Po
 
     std::istream& rs = psession->receiveResponse(response);
     auto logger = Log::trace();
-    logger << "WOPI::CheckFileInfo header for URI [" << uriPublic.toString() << "]:\n";
+    logger << "WOPI::CheckFileInfo header for URI [" << uriObject.toString() << "]:\n";
     for (auto& pair : response)
     {
         logger << '\t' + pair.first + ": " + pair.second << " / ";
@@ -432,12 +453,13 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Po
 }
 
 /// uri format: http://server/<...>/wopi*/files/<id>/content
-std::string WopiStorage::loadStorageFileToLocal()
+std::string WopiStorage::loadStorageFileToLocal(const std::string& accessToken)
 {
     // WOPI URI to download files ends in '/contents'.
     // Add it here to get the payload instead of file info.
     Poco::URI uriObject(_uri);
     uriObject.setPath(uriObject.getPath() + "/contents");
+    setQueryParameter(uriObject, "access_token", accessToken);
     Log::debug("Wopi requesting: " + uriObject.toString());
 
     const auto startTime = std::chrono::steady_clock::now();
@@ -477,14 +499,14 @@ std::string WopiStorage::loadStorageFileToLocal()
     return Poco::Path(_jailPath, _fileInfo._filename).toString();
 }
 
-StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Poco::URI& uriPublic)
+StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const std::string& accessToken)
 {
-    LOG_INF("Uploading URI [" << uriPublic.toString() << "] from [" << _jailedFilePath + "].");
     // TODO: Check if this URI has write permission (canWrite = true)
     const auto size = getFileSize(_jailedFilePath);
 
-    Poco::URI uriObject(uriPublic);
+    Poco::URI uriObject(_uri);
     uriObject.setPath(uriObject.getPath() + "/contents");
+    setQueryParameter(uriObject, "access_token", accessToken);
     Log::debug("Wopi posting: " + uriObject.toString());
 
     std::unique_ptr<Poco::Net::HTTPClientSession> psession(getHTTPClientSession(uriObject));
@@ -521,14 +543,14 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Poco::URI& uri
     return saveResult;
 }
 
-std::string WebDAVStorage::loadStorageFileToLocal()
+std::string WebDAVStorage::loadStorageFileToLocal(const std::string& /*accessToken*/)
 {
     // TODO: implement webdav GET.
     _isLoaded = true;
     return _uri.toString();
 }
 
-StorageBase::SaveResult WebDAVStorage::saveLocalFileToStorage(const Poco::URI& /*uriPublic*/)
+StorageBase::SaveResult WebDAVStorage::saveLocalFileToStorage(const std::string& /*accessToken*/)
 {
     // TODO: implement webdav PUT.
     return StorageBase::SaveResult::OK;
diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp
index 053332fc3..d6d54af24 100644
--- a/wsd/Storage.hpp
+++ b/wsd/Storage.hpp
@@ -85,10 +85,10 @@ public:
 
     /// Returns a local file path for the given URI.
     /// If necessary copies the file locally first.
-    virtual std::string loadStorageFileToLocal() = 0;
+    virtual std::string loadStorageFileToLocal(const std::string& accessToken) = 0;
 
     /// Writes the contents of the file back to the source.
-    virtual SaveResult saveLocalFileToStorage(const Poco::URI& uriPublic) = 0;
+    virtual SaveResult saveLocalFileToStorage(const std::string& accessToken) = 0;
 
     static size_t getFileSize(const std::string& filename);
 
@@ -148,11 +148,11 @@ public:
     /// Returns the URI specific file data
     /// Also stores the basic file information which can then be
     /// obtained using getFileInfo method
-    std::unique_ptr<LocalFileInfo> getLocalFileInfo(const Poco::URI& uriPublic);
+    std::unique_ptr<LocalFileInfo> getLocalFileInfo();
 
-    std::string loadStorageFileToLocal() override;
+    std::string loadStorageFileToLocal(const std::string& accessToken) override;
 
-    SaveResult saveLocalFileToStorage(const Poco::URI& uriPublic) override;
+    SaveResult saveLocalFileToStorage(const std::string& accessToken) override;
 
 private:
     /// True if the jailed file is not linked but copied.
@@ -230,15 +230,16 @@ public:
         std::chrono::duration<double> _callDuration;
     };
 
-    /// Returns the response of CheckFileInfo WOPI call for given URI
+    /// Returns the response of CheckFileInfo WOPI call for URI that was
+    /// 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 Poco::URI& uriPublic);
+    std::unique_ptr<WOPIFileInfo> getWOPIFileInfo(const std::string& accessToken);
 
     /// uri format: http://server/<...>/wopi*/files/<id>/content
-    std::string loadStorageFileToLocal() override;
+    std::string loadStorageFileToLocal(const std::string& accessToken) override;
 
-    SaveResult saveLocalFileToStorage(const Poco::URI& uriPublic) override;
+    SaveResult saveLocalFileToStorage(const std::string& accessToken) override;
 
     /// Total time taken for making WOPI calls during load
     std::chrono::duration<double> getWopiLoadDuration() const { return _wopiLoadDuration; }
@@ -266,9 +267,9 @@ public:
     // Implement me
     // WebDAVFileInfo getWebDAVFileInfo(const Poco::URI& uriPublic);
 
-    std::string loadStorageFileToLocal() override;
+    std::string loadStorageFileToLocal(const std::string& accessToken) override;
 
-    SaveResult saveLocalFileToStorage(const Poco::URI& uriPublic) override;
+    SaveResult saveLocalFileToStorage(const std::string& accessToken) override;
 
 private:
     std::unique_ptr<AuthBase> _authAgent;