AzureCliCredential: add support for expires_on field.

sdk/identity/azure-identity/CHANGELOG.md

@@ -4,6 +4,8 @@
 
 ### Features Added
 
+- [[#5116]](https://github.com/Azure/azure-sdk-for-cpp/issues/5116) `AzureCliCredential`: Added support for the new response field which represents token expiration timestamp as time zone agnostic value.
+
 ### Breaking Changes
 
 ### Bugs Fixed

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

@@ -153,8 +153,21 @@ AccessToken AzureCliCredential::GetToken(
 
       try
       {
+        // The order of elements in the vector below does matter - the code tries to find them
+        // consequently, and if finding the first one succeeds, we would not attempt to parse the
+        // second one. That is important, because the newer Azure CLI versions do have the new
+        // 'expires_on' field, which is not affected by time zone changes. The 'expiresOn' field was
+        // the only field that was present in the older versions, and it had problems, because it
+        // was a local timestamp without the time zone information.
+        // So, if only the 'expires_on' is available, we try to use it, and only if it is not
+        // available, we fall back to trying to get the value via 'expiresOn', which we also now are
+        // able to handle correctly, except when the token expiration crosses the time when the
+        // local system clock moves to and from DST.
         return TokenCredentialImpl::ParseToken(
-            azCliResult, "accessToken", "expiresIn", "expiresOn");
+            azCliResult,
+            "accessToken",
+            "expiresIn",
+            std::vector<std::string>{"expires_on", "expiresOn"});
       }
       catch (json::exception const&)
       {

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

@@ -59,6 +59,31 @@ namespace Azure { namespace Identity { namespace _detail {
         bool asResource,
         bool urlEncode = true);
 
+    /**
+     * @brief Parses JSON that contains access token and its expiration.
+     *
+     * @param jsonString String with a JSON object to parse.
+     * @param accessTokenPropertyName Name of a property in the JSON object that represents access
+     * token.
+     * @param expiresInPropertyName Name of a property in the JSON object that represents token
+     * expiration in number of seconds from now.
+     * @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.
+     *
+     * @return A successfully parsed access token.
+     *
+     * @throw `std::exception` if there was a problem parsing the token.
+     *
+     * @note The order of elements in \p expiresOnPropertyNames does matter: the code will first try
+     * to find a property with the name of the first element, then of a second one, and so on.
+     */
+    static Core::Credentials::AccessToken ParseToken(
+        std::string const& jsonString,
+        std::string const& accessTokenPropertyName,
+        std::string const& expiresInPropertyName,
+        std::vector<std::string> const& expiresOnPropertyNames);
+
     /**
      * @brief Parses JSON that contains access token and its expiration.
      *
@@ -79,7 +104,14 @@ namespace Azure { namespace Identity { namespace _detail {
         std::string const& jsonString,
         std::string const& accessTokenPropertyName,
         std::string const& expiresInPropertyName,
-        std::string const& expiresOnPropertyName);
+        std::string const& expiresOnPropertyName)
+    {
+      return ParseToken(
+          jsonString,
+          accessTokenPropertyName,
+          expiresInPropertyName,
+          std::vector<std::string>{expiresOnPropertyName});
+    }
 
     /**
      * @brief Holds `#Azure::Core::Http::Request` and all the associated resources for the HTTP

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

@@ -10,7 +10,9 @@
 #include <azure/core/internal/strings.hpp>
 #include <azure/core/url.hpp>
 
+#include <algorithm>
 #include <chrono>
+#include <iterator>
 #include <limits>
 #include <map>
 
@@ -167,22 +169,25 @@ std::string TokenAsDiagnosticString(
     json const& jsonObject,
     std::string const& accessTokenPropertyName,
     std::string const& expiresInPropertyName,
-    std::string const& expiresOnPropertyName);
+    std::vector<std::string> const& expiresOnPropertyNames);
 
 [[noreturn]] void ThrowJsonPropertyError(
     std::string const& failedPropertyName,
     json const& jsonObject,
     std::string const& accessTokenPropertyName,
     std::string const& expiresInPropertyName,
-    std::string const& expiresOnPropertyName)
+    std::vector<std::string> const& expiresOnPropertyNames)
 {
   if (IdentityLog::ShouldWrite(IdentityLog::Level::Verbose))
   {
     IdentityLog::Write(
         IdentityLog::Level::Verbose,
         ParseTokenLogPrefix
             + TokenAsDiagnosticString(
-                jsonObject, accessTokenPropertyName, expiresInPropertyName, expiresOnPropertyName));
+                jsonObject,
+                accessTokenPropertyName,
+                expiresInPropertyName,
+                expiresOnPropertyNames));
   }
 
   throw std::runtime_error(
@@ -196,7 +201,7 @@ AccessToken TokenCredentialImpl::ParseToken(
     std::string const& jsonString,
     std::string const& accessTokenPropertyName,
     std::string const& expiresInPropertyName,
-    std::string const& expiresOnPropertyName)
+    std::vector<std::string> const& expiresOnPropertyNames)
 {
   json parsedJson;
   try
@@ -220,7 +225,7 @@ AccessToken TokenCredentialImpl::ParseToken(
         parsedJson,
         accessTokenPropertyName,
         expiresInPropertyName,
-        expiresOnPropertyName);
+        expiresOnPropertyNames);
   }
 
   AccessToken accessToken;
@@ -274,77 +279,88 @@ AccessToken TokenCredentialImpl::ParseToken(
     }
   }
 
-  if (expiresOnPropertyName.empty())
+  std::vector<std::string> nonEmptyExpiresOnPropertyNames;
+  std::copy_if(
+      expiresOnPropertyNames.begin(),
+      expiresOnPropertyNames.end(),
+      std::back_inserter(nonEmptyExpiresOnPropertyNames),
+      [](auto const& propertyName) { return !propertyName.empty(); });
+
+  if (nonEmptyExpiresOnPropertyNames.empty())
   {
-    // 'expires_in' is undefined, 'expires_on' is not expected.
+    // The code was not able to parse the value of 'expires_in', and the caller did not pass any
+    // 'expires_on' for us to find and try parse.
     ThrowJsonPropertyError(
         expiresInPropertyName,
         parsedJson,
         accessTokenPropertyName,
         expiresInPropertyName,
-        expiresOnPropertyName);
+        nonEmptyExpiresOnPropertyNames);
   }
 
-  if (parsedJson.contains(expiresOnPropertyName))
+  for (auto const& expiresOnPropertyName : nonEmptyExpiresOnPropertyNames)
   {
-    auto const& expiresOn = parsedJson[expiresOnPropertyName];
-
-    if (expiresOn.is_number_unsigned())
+    if (parsedJson.contains(expiresOnPropertyName))
     {
-      try
-      {
-        // 'expires_on' as number (posix time representing an absolute timestamp)
-        auto const value = expiresOn.get<std::int64_t>();
-        if (value <= MaxPosixTimestamp)
-        {
-          accessToken.ExpiresOn = PosixTimeConverter::PosixTimeToDateTime(value);
-          return accessToken;
-        }
-      }
-      catch (std::exception const&)
-      {
-        // expiresIn.get<std::int64_t>() has thrown, we may throw later.
-      }
-    }
+      auto const& expiresOn = parsedJson[expiresOnPropertyName];
 
-    if (expiresOn.is_string())
-    {
-      auto const expiresOnAsString = expiresOn.get<std::string>();
-      for (auto const& parse : {
-               std::function<DateTime(std::string const&)>([](auto const& s) {
-                 // 'expires_on' as RFC3339 date string (absolute timestamp)
-                 return DateTime::Parse(s, DateTime::DateFormat::Rfc3339);
-               }),
-               std::function<DateTime(std::string const&)>([](auto const& s) {
-                 // 'expires_on' as numeric string (posix time representing an absolute timestamp)
-                 return PosixTimeConverter::PosixTimeToDateTime(
-                     ParseNumericExpiration(s, MaxPosixTimestamp));
-               }),
-               std::function<DateTime(std::string const&)>([](auto const& s) {
-                 // 'expires_on' as RFC1123 date string (absolute timestamp)
-                 return DateTime::Parse(s, DateTime::DateFormat::Rfc1123);
-               }),
-           })
+      if (expiresOn.is_number_unsigned())
       {
         try
         {
-          accessToken.ExpiresOn = parse(expiresOnAsString);
-          return accessToken;
+          // 'expires_on' as number (posix time representing an absolute timestamp)
+          auto const value = expiresOn.get<std::int64_t>();
+          if (value <= MaxPosixTimestamp)
+          {
+            accessToken.ExpiresOn = PosixTimeConverter::PosixTimeToDateTime(value);
+            return accessToken;
+          }
         }
         catch (std::exception const&)
         {
-          // parse() has thrown, we may throw later.
+          // expiresIn.get<std::int64_t>() has thrown, we may throw later.
+        }
+      }
+
+      if (expiresOn.is_string())
+      {
+        auto const expiresOnAsString = expiresOn.get<std::string>();
+        for (auto const& parse : {
+                 std::function<DateTime(std::string const&)>([](auto const& s) {
+                   // 'expires_on' as RFC3339 date string (absolute timestamp)
+                   return DateTime::Parse(s, DateTime::DateFormat::Rfc3339);
+                 }),
+                 std::function<DateTime(std::string const&)>([](auto const& s) {
+                   // 'expires_on' as numeric string (posix time representing an absolute timestamp)
+                   return PosixTimeConverter::PosixTimeToDateTime(
+                       ParseNumericExpiration(s, MaxPosixTimestamp));
+                 }),
+                 std::function<DateTime(std::string const&)>([](auto const& s) {
+                   // 'expires_on' as RFC1123 date string (absolute timestamp)
+                   return DateTime::Parse(s, DateTime::DateFormat::Rfc1123);
+                 }),
+             })
+        {
+          try
+          {
+            accessToken.ExpiresOn = parse(expiresOnAsString);
+            return accessToken;
+          }
+          catch (std::exception const&)
+          {
+            // parse() has thrown, we may throw later.
+          }
         }
       }
     }
   }
 
   ThrowJsonPropertyError(
-      expiresOnPropertyName,
+      nonEmptyExpiresOnPropertyNames.back(),
       parsedJson,
       accessTokenPropertyName,
       expiresInPropertyName,
-      expiresOnPropertyName);
+      nonEmptyExpiresOnPropertyNames);
 }
 
 namespace {
@@ -433,7 +449,7 @@ std::string TokenAsDiagnosticString(
     json const& jsonObject,
     std::string const& accessTokenPropertyName,
     std::string const& expiresInPropertyName,
-    std::string const& expiresOnPropertyName)
+    std::vector<std::string> const& expiresOnPropertyNames)
 {
   std::string result = "Please report an issue with the following details:\nToken JSON";
 
@@ -456,30 +472,38 @@ std::string TokenAsDiagnosticString(
           : PrintSanitizedJsonObject(accessTokenProperty, false);
     }
 
-    for (auto const& p : {
-             std::pair<char const*, std::string const*>{"relative", &expiresInPropertyName},
-             std::pair<char const*, std::string const*>{"absolute", &expiresOnPropertyName},
-         })
     {
-      result += ", ";
-      result += p.first;
-      result += " expiration property ('" + *p.second + "'): ";
+      std::vector<std::pair<char const*, std::string const*>> expirationProperties{
+          {"relative", &expiresInPropertyName}};
 
-      if (!jsonObject.contains(*p.second))
+      for (auto const& ap : expiresOnPropertyNames)
       {
-        result += "undefined";
+        expirationProperties.emplace_back(std::make_pair("absolute", &ap));
       }
-      else
+
+      for (auto const& p : expirationProperties)
       {
-        result += PrintSanitizedJsonObject(jsonObject[*p.second], true);
+        result += ", ";
+        result += p.first;
+        result += " expiration property ('" + *p.second + "'): ";
+
+        if (!jsonObject.contains(*p.second))
+        {
+          result += "undefined";
+        }
+        else
+        {
+          result += PrintSanitizedJsonObject(jsonObject[*p.second], true);
+        }
       }
     }
 
     std::map<std::string, json> otherProperties;
     for (auto const& property : jsonObject.items())
     {
       if (property.key() != accessTokenPropertyName && property.key() != expiresInPropertyName
-          && property.key() != expiresOnPropertyName)
+          && std::find(expiresOnPropertyNames.begin(), expiresOnPropertyNames.end(), property.key())
+              == expiresOnPropertyNames.end())
       {
         otherProperties[property.key()] = property.value();
       }

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

@@ -221,6 +221,56 @@ TEST(AzureCliCredential, ExpiresIn)
   EXPECT_LE(token.ExpiresOn, timestampAfter + std::chrono::seconds(30));
 }
 
+TEST(AzureCliCredential, ExpiresOnUnixTimestampInt)
+{
+  // 'expires_on' is 1700692424, which is a Unix timestamp of a date in 2023.
+  // 'ExpiresOn' is a date in 2022.
+  // The test checks that when both are present, 'expires_on' value (2023) is taken,
+  // and not that of 'ExpiresOn'.
+  constexpr auto Token = "{\"accessToken\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\","
+                         "\"expiresOn\":\"2022-08-24 00:43:08.000000\"," // <-- 2022
+                         "\"expires_on\":1700692424}"; // <-- 2023
+
+  AzureCliTestCredential const azCliCred(EchoCommand(Token));
+
+  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-11-22T22:33:44.000000Z", DateTime::DateFormat::Rfc3339));
+}
+
+TEST(AzureCliCredential, ExpiresOnUnixTimestampString)
+{
+  // 'expires_on' is 1700692424, which is a Unix timestamp of a date in 2023.
+  // 'expiresOn' is a date in 2022.
+  // The test checks that when both are present, 'expires_on' value (2023) is taken,
+  // and not that of 'expiresOn'.
+  // The test is similar to the one above, but the Unix timestamp is represented as string
+  // containing an integer.
+  constexpr auto Token = "{\"accessToken\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\","
+                         "\"expiresOn\":\"2022-08-24 00:43:08.000000\"," // <-- 2022
+                         "\"expires_on\":\"1700692424\"}"; // <-- 2023
+
+  AzureCliTestCredential const azCliCred(EchoCommand(Token));
+
+  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-11-22T22:33:44.000000Z", DateTime::DateFormat::Rfc3339));
+}
+
 TEST(AzureCliCredential, TimedOut)
 {
   AzureCliCredentialOptions options;