From 8ea64b3403dec3808fe59cb669be35bfbd10c374 Mon Sep 17 00:00:00 2001 From: Diego Molteni Date: Thu, 10 Feb 2022 19:33:13 +0100 Subject: [PATCH 1/3] fix: added debug logs --- src/src/lib/cloud/SeismicStore.cc | 16 +++++++++++----- src/src/lib/http/http_request.cc | 10 ++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/src/lib/cloud/SeismicStore.cc b/src/src/lib/cloud/SeismicStore.cc index e1917a8..baea43e 100644 --- a/src/src/lib/cloud/SeismicStore.cc +++ b/src/src/lib/cloud/SeismicStore.cc @@ -120,8 +120,8 @@ namespace seismicdrive return false; } - Logger(2) << ("[auth-debug] check if the response can be retried"); - Logger(2) << ("[auth-debug] the response code: " + std::to_string(code)); + std::cout << ("[auth-debug] check if the response can be retried"); + std::cout << ("[auth-debug] the response code: " + std::to_string(code)); // the code is a 401 or 403, retry only for well formed json, with message field with a value matching the below. try @@ -146,7 +146,7 @@ namespace seismicdrive msg = http.get_response(); } - Logger(2) << ("[auth-debug] the response message: " + msg); + std::cout << ("[auth-debug] the response message: " + msg); bool reason401 = (code == 401) && retryable(msg, k401RetryableMessages); bool reason403 = (code == 403) && retryable(msg, k403RetryableMessages); @@ -165,7 +165,7 @@ namespace seismicdrive bool res = http.response_code() == 401 || http.response_code() == 403; if (res) { - Logger(2) << ("[auth-debug]: credential needs to be refreshed: " + std::to_string(http.response_code())); + std::cout << ("[auth-debug] credential needs to be refreshed: " + std::to_string(http.response_code())); } return res; } @@ -226,7 +226,13 @@ namespace seismicdrive { http.set_auth_callback([this]() -> std::string { - return sdutils::prepBearer(_sdmanager->getIDToken()); + std::string res; + try { + res = sdutils::prepBearer(_sdmanager->getIDToken()); + } catch(std::exception &e) { + std::cout << ("[auth-debug] callback exception: " + std::string(e.what())); + } + return res; }); // these are API dependent, can't be hard-coded in HTTPRequest http.set_auth_response_parser(needs_credentials_refresh); diff --git a/src/src/lib/http/http_request.cc b/src/src/lib/http/http_request.cc index d0c9e65..2fc386e 100644 --- a/src/src/lib/http/http_request.cc +++ b/src/src/lib/http/http_request.cc @@ -518,7 +518,17 @@ namespace seismicdrive { if (http.credentials_need_refresh()) { + if(http.request_headers().count("authorization")) { + std::cout<<"[auth-debug] the failed request had the following auth header: " << http.request_headers().at("authorization") << '\n'; + } else { + std::cout<<"[auth-debug] the failed request had no auth header set\n"; + } http.apply_auth_callback(); + if(http.request_headers().count("authorization")) { + std::cout<<"[auth-debug] after refresh we have the following auth header: " << http.request_headers().at("authorization") << '\n'; + } else { + std::cout<<"[auth-debug] after refresh we have no have auth header set\n"; + } } } -- GitLab From 508624969199a0e6002e1010b451151c2ac1b726 Mon Sep 17 00:00:00 2001 From: Diego Molteni Date: Fri, 11 Feb 2022 07:38:54 +0100 Subject: [PATCH 2/3] fix: removed silent catch --- src/src/lib/cloud/SeismicStore.cc | 17 ++++++--------- src/src/lib/http/http_manager.h | 35 +++++++++++++------------------ src/src/lib/http/http_request.cc | 10 --------- 3 files changed, 20 insertions(+), 42 deletions(-) diff --git a/src/src/lib/cloud/SeismicStore.cc b/src/src/lib/cloud/SeismicStore.cc index baea43e..6a79cf9 100644 --- a/src/src/lib/cloud/SeismicStore.cc +++ b/src/src/lib/cloud/SeismicStore.cc @@ -120,8 +120,9 @@ namespace seismicdrive return false; } - std::cout << ("[auth-debug] check if the response can be retried"); - std::cout << ("[auth-debug] the response code: " + std::to_string(code)); + Logger log; + log(2) << ("[auth-debug] check if the response can be retried"); + log(2) << ("[auth-debug] the response code: " + std::to_string(code)); // the code is a 401 or 403, retry only for well formed json, with message field with a value matching the below. try @@ -146,7 +147,7 @@ namespace seismicdrive msg = http.get_response(); } - std::cout << ("[auth-debug] the response message: " + msg); + log(2) << ("[auth-debug] the response message: " + msg); bool reason401 = (code == 401) && retryable(msg, k401RetryableMessages); bool reason403 = (code == 403) && retryable(msg, k403RetryableMessages); @@ -165,7 +166,7 @@ namespace seismicdrive bool res = http.response_code() == 401 || http.response_code() == 403; if (res) { - std::cout << ("[auth-debug] credential needs to be refreshed: " + std::to_string(http.response_code())); + Logger()(2) << ("[auth-debug] credential needs to be refreshed: " + std::to_string(http.response_code())); } return res; } @@ -226,13 +227,7 @@ namespace seismicdrive { http.set_auth_callback([this]() -> std::string { - std::string res; - try { - res = sdutils::prepBearer(_sdmanager->getIDToken()); - } catch(std::exception &e) { - std::cout << ("[auth-debug] callback exception: " + std::string(e.what())); - } - return res; + return sdutils::prepBearer(_sdmanager->getIDToken()); }); // these are API dependent, can't be hard-coded in HTTPRequest http.set_auth_response_parser(needs_credentials_refresh); diff --git a/src/src/lib/http/http_manager.h b/src/src/lib/http/http_manager.h index 8828b6a..c08ce70 100644 --- a/src/src/lib/http/http_manager.h +++ b/src/src/lib/http/http_manager.h @@ -182,30 +182,23 @@ namespace seismicdrive void apply_auth_callback() { - try + // this makes a network call, and has error modes. + // consider sending a pair of status, value... + std::string token = _auth_callback(); + if (!token.empty()) { - // this makes a network call, and has error modes. - // consider sending a pair of status, value... - std::string token = _auth_callback(); - if (!token.empty()) + // In case of impersonation token the credential is returned as "Token@Context". + // In this case the context must be set as separate header. + auto pos = token.find('@'); + if (pos == std::string::npos) { - // In case of impersonation token the credential is returned as "Token@Context". - // In this case the context must be set as separate header. - auto pos = token.find('@'); - if (pos == std::string::npos) - { - _request_headers["authorization"] = token; - } - else - { - _request_headers["authorization"] = token.substr(0, pos); - _request_headers["impersonation-token-context"] = token.substr(pos + 1); - } + _request_headers["authorization"] = token; + } + else + { + _request_headers["authorization"] = token.substr(0, pos); + _request_headers["impersonation-token-context"] = token.substr(pos + 1); } - } - catch (...) - { - // log } } diff --git a/src/src/lib/http/http_request.cc b/src/src/lib/http/http_request.cc index 2fc386e..d0c9e65 100644 --- a/src/src/lib/http/http_request.cc +++ b/src/src/lib/http/http_request.cc @@ -518,17 +518,7 @@ namespace seismicdrive { if (http.credentials_need_refresh()) { - if(http.request_headers().count("authorization")) { - std::cout<<"[auth-debug] the failed request had the following auth header: " << http.request_headers().at("authorization") << '\n'; - } else { - std::cout<<"[auth-debug] the failed request had no auth header set\n"; - } http.apply_auth_callback(); - if(http.request_headers().count("authorization")) { - std::cout<<"[auth-debug] after refresh we have the following auth header: " << http.request_headers().at("authorization") << '\n'; - } else { - std::cout<<"[auth-debug] after refresh we have no have auth header set\n"; - } } } -- GitLab From b95a1ab0086c708a43e753699e4ba484592f14fe Mon Sep 17 00:00:00 2001 From: Diego Molteni Date: Fri, 11 Feb 2022 10:50:26 +0100 Subject: [PATCH 3/3] fix: added comment to change of behaviour --- src/src/lib/http/http_manager.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/src/lib/http/http_manager.h b/src/src/lib/http/http_manager.h index c08ce70..a6d1c74 100644 --- a/src/src/lib/http/http_manager.h +++ b/src/src/lib/http/http_manager.h @@ -164,13 +164,16 @@ namespace seismicdrive /** * @details use an user-provided function to produce a valid HTTP Authorization header * so for an HTTP request requiring a basic scheme ( https://datatracker.ietf.org/doc/html/rfc7617 ) - * it need to return "Basic token_value" + * it need to return "Basic token_value" * - * for an HTTP request requiring an OAuth2 bearer token ( https://datatracker.ietf.org/doc/html/rfc6750 ) - * it needs to return "Bearer token_value" + * for an HTTP request requiring an OAuth2 bearer token ( https://datatracker.ietf.org/doc/html/rfc6750 ) + * it needs to return "Bearer token_value" * - * ditto for a Digest, AWS4-HMAC-SHA256 or whatever other authentication framework. + * ditto for a Digest, AWS4-HMAC-SHA256 or whatever other authentication framework. * + * 10/02/2022 the behavior of this method changed. If the applied callback function throw the exception + * is not longer silently ignored. + * * @param callback * * @todo : describe failure modes. (this issues an authentication call and returns the value of an authorization header) -- GitLab