From 4a5b6a77edb85893c7ea9e74df08d960f84d0abd Mon Sep 17 00:00:00 2001 From: Marco Hutter Date: Fri, 16 Apr 2021 21:34:25 +0200 Subject: [PATCH 1/3] Connection error message improvements --- CesiumIonClient/src/Connection.cpp | 219 +++++++++++++---------------- 1 file changed, 95 insertions(+), 124 deletions(-) diff --git a/CesiumIonClient/src/Connection.cpp b/CesiumIonClient/src/Connection.cpp index 5705eca83..7381b3917 100644 --- a/CesiumIonClient/src/Connection.cpp +++ b/CesiumIonClient/src/Connection.cpp @@ -50,6 +50,35 @@ std::string encodeBase64(const std::vector& bytes) { } } // namespace + +static std::string createSuccessHtml(const std::string& applicationName) +{ + return std::string("") + + "

Successfully authorized!


" + + "
Please close this window and return to " + applicationName + ".
" + + ""; +} + + +static std::string createInvalidStateHtml(const std::string& applicationName) +{ + return std::string("") + + "

Invalid state!


" + + "
Please close this window and return to " + applicationName + " to try again.
" + + ""; +} + +static std::string createAuthorizationErrorHtml(const std::string& applicationName, const std::exception& exception) +{ + return std::string("") + + "

Not authorized!


" + + "
The authorization failed with the following error message: " + exception.what() + ".

" + + "
Please close this window and return to " + applicationName + ".

