AzureCliCredential to treat datetime without TZ info as local time. (#…

…5179)

* AzureCliCredential to treat datetime without TZ info as local time.

* Fix spelling

* CLang-format

* Parse as local by appending TimeZoneOffsetAsString()

* Fix bad merge

* PR Feedback

* Merge

* Merge

---------

Co-authored-by: Anton Kolesnyk <[email protected]>

sdk/identity/azure-identity/CHANGELOG.md

@@ -10,6 +10,8 @@
 
 ### Bugs Fixed
 
+- [[#5075]](https://github.com/Azure/azure-sdk-for-cpp/issues/5075) `AzureCliCredential` assumes token expiration time without local time zone adjustment.
+
 ### Other Changes
 
 - [[#5141]](https://github.com/Azure/azure-sdk-for-cpp/issues/5141) Added error response details to the `AuthenticationException` thrown when the authority host returns error response.

sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp

@@ -112,6 +112,7 @@ namespace Azure { namespace Identity {
   protected:
 #endif
     virtual std::string GetAzCommand(std::string const& scopes, std::string const& tenantId) const;
+    virtual int GetLocalTimeToUtcDiffSeconds() const;
   };
 
 }} // namespace Azure::Identity

sdk/identity/azure-identity/src/azure_cli_credential.cpp

@@ -13,7 +13,9 @@
 #include <azure/core/internal/unique_handle.hpp>
 #include <azure/core/platform.hpp>
 
+#include <chrono>
 #include <cstdio>
+#include <ctime>
 #include <stdexcept>
 #include <thread>
 #include <type_traits>
@@ -127,6 +129,27 @@ std::string AzureCliCredential::GetAzCommand(std::string const& scopes, std::str
   return command;
 }
 
+int AzureCliCredential::GetLocalTimeToUtcDiffSeconds() const
+{
+#ifdef _MSC_VER
+#pragma warning(push)
+// warning C4996: 'localtime': This function or variable may be unsafe. Consider using localtime_s
+// instead.
+#pragma warning(disable : 4996)
+#endif
+  // LCOV_EXCL_START
+  auto const timeTNow = std::chrono::system_clock::to_time_t(std::chrono::system_clock::now());
+
+  // std::difftime() returns difference in seconds.
+  // We do not expect any fractional parts, but should there be any - we do not care about them.
+  return static_cast<int>(
+      std::difftime(std::mktime(std::localtime(&timeTNow)), std::mktime(std::gmtime(&timeTNow))));
+  // LCOV_EXCL_STOP
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif
+}
+
 namespace {
 std::string RunShellCommand(
     std::string const& command,
@@ -167,7 +190,8 @@ AccessToken AzureCliCredential::GetToken(
             azCliResult,
             "accessToken",
             "expiresIn",
-            std::vector<std::string>{"expires_on", "expiresOn"});
+            std::vector<std::string>{"expires_on", "expiresOn"},
+            GetLocalTimeToUtcDiffSeconds());
       }
       catch (json::exception const&)
       {

sdk/identity/azure-identity/src/private/token_credential_impl.hpp

@@ -70,6 +70,11 @@ namespace Azure { namespace Identity { namespace _detail {
      * @param expiresOnPropertyNames Names of properties in the JSON object that represent token
      * expiration as absolute date-time stamp. Can be empty, in which case no attempt to parse the
      * corresponding property will be made. Empty string elements will be ignored.
+     * @param utcDiffSeconds Optional. If not 0, it represents the difference between the UTC and a
+     * desired time zone, in seconds. Then, should an RFC3339 timestamp come without a time zone
+     * information, a corresponding time zone offset will be applied to such timestamp.
+     * No credential other than AzureCliCredential (which gets timestamps as local time without time
+     * zone) should need to set it.
      *
      * @return A successfully parsed access token.
      *
@@ -82,7 +87,8 @@ namespace Azure { namespace Identity { namespace _detail {
         std::string const& jsonString,
         std::string const& accessTokenPropertyName,
         std::string const& expiresInPropertyName,
-        std::vector<std::string> const& expiresOnPropertyNames);
+        std::vector<std::string> const& expiresOnPropertyNames,
+        int utcDiffSeconds = 0);
 
     /**
      * @brief Parses JSON that contains access token and its expiration.

sdk/identity/azure-identity/src/token_credential_impl.cpp

@@ -12,9 +12,11 @@
 
 #include <algorithm>
 #include <chrono>
+#include <iomanip>
 #include <iterator>
 #include <limits>
 #include <map>
+#include <sstream>
 
 using Azure::Identity::_detail::TokenCredentialImpl;
 
@@ -197,13 +199,39 @@ std::string TokenAsDiagnosticString(
       + "\' property.\nSee Azure::Core::Diagnostics::Logger for details"
         " (https://aka.ms/azsdk/cpp/identity/troubleshooting).");
 }
+
+std::string TimeZoneOffsetAsString(int utcDiffSeconds)
+{
+  if (utcDiffSeconds == 0)
+  {
+    return {};
+  }
+
+  std::ostringstream os;
+  if (utcDiffSeconds >= 0)
+  {
+    os << '+';
+  }
+  else
+  {
+    os << '-';
+    utcDiffSeconds = -utcDiffSeconds;
+  }
+
+  os << std::setw(2) << std::setfill('0') << (utcDiffSeconds / (60 * 60)) << ':';
+  os << std::setw(2) << std::setfill('0') << ((utcDiffSeconds % (60 * 60)) / 60);
+
+  return os.str();
+}
+
 } // namespace
 
 AccessToken TokenCredentialImpl::ParseToken(
     std::string const& jsonString,
     std::string const& accessTokenPropertyName,
     std::string const& expiresInPropertyName,
-    std::vector<std::string> const& expiresOnPropertyNames)
+    std::vector<std::string> const& expiresOnPropertyNames,
+    int utcDiffSeconds)
 {
   json parsedJson;
   try
@@ -324,13 +352,14 @@ AccessToken TokenCredentialImpl::ParseToken(
         }
       }
 
+      auto const tzOffsetStr = TimeZoneOffsetAsString(utcDiffSeconds);
       if (expiresOn.is_string())
       {
         auto const expiresOnAsString = expiresOn.get<std::string>();
         for (auto const& parse : {
-                 std::function<DateTime(std::string const&)>([](auto const& s) {
+                 std::function<DateTime(std::string const&)>([&](auto const& s) {
                    // 'expires_on' as RFC3339 date string (absolute timestamp)
-                   return DateTime::Parse(s, DateTime::DateFormat::Rfc3339);
+                   return DateTime::Parse(s + tzOffsetStr, DateTime::DateFormat::Rfc3339);
                  }),
                  std::function<DateTime(std::string const&)>([](auto const& s) {
                    // 'expires_on' as numeric string (posix time representing an absolute timestamp)

sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp

@@ -51,6 +51,7 @@ std::string EchoCommand(std::string const text)
 class AzureCliTestCredential : public AzureCliCredential {
 private:
   std::string m_command;
+  int m_localTimeToUtcDiffSeconds = 0;
 
   std::string GetAzCommand(std::string const& resource, std::string const& tenantId) const override
   {
@@ -60,6 +61,8 @@ class AzureCliTestCredential : public AzureCliCredential {
     return m_command;
   }
 
+  int GetLocalTimeToUtcDiffSeconds() const override { return m_localTimeToUtcDiffSeconds; }
+
 public:
   explicit AzureCliTestCredential(std::string command) : m_command(std::move(command)) {}
 
@@ -80,6 +83,8 @@ class AzureCliTestCredential : public AzureCliCredential {
 
   decltype(m_tenantId) const& GetTenantId() const { return m_tenantId; }
   decltype(m_cliProcessTimeout) const& GetCliProcessTimeout() const { return m_cliProcessTimeout; }
+
+  void SetLocalTimeToUtcDiffSeconds(int diff) { m_localTimeToUtcDiffSeconds = diff; }
 };
 } // namespace
 
@@ -620,6 +625,40 @@ TEST(AzureCliCredential, StrictIso8601TimeFormat)
       DateTime::Parse("2022-08-24T00:43:08.000000Z", DateTime::DateFormat::Rfc3339));
 }
 
+TEST(AzureCliCredential, LocalTime)
+{
+  constexpr auto Token = "{\"accessToken\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\","
+                         "\"expiresOn\":\"2023-12-07 00:43:08\"}";
+
+  {
+    AzureCliTestCredential azCliCred(EchoCommand(Token));
+    azCliCred.SetLocalTimeToUtcDiffSeconds(-28800); // Redmond (no DST)
+
+    TokenRequestContext trc;
+    trc.Scopes.push_back("https://storage.azure.com/.default");
+    auto const token = azCliCred.GetToken(trc, {});
+
+    EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ");
+
+    EXPECT_EQ(
+        token.ExpiresOn, DateTime::Parse("2023-12-07T08:43:08Z", DateTime::DateFormat::Rfc3339));
+  }
+
+  {
+    AzureCliTestCredential azCliCred(EchoCommand(Token));
+    azCliCred.SetLocalTimeToUtcDiffSeconds(7200); // Kyiv (no DST)
+
+    TokenRequestContext trc;
+    trc.Scopes.push_back("https://storage.azure.com/.default");
+    auto const token = azCliCred.GetToken(trc, {});
+
+    EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ");
+
+    EXPECT_EQ(
+        token.ExpiresOn, DateTime::Parse("2023-12-06T22:43:08Z", DateTime::DateFormat::Rfc3339));
+  }
+}
+
 TEST(AzureCliCredential, Diagnosability)
 {
   {