wsd/ClientSession.cpp | 13 +++++++++
wsd/ClientSession.hpp | 7 +++++
wsd/DocumentBroker.cpp | 31 ++++++++++-------------
wsd/Storage.cpp | 65 +++++++++++++++++++++++++++++++++----------------
wsd/Storage.hpp | 23 +++++++++--------
5 files changed, 90 insertions(+), 49 deletions(-)
New commits:
commit 95e892168ce3b6e56ded62bb9ce2c02ada627dba
Author: Jan Holesovsky <ke...@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
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 55b17d64..5ef70522 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -726,6 +726,19 @@ bool ClientSession::forwardToClient(const
std::shared_ptr<Message>& payload)
return true;
}
+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();
+}
+
void ClientSession::onDisconnect()
{
LOG_INF(getName() << " Disconnected, current number of connections: " <<
LOOLWSD::NumConnections);
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 22fad016..8c791c88 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -90,8 +90,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 21f6965f..bb7b9c5e 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -383,9 +383,6 @@ bool DocumentBroker::load(const
std::shared_ptr<ClientSession>& session, const s
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.
@@ -402,7 +399,9 @@ bool DocumentBroker::load(const
std::shared_ptr<ClientSession>& session, const s
{
// Pass the public URI to storage as it needs to load using the token
// and other storage-specific data provided in the URI.
- LOG_DBG("Creating new storage instance for URI [" <<
uriPublic.toString() << "].");
+ 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)
{
@@ -421,8 +420,7 @@ 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(uriPublic);
+ std::unique_ptr<WopiStorage::WOPIFileInfo> wopifileinfo =
wopiStorage->getWOPIFileInfo(session->getAccessToken());
userid = wopifileinfo->_userid;
username = wopifileinfo->_username;
@@ -473,8 +471,7 @@ bool DocumentBroker::load(const
std::shared_ptr<ClientSession>& session, const s
LocalStorage* localStorage =
dynamic_cast<LocalStorage*>(_storage.get());
if (localStorage != nullptr)
{
- std::unique_ptr<LocalStorage::LocalFileInfo> localfileinfo =
-
localStorage->getLocalFileInfo(uriPublic);
+ std::unique_ptr<LocalStorage::LocalFileInfo> localfileinfo =
localStorage->getLocalFileInfo();
userid = localfileinfo->_userid;
username = localfileinfo->_username;
}
@@ -488,7 +485,7 @@ bool DocumentBroker::load(const
std::shared_ptr<ClientSession>& session, const s
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;
}
@@ -509,7 +506,7 @@ bool DocumentBroker::load(const
std::shared_ptr<ClientSession>& session, const s
fileInfo._modifiedTime != Zero &&
_documentLastModifiedTime != fileInfo._modifiedTime)
{
- LOG_ERR("Document has been modified behind our back, URI [" <<
uriPublic.toString() << "].");
+ LOG_ERR("Document has been modified behind our back, URI [" <<
session->getPublicUri().toString() << "].");
// What do do?
}
}
@@ -517,7 +514,7 @@ bool DocumentBroker::load(const
std::shared_ptr<ClientSession>& session, const s
// Let's load the document now, if not loaded.
if (!_storage->isLoaded())
{
- const auto localPath = _storage->loadStorageFileToLocal();
+ const auto localPath =
_storage->loadStorageFileToLocal(session->getAccessToken());
std::ifstream istr(localPath, std::ios::binary);
Poco::SHA1Engine sha1;
@@ -532,7 +529,7 @@ bool DocumentBroker::load(const
std::shared_ptr<ClientSession>& session, const s
// 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));
}
LOOLWSD::dumpNewSessionTrace(getJailId(), sessionId, _uriOrig,
_storage->getRootFilePath());
@@ -598,8 +595,8 @@ bool DocumentBroker::saveToStorageInternal(const
std::string& sessionId,
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 we aren't destroying the last editable session just yet,
// and the file timestamp hasn't changed, skip saving.
@@ -620,7 +617,7 @@ bool DocumentBroker::saveToStorageInternal(const
std::string& sessionId,
// storage behind our backs.
assert(_storage && _tileCache);
- StorageBase::SaveResult storageSaveResult =
_storage->saveLocalFileToStorage(uriPublic);
+ StorageBase::SaveResult storageSaveResult =
_storage->saveLocalFileToStorage(accessToken);
if (storageSaveResult == StorageBase::SaveResult::OK)
{
_isModified = false;
@@ -637,11 +634,11 @@ bool DocumentBroker::saveToStorageInternal(const
std::string& sessionId,
// dynamic_cast dance.
if (dynamic_cast<WopiStorage*>(_storage.get()) != nullptr)
{
- auto wopiFileInfo =
static_cast<WopiStorage*>(_storage.get())->getWOPIFileInfo(uriPublic);
+ auto wopiFileInfo =
static_cast<WopiStorage*>(_storage.get())->getWOPIFileInfo(accessToken);
}
else if (dynamic_cast<LocalStorage*>(_storage.get()) != nullptr)
{
- auto localFileInfo =
static_cast<LocalStorage*>(_storage.get())->getLocalFileInfo(uriPublic);
+ auto localFileInfo =
static_cast<LocalStorage*>(_storage.get())->getLocalFileInfo();
}
// So set _documentLastModifiedTime then
_documentLastModifiedTime = _storage->getFileInfo()._modifiedTime;
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index f0f525eb..10849e48 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -214,10 +214,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_DBG("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() << "].");
const auto& filename = path.getFileName();
const auto file = Poco::File(path);
@@ -230,7 +230,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();
@@ -281,20 +281,20 @@ std::string LocalStorage::loadStorageFileToLocal()
#endif
}
-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;
}
@@ -387,20 +387,41 @@ 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_DBG("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_DBG("Getting info for wopi uri [" << uriObject.toString() << "].");
std::string resMsg;
const auto startTime = std::chrono::steady_clock::now();
std::chrono::duration<double> callDuration(0);
try
{
- 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);
@@ -411,7 +432,7 @@ std::unique_ptr<WopiStorage::WOPIFileInfo>
WopiStorage::getWOPIFileInfo(const Po
auto logger = Log::trace();
if (logger.enabled())
{
- logger << "WOPI::CheckFileInfo header for URI [" <<
uriPublic.toString() << "]:\n";
+ logger << "WOPI::CheckFileInfo header for URI [" <<
uriObject.toString() << "]:\n";
for (const auto& pair : response)
{
logger << '\t' << pair.first << ": " << pair.second << " / ";
@@ -424,7 +445,7 @@ std::unique_ptr<WopiStorage::WOPIFileInfo>
WopiStorage::getWOPIFileInfo(const Po
}
catch(const Poco::Exception& pexc)
{
- LOG_ERR("Cannot get file info from WOPI storage uri [" +
uriPublic.toString() + "]. Error: " << pexc.displayText() <<
+ LOG_ERR("Cannot get file info from WOPI storage uri [" +
uriObject.toString() + "]. Error: " << pexc.displayText() <<
(pexc.nested() ? " (" + pexc.nested()->displayText() + ")" :
""));
throw;
}
@@ -503,12 +524,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_DBG("Wopi requesting: " << uriObject.toString());
const auto startTime = std::chrono::steady_clock::now();
@@ -559,15 +581,16 @@ 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");
- LOG_DBG("Wopi posting: " + uriObject.toString());
+ setQueryParameter(uriObject, "access_token", accessToken);
+
+ LOG_INF("Uploading URI via WOPI [" << uriObject.toString() << "] from ["
<< _jailedFilePath + "].");
std::ostringstream oss;
StorageBase::SaveResult saveResult = StorageBase::SaveResult::FAILED;
@@ -611,14 +634,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 7def9822..6613fbbc 100644
--- a/wsd/Storage.hpp
+++ b/wsd/Storage.hpp
@@ -86,10 +86,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);
@@ -150,11 +150,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.
@@ -232,15 +232,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; }
@@ -268,9 +269,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;
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