" + + "
If the problem persists, contact our support at support@cesium.com.
" + + ""; +} + + /*static*/ CesiumAsync::Future Connection::authorize( const CesiumAsync::AsyncSystem& asyncSystem, const std::shared_ptr& pAssetAccessor, @@ -128,8 +157,7 @@ std::string encodeBase64(const std::vector& bytes) { if (state != expectedState) { response.set_content( - "Invalid state! Please close this window and return to " + - friendlyApplicationName + " to try again.", + createInvalidStateHtml(friendlyApplicationName), "text/html"); promise.reject(std::runtime_error("Received an invalid state.")); return; @@ -155,17 +183,13 @@ std::string encodeBase64(const std::vector& bytes) { void operator()(Connection& connection) { response.set_content( - "Successfully authorized! Please close this window and " - "return to " + - friendlyApplicationName + ".", + createSuccessHtml(friendlyApplicationName), "text/html"); promise.resolve(std::move(connection)); } void operator()(std::exception& exception) { - response.set_content( - "Not authorized! Please close this window and return to " + - friendlyApplicationName + ".", + response.set_content(createAuthorizationErrorHtml(friendlyApplicationName, exception), "text/html"); promise.reject(std::move(exception)); } @@ -196,15 +220,52 @@ Connection::Connection( _accessToken(accessToken), _apiUrl(apiUrl) {} +template +static Response createEmptyResponse() { + return Response{ + std::nullopt, + 0, + "NoResponse", + "The server did not return a response."}; +} + template static Response createErrorResponse(const IAssetResponse* pResponse) { return Response{ std::nullopt, pResponse->statusCode(), - "TODO", - "Fill in this message"}; + std::to_string(pResponse->statusCode()), + "Received response code " + std::to_string(pResponse->statusCode()) }; +} + +template +static Response createJsonErrorResponse(const IAssetResponse* pResponse, rapidjson::Document& d) { + return Response{ + std::nullopt, + pResponse->statusCode(), + "ParseError", + std::string("Failed to parse JSON response: ") + + rapidjson::GetParseError_En(d.GetParseError())}; +} + +template +static Response createJsonTypeResponse(const IAssetResponse* pResponse, const std::string& expectedType) { + return Response{ + std::nullopt, + pResponse->statusCode(), + "ParseError", + "Response is not a JSON " + expectedType + "."}; } +static bool parseJsonObject(const IAssetResponse* pResponse, rapidjson::Document& d) { + d.Parse( + reinterpret_cast(pResponse->data().data()), + pResponse->data().size()); + return !d.HasParseError(); +} + + + CesiumAsync::Future> Connection::me() const { return this->_pAssetAccessor ->requestAsset( @@ -216,11 +277,7 @@ CesiumAsync::Future> Connection::me() const { [](std::shared_ptr&& pRequest) { const IAssetResponse* pResponse = pRequest->response(); if (!pResponse) { - return Response{ - std::nullopt, - 0, - "NoResponse", - "The server did not return a response."}; + return createEmptyResponse(); } if (pResponse->statusCode() < 200 || @@ -229,24 +286,11 @@ CesiumAsync::Future> Connection::me() const { } rapidjson::Document d; - d.Parse( - reinterpret_cast(pResponse->data().data()), - pResponse->data().size()); - if (d.HasParseError()) { - return Response{ - std::nullopt, - pResponse->statusCode(), - "ParseError", - std::string("Failed to parse JSON response: ") + - rapidjson::GetParseError_En(d.GetParseError())}; + if (!parseJsonObject(pResponse, d)) { + return createJsonErrorResponse(pResponse, d); } - if (!d.IsObject()) { - return Response{ - std::nullopt, - pResponse->statusCode(), - "ParseError", - "Response is not a JSON object."}; + return createJsonTypeResponse(pResponse, "object"); } Profile result; @@ -309,11 +353,7 @@ CesiumAsync::Future> Connection::assets() const { [](std::shared_ptr&& pRequest) { const IAssetResponse* pResponse = pRequest->response(); if (!pResponse) { - return Response{ - std::nullopt, - 0, - "NoResponse", - "The server did not return a response."}; + return createEmptyResponse(); } if (pResponse->statusCode() < 200 || @@ -322,24 +362,11 @@ CesiumAsync::Future> Connection::assets() const { } rapidjson::Document d; - d.Parse( - reinterpret_cast(pResponse->data().data()), - pResponse->data().size()); - if (d.HasParseError()) { - return Response{ - std::nullopt, - pResponse->statusCode(), - "ParseError", - std::string("Failed to parse JSON response: ") + - rapidjson::GetParseError_En(d.GetParseError())}; + if (!parseJsonObject(pResponse, d)) { + return createJsonErrorResponse(pResponse, d); } - if (!d.IsObject()) { - return Response{ - std::nullopt, - pResponse->statusCode(), - "ParseError", - "Response is not a JSON object."}; + return createJsonTypeResponse(pResponse, "object"); } Assets result; @@ -365,10 +392,7 @@ CesiumAsync::Future> Connection::assets() const { }); } -static std::optional tokenFromJson(const rapidjson::Value& json) { - if (!json.IsObject()) { - return std::nullopt; - } +static Token tokenFromJson(const rapidjson::Value& json) { Token token; token.jti = JsonHelpers::getStringOrDefault(json, "jti", ""); @@ -399,11 +423,7 @@ CesiumAsync::Future>> Connection::tokens() const { [](std::shared_ptr&& pRequest) { const IAssetResponse* pResponse = pRequest->response(); if (!pResponse) { - return Response>{ - std::nullopt, - 0, - "NoResponse", - "The server did not return a response."}; + return createEmptyResponse>(); } if (pResponse->statusCode() < 200 || @@ -412,28 +432,14 @@ CesiumAsync::Future>> Connection::tokens() const { } rapidjson::Document d; - d.Parse( - reinterpret_cast(pResponse->data().data()), - pResponse->data().size()); - if (d.HasParseError()) { - return Response>{ - std::nullopt, - pResponse->statusCode(), - "ParseError", - std::string("Failed to parse JSON response: ") + - rapidjson::GetParseError_En(d.GetParseError())}; + if (!parseJsonObject(pResponse, d)) { + return createJsonErrorResponse>(pResponse, d); } - if (!d.IsArray()) { - return Response>{ - std::nullopt, - pResponse->statusCode(), - "ParseError", - "Response is not a JSON array."}; + return createJsonTypeResponse>(pResponse, "array"); } std::vector result; - for (auto it = d.Begin(); it != d.End(); ++it) { std::optional token = tokenFromJson(*it); if (!token) { @@ -465,11 +471,7 @@ CesiumAsync::Future> Connection::asset(int64_t assetID) const { [](std::shared_ptr&& pRequest) { const IAssetResponse* pResponse = pRequest->response(); if (!pResponse) { - return Response{ - std::nullopt, - 0, - "NoResponse", - "The server did not return a response."}; + return createEmptyResponse(); } if (pResponse->statusCode() < 200 || @@ -478,24 +480,11 @@ CesiumAsync::Future> Connection::asset(int64_t assetID) const { } rapidjson::Document d; - d.Parse( - reinterpret_cast(pResponse->data().data()), - pResponse->data().size()); - if (d.HasParseError()) { - return Response{ - std::nullopt, - pResponse->statusCode(), - "ParseError", - std::string("Failed to parse JSON response: ") + - rapidjson::GetParseError_En(d.GetParseError())}; + if (!parseJsonObject(pResponse, d)) { + return createJsonErrorResponse(pResponse, d); } - if (!d.IsObject()) { - return Response{ - std::nullopt, - pResponse->statusCode(), - "ParseError", - "Response is not a JSON object."}; + return createJsonTypeResponse(pResponse, "object"); } return Response{ @@ -549,11 +538,7 @@ CesiumAsync::Future> Connection::createToken( [](std::shared_ptr&& pRequest) { const IAssetResponse* pResponse = pRequest->response(); if (!pResponse) { - return Response{ - std::nullopt, - 0, - "NoResponse", - "The server did not return a response."}; + return createEmptyResponse(); } if (pResponse->statusCode() < 200 || @@ -562,30 +547,16 @@ CesiumAsync::Future> Connection::createToken( } rapidjson::Document d; - d.Parse( - reinterpret_cast(pResponse->data().data()), - pResponse->data().size()); - if (d.HasParseError()) { - return Response{ - std::nullopt, - pResponse->statusCode(), - "ParseError", - std::string("Failed to parse JSON response: ") + - rapidjson::GetParseError_En(d.GetParseError())}; + if (!parseJsonObject(pResponse, d)) { + return createJsonErrorResponse(pResponse, d); } - - std::optional maybeToken = tokenFromJson(d); - - if (!maybeToken) { - return Response{ - std::nullopt, - pResponse->statusCode(), - "ParseError", - "Response is not a JSON object."}; + if (!d.IsObject()) { + return createJsonTypeResponse(pResponse, "object"); } + Token token = tokenFromJson(d); return Response{ - std::move(maybeToken), + std::move(token), pResponse->statusCode(), std::string(), std::string()}; From bbf2400baac18d01d04d7a8660e7f099b341527a Mon Sep 17 00:00:00 2001 From: Marco Hutter Date: Fri, 16 Apr 2021 21:55:27 +0200 Subject: [PATCH 2/3] Handle denied access with sensible message --- CesiumIonClient/src/Connection.cpp | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/CesiumIonClient/src/Connection.cpp b/CesiumIonClient/src/Connection.cpp index 7381b3917..8b8a1cda7 100644 --- a/CesiumIonClient/src/Connection.cpp +++ b/CesiumIonClient/src/Connection.cpp @@ -60,10 +60,11 @@ static std::string createSuccessHtml(const std::string& applicationName) } -static std::string createInvalidStateHtml(const std::string& applicationName) +static std::string createGenericErrorHtml(const std::string& applicationName, const std::string& errorMessage, const std::string& errorDescription) { return std::string("") + - "

