BUG#35468541 HEAD request is not retried if it fails with an authorization error

If an AWS HEAD request failed with an authorization error, it was not
retried, as response did not contain body, and retry mechanism uses body
to recognize AWS authorization errors.

As a workaround, whenever an AWS HEAD request fails with a 400 HTTP
error code, it is retried.

Additionally, the expiration time of AWS credentials (if present) is
adjusted, so that credentials are refreshed earlier, five minutes before
the required time.

Change-Id: I0a622383e01258284ac1f05e8d30f3f956d01e68

Author: Paweł Andruszkiewicz

mysqlshdk/libs/aws/aws_credentials_provider.cc

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2023, Oracle and/or its affiliates.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License, version 2.0,
@@ -23,10 +23,14 @@
 
 #include "mysqlshdk/libs/aws/aws_credentials_provider.h"
 
+#include <chrono>
 #include <mutex>
 #include <stdexcept>
 #include <utility>
 
+#include "mysqlshdk/libs/utils/logger.h"
+#include "mysqlshdk/libs/utils/utils_time.h"
+
 namespace mysqlshdk {
 namespace aws {
 
@@ -96,10 +100,33 @@ std::shared_ptr<Aws_credentials> Aws_credentials_provider::get_credentials() {
 
   if (credentials.access_key_id.has_value() &&
       credentials.secret_access_key.has_value()) {
+    Aws_credentials::Time_point expiration = Aws_credentials::NO_EXPIRATION;
+
+    if (credentials.expiration.has_value()) {
+      log_info("The AWS credentials are set to expire at: %s",
+               credentials.expiration->c_str());
+
+      try {
+        expiration = shcore::rfc3339_to_time_point(*credentials.expiration);
+      } catch (const std::exception &e) {
+        throw std::runtime_error("Failed to parse 'Expiration' value '" +
+                                 *credentials.expiration + "' in " + name());
+      }
+
+      // we adjust the expiration time so credentials are refreshed before they
+      // actually expire; the minimum duration specified by AWS docs is 15
+      // minutes, but we check if we don't end up with a value that's in the
+      // past, as some of our tests use values smaller than that
+      if (const auto adjusted_expiration = expiration - std::chrono::minutes(5);
+          Aws_credentials::Clock::now() < adjusted_expiration) {
+        log_info("The expiration of AWS credentials has been adjusted");
+        expiration = adjusted_expiration;
+      }
+    }
+
     return std::make_shared<Aws_credentials>(
         *credentials.access_key_id, *credentials.secret_access_key,
-        credentials.session_token.value_or(std::string{}),
-        credentials.expiration);
+        credentials.session_token.value_or(std::string{}), expiration);
   }
 
   return {};

mysqlshdk/libs/aws/aws_credentials_provider.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2023, Oracle and/or its affiliates.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License, version 2.0,
@@ -63,7 +63,7 @@ class Aws_credentials_provider {
     std::optional<std::string> access_key_id;
     std::optional<std::string> secret_access_key;
     std::optional<std::string> session_token;
-    Aws_credentials::Time_point expiration = Aws_credentials::NO_EXPIRATION;
+    std::optional<std::string> expiration;
   };
 
   struct Context {

mysqlshdk/libs/aws/aws_signer.cc

@@ -271,8 +271,15 @@ bool Aws_signer::auth_data_expired(time_t now) const {
   return m_credentials->expired(now);
 }
 
-bool Aws_signer::is_authorization_error(const rest::Response &response) const {
+bool Aws_signer::is_authorization_error(const rest::Signed_request &request,
+                                        const rest::Response &response) const {
   if (rest::Response::Status_code::BAD_REQUEST == response.status) {
+    if (rest::Type::HEAD == request.type) {
+      // if this was a HEAD request, then we won't get the body and the error
+      // code, retry just in case, it's a lightweight request
+      return true;
+    }
+
     if (const auto error = response.get_error(); error.has_value()) {
       if ("ExpiredToken" == error->code() ||
           "TokenRefreshRequired" == error->code()) {
@@ -283,7 +290,7 @@ bool Aws_signer::is_authorization_error(const rest::Response &response) const {
     }
   }
 
-  return Signer::is_authorization_error(response);
+  return Signer::is_authorization_error(request, response);
 }
 
 bool Aws_signer::update_credentials() {

mysqlshdk/libs/aws/aws_signer.h

@@ -68,7 +68,8 @@ class Aws_signer : public rest::Signer {
 
   bool auth_data_expired(time_t now) const override;
 
-  bool is_authorization_error(const rest::Response &response) const override;
+  bool is_authorization_error(const rest::Signed_request &request,
+                              const rest::Response &response) const override;
 
  private:
 #ifdef FRIEND_TEST

mysqlshdk/libs/aws/process_credentials_provider.cc

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2023, Oracle and/or its affiliates.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License, version 2.0,
@@ -35,7 +35,6 @@
 #include "mysqlshdk/libs/utils/utils_general.h"
 #include "mysqlshdk/libs/utils/utils_lexing.h"
 #include "mysqlshdk/libs/utils/utils_string.h"
-#include "mysqlshdk/libs/utils/utils_time.h"
 
 #ifdef _WIN32
 #define pclose _pclose
@@ -235,15 +234,7 @@ Aws_credentials_provider::Credentials Process_credentials_provider::parse_json(
   creds.access_key_id = required(access_key_id());
   creds.secret_access_key = required(secret_access_key());
   creds.session_token = optional("SessionToken");
-
-  if (const auto expiration = optional("Expiration"); expiration.has_value()) {
-    try {
-      creds.expiration = shcore::rfc3339_to_time_point(*expiration);
-    } catch (const std::exception &e) {
-      handle_error(std::string{"failed to parse 'Expiration' value: "} +
-                   e.what());
-    }
-  }
+  creds.expiration = optional("Expiration");
 
   return creds;
 }

mysqlshdk/libs/rest/signed_rest_service.cc

@@ -243,7 +243,7 @@ Response::Status_code Signed_rest_service::execute(Signed_request *request,
   do {
     code = rest->execute(request, response);
     retry = Response::is_error(code) &&
-            m_signer->is_authorization_error(*response) &&
+            m_signer->is_authorization_error(*request, *response) &&
             ++retries <= k_authorization_retry_limit;
 
     if (retry) {

mysqlshdk/libs/rest/signed_rest_service.h

@@ -77,7 +77,8 @@ class Signer {
 
   virtual bool auth_data_expired(time_t now) const = 0;
 
-  virtual bool is_authorization_error(const Response &response) const {
+  virtual bool is_authorization_error(const Signed_request &,
+                                      const Response &response) const {
     return Response::Status_code::UNAUTHORIZED == response.status;
   }
 };

unittest/mysqlshdk/libs/aws/aws_credentials_provider_t.cc

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2023, Oracle and/or its affiliates.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License, version 2.0,
@@ -27,10 +27,12 @@
 
 #include "unittest/gtest_clean.h"
 
+#include <chrono>
 #include <thread>
 #include <vector>
 
 #include "mysqlshdk/libs/utils/utils_general.h"
+#include "mysqlshdk/libs/utils/utils_time.h"
 
 namespace mysqlshdk {
 namespace aws {
@@ -51,14 +53,30 @@ TEST(Aws_credentials_provider_test, temporary_credentials) {
 
       result.access_key_id = access_key_id() + std::to_string(m_called);
       result.secret_access_key = secret_access_key() + std::to_string(m_called);
-      result.expiration =
-          Aws_credentials::Clock::now() + std::chrono::milliseconds(100);
+      result.expiration = expiration();
 
       ++m_called;
 
       return result;
     }
 
+    static std::string expiration() {
+      using std::chrono::milliseconds;
+      using std::chrono::time_point_cast;
+
+      const auto tp = time_point_cast<milliseconds>(
+          Aws_credentials::Clock::now() + milliseconds(100));
+      auto result = shcore::time_point_to_rfc3339(tp);
+      if (std::string::npos == result.find('.')) {
+        // add milliseconds
+        auto ms = std::to_string(tp.time_since_epoch().count() %
+                                 milliseconds::period::den);
+        ms = "." + std::string(3 - ms.length(), '0') + ms;
+        result = result.substr(0, 19) + ms + result.substr(19);
+      }
+      return result;
+    }
+
     int m_called = 0;
   };
 

unittest/mysqlshdk/libs/aws/s3_bucket_config_t.cc

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2023, Oracle and/or its affiliates.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License, version 2.0,
@@ -875,12 +875,14 @@ TEST_F(Aws_s3_bucket_config_test, process_credentials) {
   }
 
   const auto EXPECT_EXPIRATION = [](Aws_credentials::Time_point expected,
-                                    Aws_credentials::Time_point actual) {
+                                    Aws_credentials::Time_point actual,
+                                    bool adjusted = false) {
     // shcore::time_point_to_rfc3339() does not include information about
     // fractions of seconds, these need to be converted to seconds before we can
     // compare them
     EXPECT_EQ(std::chrono::floor<std::chrono::seconds>(expected),
-              std::chrono::floor<std::chrono::seconds>(actual));
+              std::chrono::floor<std::chrono::seconds>(actual) +
+                  std::chrono::minutes(adjusted ? 5 : 0));
   };
 
   {
@@ -947,7 +949,7 @@ TEST_F(Aws_s3_bucket_config_test, process_credentials) {
     EXPECT_EQ(k_session_token, credentials->session_token());
     EXPECT_TRUE(credentials->temporary());
     EXPECT_FALSE(credentials->expired());
-    EXPECT_EXPIRATION(expiration, credentials->expiration());
+    EXPECT_EXPIRATION(expiration, credentials->expiration(), true);
   }
 
   {
@@ -969,7 +971,7 @@ TEST_F(Aws_s3_bucket_config_test, process_credentials) {
     EXPECT_EQ("", credentials->session_token());
     EXPECT_TRUE(credentials->temporary());
     EXPECT_FALSE(credentials->expired());
-    EXPECT_EXPIRATION(expiration, credentials->expiration());
+    EXPECT_EXPIRATION(expiration, credentials->expiration(), true);
   }
 
   {

unittest/scripts/auto/py_aws/scripts/util_dump_and_load_aws_norecord.py

@@ -516,17 +516,38 @@ def TEST_BUG35027093(http_server, expected_error):
     EXPECT_SHELL_LOG_CONTAINS_COUNT("Refreshing authentication data", 2)
     EXPECT_SHELL_LOG_CONTAINS_COUNT("Retrying a request which failed due to an authorization error", 2)
 
+# BUG#35468541 - expired HEAD request
+class FailHeadRequest(BaseHTTPRequestHandler):
+    protocol_version = "HTTP/1.1"
+    def do_HEAD(self):
+        self.send_response(400)
+        self.send_header('Content-Type', 'application/xml')
+        self.send_header('Content-Length', 123)
+        self.end_headers()
+        self.close_connection = True
+    def log_message(self, format, *args):
+        pass
+
+def TEST_BUG35468541(http_server, expected_error):
+    WIPE_SHELL_LOG()
+    EXPECT_THROWS(lambda: util.load_dump(dump_dir, get_options({ "s3EndpointOverride": test_server_url(http_server), "s3Profile": local_aws_profile, "s3ConfigFile": local_aws_config_file })), expected_error)
+    EXPECT_SHELL_LOG_CONTAINS_COUNT("Refreshing authentication data", 2)
+    EXPECT_SHELL_LOG_CONTAINS_COUNT("Retrying a request which failed due to an authorization error", 2)
+
 expired_token_server = start_test_server(ExpiredToken)
 token_refresh_server = start_test_server(TokenRefreshRequired)
+fail_head_server = start_test_server(FailHeadRequest)
 
-#@<> BUG#35027093 - test
+#@<> BUG#35027093 + BUG#35468541 - test
 with write_profile(local_aws_config_file, "profile " + local_aws_profile, { "credential_process": f"{__mysqlsh} --py --file {creds_script}" }):
     TEST_BUG35027093(expired_token_server, "The provided token has expired. (400)")
     TEST_BUG35027093(token_refresh_server, "The provided token must be refreshed. (400)")
+    TEST_BUG35468541(fail_head_server, "Bad Request (400)")
 
-#@<> BUG#35027093 - cleanup
+#@<> BUG#35027093 + BUG#35468541 - cleanup
 stop_test_server(expired_token_server)
 stop_test_server(token_refresh_server)
+stop_test_server(fail_head_server)
 
 if os.path.exists(creds_script):
     os.remove(creds_script)