[Libreoffice-commits] online.git: android/lib common/Authorization.cpp common/Authorization.hpp test/Makefile.am test/WhiteBoxTests.cpp wsd/Admin.cpp wsd/ClientSession.cpp wsd/ClientSession.hpp
Ashod Nakashian (via logerrit)
logerrit at kemper.freedesktop.org
Wed Jul 1 05:34:17 UTC 2020
android/lib/src/main/cpp/CMakeLists.txt.in | 5 -
common/Authorization.cpp | 22 ++++++
common/Authorization.hpp | 10 ++
test/Makefile.am | 2
test/WhiteBoxTests.cpp | 105 ++++++++++++++++++++++++++++-
wsd/Admin.cpp | 1
wsd/ClientSession.cpp | 31 --------
wsd/ClientSession.hpp | 4 -
8 files changed, 142 insertions(+), 38 deletions(-)
New commits:
commit fa96934861b075cffd980cdfa98f61dee82fc012
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
AuthorDate: Sat Jun 20 14:09:21 2020 -0400
Commit: Ashod Nakashian <ashnakash at gmail.com>
CommitDate: Wed Jul 1 07:33:57 2020 +0200
wsd: Authorization parsing and creation improvements
Authorization class now handles the parsing and creation
of its instances, which makes it centralized.
We also avoid repeatedly constructing Authorization objects
in ClientSession and instead do it once at construction
and cache it.
A bunch of new unit-tests added.
Change-Id: I9b5939be51a5957214d07ed8f1096efd179686c6
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/96825
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/android/lib/src/main/cpp/CMakeLists.txt.in b/android/lib/src/main/cpp/CMakeLists.txt.in
index 56b5a8a52..53a3fafd7 100644
--- a/android/lib/src/main/cpp/CMakeLists.txt.in
+++ b/android/lib/src/main/cpp/CMakeLists.txt.in
@@ -3,12 +3,13 @@ cmake_minimum_required(VERSION 3.4.1)
add_library(androidapp SHARED
androidapp.cpp
+ ../../../../../common/Authorization.cpp
../../../../../common/FileUtil.cpp
../../../../../common/JailUtil.cpp
../../../../../common/Log.cpp
../../../../../common/MessageQueue.cpp
../../../../../common/Protocol.cpp
- ../../../../../common/StringVector.cpp
+ ../../../../../common/StringVector.cpp
../../../../../common/Session.cpp
../../../../../common/SigUtil.cpp
../../../../../common/SpookyV2.cpp
@@ -21,7 +22,7 @@ add_library(androidapp SHARED
../../../../../wsd/ClientSession.cpp
../../../../../wsd/DocumentBroker.cpp
../../../../../wsd/LOOLWSD.cpp
- ../../../../../wsd/RequestDetails.cpp
+ ../../../../../wsd/RequestDetails.cpp
../../../../../wsd/Storage.cpp
../../../../../wsd/TileCache.cpp)
diff --git a/common/Authorization.cpp b/common/Authorization.cpp
index ad4381ef5..93c5704fc 100644
--- a/common/Authorization.cpp
+++ b/common/Authorization.cpp
@@ -88,4 +88,26 @@ void Authorization::authorizeRequest(Poco::Net::HTTPRequest& request) const
}
}
+Authorization Authorization::create(const Poco::URI::QueryParameters& queryParams)
+{
+ // prefer the access_token
+ std::string decoded;
+ for (const auto& param : queryParams)
+ {
+ if (param.first == "access_token")
+ {
+ Poco::URI::decode(param.second, decoded);
+ return Authorization(Authorization::Type::Token, decoded);
+ }
+
+ if (param.first == "access_header")
+ Poco::URI::decode(param.second, decoded);
+ }
+
+ if (!decoded.empty())
+ return Authorization(Authorization::Type::Header, decoded);
+
+ return Authorization();
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/common/Authorization.hpp b/common/Authorization.hpp
index 4fd2f3671..14accb236 100644
--- a/common/Authorization.hpp
+++ b/common/Authorization.hpp
@@ -28,8 +28,8 @@ public:
};
private:
- Type _type;
- std::string _data;
+ const Type _type;
+ const std::string _data;
public:
Authorization()
@@ -43,6 +43,12 @@ public:
{
}
+ /// Create an Authorization instance from the URI query parameters.
+ /// Expects access_token (preferred) or access_header.
+ static Authorization create(const Poco::URI::QueryParameters& queryParams);
+ static Authorization create(const Poco::URI& uri) { return create(uri.getQueryParameters()); }
+ static Authorization create(const std::string& uri) { return create(Poco::URI(uri)); }
+
/// Set the access_token parametr to the given uri.
void authorizeURI(Poco::URI& uri) const;
diff --git a/test/Makefile.am b/test/Makefile.am
index ad48899a1..e05dfdf3f 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -75,7 +75,7 @@ test_base_source = \
WopiProofTests.cpp \
$(wsd_sources)
-unittest_CPPFLAGS = -I$(top_srcdir) -DBUILDING_TESTS -DSTANDALONE_CPPUNIT
+unittest_CPPFLAGS = -I$(top_srcdir) -DBUILDING_TESTS -DSTANDALONE_CPPUNIT -g
unittest_SOURCES = \
$(test_base_source) \
../common/Log.cpp \
diff --git a/test/WhiteBoxTests.cpp b/test/WhiteBoxTests.cpp
index 47023d2fa..dd32dbdd6 100644
--- a/test/WhiteBoxTests.cpp
+++ b/test/WhiteBoxTests.cpp
@@ -722,13 +722,15 @@ void WhiteBoxTests::testAuthorization()
auth3.authorizeRequest(req3);
LOK_ASSERT_EQUAL(std::string("Basic huhu=="), req3.get("Authorization"));
- Authorization auth4(Authorization::Type::Header, " Authorization: Basic blah== \n\r X-Something: additional ");
+ Authorization auth4(Authorization::Type::Header, " Authorization: Basic blah== \n\rX-Something: additional ");
Poco::Net::HTTPRequest req4;
auth4.authorizeRequest(req4);
+ LOK_ASSERT_MESSAGE("Exected request to have Authorization header", req4.has("Authorization"));
LOK_ASSERT_EQUAL(std::string("Basic blah=="), req4.get("Authorization"));
+ LOK_ASSERT_MESSAGE("Exected request to have X-Something header", req4.has("X-Something"));
LOK_ASSERT_EQUAL(std::string("additional"), req4.get("X-Something"));
- Authorization auth5(Authorization::Type::Header, " Authorization: Basic huh== \n\r X-Something-More: else \n\r");
+ Authorization auth5(Authorization::Type::Header, " Authorization: Basic huh== \n\rX-Something-More: else \n\r");
Poco::Net::HTTPRequest req5;
auth5.authorizeRequest(req5);
LOK_ASSERT_EQUAL(std::string("Basic huh=="), req5.get("Authorization"));
@@ -737,6 +739,40 @@ void WhiteBoxTests::testAuthorization()
Authorization auth6(Authorization::Type::None, "Authorization: basic huh==");
Poco::Net::HTTPRequest req6;
CPPUNIT_ASSERT_THROW(auth6.authorizeRequest(req6), BadRequestException);
+
+ {
+ const std::string WorkingDocumentURI
+ = "https://example.com:8443/rest/files/wopi/files/"
+ "8ac75551de4d89e60002?access_header=Authorization%3A%2520Bearer%25201hpoiuytrewq%"
+ "250D%250A%250D%250AX-Requested-With%3A%2520XMLHttpRequest&reuse_cookies=lang%3Den-"
+ "us%3A_xx_%3DGS1.1.%3APublicToken%"
+ "3DeyJzdWIiOiJhZG1pbiIsImV4cCI6MTU4ODkxNzc3NCwiaWF0IjoxNTg4OTE2ODc0LCJqdGkiOiI4OGZhN2"
+ "E3ZC1lMzU5LTQ2OWEtYjg3Zi02NmFhNzI0ZGFkNTcifQ%3AZNPCQ003-32383700%3De9c71c3b%"
+ "3AJSESSIONID%3Dnode019djohorurnaf1eo6f57ejhg0520.node0&permission=edit";
+
+ const std::string AuthorizationParam = "Bearer 1hpoiuytrewq";
+
+ Authorization auth(Authorization::create(WorkingDocumentURI));
+ Poco::Net::HTTPRequest req;
+ auth.authorizeRequest(req);
+ LOK_ASSERT_EQUAL(AuthorizationParam, req.get("Authorization"));
+ LOK_ASSERT_EQUAL(std::string("XMLHttpRequest"), req.get("X-Requested-With"));
+ }
+
+ {
+ const std::string URI
+ = "https://example.com:8443/rest/files/wopi/files/"
+ "24e3f0a17230cca5017230fb6861000c?access_header=Authorization%3A%20Bearer%"
+ "201hpoiuytrewq%0D%0A%0D%0AX-Requested-With%3A%20XMLHttpRequest";
+
+ const std::string AuthorizationParam = "Bearer 1hpoiuytrewq";
+
+ Authorization auth7(Authorization::create(URI));
+ Poco::Net::HTTPRequest req7;
+ auth7.authorizeRequest(req7);
+ LOK_ASSERT_EQUAL(AuthorizationParam, req7.get("Authorization"));
+ LOK_ASSERT_EQUAL(std::string("XMLHttpRequest"), req7.get("X-Requested-With"));
+ }
}
void WhiteBoxTests::testJson()
@@ -1432,6 +1468,71 @@ void WhiteBoxTests::testRequestDetails()
LOK_ASSERT_EQUAL(std::string(""), details.getField(RequestDetails::Field::Serial));
LOK_ASSERT(details.equals(RequestDetails::Field::Serial, ""));
}
+
+ {
+ static const std::string URI
+ = "/lool/"
+ "https%3A%2F%2Fexample.com%3A8443%2Frest%2Ffiles%2Fwopi%2Ffiles%"
+ "2F8ac75551de4d89e60002%3Faccess_header%3DAuthorization%253A%252520Bearer%"
+ "252520poiuytrewq%25250D%25250A%25250D%25250AX-Requested-"
+ "With%253A%252520XMLHttpRequest%26reuse_cookies%3Dlang%253Den-us%253A_ga_"
+ "LMX4TVJ02K%253DGS1.1%"
+ "253AToken%253DeyJhbGciOiJIUzUxMiJ9.vajknfkfajksdljfiwjek-"
+ "W90fmgVb3C-00-eSkJBDqDNSYA%253APublicToken%"
+ "253Dabc%253AZNPCQ003-32383700%253De9c71c3b%"
+ "253AJSESSIONID%253Dnode0.node0%26permission%3Dedit/"
+ "ws?WOPISrc=https://example.com:8443/rest/files/wopi/files/"
+ "8c74c1deff7dede002&compat=/ws";
+
+ Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, URI,
+ Poco::Net::HTTPMessage::HTTP_1_1);
+ request.setHost(Root);
+ request.set("User-Agent", WOPI_AGENT_STRING);
+ request.set("ProxyPrefix", ProxyPrefix);
+
+ RequestDetails details(request, "");
+ LOK_ASSERT_EQUAL(true, details.isProxy());
+ LOK_ASSERT_EQUAL(ProxyPrefix, details.getProxyPrefix());
+
+ LOK_ASSERT_EQUAL(Root, details.getHostUntrusted());
+ LOK_ASSERT_EQUAL(false, details.isWebSocket());
+ LOK_ASSERT_EQUAL(true, details.isGet());
+
+ const std::string docUri
+ = "https://example.com:8443/rest/files/wopi/files/"
+ "8ac75551de4d89e60002?access_header=Authorization%3A%2520Bearer%2520poiuytrewq%250D%"
+ "250A%250D%250AX-Requested-With%3A%2520XMLHttpRequest&reuse_cookies=lang%3Den-us%3A_"
+ "ga_LMX4TVJ02K%3DGS1.1%3AToken%3DeyJhbGciOiJIUzUxMiJ9.vajknfkfajksdljfiwjek-"
+ "W90fmgVb3C-00-eSkJBDqDNSYA%3APublicToken%3Dabc%3AZNPCQ003-32383700%3De9c71c3b%"
+ "3AJSESSIONID%3Dnode0.node0&permission=edit";
+
+ // LOK_ASSERT_EQUAL(docUri, details.getLegacyDocumentURI()); // Broken.
+ LOK_ASSERT_EQUAL(docUri, details.getDocumentURI());
+
+ LOK_ASSERT_EQUAL(static_cast<std::size_t>(11), details.size());
+ LOK_ASSERT_EQUAL(std::string("lool"), details[0]);
+ LOK_ASSERT(details.equals(0, "lool"));
+
+ const std::string encodedDocUri
+ = "https%3A%2F%2Fexample.com%3A8443%2Frest%2Ffiles%2Fwopi%2Ffiles%"
+ "2F8ac75551de4d89e60002%3Faccess_header%3DAuthorization%253A%252520Bearer%"
+ "252520poiuytrewq%25250D%25250A%25250D%25250AX-Requested-With%253A%"
+ "252520XMLHttpRequest%26reuse_cookies%3Dlang%253Den-us%253A_ga_LMX4TVJ02K%253DGS1.1%"
+ "253AToken%253DeyJhbGciOiJIUzUxMiJ9.vajknfkfajksdljfiwjek-W90fmgVb3C-00-eSkJBDqDNSYA%"
+ "253APublicToken%253Dabc%253AZNPCQ003-32383700%253De9c71c3b%253AJSESSIONID%253Dnode0."
+ "node0%26permission%3Dedit";
+
+ LOK_ASSERT_EQUAL(encodedDocUri, details[1]);
+
+ LOK_ASSERT_EQUAL(std::string("lool"), details.getField(RequestDetails::Field::Type));
+ LOK_ASSERT(details.equals(RequestDetails::Field::Type, "lool"));
+ LOK_ASSERT_EQUAL(std::string(""), details.getField(RequestDetails::Field::SessionId));
+ LOK_ASSERT(details.equals(RequestDetails::Field::SessionId, ""));
+ LOK_ASSERT_EQUAL(std::string(""), details.getField(RequestDetails::Field::Command));
+ LOK_ASSERT(details.equals(RequestDetails::Field::Command, ""));
+ LOK_ASSERT_EQUAL(std::string(""), details.getField(RequestDetails::Field::Serial));
+ LOK_ASSERT(details.equals(RequestDetails::Field::Serial, ""));
+ }
}
CPPUNIT_TEST_SUITE_REGISTRATION(WhiteBoxTests);
diff --git a/wsd/Admin.cpp b/wsd/Admin.cpp
index 8d0c9373d..996e7b75c 100644
--- a/wsd/Admin.cpp
+++ b/wsd/Admin.cpp
@@ -35,7 +35,6 @@
#include <net/WebSocketHandler.hpp>
#include <common/SigUtil.hpp>
-#include <common/Authorization.hpp>
using namespace LOOLProtocol;
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 614a75fed..26f9e9c8b 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -53,6 +53,7 @@ ClientSession::ClientSession(
Session(ws, "ToClient-" + id, id, readOnly),
_docBroker(docBroker),
_uriPublic(uriPublic),
+ _auth(Authorization::create(uriPublic)),
_isDocumentOwner(false),
_state(SessionState::DETACHED),
_keyEvents(1),
@@ -76,7 +77,7 @@ ClientSession::ClientSession(
{
// Cache the cookies to avoid re-parsing the URI again.
_cookies = param.second;
- LOG_INF("ClientSession [" << getName() << "] has cookies: " << _cookies);
+ LOG_INF("ClientSession [" << getName() << "] has cookies: [" << _cookies << "].");
break;
}
}
@@ -1571,34 +1572,6 @@ void ClientSession::enqueueSendMessage(const std::shared_ptr<Message>& data)
}
}
-Authorization ClientSession::getAuthorization() const
-{
- Poco::URI::QueryParameters queryParams = _uriPublic.getQueryParameters();
-
- // prefer the access_token
- for (auto& param: queryParams)
- {
- if (param.first == "access_token")
- {
- std::string decodedToken;
- Poco::URI::decode(param.second, decodedToken);
- return Authorization(Authorization::Type::Token, decodedToken);
- }
- }
-
- for (auto& param: queryParams)
- {
- if (param.first == "access_header")
- {
- std::string decodedHeader;
- Poco::URI::decode(param.second, decodedHeader);
- return Authorization(Authorization::Type::Header, decodedHeader);
- }
- }
-
- return Authorization();
-}
-
void ClientSession::addTileOnFly(const TileDesc& tile)
{
_tilesOnFly.push_back({tile.generateID(), std::chrono::steady_clock::now()});
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 6103d0550..8922b924e 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -117,7 +117,7 @@ public:
const Poco::URI& getPublicUri() const { return _uriPublic; }
/// The access token of this session.
- Authorization getAuthorization() const;
+ Authorization getAuthorization() const { return _auth; }
const std::string& getCookies() const { return _cookies; }
@@ -226,6 +226,8 @@ private:
/// URI with which client made request to us
const Poco::URI _uriPublic;
+ const Authorization _auth;
+
/// The cookies we should pass on to the storage on saving.
std::string _cookies;
More information about the Libreoffice-commits
mailing list