Invalid state!


" + + "

" + errorMessage + "


" + + "
" + errorDescription + ".

" + "
Please close this window and return to " + applicationName + " to try again.
" + ""; } @@ -152,12 +153,30 @@ static std::string createAuthorizationErrorHtml(const std::string& applicationNa httplib::Response& response) { pServer->stop(); + + std::string error = request.get_param_value("error"); + std::string errorDescription = request.get_param_value("error_description"); + if (!error.empty()) { + std::string errorMessage = "Error"; + std::string errorDescriptionMessage = "An unknown error occurred"; + if (error == "access_denied") { + errorMessage = "Access denied"; + } + if (!errorDescription.empty()) { + errorDescriptionMessage = errorDescription; + } + response.set_content( + createGenericErrorHtml(friendlyApplicationName, errorMessage, errorDescriptionMessage), + "text/html"); + promise.reject(std::runtime_error("Received an error message")); + return; + } + std::string code = request.get_param_value("code"); std::string state = request.get_param_value("state"); - if (state != expectedState) { response.set_content( - createInvalidStateHtml(friendlyApplicationName), + createGenericErrorHtml(friendlyApplicationName, "Invalid state", "The redirection received an invalid state"), "text/html"); promise.reject(std::runtime_error("Received an invalid state.")); return; @@ -174,8 +193,6 @@ static std::string createAuthorizationErrorHtml(const std::string& applicationNa codeVerifier) .wait(); - // TODO: nicer HTML - struct Operation { const std::string& friendlyApplicationName; httplib::Response& response; From fbea991d2d46e46c52309042fc5cb3a3e8b9758b Mon Sep 17 00:00:00 2001 From: Marco Hutter Date: Fri, 16 Apr 2021 21:56:15 +0200 Subject: [PATCH 3/3] Formatting --- CesiumIonClient/src/Connection.cpp | 95 ++++++++++++++++++------------ 1 file changed, 58 insertions(+), 37 deletions(-) diff --git a/CesiumIonClient/src/Connection.cpp b/CesiumIonClient/src/Connection.cpp index 8b8a1cda7..4324174b6 100644 --- a/CesiumIonClient/src/Connection.cpp +++ b/CesiumIonClient/src/Connection.cpp @@ -50,36 +50,44 @@ std::string encodeBase64(const std::vector& bytes) { } } // namespace - -static std::string createSuccessHtml(const std::string& applicationName) -{ +static std::string createSuccessHtml(const std::string& applicationName) { return std::string("") + - "

