[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-2-0' - wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/Storage.cpp wsd/Storage.hpp
Jan Holesovsky
kendy at collabora.com
Fri Jun 2 12:24:20 UTC 2017
wsd/ClientSession.cpp | 13 ++++++++++
wsd/ClientSession.hpp | 7 +++++
wsd/DocumentBroker.cpp | 28 ++++++++++------------
wsd/Storage.cpp | 60 +++++++++++++++++++++++++++++++++----------------
wsd/Storage.hpp | 23 +++++++++---------
5 files changed, 86 insertions(+), 45 deletions(-)
New commits:
commit a9f90e49df5b0672239051003e06650294b1e446
Author: Jan Holesovsky <kendy at collabora.com>
Date: Fri May 12 17:42:03 2017 +0200
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 at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index a6e94b9e..c3fcc188 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 36a1f857..f2f5208c 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 423474e1..96ac6273 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 d4426d72..0d4ef129 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 053332fc..d6d54af2 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;
More information about the Libreoffice-commits
mailing list