Change handling token_endpoint_url (#1284)

Move removing url handling from TokenEndpoint to TokenEndpointImpl
Added reading token_endpoint_url from credentials.properties
Add checking network protocol

Resolves: OLPEDGE-1029

Signed-off-by: Yevhenii Dudnyk <[email protected]>

Author: dudnyk

olp-cpp-sdk-authentication/include/olp/authentication/AuthenticationCredentials.h

@@ -94,6 +94,16 @@ class AUTHENTICATION_API AuthenticationCredentials {
    */
   AuthenticationCredentials(std::string key, std::string secret);
 
+  /**
+   * @brief Creates the `AuthenticationCredentials` instance.
+   *
+   * @param key The access key ID.
+   * @param secret The access key secret.
+   * @param endpoint_url The token endpoint URL.
+   */
+  AuthenticationCredentials(std::string key, std::string secret,
+                            std::string endpoint_url);
+
   /**
    * @brief Gets the access key ID from the `AuthenticationCredentials`
    * instance.
@@ -110,9 +120,18 @@ class AUTHENTICATION_API AuthenticationCredentials {
    */
   const std::string& GetSecret() const;
 
+  /**
+   * @brief Gets the token endpoint URL from the `AuthenticationCredentials`
+   * instance.
+   *
+   * @return A const reference to the access token endpoint URL member.
+   */
+  const std::string& GetEndpointUrl() const;
+
  private:
   std::string key_;
   std::string secret_;
+  std::string endpoint_url_;
 };
 
 }  // namespace authentication

olp-cpp-sdk-authentication/src/AuthenticationCredentials.cpp

@@ -28,6 +28,7 @@ namespace {
 const std::regex kRegex{"\\s*=\\s*"};
 const std::string kHereAccessKeyId = "here.access.key.id";
 const std::string kHereAccessKeySecret = "here.access.key.secret";
+const std::string kHereTokenEndpointUrl = "here.token.endpoint.url";
 
 /// Forms a default path that is valid for the current OS.
 std::string GetDefaultPath() {
@@ -57,6 +58,7 @@ boost::optional<AuthenticationCredentials>
 AuthenticationCredentials::ReadFromStream(std::istream& stream) {
   std::string access_key_id;
   std::string access_key_secret;
+  std::string token_endpoint_url;
   std::string line;
 
   while (std::getline(stream, line)) {
@@ -72,12 +74,15 @@ AuthenticationCredentials::ReadFromStream(std::istream& stream) {
       access_key_id = token[1];
     } else if (token[0] == kHereAccessKeySecret) {
       access_key_secret = token[1];
+    } else if (token[0] == kHereTokenEndpointUrl) {
+      token_endpoint_url = token[1];
     }
   }
 
   return (!access_key_id.empty() && !access_key_secret.empty())
              ? boost::make_optional<AuthenticationCredentials>(
-                   {std::move(access_key_id), std::move(access_key_secret)})
+                   {std::move(access_key_id), std::move(access_key_secret),
+                    std::move(token_endpoint_url)})
              : boost::none;
 }
 
@@ -95,10 +100,22 @@ AuthenticationCredentials::AuthenticationCredentials(std::string key,
                                                      std::string secret)
     : key_(std::move(key)), secret_(std::move(secret)) {}
 
+AuthenticationCredentials::AuthenticationCredentials(std::string key,
+                                                     std::string secret,
+                                                     std::string endpoint_url)
+    : key_(std::move(key)),
+      secret_(std::move(secret)),
+      endpoint_url_(std::move(endpoint_url)) {}
+
 const std::string& AuthenticationCredentials::GetKey() const { return key_; }
 
 const std::string& AuthenticationCredentials::GetSecret() const {
   return secret_;
 }
+
+const std::string& AuthenticationCredentials::GetEndpointUrl() const {
+  return endpoint_url_;
+}
+
 }  // namespace authentication
 }  // namespace olp

olp-cpp-sdk-authentication/src/AutoRefreshingToken.cpp