Successfully authorized!


" + - "
Please close this window and return to " + applicationName + ".
" + - ""; + "

Successfully " + "authorized!


" + + "
Please close this window and " + "return to " + + applicationName + ".
" + ""; } - -static std::string createGenericErrorHtml(const std::string& applicationName, const std::string& errorMessage, const std::string& errorDescription) -{ - return std::string("") + - "

" + errorMessage + "


" + - "
" + errorDescription + ".

" + - "
Please close this window and return to " + applicationName + " to try again.
" + - ""; +static std::string createGenericErrorHtml( + const std::string& applicationName, + const std::string& errorMessage, + const std::string& errorDescription) { + return std::string("") + "

" + + errorMessage + "


" + "
" + + errorDescription + ".

" + + "
Please close this window and " + "return to " + + applicationName + " to try again.
" + ""; } -static std::string createAuthorizationErrorHtml(const std::string& applicationName, const std::exception& exception) -{ +static std::string createAuthorizationErrorHtml( + const std::string& applicationName, + const std::exception& exception) { return std::string("") + - "

Not authorized!


" + - "
The authorization failed with the following error message: " + exception.what() + ".

" + - "
Please close this window and return to " + applicationName + ".

" + - "
If the problem persists, contact our support at support@cesium.com.
" + - ""; + "

Not authorized!


" + + "
The authorization failed with the " + "following error message: " + + exception.what() + ".

" + + "
Please close this window and " + "return to " + + applicationName + ".

