[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-2-1' - test/Makefile.am test/WhiteBoxTests.cpp wsd/Auth.cpp wsd/Auth.hpp wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/Storage.cpp wsd/Storage.hpp
Jan Holesovsky
kendy at collabora.com
Thu Aug 17 08:05:51 UTC 2017
test/Makefile.am | 1
test/WhiteBoxTests.cpp | 45 +++++++++++++++++++++++++++++++++++++++
wsd/Auth.cpp | 56 +++++++++++++++++++++++++++++++++++++++++++++++++
wsd/Auth.hpp | 36 +++++++++++++++++++++++++++++++
wsd/ClientSession.cpp | 14 +++++++++---
wsd/ClientSession.hpp | 2 -
wsd/DocumentBroker.cpp | 8 +++----
wsd/Storage.cpp | 45 ++++++++++++---------------------------
wsd/Storage.hpp | 18 +++++++--------
9 files changed, 177 insertions(+), 48 deletions(-)
New commits:
commit 260fe8a30fa84c01778f2fa5210ff5cff5fef1ab
Author: Jan Holesovsky <kendy at collabora.com>
Date: Wed Aug 16 16:38:00 2017 +0200
access_header: Infrastructure for providing custom headers for authentication.
Change-Id: I52e61dc01dbad0d501471e663aaf364d9bc23c52
Reviewed-on: https://gerrit.libreoffice.org/41223
Reviewed-by: Miklos Vajna <vmiklos at collabora.co.uk>
Reviewed-by: pranavk <pranavk at collabora.co.uk>
Tested-by: pranavk <pranavk at collabora.co.uk>
diff --git a/test/Makefile.am b/test/Makefile.am
index 04f4a293..8f611d2e 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -37,6 +37,7 @@ wsd_sources = \
../common/Util.cpp \
../common/MessageQueue.cpp \
../kit/Kit.cpp \
+ ../wsd/Auth.cpp \
../wsd/TileCache.cpp \
../wsd/TestStubs.cpp \
../common/Unit.cpp \
diff --git a/test/WhiteBoxTests.cpp b/test/WhiteBoxTests.cpp
index 825ebb4e..07c630b8 100644
--- a/test/WhiteBoxTests.cpp
+++ b/test/WhiteBoxTests.cpp
@@ -11,6 +11,7 @@
#include <cppunit/extensions/HelperMacros.h>
+#include <Auth.hpp>
#include <ChildSession.hpp>
#include <Common.hpp>
#include <Kit.hpp>
@@ -31,6 +32,7 @@ class WhiteBoxTests : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testRegexListMatcher_Init);
CPPUNIT_TEST(testEmptyCellCursor);
CPPUNIT_TEST(testRectanglesIntersect);
+ CPPUNIT_TEST(testAuthorization);
CPPUNIT_TEST_SUITE_END();
@@ -41,6 +43,7 @@ class WhiteBoxTests : public CPPUNIT_NS::TestFixture
void testRegexListMatcher_Init();
void testEmptyCellCursor();
void testRectanglesIntersect();
+ void testAuthorization();
};
void WhiteBoxTests::testLOOLProtocolFunctions()
@@ -398,6 +401,48 @@ void WhiteBoxTests::testRectanglesIntersect()
1000, 1000, 2000, 1000));
}
+void WhiteBoxTests::testAuthorization()
+{
+ Authorization auth1(Authorization::Type::Token, "abc");
+ Poco::URI uri1("http://localhost");
+ auth1.authorizeURI(uri1);
+ CPPUNIT_ASSERT_EQUAL(uri1.toString(), std::string("http://localhost/?access_token=abc"));
+ Poco::Net::HTTPRequest req1;
+ auth1.authorizeRequest(req1);
+ CPPUNIT_ASSERT_EQUAL(req1.get("Authorization"), std::string("Bearer abc"));
+
+ Authorization auth1modify(Authorization::Type::Token, "modified");
+ // still the same uri1, currently "http://localhost/?access_token=abc"
+ auth1modify.authorizeURI(uri1);
+ CPPUNIT_ASSERT_EQUAL(uri1.toString(), std::string("http://localhost/?access_token=modified"));
+
+ Authorization auth2(Authorization::Type::Header, "def");
+ Poco::Net::HTTPRequest req2;
+ auth2.authorizeRequest(req2);
+ CPPUNIT_ASSERT(!req2.has("Authorization"));
+
+ Authorization auth3(Authorization::Type::Header, "Authorization: Basic huhu== ");
+ Poco::URI uri2("http://localhost");
+ auth3.authorizeURI(uri2);
+ // nothing added with the Authorization header approach
+ CPPUNIT_ASSERT_EQUAL(uri2.toString(), std::string("http://localhost"));
+ Poco::Net::HTTPRequest req3;
+ auth3.authorizeRequest(req3);
+ CPPUNIT_ASSERT_EQUAL(req3.get("Authorization"), std::string("Basic huhu=="));
+
+ Authorization auth4(Authorization::Type::Header, " Authorization: Basic blah== \n\r X-Something: additional ");
+ Poco::Net::HTTPRequest req4;
+ auth4.authorizeRequest(req4);
+ CPPUNIT_ASSERT_EQUAL(req4.get("Authorization"), std::string("Basic blah=="));
+ CPPUNIT_ASSERT_EQUAL(req4.get("X-Something"), std::string("additional"));
+
+ Authorization auth5(Authorization::Type::Header, " Authorization: Basic huh== \n\r X-Something-More: else \n\r");
+ Poco::Net::HTTPRequest req5;
+ auth5.authorizeRequest(req5);
+ CPPUNIT_ASSERT_EQUAL(req5.get("Authorization"), std::string("Basic huh=="));
+ CPPUNIT_ASSERT_EQUAL(req5.get("X-Something-More"), std::string("else"));
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(WhiteBoxTests);
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/Auth.cpp b/wsd/Auth.cpp
index 0def0fef..088719d7 100644
--- a/wsd/Auth.cpp
+++ b/wsd/Auth.cpp
@@ -37,6 +37,62 @@ using Poco::Base64Decoder;
using Poco::Base64Encoder;
using Poco::OutputLineEndingConverter;
+void Authorization::authorizeURI(Poco::URI& uri) const
+{
+ if (_type == Authorization::Type::Token)
+ {
+ static const std::string key("access_token");
+
+ Poco::URI::QueryParameters queryParams = uri.getQueryParameters();
+ for (auto& param: queryParams)
+ {
+ if (param.first == key)
+ {
+ param.second = _data;
+ uri.setQueryParameters(queryParams);
+ return;
+ }
+ }
+
+ // it did not exist yet
+ uri.addQueryParameter(key, _data);
+ }
+}
+
+void Authorization::authorizeRequest(Poco::Net::HTTPRequest& request) const
+{
+ switch (_type)
+ {
+ case Type::Token:
+ request.set("Authorization", "Bearer " + _data);
+ break;
+ case Type::Header:
+ {
+ // there might be more headers in here; like
+ // Authorization: Basic ....
+ // X-Something-Custom: Huh
+ Poco::StringTokenizer tokens(_data, "\n\r", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
+ for (const auto& token : tokens)
+ {
+ size_t i = token.find_first_of(':');
+ if (i != std::string::npos)
+ {
+ size_t separator = i;
+ for (++i; i < token.length() && token[i] == ' ';)
+ ++i;
+
+ // set the header
+ if (i < token.length())
+ request.set(token.substr(0, separator), token.substr(i));
+ }
+ }
+ break;
+ }
+ default:
+ assert(false);
+ }
+}
+
const std::string JWTAuth::getAccessToken()
{
std::string encodedHeader = createHeader();
diff --git a/wsd/Auth.hpp b/wsd/Auth.hpp
index 87aa54b7..de41aeb6 100644
--- a/wsd/Auth.hpp
+++ b/wsd/Auth.hpp
@@ -11,10 +11,46 @@
#ifndef INCLUDED_AUTH_HPP
#define INCLUDED_AUTH_HPP
+#include <cassert>
#include <string>
#include <Poco/Crypto/RSADigestEngine.h>
#include <Poco/Crypto/RSAKey.h>
+#include <Poco/Net/HTTPRequest.h>
+#include <Poco/URI.h>
+
+/// Class to keep the authorization data.
+class Authorization
+{
+public:
+ enum class Type {
+ None,
+ Token,
+ Header
+ };
+
+private:
+ Type _type;
+ std::string _data;
+
+public:
+ Authorization()
+ : _type(Type::None)
+ {
+ }
+
+ Authorization(Type type, const std::string& data)
+ : _type(type)
+ , _data(data)
+ {
+ }
+
+ /// Set the access_token parametr to the given uri.
+ void authorizeURI(Poco::URI& uri) const;
+
+ /// Set the Authorization: header in request.
+ void authorizeRequest(Poco::Net::HTTPRequest& request) const;
+};
/// Base class of all Authentication/Authorization implementations.
class AuthBase
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 61509277..8fa41b12 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -767,17 +767,25 @@ bool ClientSession::forwardToClient(const std::shared_ptr<Message>& payload)
return true;
}
-std::string ClientSession::getAccessToken() const
+Authorization ClientSession::getAuthorization() const
{
std::string accessToken;
Poco::URI::QueryParameters queryParams = _uriPublic.getQueryParameters();
+
+ // prefer the access_token
for (auto& param: queryParams)
{
if (param.first == "access_token")
- return param.second;
+ return Authorization(Authorization::Type::Token, param.second);
+ }
+
+ for (auto& param: queryParams)
+ {
+ if (param.first == "access_header")
+ return Authorization(Authorization::Type::Header, param.second);
}
- return std::string();
+ return Authorization();
}
void ClientSession::onDisconnect()
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 0002d615..fd3a05b8 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -98,7 +98,7 @@ public:
const Poco::URI& getPublicUri() const { return _uriPublic; }
/// The access token of this session.
- std::string getAccessToken() const;
+ Authorization getAuthorization() 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 eb3fc099..c18526cd 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -403,7 +403,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(session->getAccessToken());
+ std::unique_ptr<WopiStorage::WOPIFileInfo> wopifileinfo = wopiStorage->getWOPIFileInfo(session->getAuthorization());
userid = wopifileinfo->_userid;
username = wopifileinfo->_username;
userExtraInfo = wopifileinfo->_userExtraInfo;
@@ -527,7 +527,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(session->getAccessToken());
+ const auto localPath = _storage->loadStorageFileToLocal(session->getAuthorization());
std::ifstream istr(localPath, std::ios::binary);
Poco::SHA1Engine sha1;
@@ -613,7 +613,7 @@ bool DocumentBroker::saveToStorageInternal(const std::string& sessionId,
return false;
}
- const std::string accessToken = it->second->getAccessToken();
+ const Authorization auth = it->second->getAuthorization();
const auto uri = it->second->getPublicUri().toString();
// If we aren't destroying the last editable session just yet,
@@ -635,7 +635,7 @@ bool DocumentBroker::saveToStorageInternal(const std::string& sessionId,
// storage behind our backs.
assert(_storage && _tileCache);
- StorageBase::SaveResult storageSaveResult = _storage->saveLocalFileToStorage(accessToken);
+ StorageBase::SaveResult storageSaveResult = _storage->saveLocalFileToStorage(auth);
if (storageSaveResult == StorageBase::SaveResult::OK)
{
setModified(false);
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 39e56fc0..b554bd5f 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -246,7 +246,7 @@ std::unique_ptr<LocalStorage::LocalFileInfo> LocalStorage::getLocalFileInfo()
return std::unique_ptr<LocalStorage::LocalFileInfo>(new LocalFileInfo({"localhost" + std::to_string(LastLocalStorageId), "Local Host #" + std::to_string(LastLocalStorageId++)}));
}
-std::string LocalStorage::loadStorageFileToLocal(const std::string& /*accessToken*/)
+std::string LocalStorage::loadStorageFileToLocal(const Authorization& /*auth*/)
{
// /chroot/jailId/user/doc/childId/file.ext
const auto filename = Poco::Path(_uri.getPath()).getFileName();
@@ -297,7 +297,7 @@ std::string LocalStorage::loadStorageFileToLocal(const std::string& /*accessToke
#endif
}
-StorageBase::SaveResult LocalStorage::saveLocalFileToStorage(const std::string& /*accessToken*/)
+StorageBase::SaveResult LocalStorage::saveLocalFileToStorage(const Authorization& /*auth*/)
{
try
{
@@ -440,24 +440,6 @@ bool parseJSON(const std::string& json, Poco::JSON::Object::Ptr& object)
return success;
}
-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);
-}
-
-
void addStorageDebugCookie(Poco::Net::HTTPRequest& request)
{
(void) request;
@@ -499,11 +481,11 @@ Poco::Timestamp iso8601ToTimestamp(const std::string& iso8601Time)
} // anonymous namespace
-std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const std::string& accessToken)
+std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Authorization& auth)
{
// update the access_token to the one matching to the session
Poco::URI uriObject(_uri);
- setQueryParameter(uriObject, "access_token", accessToken);
+ auth.authorizeURI(uriObject);
LOG_DBG("Getting info for wopi uri [" << uriObject.toString() << "].");
@@ -516,7 +498,7 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const st
Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, uriObject.getPathAndQuery(), Poco::Net::HTTPMessage::HTTP_1_1);
request.set("User-Agent", WOPI_AGENT_STRING);
- request.set("Authorization", "Bearer " + accessToken);
+ auth.authorizeRequest(request);
addStorageDebugCookie(request);
psession->sendRequest(request);
@@ -603,13 +585,14 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const st
}
/// uri format: http://server/<...>/wopi*/files/<id>/content
-std::string WopiStorage::loadStorageFileToLocal(const std::string& accessToken)
+std::string WopiStorage::loadStorageFileToLocal(const Authorization& auth)
{
// 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);
+ auth.authorizeURI(uriObject);
+
LOG_DBG("Wopi requesting: " << uriObject.toString());
const auto startTime = std::chrono::steady_clock::now();
@@ -619,7 +602,7 @@ std::string WopiStorage::loadStorageFileToLocal(const std::string& accessToken)
Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, uriObject.getPathAndQuery(), Poco::Net::HTTPMessage::HTTP_1_1);
request.set("User-Agent", WOPI_AGENT_STRING);
- request.set("Authorization", "Bearer " + accessToken);
+ auth.authorizeRequest(request);
addStorageDebugCookie(request);
psession->sendRequest(request);
@@ -670,14 +653,14 @@ std::string WopiStorage::loadStorageFileToLocal(const std::string& accessToken)
return "";
}
-StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const std::string& accessToken)
+StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Authorization& auth)
{
// TODO: Check if this URI has write permission (canWrite = true)
const auto size = getFileSize(_jailedFilePath);
Poco::URI uriObject(_uri);
uriObject.setPath(uriObject.getPath() + "/contents");
- setQueryParameter(uriObject, "access_token", accessToken);
+ auth.authorizeURI(uriObject);
LOG_INF("Uploading URI via WOPI [" << uriObject.toString() << "] from [" << _jailedFilePath + "].");
@@ -689,7 +672,7 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const std::string& a
Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, uriObject.getPathAndQuery(), Poco::Net::HTTPMessage::HTTP_1_1);
request.set("X-WOPI-Override", "PUT");
- request.set("Authorization", "Bearer " + accessToken);
+ auth.authorizeRequest(request);
if (!_forceSave)
{
// Request WOPI host to not overwrite if timestamps mismatch
@@ -768,14 +751,14 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const std::string& a
return saveResult;
}
-std::string WebDAVStorage::loadStorageFileToLocal(const std::string& /*accessToken*/)
+std::string WebDAVStorage::loadStorageFileToLocal(const Authorization& /*auth*/)
{
// TODO: implement webdav GET.
_isLoaded = true;
return _uri.toString();
}
-StorageBase::SaveResult WebDAVStorage::saveLocalFileToStorage(const std::string& /*accessToken*/)
+StorageBase::SaveResult WebDAVStorage::saveLocalFileToStorage(const Authorization& /*auth*/)
{
// TODO: implement webdav PUT.
return StorageBase::SaveResult::OK;
diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp
index 09002bde..6b26c066 100644
--- a/wsd/Storage.hpp
+++ b/wsd/Storage.hpp
@@ -101,10 +101,10 @@ public:
/// Returns a local file path for the given URI.
/// If necessary copies the file locally first.
- virtual std::string loadStorageFileToLocal(const std::string& accessToken) = 0;
+ virtual std::string loadStorageFileToLocal(const Authorization& auth) = 0;
/// Writes the contents of the file back to the source.
- virtual SaveResult saveLocalFileToStorage(const std::string& accessToken) = 0;
+ virtual SaveResult saveLocalFileToStorage(const Authorization& auth) = 0;
static size_t getFileSize(const std::string& filename);
@@ -168,9 +168,9 @@ public:
/// obtained using getFileInfo method
std::unique_ptr<LocalFileInfo> getLocalFileInfo();
- std::string loadStorageFileToLocal(const std::string& accessToken) override;
+ std::string loadStorageFileToLocal(const Authorization& auth) override;
- SaveResult saveLocalFileToStorage(const std::string& accessToken) override;
+ SaveResult saveLocalFileToStorage(const Authorization& auth) override;
private:
/// True if the jailed file is not linked but copied.
@@ -256,12 +256,12 @@ 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 std::string& accessToken);
+ std::unique_ptr<WOPIFileInfo> getWOPIFileInfo(const Authorization& auth);
/// uri format: http://server/<...>/wopi*/files/<id>/content
- std::string loadStorageFileToLocal(const std::string& accessToken) override;
+ std::string loadStorageFileToLocal(const Authorization& auth) override;
- SaveResult saveLocalFileToStorage(const std::string& accessToken) override;
+ SaveResult saveLocalFileToStorage(const Authorization& auth) override;
/// Total time taken for making WOPI calls during load
std::chrono::duration<double> getWopiLoadDuration() const { return _wopiLoadDuration; }
@@ -289,9 +289,9 @@ public:
// Implement me
// WebDAVFileInfo getWebDAVFileInfo(const Poco::URI& uriPublic);
- std::string loadStorageFileToLocal(const std::string& accessToken) override;
+ std::string loadStorageFileToLocal(const Authorization& auth) override;
- SaveResult saveLocalFileToStorage(const std::string& accessToken) override;
+ SaveResult saveLocalFileToStorage(const Authorization& auth) override;
private:
std::unique_ptr<AuthBase> _authAgent;
More information about the Libreoffice-commits
mailing list