From d6a0b193053f7f7bd9349f13710d6a60f0a0e56e Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk Date: Wed, 22 Nov 2023 20:35:42 -0800 Subject: [PATCH 1/8] AzureCliCredential to treat datetime without TZ info as local time. --- sdk/core/azure-core/CHANGELOG.md | 2 + .../azure-core/inc/azure/core/datetime.hpp | 136 +++++++++++---- sdk/core/azure-core/src/datetime.cpp | 41 ++++- sdk/core/azure-core/test/ut/datetime_test.cpp | 161 ++++++++++++++++++ sdk/identity/azure-identity/CHANGELOG.md | 2 + .../azure/identity/azure_cli_credential.hpp | 1 + .../src/azure_cli_credential.cpp | 7 +- .../src/private/token_credential_impl.hpp | 8 +- .../src/token_credential_impl.cpp | 12 +- .../test/ut/azure_cli_credential_test.cpp | 12 +- 10 files changed, 336 insertions(+), 46 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 373ea4ab64..0e3180fa87 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -10,6 +10,8 @@ ### Other Changes +- Added internal provisions for extended `Azure::DateTime` parsing. + ## 1.11.0-beta.2 (2023-11-02) ### Features Added diff --git a/sdk/core/azure-core/inc/azure/core/datetime.hpp b/sdk/core/azure-core/inc/azure/core/datetime.hpp index 964b4cfaa2..9d9ec6f139 100644 --- a/sdk/core/azure-core/inc/azure/core/datetime.hpp +++ b/sdk/core/azure-core/inc/azure/core/datetime.hpp @@ -43,6 +43,10 @@ namespace _detail { }; } // namespace _detail +namespace Core { namespace _internal { + class DateTimeExtensions; +}} // namespace Core::_internal + /** * @brief Manages date and time in standardized string formats. * @details Supports date range from year 0001 to end of year 9999 with 100ns (7 decimal places @@ -52,6 +56,7 @@ namespace _detail { * @remark This class is supposed to be able to handle a DateTime that comes over the wire. */ class DateTime final : public _detail::Clock::time_point { + friend class Core::_internal::DateTimeExtensions; private: AZ_CORE_DLLEXPORT static DateTime const SystemClockEpoch; @@ -168,6 +173,14 @@ class DateTime final : public _detail::Clock::time_point { Rfc3339, }; +private: + static DateTime Parse( + std::string const& dateTime, + DateFormat format, + bool rfc3339NoTimeZoneMeansLocal, + int const* localTimeToUtcDiffSeconds); + +public: /** * @brief Create #Azure::DateTime from a string representing time in UTC in the specified * format. @@ -179,7 +192,10 @@ class DateTime final : public _detail::Clock::time_point { * * @throw std::invalid_argument If \p format is not recognized, or if parsing error. */ - static DateTime Parse(std::string const& dateTime, DateFormat format); + static DateTime Parse(std::string const& dateTime, DateFormat format) + { + return Parse(dateTime, format, false, nullptr); + } /** * @brief Get a string representation of the #Azure::DateTime. @@ -281,49 +297,95 @@ inline bool operator>=(std::chrono::system_clock::time_point const& tp, DateTime } namespace Core { namespace _internal { - /** - * @brief Provides convertion methods for POSIX time to an #Azure::DateTime. - * - */ - class PosixTimeConverter final { - public: - /** - * @brief Converts POSIX time to an #Azure::DateTime. - * - * @param posixTime The number of seconds since 1970. - * @return Calculated #Azure::DateTime. - */ - static DateTime PosixTimeToDateTime(int64_t posixTime) - { - return {DateTime(1970) + std::chrono::seconds(posixTime)}; - } - /** - * @brief Converts a DateTime to POSIX time. + * @brief Provides convertion methods for POSIX time to an #Azure::DateTime. * - * @param dateTime The `%DateTime` to convert. - * @return The number of seconds since 1970. */ - static int64_t DateTimeToPosixTime(DateTime const& dateTime) - { - // This count starts at the POSIX epoch which is January 1st, 1970 UTC. - return std::chrono::duration_cast(dateTime - DateTime(1970)).count(); - } + class PosixTimeConverter final { + public: + /** + * @brief Converts POSIX time to an #Azure::DateTime. + * + * @param posixTime The number of seconds since 1970. + * @return Calculated #Azure::DateTime. + */ + static DateTime PosixTimeToDateTime(int64_t posixTime) + { + return {DateTime(1970) + std::chrono::seconds(posixTime)}; + } + + /** + * @brief Converts a DateTime to POSIX time. + * + * @param dateTime The `%DateTime` to convert. + * @return The number of seconds since 1970. + */ + static int64_t DateTimeToPosixTime(DateTime const& dateTime) + { + // This count starts at the POSIX epoch which is January 1st, 1970 UTC. + return std::chrono::duration_cast(dateTime - DateTime(1970)).count(); + } + + private: + /** + * @brief An instance of `%PosixTimeConverter` class cannot be created. + * + */ + PosixTimeConverter() = delete; + + /** + * @brief An instance of `%PosixTimeConverter` class cannot be destructed, because no instance + * can be created. + * + */ + ~PosixTimeConverter() = delete; + }; - private: /** - * @brief An instance of `%PosixTimeConverter` class cannot be created. + * @brief Provides addtional methods for #Azure::DateTime. * */ - PosixTimeConverter() = delete; - - /** - * @brief An instance of `%PosixTimeConverter` class cannot be destructed, because no instance - * can be created. - * - */ - ~PosixTimeConverter() = delete; - }; + class DateTimeExtensions final { + public: + /** + * @brief Create #Azure::DateTime from a string representing time in UTC in the specified + * format. + * + * @param dateTime A string with the date and time. + * @param format A format to which \p dateTime string adheres to. + * @param rfc3339NoTimeZoneMeansLocal If time zone is not specified, treat the \p dateTime as + * local time. + * @param localTimeToUtcDiffSeconds Assume this offset in seconds between local time and UTC + * time, for unit test reproduceability. Detect current time zone if `nullptr`. + * + * @return #Azure::DateTime that was constructed from the \p dateTime string. + * + * @throw std::invalid_argument If \p format is not recognized, or if parsing error. + */ + static DateTime Parse( + std::string const& dateTime, + DateTime::DateFormat format, + bool rfc3339NoTimeZoneMeansLocal, + int const* localTimeToUtcDiffSeconds) + { + return DateTime::Parse( + dateTime, format, rfc3339NoTimeZoneMeansLocal, localTimeToUtcDiffSeconds); + } + + private: + /** + * @brief An instance of `%DateTimeExtensions` class cannot be created. + * + */ + DateTimeExtensions() = delete; + + /** + * @brief An instance of `%DateTimeExtensions` class cannot be destructed, because no instance + * can be created. + * + */ + ~DateTimeExtensions() = delete; + }; }} // namespace Core::_internal } // namespace Azure diff --git a/sdk/core/azure-core/src/datetime.cpp b/sdk/core/azure-core/src/datetime.cpp index 2946fa3ce1..51e163ab31 100644 --- a/sdk/core/azure-core/src/datetime.cpp +++ b/sdk/core/azure-core/src/datetime.cpp @@ -447,7 +447,11 @@ DateTime::operator std::chrono::system_clock::time_point() const + std::chrono::duration_cast(*this - SystemClockEpoch); } -DateTime DateTime::Parse(std::string const& dateTime, DateFormat format) +DateTime DateTime::Parse( + std::string const& dateTime, + DateFormat format, + bool rfc3339NoTimeZoneMeansLocal, + int const* localTimeToUtcDiffSeconds) { // The values that are not supposed to be read before they are written are set to -123... to avoid // warnings on some compilers, yet provide a clearly bad value to make it obvious if things don't @@ -464,6 +468,7 @@ DateTime DateTime::Parse(std::string const& dateTime, DateFormat format) int8_t localDiffHours = 0; int8_t localDiffMinutes = 0; bool roundFracSecUp = false; + bool adjustAsLocal = false; { std::string::size_type const DateTimeLength = dateTime.length(); std::string::size_type minDateTimeLength = 0; @@ -721,6 +726,16 @@ DateTime DateTime::Parse(std::string const& dateTime, DateFormat format) &cursor, dateTime, DateTimeLength, parsingArea, 2, 2); } } + else if (rfc3339NoTimeZoneMeansLocal) // Fractional seconds were present, but not the time + // zone + { + adjustAsLocal = true; + } + } + else if ( // No fractional seconds and no time zone were present + rfc3339NoTimeZoneMeansLocal && cursor == DateTimeLength) + { + adjustAsLocal = true; } } } @@ -729,6 +744,30 @@ DateTime DateTime::Parse(std::string const& dateTime, DateFormat format) throw std::invalid_argument("Unrecognized date format."); } + if (adjustAsLocal) + { + auto const timeTNow = std::chrono::system_clock::to_time_t(std::chrono::system_clock::now()); + +#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 utcDiffSeconds = localTimeToUtcDiffSeconds != nullptr + ? *localTimeToUtcDiffSeconds + : static_cast(std::difftime( + std::mktime(std::localtime(&timeTNow)), std::mktime(std::gmtime(&timeTNow)))); + // LCOV_EXCL_STOP +#ifdef _MSC_VER +#pragma warning(pop) +#endif + + localDiffHours = static_cast(utcDiffSeconds / (60 * 60)); + localDiffMinutes = static_cast((utcDiffSeconds % (60 * 60)) / 60); + } + return DateTime( year, month, diff --git a/sdk/core/azure-core/test/ut/datetime_test.cpp b/sdk/core/azure-core/test/ut/datetime_test.cpp index d68f7d6f9f..a33e5dee6d 100644 --- a/sdk/core/azure-core/test/ut/datetime_test.cpp +++ b/sdk/core/azure-core/test/ut/datetime_test.cpp @@ -902,3 +902,164 @@ TEST(DateTime, Rfc3339Space) = DateTime::Parse("2022-08-24 00:43:08.0004308Z", DateTime::DateFormat::Rfc3339); EXPECT_EQ(datetime.ToString(DateTime::DateFormat::Rfc3339), "2022-08-24T00:43:08.0004308Z"); } + +TEST(DateTime, LocalTime) +{ + using Azure::Core::_internal::DateTimeExtensions; + + { + auto const uniformOffsetSeconds = -28800; // Redmond (no DST) + + // No fractions, no time zone + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44", DateTime::DateFormat::Rfc3339, true, &uniformOffsetSeconds) + .ToString(), + "2023-11-23T02:33:44Z"); // Local time adjustment + + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44", DateTime::DateFormat::Rfc3339, false, &uniformOffsetSeconds) + .ToString(), + "2023-11-22T18:33:44Z"); // no adjustment + + // With fractions, no time zone + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44.4308", DateTime::DateFormat::Rfc3339, true, &uniformOffsetSeconds) + .ToString(), + "2023-11-23T02:33:44.4308Z"); // Local time adjustment + + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44.4308", DateTime::DateFormat::Rfc3339, false, &uniformOffsetSeconds) + .ToString(), + "2023-11-22T18:33:44.4308Z"); // no adjustment + + // No fractions, with time zone + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44-05:00", DateTime::DateFormat::Rfc3339, true, &uniformOffsetSeconds) + .ToString(), + "2023-11-22T23:33:44Z"); // no local time adjustment, just the regular time zone adjustment + + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44-05:00", + DateTime::DateFormat::Rfc3339, + false, + &uniformOffsetSeconds) + .ToString(), + "2023-11-22T23:33:44Z"); // no local time adjustment, just the regular time zone adjustment + + // With fractions, with time zone + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44.4308-05:00", + DateTime::DateFormat::Rfc3339, + true, + &uniformOffsetSeconds) + .ToString(), + "2023-11-22T23:33:44.4308Z"); // no local time adjustment, just the regular time zone + // adjustment + + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44.4308-05:00", + DateTime::DateFormat::Rfc3339, + false, + &uniformOffsetSeconds) + .ToString(), + "2023-11-22T23:33:44.4308Z"); // no local time adjustment, just the regular time zone + // adjustment + + // No fractions, Zulu time zone + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44Z", DateTime::DateFormat::Rfc3339, true, &uniformOffsetSeconds) + .ToString(), + "2023-11-22T18:33:44Z"); // no adjustment + + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44Z", DateTime::DateFormat::Rfc3339, false, &uniformOffsetSeconds) + .ToString(), + "2023-11-22T18:33:44Z"); // no adjustment + + // With fractions, Zulu time zone + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44.4308Z", DateTime::DateFormat::Rfc3339, true, &uniformOffsetSeconds) + .ToString(), + "2023-11-22T18:33:44.4308Z"); // no adjustment + + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44.4308Z", + DateTime::DateFormat::Rfc3339, + false, + &uniformOffsetSeconds) + .ToString(), + "2023-11-22T18:33:44.4308Z"); // no adjustment + } + + { + auto const bravoOffsetSeconds = 7200; // Kyiv (no DST) + + // No fractions, no time zone + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44", DateTime::DateFormat::Rfc3339, true, &bravoOffsetSeconds) + .ToString(), + "2023-11-22T16:33:44Z"); // Local time adjustment + + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44", DateTime::DateFormat::Rfc3339, false, &bravoOffsetSeconds) + .ToString(), + "2023-11-22T18:33:44Z"); // no adjustment + + // With fractions, no time zone + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44.4308", DateTime::DateFormat::Rfc3339, true, &bravoOffsetSeconds) + .ToString(), + "2023-11-22T16:33:44.4308Z"); // Local time adjustment + + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44.4308", DateTime::DateFormat::Rfc3339, false, &bravoOffsetSeconds) + .ToString(), + "2023-11-22T18:33:44.4308Z"); // no adjustment + } + + { + auto const zuluOffsetSeconds = 0; // London (no DST) + + // No fractions, no time zone + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44", DateTime::DateFormat::Rfc3339, true, &zuluOffsetSeconds) + .ToString(), + "2023-11-22T18:33:44Z"); // Local time adjustment, no effect + + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44", DateTime::DateFormat::Rfc3339, false, &zuluOffsetSeconds) + .ToString(), + "2023-11-22T18:33:44Z"); // no adjustment + + // With fractions, no time zone + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44.4308", DateTime::DateFormat::Rfc3339, true, &zuluOffsetSeconds) + .ToString(), + "2023-11-22T18:33:44.4308Z"); // Local time adjustment, no effect + + EXPECT_EQ( + DateTimeExtensions::Parse( + "2023-11-22T18:33:44.4308", DateTime::DateFormat::Rfc3339, false, &zuluOffsetSeconds) + .ToString(), + "2023-11-22T18:33:44.4308Z"); // no adjustment + } +} diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index a024f17a03..aeb06d8c9a 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -8,6 +8,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 ## 1.6.0 (2023-11-10) diff --git a/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp b/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp index b7b563619f..0bd0837ab0 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp +++ b/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 const* GetLocalTimeToUtcDiffSeconds() const { return nullptr; } }; }} // namespace Azure::Identity diff --git a/sdk/identity/azure-identity/src/azure_cli_credential.cpp b/sdk/identity/azure-identity/src/azure_cli_credential.cpp index 911fee6833..7f7112d405 100644 --- a/sdk/identity/azure-identity/src/azure_cli_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_cli_credential.cpp @@ -154,7 +154,12 @@ AccessToken AzureCliCredential::GetToken( try { return TokenCredentialImpl::ParseToken( - azCliResult, "accessToken", "expiresIn", "expiresOn"); + azCliResult, + "accessToken", + "expiresIn", + "expiresOn", + true, + GetLocalTimeToUtcDiffSeconds()); } catch (json::exception const&) { diff --git a/sdk/identity/azure-identity/src/private/token_credential_impl.hpp b/sdk/identity/azure-identity/src/private/token_credential_impl.hpp index 4f8fa04cf7..86f91cc6f5 100644 --- a/sdk/identity/azure-identity/src/private/token_credential_impl.hpp +++ b/sdk/identity/azure-identity/src/private/token_credential_impl.hpp @@ -70,6 +70,10 @@ namespace Azure { namespace Identity { namespace _detail { * @param expiresOnPropertyName Name of a property in the JSON object that represents token * expiration as absolute date-time stamp. Can be empty, in which case no attempt to parse it is * made. + * @param rfc33339NoTimeZoneMeansLocal If an RFC3339 datetime has no time zone information, + * treat it as local time. + * @param localTimeToUtcDiffSeconds Assume this offset in seconds between local time and UTC + * time, for unit test reproduceability. Detect current time zone if `nullptr`. * * @return A successfully parsed access token. * @@ -79,7 +83,9 @@ 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, + bool rfc33339NoTimeZoneMeansLocal = false, + int const* localTimeToUtcDiffSeconds = nullptr); /** * @brief Holds `#Azure::Core::Http::Request` and all the associated resources for the HTTP diff --git a/sdk/identity/azure-identity/src/token_credential_impl.cpp b/sdk/identity/azure-identity/src/token_credential_impl.cpp index aeb8b1397d..4389a07d6c 100644 --- a/sdk/identity/azure-identity/src/token_credential_impl.cpp +++ b/sdk/identity/azure-identity/src/token_credential_impl.cpp @@ -196,7 +196,9 @@ AccessToken TokenCredentialImpl::ParseToken( std::string const& jsonString, std::string const& accessTokenPropertyName, std::string const& expiresInPropertyName, - std::string const& expiresOnPropertyName) + std::string const& expiresOnPropertyName, + bool rfc33339NoTimeZoneMeansLocal, + int const* localTimeToUtcDiffSeconds) { json parsedJson; try @@ -311,9 +313,13 @@ AccessToken TokenCredentialImpl::ParseToken( { auto const expiresOnAsString = expiresOn.get(); for (auto const& parse : { - std::function([](auto const& s) { + std::function([&](auto const& s) { // 'expires_on' as RFC3339 date string (absolute timestamp) - return DateTime::Parse(s, DateTime::DateFormat::Rfc3339); + return Core::_internal::DateTimeExtensions::Parse( + s, + DateTime::DateFormat::Rfc3339, + rfc33339NoTimeZoneMeansLocal, + localTimeToUtcDiffSeconds); }), std::function([](auto const& s) { // 'expires_on' as numeric string (posix time representing an absolute timestamp) diff --git a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp index da4853882f..7faac1070f 100644 --- a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp +++ b/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 = -28800; // Redmond (no DST) std::string GetAzCommand(std::string const& resource, std::string const& tenantId) const override { @@ -60,6 +61,11 @@ class AzureCliTestCredential : public AzureCliCredential { return m_command; } + int const* GetLocalTimeToUtcDiffSeconds() const override + { + return &m_localTimeToUtcDiffSeconds; + } + public: explicit AzureCliTestCredential(std::string command) : m_command(std::move(command)) {} @@ -107,7 +113,7 @@ TEST(AzureCliCredential, NotAvailable) EXPECT_EQ( token.ExpiresOn, - DateTime::Parse("2022-08-24T00:43:08.000000Z", DateTime::DateFormat::Rfc3339)); + DateTime::Parse("2022-08-24T08:43:08.000000Z", DateTime::DateFormat::Rfc3339)); #else // UWP // The credential should throw during GetToken() and not during construction, because it allows // customers to put it into ChainedTokenCredential and successfully use it there without writing @@ -199,7 +205,7 @@ TEST(AzureCliCredential, BigToken) EXPECT_EQ( token.ExpiresOn, - DateTime::Parse("2022-08-24T00:43:08.000000Z", DateTime::DateFormat::Rfc3339)); + DateTime::Parse("2022-08-24T08:43:08.000000Z", DateTime::DateFormat::Rfc3339)); } TEST(AzureCliCredential, ExpiresIn) @@ -567,7 +573,7 @@ TEST(AzureCliCredential, StrictIso8601TimeFormat) EXPECT_EQ( token.ExpiresOn, - DateTime::Parse("2022-08-24T00:43:08.000000Z", DateTime::DateFormat::Rfc3339)); + DateTime::Parse("2022-08-24T08:43:08.000000Z", DateTime::DateFormat::Rfc3339)); } TEST(AzureCliCredential, Diagnosability) From 1b126bbebd93e768b30f551923ae7221a0d6e04b Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk Date: Wed, 22 Nov 2023 21:06:38 -0800 Subject: [PATCH 2/8] Fix spelling --- sdk/core/azure-core/inc/azure/core/datetime.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core/inc/azure/core/datetime.hpp b/sdk/core/azure-core/inc/azure/core/datetime.hpp index 9d9ec6f139..058aace087 100644 --- a/sdk/core/azure-core/inc/azure/core/datetime.hpp +++ b/sdk/core/azure-core/inc/azure/core/datetime.hpp @@ -342,7 +342,7 @@ namespace Core { namespace _internal { }; /** - * @brief Provides addtional methods for #Azure::DateTime. + * @brief Provides additional methods for #Azure::DateTime. * */ class DateTimeExtensions final { From 293acce2d96ef400c53f63ff300e222548c979e5 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk Date: Wed, 22 Nov 2023 21:28:39 -0800 Subject: [PATCH 3/8] CLang-format --- .../azure-core/inc/azure/core/datetime.hpp | 166 +++++++++--------- .../test/ut/azure_cli_credential_test.cpp | 5 +- 2 files changed, 84 insertions(+), 87 deletions(-) diff --git a/sdk/core/azure-core/inc/azure/core/datetime.hpp b/sdk/core/azure-core/inc/azure/core/datetime.hpp index 058aace087..9dbd14e557 100644 --- a/sdk/core/azure-core/inc/azure/core/datetime.hpp +++ b/sdk/core/azure-core/inc/azure/core/datetime.hpp @@ -44,7 +44,7 @@ namespace _detail { } // namespace _detail namespace Core { namespace _internal { - class DateTimeExtensions; + class DateTimeExtensions; }} // namespace Core::_internal /** @@ -297,95 +297,95 @@ inline bool operator>=(std::chrono::system_clock::time_point const& tp, DateTime } namespace Core { namespace _internal { + /** + * @brief Provides convertion methods for POSIX time to an #Azure::DateTime. + * + */ + class PosixTimeConverter final { + public: + /** + * @brief Converts POSIX time to an #Azure::DateTime. + * + * @param posixTime The number of seconds since 1970. + * @return Calculated #Azure::DateTime. + */ + static DateTime PosixTimeToDateTime(int64_t posixTime) + { + return {DateTime(1970) + std::chrono::seconds(posixTime)}; + } + /** - * @brief Provides convertion methods for POSIX time to an #Azure::DateTime. + * @brief Converts a DateTime to POSIX time. * + * @param dateTime The `%DateTime` to convert. + * @return The number of seconds since 1970. */ - class PosixTimeConverter final { - public: - /** - * @brief Converts POSIX time to an #Azure::DateTime. - * - * @param posixTime The number of seconds since 1970. - * @return Calculated #Azure::DateTime. - */ - static DateTime PosixTimeToDateTime(int64_t posixTime) - { - return {DateTime(1970) + std::chrono::seconds(posixTime)}; - } - - /** - * @brief Converts a DateTime to POSIX time. - * - * @param dateTime The `%DateTime` to convert. - * @return The number of seconds since 1970. - */ - static int64_t DateTimeToPosixTime(DateTime const& dateTime) - { - // This count starts at the POSIX epoch which is January 1st, 1970 UTC. - return std::chrono::duration_cast(dateTime - DateTime(1970)).count(); - } - - private: - /** - * @brief An instance of `%PosixTimeConverter` class cannot be created. - * - */ - PosixTimeConverter() = delete; - - /** - * @brief An instance of `%PosixTimeConverter` class cannot be destructed, because no instance - * can be created. - * - */ - ~PosixTimeConverter() = delete; - }; + static int64_t DateTimeToPosixTime(DateTime const& dateTime) + { + // This count starts at the POSIX epoch which is January 1st, 1970 UTC. + return std::chrono::duration_cast(dateTime - DateTime(1970)).count(); + } + private: /** - * @brief Provides additional methods for #Azure::DateTime. + * @brief An instance of `%PosixTimeConverter` class cannot be created. * */ - class DateTimeExtensions final { - public: - /** - * @brief Create #Azure::DateTime from a string representing time in UTC in the specified - * format. - * - * @param dateTime A string with the date and time. - * @param format A format to which \p dateTime string adheres to. - * @param rfc3339NoTimeZoneMeansLocal If time zone is not specified, treat the \p dateTime as - * local time. - * @param localTimeToUtcDiffSeconds Assume this offset in seconds between local time and UTC - * time, for unit test reproduceability. Detect current time zone if `nullptr`. - * - * @return #Azure::DateTime that was constructed from the \p dateTime string. - * - * @throw std::invalid_argument If \p format is not recognized, or if parsing error. - */ - static DateTime Parse( - std::string const& dateTime, - DateTime::DateFormat format, - bool rfc3339NoTimeZoneMeansLocal, - int const* localTimeToUtcDiffSeconds) - { - return DateTime::Parse( - dateTime, format, rfc3339NoTimeZoneMeansLocal, localTimeToUtcDiffSeconds); - } - - private: - /** - * @brief An instance of `%DateTimeExtensions` class cannot be created. - * - */ - DateTimeExtensions() = delete; - - /** - * @brief An instance of `%DateTimeExtensions` class cannot be destructed, because no instance - * can be created. - * - */ - ~DateTimeExtensions() = delete; - }; + PosixTimeConverter() = delete; + + /** + * @brief An instance of `%PosixTimeConverter` class cannot be destructed, because no instance + * can be created. + * + */ + ~PosixTimeConverter() = delete; + }; + + /** + * @brief Provides additional methods for #Azure::DateTime. + * + */ + class DateTimeExtensions final { + public: + /** + * @brief Create #Azure::DateTime from a string representing time in UTC in the specified + * format. + * + * @param dateTime A string with the date and time. + * @param format A format to which \p dateTime string adheres to. + * @param rfc3339NoTimeZoneMeansLocal If time zone is not specified, treat the \p dateTime as + * local time. + * @param localTimeToUtcDiffSeconds Assume this offset in seconds between local time and UTC + * time, for unit test reproduceability. Detect current time zone if `nullptr`. + * + * @return #Azure::DateTime that was constructed from the \p dateTime string. + * + * @throw std::invalid_argument If \p format is not recognized, or if parsing error. + */ + static DateTime Parse( + std::string const& dateTime, + DateTime::DateFormat format, + bool rfc3339NoTimeZoneMeansLocal, + int const* localTimeToUtcDiffSeconds) + { + return DateTime::Parse( + dateTime, format, rfc3339NoTimeZoneMeansLocal, localTimeToUtcDiffSeconds); + } + + private: + /** + * @brief An instance of `%DateTimeExtensions` class cannot be created. + * + */ + DateTimeExtensions() = delete; + + /** + * @brief An instance of `%DateTimeExtensions` class cannot be destructed, because no instance + * can be created. + * + */ + ~DateTimeExtensions() = delete; + }; }} // namespace Core::_internal } // namespace Azure diff --git a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp index 7faac1070f..b508ce696b 100644 --- a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp @@ -61,10 +61,7 @@ class AzureCliTestCredential : public AzureCliCredential { return m_command; } - int const* GetLocalTimeToUtcDiffSeconds() const override - { - return &m_localTimeToUtcDiffSeconds; - } + int const* GetLocalTimeToUtcDiffSeconds() const override { return &m_localTimeToUtcDiffSeconds; } public: explicit AzureCliTestCredential(std::string command) : m_command(std::move(command)) {} From bcdf95844c190e49ade8b483168005853e8dd351 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk Date: Thu, 7 Dec 2023 16:26:46 -0800 Subject: [PATCH 4/8] Parse as local by appending TimeZoneOffsetAsString() --- sdk/core/azure-core/CHANGELOG.md | 1 - .../azure-core/inc/azure/core/datetime.hpp | 64 +------ sdk/core/azure-core/src/datetime.cpp | 41 +---- sdk/core/azure-core/test/ut/datetime_test.cpp | 161 ------------------ .../azure/identity/azure_cli_credential.hpp | 2 +- .../src/azure_cli_credential.cpp | 28 ++- .../src/private/token_credential_impl.hpp | 12 +- .../src/token_credential_impl.cpp | 37 +++- .../test/ut/azure_cli_credential_test.cpp | 46 ++++- 9 files changed, 102 insertions(+), 290 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 3cd1cc2f5f..c5b9b56ff0 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -14,7 +14,6 @@ ### Other Changes - [[#4756]] (https://github.com/Azure/azure-sdk-for-cpp/issues/4756) `BearerTokenAuthenticationPolicy` now uses shared mutex lock for read operations. -- Added internal provisions for extended `Azure::DateTime` parsing. ## 1.11.0-beta.2 (2023-11-02) diff --git a/sdk/core/azure-core/inc/azure/core/datetime.hpp b/sdk/core/azure-core/inc/azure/core/datetime.hpp index 9dbd14e557..964b4cfaa2 100644 --- a/sdk/core/azure-core/inc/azure/core/datetime.hpp +++ b/sdk/core/azure-core/inc/azure/core/datetime.hpp @@ -43,10 +43,6 @@ namespace _detail { }; } // namespace _detail -namespace Core { namespace _internal { - class DateTimeExtensions; -}} // namespace Core::_internal - /** * @brief Manages date and time in standardized string formats. * @details Supports date range from year 0001 to end of year 9999 with 100ns (7 decimal places @@ -56,7 +52,6 @@ namespace Core { namespace _internal { * @remark This class is supposed to be able to handle a DateTime that comes over the wire. */ class DateTime final : public _detail::Clock::time_point { - friend class Core::_internal::DateTimeExtensions; private: AZ_CORE_DLLEXPORT static DateTime const SystemClockEpoch; @@ -173,14 +168,6 @@ class DateTime final : public _detail::Clock::time_point { Rfc3339, }; -private: - static DateTime Parse( - std::string const& dateTime, - DateFormat format, - bool rfc3339NoTimeZoneMeansLocal, - int const* localTimeToUtcDiffSeconds); - -public: /** * @brief Create #Azure::DateTime from a string representing time in UTC in the specified * format. @@ -192,10 +179,7 @@ class DateTime final : public _detail::Clock::time_point { * * @throw std::invalid_argument If \p format is not recognized, or if parsing error. */ - static DateTime Parse(std::string const& dateTime, DateFormat format) - { - return Parse(dateTime, format, false, nullptr); - } + static DateTime Parse(std::string const& dateTime, DateFormat format); /** * @brief Get a string representation of the #Azure::DateTime. @@ -340,52 +324,6 @@ namespace Core { namespace _internal { */ ~PosixTimeConverter() = delete; }; - - /** - * @brief Provides additional methods for #Azure::DateTime. - * - */ - class DateTimeExtensions final { - public: - /** - * @brief Create #Azure::DateTime from a string representing time in UTC in the specified - * format. - * - * @param dateTime A string with the date and time. - * @param format A format to which \p dateTime string adheres to. - * @param rfc3339NoTimeZoneMeansLocal If time zone is not specified, treat the \p dateTime as - * local time. - * @param localTimeToUtcDiffSeconds Assume this offset in seconds between local time and UTC - * time, for unit test reproduceability. Detect current time zone if `nullptr`. - * - * @return #Azure::DateTime that was constructed from the \p dateTime string. - * - * @throw std::invalid_argument If \p format is not recognized, or if parsing error. - */ - static DateTime Parse( - std::string const& dateTime, - DateTime::DateFormat format, - bool rfc3339NoTimeZoneMeansLocal, - int const* localTimeToUtcDiffSeconds) - { - return DateTime::Parse( - dateTime, format, rfc3339NoTimeZoneMeansLocal, localTimeToUtcDiffSeconds); - } - - private: - /** - * @brief An instance of `%DateTimeExtensions` class cannot be created. - * - */ - DateTimeExtensions() = delete; - - /** - * @brief An instance of `%DateTimeExtensions` class cannot be destructed, because no instance - * can be created. - * - */ - ~DateTimeExtensions() = delete; - }; }} // namespace Core::_internal } // namespace Azure diff --git a/sdk/core/azure-core/src/datetime.cpp b/sdk/core/azure-core/src/datetime.cpp index 51e163ab31..2946fa3ce1 100644 --- a/sdk/core/azure-core/src/datetime.cpp +++ b/sdk/core/azure-core/src/datetime.cpp @@ -447,11 +447,7 @@ DateTime::operator std::chrono::system_clock::time_point() const + std::chrono::duration_cast(*this - SystemClockEpoch); } -DateTime DateTime::Parse( - std::string const& dateTime, - DateFormat format, - bool rfc3339NoTimeZoneMeansLocal, - int const* localTimeToUtcDiffSeconds) +DateTime DateTime::Parse(std::string const& dateTime, DateFormat format) { // The values that are not supposed to be read before they are written are set to -123... to avoid // warnings on some compilers, yet provide a clearly bad value to make it obvious if things don't @@ -468,7 +464,6 @@ DateTime DateTime::Parse( int8_t localDiffHours = 0; int8_t localDiffMinutes = 0; bool roundFracSecUp = false; - bool adjustAsLocal = false; { std::string::size_type const DateTimeLength = dateTime.length(); std::string::size_type minDateTimeLength = 0; @@ -726,16 +721,6 @@ DateTime DateTime::Parse( &cursor, dateTime, DateTimeLength, parsingArea, 2, 2); } } - else if (rfc3339NoTimeZoneMeansLocal) // Fractional seconds were present, but not the time - // zone - { - adjustAsLocal = true; - } - } - else if ( // No fractional seconds and no time zone were present - rfc3339NoTimeZoneMeansLocal && cursor == DateTimeLength) - { - adjustAsLocal = true; } } } @@ -744,30 +729,6 @@ DateTime DateTime::Parse( throw std::invalid_argument("Unrecognized date format."); } - if (adjustAsLocal) - { - auto const timeTNow = std::chrono::system_clock::to_time_t(std::chrono::system_clock::now()); - -#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 utcDiffSeconds = localTimeToUtcDiffSeconds != nullptr - ? *localTimeToUtcDiffSeconds - : static_cast(std::difftime( - std::mktime(std::localtime(&timeTNow)), std::mktime(std::gmtime(&timeTNow)))); - // LCOV_EXCL_STOP -#ifdef _MSC_VER -#pragma warning(pop) -#endif - - localDiffHours = static_cast(utcDiffSeconds / (60 * 60)); - localDiffMinutes = static_cast((utcDiffSeconds % (60 * 60)) / 60); - } - return DateTime( year, month, diff --git a/sdk/core/azure-core/test/ut/datetime_test.cpp b/sdk/core/azure-core/test/ut/datetime_test.cpp index 4550d863b9..cb057debb3 100644 --- a/sdk/core/azure-core/test/ut/datetime_test.cpp +++ b/sdk/core/azure-core/test/ut/datetime_test.cpp @@ -902,164 +902,3 @@ TEST(DateTime, Rfc3339Space) = DateTime::Parse("2022-08-24 00:43:08.0004308Z", DateTime::DateFormat::Rfc3339); EXPECT_EQ(datetime.ToString(DateTime::DateFormat::Rfc3339), "2022-08-24T00:43:08.0004308Z"); } - -TEST(DateTime, LocalTime) -{ - using Azure::Core::_internal::DateTimeExtensions; - - { - auto const uniformOffsetSeconds = -28800; // Redmond (no DST) - - // No fractions, no time zone - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44", DateTime::DateFormat::Rfc3339, true, &uniformOffsetSeconds) - .ToString(), - "2023-11-23T02:33:44Z"); // Local time adjustment - - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44", DateTime::DateFormat::Rfc3339, false, &uniformOffsetSeconds) - .ToString(), - "2023-11-22T18:33:44Z"); // no adjustment - - // With fractions, no time zone - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44.4308", DateTime::DateFormat::Rfc3339, true, &uniformOffsetSeconds) - .ToString(), - "2023-11-23T02:33:44.4308Z"); // Local time adjustment - - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44.4308", DateTime::DateFormat::Rfc3339, false, &uniformOffsetSeconds) - .ToString(), - "2023-11-22T18:33:44.4308Z"); // no adjustment - - // No fractions, with time zone - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44-05:00", DateTime::DateFormat::Rfc3339, true, &uniformOffsetSeconds) - .ToString(), - "2023-11-22T23:33:44Z"); // no local time adjustment, just the regular time zone adjustment - - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44-05:00", - DateTime::DateFormat::Rfc3339, - false, - &uniformOffsetSeconds) - .ToString(), - "2023-11-22T23:33:44Z"); // no local time adjustment, just the regular time zone adjustment - - // With fractions, with time zone - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44.4308-05:00", - DateTime::DateFormat::Rfc3339, - true, - &uniformOffsetSeconds) - .ToString(), - "2023-11-22T23:33:44.4308Z"); // no local time adjustment, just the regular time zone - // adjustment - - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44.4308-05:00", - DateTime::DateFormat::Rfc3339, - false, - &uniformOffsetSeconds) - .ToString(), - "2023-11-22T23:33:44.4308Z"); // no local time adjustment, just the regular time zone - // adjustment - - // No fractions, Zulu time zone - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44Z", DateTime::DateFormat::Rfc3339, true, &uniformOffsetSeconds) - .ToString(), - "2023-11-22T18:33:44Z"); // no adjustment - - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44Z", DateTime::DateFormat::Rfc3339, false, &uniformOffsetSeconds) - .ToString(), - "2023-11-22T18:33:44Z"); // no adjustment - - // With fractions, Zulu time zone - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44.4308Z", DateTime::DateFormat::Rfc3339, true, &uniformOffsetSeconds) - .ToString(), - "2023-11-22T18:33:44.4308Z"); // no adjustment - - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44.4308Z", - DateTime::DateFormat::Rfc3339, - false, - &uniformOffsetSeconds) - .ToString(), - "2023-11-22T18:33:44.4308Z"); // no adjustment - } - - { - auto const bravoOffsetSeconds = 7200; // Kyiv (no DST) - - // No fractions, no time zone - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44", DateTime::DateFormat::Rfc3339, true, &bravoOffsetSeconds) - .ToString(), - "2023-11-22T16:33:44Z"); // Local time adjustment - - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44", DateTime::DateFormat::Rfc3339, false, &bravoOffsetSeconds) - .ToString(), - "2023-11-22T18:33:44Z"); // no adjustment - - // With fractions, no time zone - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44.4308", DateTime::DateFormat::Rfc3339, true, &bravoOffsetSeconds) - .ToString(), - "2023-11-22T16:33:44.4308Z"); // Local time adjustment - - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44.4308", DateTime::DateFormat::Rfc3339, false, &bravoOffsetSeconds) - .ToString(), - "2023-11-22T18:33:44.4308Z"); // no adjustment - } - - { - auto const zuluOffsetSeconds = 0; // London (no DST) - - // No fractions, no time zone - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44", DateTime::DateFormat::Rfc3339, true, &zuluOffsetSeconds) - .ToString(), - "2023-11-22T18:33:44Z"); // Local time adjustment, no effect - - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44", DateTime::DateFormat::Rfc3339, false, &zuluOffsetSeconds) - .ToString(), - "2023-11-22T18:33:44Z"); // no adjustment - - // With fractions, no time zone - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44.4308", DateTime::DateFormat::Rfc3339, true, &zuluOffsetSeconds) - .ToString(), - "2023-11-22T18:33:44.4308Z"); // Local time adjustment, no effect - - EXPECT_EQ( - DateTimeExtensions::Parse( - "2023-11-22T18:33:44.4308", DateTime::DateFormat::Rfc3339, false, &zuluOffsetSeconds) - .ToString(), - "2023-11-22T18:33:44.4308Z"); // no adjustment - } -} diff --git a/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp b/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp index 0bd0837ab0..981296692b 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp @@ -112,7 +112,7 @@ namespace Azure { namespace Identity { protected: #endif virtual std::string GetAzCommand(std::string const& scopes, std::string const& tenantId) const; - virtual int const* GetLocalTimeToUtcDiffSeconds() const { return nullptr; } + virtual int GetLocalTimeToUtcDiffSeconds() const; }; }} // namespace Azure::Identity diff --git a/sdk/identity/azure-identity/src/azure_cli_credential.cpp b/sdk/identity/azure-identity/src/azure_cli_credential.cpp index 7f7112d405..8ca250d2b1 100644 --- a/sdk/identity/azure-identity/src/azure_cli_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_cli_credential.cpp @@ -13,7 +13,9 @@ #include #include +#include #include +#include #include #include #include @@ -127,6 +129,25 @@ 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()); + + return static_cast( + 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, @@ -154,12 +175,7 @@ AccessToken AzureCliCredential::GetToken( try { return TokenCredentialImpl::ParseToken( - azCliResult, - "accessToken", - "expiresIn", - "expiresOn", - true, - GetLocalTimeToUtcDiffSeconds()); + azCliResult, "accessToken", "expiresIn", "expiresOn", GetLocalTimeToUtcDiffSeconds()); } catch (json::exception const&) { diff --git a/sdk/identity/azure-identity/src/private/token_credential_impl.hpp b/sdk/identity/azure-identity/src/private/token_credential_impl.hpp index 86f91cc6f5..2a2a087266 100644 --- a/sdk/identity/azure-identity/src/private/token_credential_impl.hpp +++ b/sdk/identity/azure-identity/src/private/token_credential_impl.hpp @@ -70,10 +70,11 @@ namespace Azure { namespace Identity { namespace _detail { * @param expiresOnPropertyName Name of a property in the JSON object that represents token * expiration as absolute date-time stamp. Can be empty, in which case no attempt to parse it is * made. - * @param rfc33339NoTimeZoneMeansLocal If an RFC3339 datetime has no time zone information, - * treat it as local time. - * @param localTimeToUtcDiffSeconds Assume this offset in seconds between local time and UTC - * time, for unit test reproduceability. Detect current time zone if `nullptr`. + * @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. * @@ -84,8 +85,7 @@ namespace Azure { namespace Identity { namespace _detail { std::string const& accessTokenPropertyName, std::string const& expiresInPropertyName, std::string const& expiresOnPropertyName, - bool rfc33339NoTimeZoneMeansLocal = false, - int const* localTimeToUtcDiffSeconds = nullptr); + int utcDiffSeconds = 0); /** * @brief Holds `#Azure::Core::Http::Request` and all the associated resources for the HTTP diff --git a/sdk/identity/azure-identity/src/token_credential_impl.cpp b/sdk/identity/azure-identity/src/token_credential_impl.cpp index 254e0311ec..e9695a722e 100644 --- a/sdk/identity/azure-identity/src/token_credential_impl.cpp +++ b/sdk/identity/azure-identity/src/token_credential_impl.cpp @@ -11,8 +11,10 @@ #include #include +#include #include #include +#include using Azure::Identity::_detail::TokenCredentialImpl; @@ -192,6 +194,31 @@ 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( @@ -199,8 +226,7 @@ AccessToken TokenCredentialImpl::ParseToken( std::string const& accessTokenPropertyName, std::string const& expiresInPropertyName, std::string const& expiresOnPropertyName, - bool rfc33339NoTimeZoneMeansLocal, - int const* localTimeToUtcDiffSeconds) + int utcDiffSeconds) { json parsedJson; try @@ -311,17 +337,14 @@ AccessToken TokenCredentialImpl::ParseToken( } } + auto const tzOffsetStr = TimeZoneOffsetAsString(utcDiffSeconds); if (expiresOn.is_string()) { auto const expiresOnAsString = expiresOn.get(); for (auto const& parse : { std::function([&](auto const& s) { // 'expires_on' as RFC3339 date string (absolute timestamp) - return Core::_internal::DateTimeExtensions::Parse( - s, - DateTime::DateFormat::Rfc3339, - rfc33339NoTimeZoneMeansLocal, - localTimeToUtcDiffSeconds); + return DateTime::Parse(s + tzOffsetStr, DateTime::DateFormat::Rfc3339); }), std::function([](auto const& s) { // 'expires_on' as numeric string (posix time representing an absolute timestamp) diff --git a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp index b508ce696b..1911b28ebd 100644 --- a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp @@ -51,7 +51,7 @@ std::string EchoCommand(std::string const text) class AzureCliTestCredential : public AzureCliCredential { private: std::string m_command; - int m_localTimeToUtcDiffSeconds = -28800; // Redmond (no DST) + int m_localTimeToUtcDiffSeconds = 0; std::string GetAzCommand(std::string const& resource, std::string const& tenantId) const override { @@ -61,7 +61,7 @@ class AzureCliTestCredential : public AzureCliCredential { return m_command; } - int const* GetLocalTimeToUtcDiffSeconds() const override { return &m_localTimeToUtcDiffSeconds; } + int GetLocalTimeToUtcDiffSeconds() const override { return m_localTimeToUtcDiffSeconds; } public: explicit AzureCliTestCredential(std::string command) : m_command(std::move(command)) {} @@ -83,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 @@ -110,7 +112,7 @@ TEST(AzureCliCredential, NotAvailable) EXPECT_EQ( token.ExpiresOn, - DateTime::Parse("2022-08-24T08:43:08.000000Z", DateTime::DateFormat::Rfc3339)); + DateTime::Parse("2022-08-24T00:43:08.000000Z", DateTime::DateFormat::Rfc3339)); #else // UWP // The credential should throw during GetToken() and not during construction, because it allows // customers to put it into ChainedTokenCredential and successfully use it there without writing @@ -202,7 +204,7 @@ TEST(AzureCliCredential, BigToken) EXPECT_EQ( token.ExpiresOn, - DateTime::Parse("2022-08-24T08:43:08.000000Z", DateTime::DateFormat::Rfc3339)); + DateTime::Parse("2022-08-24T00:43:08.000000Z", DateTime::DateFormat::Rfc3339)); } TEST(AzureCliCredential, ExpiresIn) @@ -570,7 +572,41 @@ TEST(AzureCliCredential, StrictIso8601TimeFormat) EXPECT_EQ( token.ExpiresOn, - DateTime::Parse("2022-08-24T08:43:08.000000Z", DateTime::DateFormat::Rfc3339)); + 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) From f5dc1aedfd457d7d8e6eca7a15a7a0349a12c095 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk Date: Thu, 14 Dec 2023 17:46:18 -0800 Subject: [PATCH 5/8] Fix bad merge --- sdk/identity/azure-identity/src/token_credential_impl.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/identity/azure-identity/src/token_credential_impl.cpp b/sdk/identity/azure-identity/src/token_credential_impl.cpp index a4e82858eb..dc15dafaf8 100644 --- a/sdk/identity/azure-identity/src/token_credential_impl.cpp +++ b/sdk/identity/azure-identity/src/token_credential_impl.cpp @@ -332,6 +332,8 @@ AccessToken TokenCredentialImpl::ParseToken( { if (parsedJson.contains(expiresOnPropertyName)) { + auto const& expiresOn = parsedJson[expiresOnPropertyName]; + if (expiresOn.is_number_unsigned()) { try From a7b319e30df0f489fb60b4a30352c9e5ea404a41 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk Date: Thu, 14 Dec 2023 17:54:50 -0800 Subject: [PATCH 6/8] PR Feedback --- sdk/identity/azure-identity/src/azure_cli_credential.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/identity/azure-identity/src/azure_cli_credential.cpp b/sdk/identity/azure-identity/src/azure_cli_credential.cpp index e6ffa0c48d..43bb47de66 100644 --- a/sdk/identity/azure-identity/src/azure_cli_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_cli_credential.cpp @@ -140,6 +140,8 @@ int AzureCliCredential::GetLocalTimeToUtcDiffSeconds() const // 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( std::difftime(std::mktime(std::localtime(&timeTNow)), std::mktime(std::gmtime(&timeTNow)))); // LCOV_EXCL_STOP From d3b252a7e639f523a13d857579d52cd735d3518a Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Thu, 14 Dec 2023 17:56:40 -0800 Subject: [PATCH 7/8] Merge --- .../azure-identity/src/private/token_credential_impl.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/identity/azure-identity/src/private/token_credential_impl.hpp b/sdk/identity/azure-identity/src/private/token_credential_impl.hpp index 361607aa00..ef0f2b29f6 100644 --- a/sdk/identity/azure-identity/src/private/token_credential_impl.hpp +++ b/sdk/identity/azure-identity/src/private/token_credential_impl.hpp @@ -87,7 +87,7 @@ namespace Azure { namespace Identity { namespace _detail { std::string const& jsonString, std::string const& accessTokenPropertyName, std::string const& expiresInPropertyName, - std::vector const& expiresOnPropertyNames + std::vector const& expiresOnPropertyNames, int utcDiffSeconds); /** From c9b5f305124f78ffe95585f1831b8861879f9792 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk Date: Thu, 14 Dec 2023 18:54:35 -0800 Subject: [PATCH 8/8] Merge --- .../azure-identity/src/private/token_credential_impl.hpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sdk/identity/azure-identity/src/private/token_credential_impl.hpp b/sdk/identity/azure-identity/src/private/token_credential_impl.hpp index ef0f2b29f6..b51a1fc053 100644 --- a/sdk/identity/azure-identity/src/private/token_credential_impl.hpp +++ b/sdk/identity/azure-identity/src/private/token_credential_impl.hpp @@ -70,8 +70,8 @@ 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 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 + * @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. @@ -88,7 +88,7 @@ namespace Azure { namespace Identity { namespace _detail { std::string const& accessTokenPropertyName, std::string const& expiresInPropertyName, std::vector const& expiresOnPropertyNames, - int utcDiffSeconds); + int utcDiffSeconds = 0); /** * @brief Parses JSON that contains access token and its expiration. @@ -116,8 +116,7 @@ namespace Azure { namespace Identity { namespace _detail { jsonString, accessTokenPropertyName, expiresInPropertyName, - std::vector{expiresOnPropertyName}, - 0); + std::vector{expiresOnPropertyName}); } /**