" + + "
If the problem persists, contact " + "our support at support@cesium.com.
" + + ""; } - /*static*/ CesiumAsync::Future Connection::authorize( const CesiumAsync::AsyncSystem& asyncSystem, const std::shared_ptr& pAssetAccessor, @@ -153,9 +161,9 @@ static std::string createAuthorizationErrorHtml(const std::string& applicationNa httplib::Response& response) { pServer->stop(); - std::string error = request.get_param_value("error"); - std::string errorDescription = request.get_param_value("error_description"); + std::string errorDescription = + request.get_param_value("error_description"); if (!error.empty()) { std::string errorMessage = "Error"; std::string errorDescriptionMessage = "An unknown error occurred"; @@ -166,7 +174,10 @@ static std::string createAuthorizationErrorHtml(const std::string& applicationNa errorDescriptionMessage = errorDescription; } response.set_content( - createGenericErrorHtml(friendlyApplicationName, errorMessage, errorDescriptionMessage), + createGenericErrorHtml( + friendlyApplicationName, + errorMessage, + errorDescriptionMessage), "text/html"); promise.reject(std::runtime_error("Received an error message")); return; @@ -176,7 +187,10 @@ static std::string createAuthorizationErrorHtml(const std::string& applicationNa std::string state = request.get_param_value("state"); if (state != expectedState) { response.set_content( - createGenericErrorHtml(friendlyApplicationName, "Invalid state", "The redirection received an invalid state"), + createGenericErrorHtml( + friendlyApplicationName, + "Invalid state", + "The redirection received an invalid state"), "text/html"); promise.reject(std::runtime_error("Received an invalid state.")); return; @@ -200,13 +214,16 @@ static std::string createAuthorizationErrorHtml(const std::string& applicationNa void operator()(Connection& connection) { response.set_content( - createSuccessHtml(friendlyApplicationName), + createSuccessHtml(friendlyApplicationName), "text/html"); promise.resolve(std::move(connection)); } void operator()(std::exception& exception) { - response.set_content(createAuthorizationErrorHtml(friendlyApplicationName, exception), + response.set_content( + createAuthorizationErrorHtml( + friendlyApplicationName, + exception), "text/html"); promise.reject(std::move(exception)); } @@ -237,8 +254,7 @@ Connection::Connection( _accessToken(accessToken), _apiUrl(apiUrl) {} -template -static Response createEmptyResponse() { +template static Response createEmptyResponse() { return Response{ std::nullopt, 0, @@ -252,11 +268,13 @@ static Response createErrorResponse(const IAssetResponse* pResponse) { std::nullopt, pResponse->statusCode(), std::to_string(pResponse->statusCode()), - "Received response code " + std::to_string(pResponse->statusCode()) }; + "Received response code " + std::to_string(pResponse->statusCode())}; } template -static Response createJsonErrorResponse(const IAssetResponse* pResponse, rapidjson::Document& d) { +static Response createJsonErrorResponse( + const IAssetResponse* pResponse, + rapidjson::Document& d) { return Response{ std::nullopt, pResponse->statusCode(), @@ -266,7 +284,9 @@ static Response createJsonErrorResponse(const IAssetResponse* pResponse, rapi } template -static Response createJsonTypeResponse(const IAssetResponse* pResponse, const std::string& expectedType) { +static Response createJsonTypeResponse( + const IAssetResponse* pResponse, + const std::string& expectedType) { return Response{ std::nullopt, pResponse->statusCode(), @@ -274,15 +294,14 @@ static Response createJsonTypeResponse(const IAssetResponse* pResponse, const "Response is not a JSON " + expectedType + "."}; } -static bool parseJsonObject(const IAssetResponse* pResponse, rapidjson::Document& d) { +static bool +parseJsonObject(const IAssetResponse* pResponse, rapidjson::Document& d) { d.Parse( reinterpret_cast(pResponse->data().data()), pResponse->data().size()); return !d.HasParseError(); } - - CesiumAsync::Future> Connection::me() const { return this->_pAssetAccessor ->requestAsset( @@ -453,7 +472,9 @@ CesiumAsync::Future>> Connection::tokens() const { return createJsonErrorResponse>(pResponse, d); } if (!d.IsArray()) { - return createJsonTypeResponse>(pResponse, "array"); + return createJsonTypeResponse>( + pResponse, + "array"); } std::vector result;