@@ -28,7 +28,7 @@
 #include "TokenRequest.h"
 
 namespace {
-constexpr auto kLogTag = "authentication::AutoRefreshingToken";
+constexpr auto kLogTag = "AutoRefreshingToken";
 
 std::chrono::steady_clock::time_point ComputeRefreshTime(
     const olp::authentication::TokenResponse& current_token,

olp-cpp-sdk-authentication/src/TokenEndpoint.cpp

@@ -28,28 +28,8 @@
 namespace olp {
 namespace authentication {
 
-namespace {
-static const std::string kOauth2TokenEndpoint = "/oauth2/token";
-static constexpr auto kLogTag = "TokenEndpoint";
-}  // namespace
-
-TokenEndpoint::TokenEndpoint(Settings settings) {
-  // The underlying auth library expects a base URL and appends /oauth2/token
-  // endpoint to it. Therefore if /oauth2/token is found it is stripped from the
-  // endpoint URL provided. The underlying auth library should be updated to
-  // support an arbitrary token endpoint URL.
-  auto pos = settings.token_endpoint_url.find(kOauth2TokenEndpoint);
-  if (pos != std::string::npos) {
-    settings.token_endpoint_url.erase(pos, kOauth2TokenEndpoint.size());
-  } else {
-    OLP_SDK_LOG_ERROR(
-        kLogTag,
-        "Expected '/oauth2/token' endpoint in the token_endpoint_url. Only "
-        "standard OAuth2 token endpoint URLs are supported.");
-  }
-
-  impl_ = std::make_shared<TokenEndpointImpl>(std::move(settings));
-}
+TokenEndpoint::TokenEndpoint(Settings settings)
+    : impl_(std::make_shared<TokenEndpointImpl>(std::move(settings))) {}
 
 client::CancellationToken TokenEndpoint::RequestToken(
     const TokenRequest& token_request,

olp-cpp-sdk-authentication/src/TokenEndpointImpl.cpp

@@ -51,13 +51,29 @@ constexpr auto kClientGrantType = "client_credentials";
 constexpr auto kLogTag = "TokenEndpointImpl";
 constexpr auto kErrorWrongTimestamp = 401204;
 
+std::string GetBasePath(const std::string& base_string) {
+  // Remove /oauth2/token from url to make sure only the base url is used
+  auto new_base_string = base_string;
+  auto pos = new_base_string.find(kOauthEndpoint);
+  if (pos != std::string::npos) {
+    new_base_string.erase(pos, std::strlen(kOauthEndpoint));
+  }
+
+  OLP_SDK_LOG_INFO_F(
+      kLogTag,
+      "GetBasePath: old_token_endpoint_url='%s', token_endpoint_url='%s'",
+      base_string.c_str(), new_base_string.c_str());
+
+  return new_base_string;
+}
+
 AuthenticationSettings ConvertSettings(Settings settings) {
   AuthenticationSettings auth_settings;
   auth_settings.network_proxy_settings =
       std::move(settings.network_proxy_settings);
   auth_settings.network_request_handler =
       std::move(settings.network_request_handler);
-  auth_settings.token_endpoint_url = std::move(settings.token_endpoint_url);
+  auth_settings.token_endpoint_url = GetBasePath(settings.token_endpoint_url);
   auth_settings.use_system_time = settings.use_system_time;
   auth_settings.retry_settings = std::move(settings.retry_settings);
 

olp-cpp-sdk-authentication/tests/AuthenticationCredentialsTest.cpp

@@ -23,18 +23,18 @@
 #include <gtest/gtest.h>
 #include "olp/authentication/AuthenticationCredentials.h"
 
-using namespace olp::authentication;
+namespace auth = olp::authentication;
 
 namespace {
-const std::string valid_file_path = "existing.file";
-const std::string invalid_file_path = "nonexisting.file";
-const std::string valid_credentials =
+constexpr auto valid_file_path = "existing.file";
+constexpr auto invalid_file_path = "nonexisting.file";
+constexpr auto valid_credentials =
     "here.user.id = HERE-111\n"
     "here.client.id = 123\n"
     "here.access.key.id = 234\n"
     "here.access.key.secret = 345\n"
     "here.token.endpoint.url = https://account.api.here.com/oauth2/token";
-const std::string invalid_credentials =
+constexpr auto invalid_credentials =
     "here.user.id = HERE-111\n"
     "here.client.id = 222\n"
     "here.access.key.id = 333\n"
@@ -47,7 +47,8 @@ TEST(AuthenticationCredentialsTest, ReadFromStream) {
     SCOPED_TRACE("Credentials parse succeeds");
 
     std::stringstream ss(valid_credentials);
-    const auto credentials = AuthenticationCredentials::ReadFromStream(ss);
+    const auto credentials =
+        auth::AuthenticationCredentials::ReadFromStream(ss);
 
     EXPECT_TRUE(credentials);
 
@@ -58,7 +59,8 @@ TEST(AuthenticationCredentialsTest, ReadFromStream) {
     SCOPED_TRACE("Bad content in the stream");
 
     std::stringstream ss(invalid_credentials);
-    const auto credentials = AuthenticationCredentials::ReadFromStream(ss);
+    const auto credentials =
+        auth::AuthenticationCredentials::ReadFromStream(ss);
 
     EXPECT_FALSE(credentials);
   }
@@ -74,27 +76,28 @@ TEST(AuthenticationCredentialsTest, ReadFromFile) {
     stream.close();
 
     const auto credentials =
-        AuthenticationCredentials::ReadFromFile(valid_file_path);
+        auth::AuthenticationCredentials::ReadFromFile(valid_file_path);
 
     EXPECT_TRUE(credentials);
+    EXPECT_NE(credentials.value().GetEndpointUrl(), "");
 
     // Remove the file.
-    remove(valid_file_path.c_str());
+    remove(valid_file_path);
   }
   {
     SCOPED_TRACE("Missing credential file");
 
     const auto credentials =
-        AuthenticationCredentials::ReadFromFile(invalid_file_path);
+        auth::AuthenticationCredentials::ReadFromFile(invalid_file_path);
 
     EXPECT_FALSE(credentials);
   }
 }
 
 
 TEST(AuthenticationCredentialsTest, CanBeCopied) {
-  AuthenticationCredentials credentials("test_key", "test_secred");
-  AuthenticationCredentials copy = credentials;
+  auth::AuthenticationCredentials credentials("test_key", "test_secret");
+  auth::AuthenticationCredentials copy = credentials;
   EXPECT_EQ(credentials.GetKey(), copy.GetKey());
   EXPECT_EQ(credentials.GetSecret(), copy.GetSecret());
 }

olp-cpp-sdk-core/src/client/OlpClient.cpp

@@ -38,6 +38,8 @@
 namespace {
 constexpr auto kLogTag = "OlpClient";
 constexpr auto kApiKeyParam = "apiKey=";
+constexpr auto kHttpPrefix = "http://";
+constexpr auto kHttpsPrefix = "https://";
 
 struct RequestSettings {
   explicit RequestSettings(const int initial_backdown_period_ms,
@@ -381,6 +383,8 @@ class OlpClient::OlpClientImpl {
   ParametersType default_headers_;
   OlpClientSettings settings_;
   PendingUrlRequestsPtr pending_requests_;
+
+  bool ValidateBaseUrl() const;
 };
 
 OlpClient::OlpClientImpl::OlpClientImpl()
@@ -452,6 +456,13 @@ boost::optional<client::ApiError> OlpClient::OlpClientImpl::AddBearer(
     return boost::none;
 }
 
+bool OlpClient::OlpClientImpl::ValidateBaseUrl() const {
+  return base_url_.locked([](const std::string& base_url) {
+    return base_url == "" || (base_url.find(kHttpPrefix) != std::string::npos ||
+                              base_url.find(kHttpsPrefix) != std::string::npos);
+  });
+}
+
 std::shared_ptr<http::NetworkRequest> OlpClient::OlpClientImpl::CreateRequest(
     const std::string& path, const std::string& method,
     const OlpClient::ParametersType& query_params,
@@ -505,6 +516,12 @@ CancellationToken OlpClient::OlpClientImpl::CallApi(
     return CancellationToken();
   }
 
+  if (!ValidateBaseUrl()) {
+    callback({static_cast<int>(http::ErrorCode::INVALID_URL_ERROR),
+              "Base URI does not contain a protocol"});
+    return CancellationToken();
+  }
+
   auto network_request = CreateRequest(path, method, query_params,
                                        header_params, post_body, content_type);
 
@@ -585,6 +602,12 @@ HttpResponse OlpClient::OlpClientImpl::CallApi(
                         "Network request handler is empty.");
   }
 
+  if (!ValidateBaseUrl()) {
+    return HttpResponse(
+        static_cast<int>(olp::http::ErrorCode::INVALID_URL_ERROR),
+        "Base URI does not contain a protocol");
+  }
+
   const auto& retry_settings = settings_.retry_settings;
   auto network_settings =
       http::NetworkSettings()
@@ -679,7 +702,7 @@ HttpResponse OlpClient::OlpClientImpl::CallApi(
   return response;
 }
 
-OlpClient::OlpClient() : impl_(std::make_shared<OlpClientImpl>()){};
+OlpClient::OlpClient() : impl_(std::make_shared<OlpClientImpl>()) {}
 OlpClient::OlpClient(const OlpClientSettings& settings, std::string base_url)
     : impl_(std::make_shared<OlpClientImpl>(settings, std::move(base_url))) {}
 OlpClient::~OlpClient() = default;