From f49912ee6b0e1f8c179377b2d7c7d46c9b96a9c0 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 8 Feb 2024 11:37:42 -0800 Subject: [PATCH 1/8] Classes marked as final should not have virtual methods. (cherry picked from commit 7413384c111b73be448f78527a08a544fd1f6ec6) --- .../inc/azure/identity/azure_cli_credential.hpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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 981296692b..392f8ae025 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,14 @@ namespace Azure { namespace Identity { protected: #endif virtual std::string GetAzCommand(std::string const& scopes, std::string const& tenantId) const; - virtual int GetLocalTimeToUtcDiffSeconds() const; + +#if !defined(TESTING_BUILD) + private: +#else + protected: + virtual +#endif + int GetLocalTimeToUtcDiffSeconds() const; }; }} // namespace Azure::Identity From 49cef9d785b613e7e5e9ed8d7749108c1521ae7c Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 8 Feb 2024 12:02:21 -0800 Subject: [PATCH 2/8] Update changelog. --- sdk/identity/azure-identity/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index 88e6777fb8..fa914caed9 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -10,6 +10,8 @@ ### Other Changes +- Classes marked as final no longer have virtual methods. + ## 1.7.0-beta.1 (2024-01-11) ### Features Added From a3b35f801de761483b7e6382f0d8fd047aafa8ba Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 8 Feb 2024 17:40:32 -0800 Subject: [PATCH 3/8] Address PR feedback and use named-macro. --- sdk/identity/azure-identity/CHANGELOG.md | 2 -- .../azure/identity/azure_cli_credential.hpp | 26 ++++++------------- .../inc/azure/identity/dll_import_export.hpp | 8 ++++++ 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index fa914caed9..88e6777fb8 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -10,8 +10,6 @@ ### Other Changes -- Classes marked as final no longer have virtual methods. - ## 1.7.0-beta.1 (2024-01-11) ### Features Added 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 392f8ae025..54428574fa 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 @@ -9,6 +9,7 @@ #pragma once #include "azure/identity/detail/token_cache.hpp" +#include "azure/identity/dll_import_export.hpp" #include #include @@ -22,7 +23,8 @@ namespace Azure { namespace Identity { /** * @brief Options for configuring the #Azure::Identity::AzureCliCredential. */ - struct AzureCliCredentialOptions final : public Core::Credentials::TokenCredentialOptions + struct AzureCliCredentialOptions _azure_NON_FINAL_FOR_TESTS + : public Core::Credentials::TokenCredentialOptions { /** * @brief The ID of the tenant to which the credential will authenticate by default. If not @@ -49,11 +51,7 @@ namespace Azure { namespace Identity { * @brief Enables authentication to Microsoft Entra ID using Azure CLI to obtain an access * token. */ - class AzureCliCredential -#if !defined(TESTING_BUILD) - final -#endif - : public Core::Credentials::TokenCredential { + class AzureCliCredential _azure_NON_FINAL_FOR_TESTS : public Core::Credentials::TokenCredential { protected: /** @brief The cache for the access token. */ _detail::TokenCache m_tokenCache; @@ -106,20 +104,12 @@ namespace Azure { namespace Identity { Core::Credentials::TokenRequestContext const& tokenRequestContext, Core::Context const& context) const override; -#if !defined(TESTING_BUILD) private: -#else - protected: -#endif - virtual std::string GetAzCommand(std::string const& scopes, std::string const& tenantId) const; + _azure_VIRTUAL_FOR_TESTS std::string GetAzCommand( + std::string const& scopes, + std::string const& tenantId) const; -#if !defined(TESTING_BUILD) - private: -#else - protected: - virtual -#endif - int GetLocalTimeToUtcDiffSeconds() const; + _azure_VIRTUAL_FOR_TESTS int GetLocalTimeToUtcDiffSeconds() const; }; }} // namespace Azure::Identity diff --git a/sdk/identity/azure-identity/inc/azure/identity/dll_import_export.hpp b/sdk/identity/azure-identity/inc/azure/identity/dll_import_export.hpp index 6b01515fb1..ce74ac15a8 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/dll_import_export.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/dll_import_export.hpp @@ -38,6 +38,14 @@ #undef AZ_IDENTITY_BUILT_AS_DLL +#if defined(_azure_TESTING_BUILD) +#define _azure_NON_FINAL_FOR_TESTS +#define _azure_VIRTUAL_FOR_TESTS virtual +#else +#define _azure_NON_FINAL_FOR_TESTS final +#define _azure_VIRTUAL_FOR_TESTS +#endif + /** * @brief Azure SDK abstractions. * From 39d2333d6a2de5c1f2fdb9587c161bf990aea5e9 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 8 Feb 2024 17:42:41 -0800 Subject: [PATCH 4/8] Only use the macro for testing_build final classes. --- .../azure-identity/inc/azure/identity/azure_cli_credential.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 54428574fa..ff5b03dc75 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 @@ -23,8 +23,7 @@ namespace Azure { namespace Identity { /** * @brief Options for configuring the #Azure::Identity::AzureCliCredential. */ - struct AzureCliCredentialOptions _azure_NON_FINAL_FOR_TESTS - : public Core::Credentials::TokenCredentialOptions + struct AzureCliCredentialOptions final : public Core::Credentials::TokenCredentialOptions { /** * @brief The ID of the tenant to which the credential will authenticate by default. If not From a7de704a96a8ead3fe7983af785a53d6db470c28 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 8 Feb 2024 17:44:49 -0800 Subject: [PATCH 5/8] Add a new compile definition _azure_TESTING_BUILD. --- sdk/identity/azure-identity/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/identity/azure-identity/CMakeLists.txt b/sdk/identity/azure-identity/CMakeLists.txt index a53226b396..ff07c001ab 100644 --- a/sdk/identity/azure-identity/CMakeLists.txt +++ b/sdk/identity/azure-identity/CMakeLists.txt @@ -127,6 +127,7 @@ az_rtti_setup( if(BUILD_TESTING) # define a symbol that enables some test hooks in code add_compile_definitions(TESTING_BUILD) + add_compile_definitions(_azure_TESTING_BUILD) # tests if (NOT AZ_ALL_LIBRARIES OR FETCH_SOURCE_DEPS) From d9a933369971bffbe68819fee922a788f2c74b8e Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 8 Feb 2024 18:15:15 -0800 Subject: [PATCH 6/8] Add test as a friend class and put the test in a Identity specific test namespace. --- .../azure/identity/azure_cli_credential.hpp | 11 + .../test/ut/azure_cli_credential_test.cpp | 1024 +++++++++-------- 2 files changed, 526 insertions(+), 509 deletions(-) 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 ff5b03dc75..00f0defc39 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 @@ -19,6 +19,12 @@ #include #include +#if defined(_azure_TESTING_BUILD) +namespace Azure { namespace Identity { namespace Test { + class AzureCliTestCredential; +}}} // namespace Azure::Identity::Test +#endif + namespace Azure { namespace Identity { /** * @brief Options for configuring the #Azure::Identity::AzureCliCredential. @@ -51,6 +57,11 @@ namespace Azure { namespace Identity { * token. */ class AzureCliCredential _azure_NON_FINAL_FOR_TESTS : public Core::Credentials::TokenCredential { + +#if defined(_azure_TESTING_BUILD) + friend class Azure::Identity::Test::AzureCliTestCredential; +#endif + protected: /** @brief The cache for the access token. */ _detail::TokenCache m_tokenCache; 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 cd7994a6de..0ff9087b79 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 @@ -22,617 +22,602 @@ using Azure::Core::Credentials::TokenCredentialOptions; using Azure::Core::Credentials::TokenRequestContext; using Azure::Identity::AzureCliCredentialOptions; -namespace { -constexpr auto InfiniteCommand = +namespace Azure { namespace Identity { namespace Test { + constexpr auto InfiniteCommand = #if defined(AZ_PLATFORM_WINDOWS) - "for /l %q in (0) do timeout 10"; + "for /l %q in (0) do timeout 10"; #else - "while true; do sleep 10; done" + "while true; do sleep 10; done" #endif -; + ; -constexpr auto EmptyOutputCommand = + constexpr auto EmptyOutputCommand = #if defined(AZ_PLATFORM_WINDOWS) - "rem"; + "rem"; #else - "clear" + "clear" #endif -; + ; -std::string EchoCommand(std::string const text) -{ + std::string EchoCommand(std::string const text) + { #if defined(AZ_PLATFORM_WINDOWS) - return std::string("echo ") + text; + return std::string("echo ") + text; #else - return std::string("echo \'") + text + "\'"; + return std::string("echo \'") + text + "\'"; #endif -} + } -class AzureCliTestCredential : public AzureCliCredential { -private: - std::string m_command; - int m_localTimeToUtcDiffSeconds = 0; + 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 - { - static_cast(resource); - static_cast(tenantId); + std::string GetAzCommand(std::string const& resource, std::string const& tenantId) + const override + { + static_cast(resource); + static_cast(tenantId); - return m_command; - } + return m_command; + } - int GetLocalTimeToUtcDiffSeconds() const override { return m_localTimeToUtcDiffSeconds; } + int GetLocalTimeToUtcDiffSeconds() const override { return m_localTimeToUtcDiffSeconds; } -public: - explicit AzureCliTestCredential(std::string command) : m_command(std::move(command)) {} + public: + explicit AzureCliTestCredential(std::string command) : m_command(std::move(command)) {} - explicit AzureCliTestCredential(std::string command, AzureCliCredentialOptions const& options) - : AzureCliCredential(options), m_command(std::move(command)) - { - } + explicit AzureCliTestCredential(std::string command, AzureCliCredentialOptions const& options) + : AzureCliCredential(options), m_command(std::move(command)) + { + } - explicit AzureCliTestCredential(std::string command, TokenCredentialOptions const& options) - : AzureCliCredential(options), m_command(std::move(command)) - { - } + explicit AzureCliTestCredential(std::string command, TokenCredentialOptions const& options) + : AzureCliCredential(options), m_command(std::move(command)) + { + } - std::string GetOriginalAzCommand(std::string const& resource, std::string const& tenantId) const - { - return AzureCliCredential::GetAzCommand(resource, tenantId); - } + std::string GetOriginalAzCommand(std::string const& resource, std::string const& tenantId) const + { + return AzureCliCredential::GetAzCommand(resource, tenantId); + } - decltype(m_tenantId) const& GetTenantId() const { return m_tenantId; } - decltype(m_cliProcessTimeout) const& GetCliProcessTimeout() const { return m_cliProcessTimeout; } + 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 + void SetLocalTimeToUtcDiffSeconds(int diff) { m_localTimeToUtcDiffSeconds = diff; } + }; #if !defined(AZ_PLATFORM_WINDOWS) \ || (!defined(WINAPI_PARTITION_DESKTOP) || WINAPI_PARTITION_DESKTOP) // not UWP -TEST(AzureCliCredential, Success) + TEST(AzureCliCredential, Success) #else -TEST(AzureCliCredential, NotAvailable) + TEST(AzureCliCredential, NotAvailable) #endif -{ - constexpr auto Token = "{\"accessToken\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\"," - "\"expiresOn\":\"2022-08-24 00:43:08.000000\"," - "\"tenant\":\"72f988bf-86f1-41af-91ab-2d7cd011db47\"," - "\"tokenType\":\"Bearer\"}"; + { + constexpr auto Token = "{\"accessToken\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\"," + "\"expiresOn\":\"2022-08-24 00:43:08.000000\"," + "\"tenant\":\"72f988bf-86f1-41af-91ab-2d7cd011db47\"," + "\"tokenType\":\"Bearer\"}"; - AzureCliTestCredential const azCliCred(EchoCommand(Token)); + AzureCliTestCredential const azCliCred(EchoCommand(Token)); - TokenRequestContext trc; - trc.Scopes.push_back("https://storage.azure.com/.default"); + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); #if !defined(AZ_PLATFORM_WINDOWS) \ || (!defined(WINAPI_PARTITION_DESKTOP) || WINAPI_PARTITION_DESKTOP) // not UWP - auto const token = azCliCred.GetToken(trc, {}); + auto const token = azCliCred.GetToken(trc, {}); - EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); - EXPECT_EQ( - token.ExpiresOn, - DateTime::Parse("2022-08-24T00:43:08.000000Z", DateTime::DateFormat::Rfc3339)); + EXPECT_EQ( + token.ExpiresOn, + 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 - // ifdefs for UWP. It is not too late to throw - for example, if Azure CLI is not installed, then - // the credential will also find out during GetToken() and not during construction (if we had to - // find out during the construction, we'd have to fire up some 'az' command in constructor; again, - // that would also make it hard to put the credential into ChainedTokenCredential). - EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + // 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 ifdefs for UWP. It is not too late to throw - for example, if Azure CLI is + // not installed, then the credential will also find out during GetToken() and not during + // construction (if we had to find out during the construction, we'd have to fire up some 'az' + // command in constructor; again, that would also make it hard to put the credential into + // ChainedTokenCredential). + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); #endif // UWP -} + } #if !defined(AZ_PLATFORM_WINDOWS) \ || (!defined(WINAPI_PARTITION_DESKTOP) || WINAPI_PARTITION_DESKTOP) // not UWP -TEST(AzureCliCredential, Error) -{ - using Azure::Core::Diagnostics::Logger; - using LogMsgVec = std::vector>; - LogMsgVec log; - Logger::SetLevel(Logger::Level::Informational); - Logger::SetListener([&](auto lvl, auto msg) { log.push_back(std::make_pair(lvl, msg)); }); - - AzureCliTestCredential const azCliCred( - EchoCommand("ERROR: Please run az login to setup account.")); - - EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); - EXPECT_EQ(log[0].first, Logger::Level::Informational); - EXPECT_EQ( - log[0].second, - "Identity: AzureCliCredential created." - "\nSuccessful creation does not guarantee further successful token retrieval."); - - TokenRequestContext trc; - trc.Scopes.push_back("https://storage.azure.com/.default"); - - log.clear(); - auto const errorMsg = "Identity: AzureCliCredential didn't get the token:" - " \"ERROR: Please run az login to setup account." -#if defined(AZ_PLATFORM_WINDOWS) - "\r" -#endif - "\n\""; + TEST(AzureCliCredential, Error) + { + using Azure::Core::Diagnostics::Logger; + using LogMsgVec = std::vector>; + LogMsgVec log; + Logger::SetLevel(Logger::Level::Informational); + Logger::SetListener([&](auto lvl, auto msg) { log.push_back(std::make_pair(lvl, msg)); }); - EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); - EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); - EXPECT_EQ(log[0].first, Logger::Level::Warning); - EXPECT_EQ(log[0].second, errorMsg); + AzureCliTestCredential const azCliCred( + EchoCommand("ERROR: Please run az login to setup account.")); - Logger::SetListener(nullptr); -} + EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); + EXPECT_EQ(log[0].first, Logger::Level::Informational); + EXPECT_EQ( + log[0].second, + "Identity: AzureCliCredential created." + "\nSuccessful creation does not guarantee further successful token retrieval."); -TEST(AzureCliCredential, GetCredentialName) -{ - AzureCliTestCredential const cred(EmptyOutputCommand); - EXPECT_EQ(cred.GetCredentialName(), "AzureCliCredential"); -} + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); -TEST(AzureCliCredential, EmptyOutput) -{ - AzureCliTestCredential const azCliCred(EmptyOutputCommand); + log.clear(); + auto const errorMsg = "Identity: AzureCliCredential didn't get the token:" + " \"ERROR: Please run az login to setup account." +#if defined(AZ_PLATFORM_WINDOWS) + "\r" +#endif + "\n\""; - TokenRequestContext trc; - trc.Scopes.push_back("https://storage.azure.com/.default"); + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); + EXPECT_EQ(log[0].first, Logger::Level::Warning); + EXPECT_EQ(log[0].second, errorMsg); - EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); -} + Logger::SetListener(nullptr); + } -TEST(AzureCliCredential, BigToken) -{ - std::string accessToken; + TEST(AzureCliCredential, GetCredentialName) { - std::string const tokenPart = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; - auto const nIterations = ((4 * 1024) / tokenPart.size()) + 1; - for (auto i = 0; i < static_cast(nIterations); ++i) - { - accessToken += tokenPart; - } + AzureCliTestCredential const cred(EmptyOutputCommand); + EXPECT_EQ(cred.GetCredentialName(), "AzureCliCredential"); } - AzureCliTestCredential const azCliCred(EchoCommand( - std::string("{\"accessToken\":\"") + accessToken - + "\",\"expiresOn\":\"2022-08-24 00:43:08.000000\"}")); - - TokenRequestContext trc; - trc.Scopes.push_back("https://storage.azure.com/.default"); - - auto const token = azCliCred.GetToken(trc, {}); - - EXPECT_EQ(token.Token, accessToken); - - EXPECT_EQ( - token.ExpiresOn, - DateTime::Parse("2022-08-24T00:43:08.000000Z", DateTime::DateFormat::Rfc3339)); -} - -TEST(AzureCliCredential, ExpiresIn) -{ - constexpr auto Token = "{\"accessToken\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\",\"expiresIn\":30}"; - - AzureCliTestCredential const azCliCred(EchoCommand(Token)); - - TokenRequestContext trc; - trc.Scopes.push_back("https://storage.azure.com/.default"); - - auto const timestampBefore = std::chrono::system_clock::now(); - auto const token = azCliCred.GetToken(trc, {}); - auto const timestampAfter = std::chrono::system_clock::now(); - - EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); - - EXPECT_GE(token.ExpiresOn, timestampBefore + std::chrono::seconds(30)); - 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"); + TEST(AzureCliCredential, EmptyOutput) + { + AzureCliTestCredential const azCliCred(EmptyOutputCommand); - auto const token = azCliCred.GetToken(trc, {}); + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); - EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + } - EXPECT_EQ( - token.ExpiresOn, - DateTime::Parse("2023-11-22T22:33:44.000000Z", DateTime::DateFormat::Rfc3339)); -} + TEST(AzureCliCredential, BigToken) + { + std::string accessToken; + { + std::string const tokenPart = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + auto const nIterations = ((4 * 1024) / tokenPart.size()) + 1; + for (auto i = 0; i < static_cast(nIterations); ++i) + { + accessToken += tokenPart; + } + } -TEST(AzureCliCredential, TimedOut) -{ - AzureCliCredentialOptions options; - options.CliProcessTimeout = std::chrono::seconds(2); - AzureCliTestCredential const azCliCred(InfiniteCommand, options); + AzureCliTestCredential const azCliCred(EchoCommand( + std::string("{\"accessToken\":\"") + accessToken + + "\",\"expiresOn\":\"2022-08-24 00:43:08.000000\"}")); - TokenRequestContext trc; - trc.Scopes.push_back("https://storage.azure.com/.default"); + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); - EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); -} + auto const token = azCliCred.GetToken(trc, {}); -TEST(AzureCliCredential, ContextCancelled) -{ - AzureCliCredentialOptions options; - options.CliProcessTimeout = std::chrono::hours(24); - AzureCliTestCredential const azCliCred(InfiniteCommand, options); + EXPECT_EQ(token.Token, accessToken); - TokenRequestContext trc; - trc.Scopes.push_back("https://storage.azure.com/.default"); + EXPECT_EQ( + token.ExpiresOn, + DateTime::Parse("2022-08-24T00:43:08.000000Z", DateTime::DateFormat::Rfc3339)); + } - auto context = Context::ApplicationContext.WithDeadline( - std::chrono::system_clock::now() + std::chrono::hours(24)); + TEST(AzureCliCredential, ExpiresIn) + { + constexpr auto Token = "{\"accessToken\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\",\"expiresIn\":30}"; - std::atomic thread1Started(false); + AzureCliTestCredential const azCliCred(EchoCommand(Token)); - std::thread thread1([&]() { - thread1Started = true; - EXPECT_THROW(static_cast(azCliCred.GetToken(trc, context)), AuthenticationException); - }); + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); - std::thread thread2([&]() { - while (!thread1Started) - { - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - } + auto const timestampBefore = std::chrono::system_clock::now(); + auto const token = azCliCred.GetToken(trc, {}); + auto const timestampAfter = std::chrono::system_clock::now(); - std::this_thread::sleep_for(std::chrono::seconds(2)); - context.Cancel(); - }); + EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); - thread1.join(); - thread2.join(); -} + EXPECT_GE(token.ExpiresOn, timestampBefore + std::chrono::seconds(30)); + EXPECT_LE(token.ExpiresOn, timestampAfter + std::chrono::seconds(30)); + } -TEST(AzureCliCredential, Defaults) -{ + TEST(AzureCliCredential, ExpiresOnUnixTimestampInt) { - AzureCliCredentialOptions const DefaultOptions; + // '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 azCliCred({}); - EXPECT_EQ(azCliCred.GetTenantId(), DefaultOptions.TenantId); - EXPECT_EQ(azCliCred.GetCliProcessTimeout(), DefaultOptions.CliProcessTimeout); - } + AzureCliTestCredential const azCliCred(EchoCommand(Token)); - { - TokenCredentialOptions const options; - AzureCliTestCredential azCliCred({}, options); - EXPECT_EQ(azCliCred.GetTenantId(), DefaultOptions.TenantId); - EXPECT_EQ(azCliCred.GetCliProcessTimeout(), DefaultOptions.CliProcessTimeout); - } - } + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); - { - AzureCliCredentialOptions options; - options.TenantId = "01234567-89AB-CDEF-0123-456789ABCDEF"; - options.CliProcessTimeout = std::chrono::seconds(12345); + auto const token = azCliCred.GetToken(trc, {}); - AzureCliTestCredential azCliCred({}, options); + EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); - EXPECT_EQ(azCliCred.GetTenantId(), "01234567-89AB-CDEF-0123-456789ABCDEF"); - EXPECT_EQ(azCliCred.GetCliProcessTimeout(), std::chrono::seconds(12345)); + EXPECT_EQ( + token.ExpiresOn, + DateTime::Parse("2023-11-22T22:33:44.000000Z", DateTime::DateFormat::Rfc3339)); } -} -TEST(AzureCliCredential, CmdLine) -{ - AzureCliTestCredential azCliCred({}); - - auto const cmdLineWithoutTenant - = azCliCred.GetOriginalAzCommand("https://storage.azure.com/.default", {}); + 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)); - auto const cmdLineWithTenant = azCliCred.GetOriginalAzCommand( - "https://storage.azure.com/.default", "01234567-89AB-CDEF-0123-456789ABCDEF"); + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); - EXPECT_EQ( - cmdLineWithoutTenant, - "az account get-access-token --output json --scope \"https://storage.azure.com/.default\""); + auto const token = azCliCred.GetToken(trc, {}); - EXPECT_EQ( - cmdLineWithTenant, - "az account get-access-token --output json --scope \"https://storage.azure.com/.default\"" - " --tenant \"01234567-89AB-CDEF-0123-456789ABCDEF\""); -} + EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); -TEST(AzureCliCredential, UnsafeChars) -{ - std::string const Exploit = std::string("\" | echo OWNED | ") + InfiniteCommand + " | echo \""; + EXPECT_EQ( + token.ExpiresOn, + DateTime::Parse("2023-11-22T22:33:44.000000Z", DateTime::DateFormat::Rfc3339)); + } + TEST(AzureCliCredential, TimedOut) { AzureCliCredentialOptions options; - options.TenantId = "01234567-89AB-CDEF-0123-456789ABCDEF"; - options.TenantId += Exploit; - AzureCliCredential azCliCred(options); + options.CliProcessTimeout = std::chrono::seconds(2); + AzureCliTestCredential const azCliCred(InfiniteCommand, options); TokenRequestContext trc; - trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); + trc.Scopes.push_back("https://storage.azure.com/.default"); + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); } + TEST(AzureCliCredential, ContextCancelled) { AzureCliCredentialOptions options; options.CliProcessTimeout = std::chrono::hours(24); - AzureCliCredential azCliCred(options); + AzureCliTestCredential const azCliCred(InfiniteCommand, options); TokenRequestContext trc; - trc.Scopes.push_back(std::string("https://storage.azure.com/.default") + Exploit); + trc.Scopes.push_back("https://storage.azure.com/.default"); - EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); - } -} + auto context = Context::ApplicationContext.WithDeadline( + std::chrono::system_clock::now() + std::chrono::hours(24)); -class ParameterizedTestForDisallowedChars : public ::testing::TestWithParam { -protected: - std::string value; -}; + std::atomic thread1Started(false); -TEST_P(ParameterizedTestForDisallowedChars, DisallowedCharsForScopeAndTenantId) -{ - std::string const InvalidValue = GetParam(); + std::thread thread1([&]() { + thread1Started = true; + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, context)), AuthenticationException); + }); - // Tenant ID test via AzureCliCredentialOptions directly. - { - AzureCliCredentialOptions options; - options.TenantId = "01234567-89AB-CDEF-0123-456789ABCDEF"; - options.TenantId += InvalidValue; - AzureCliCredential azCliCred(options); + std::thread thread2([&]() { + while (!thread1Started) + { + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } - TokenRequestContext trc; - trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); - EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + std::this_thread::sleep_for(std::chrono::seconds(2)); + context.Cancel(); + }); - try - { - auto const token = azCliCred.GetToken(trc, {}); - } - catch (AuthenticationException const& e) - { - EXPECT_TRUE(std::string(e.what()).find("Unsafe") != std::string::npos) << e.what(); - } + thread1.join(); + thread2.join(); } - // Tenant ID test via TokenRequestContext, using a wildcard for AdditionallyAllowedTenants. + TEST(AzureCliCredential, Defaults) { - AzureCliCredentialOptions options; - options.CliProcessTimeout = std::chrono::hours(24); - options.AdditionallyAllowedTenants.push_back("*"); - AzureCliCredential azCliCred(options); - - TokenRequestContext trc; - trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); - trc.TenantId = InvalidValue; - EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); - - try { - auto const token = azCliCred.GetToken(trc, {}); + AzureCliCredentialOptions const DefaultOptions; + + { + AzureCliTestCredential azCliCred({}); + EXPECT_EQ(azCliCred.GetTenantId(), DefaultOptions.TenantId); + EXPECT_EQ(azCliCred.GetCliProcessTimeout(), DefaultOptions.CliProcessTimeout); + } + + { + TokenCredentialOptions const options; + AzureCliTestCredential azCliCred({}, options); + EXPECT_EQ(azCliCred.GetTenantId(), DefaultOptions.TenantId); + EXPECT_EQ(azCliCred.GetCliProcessTimeout(), DefaultOptions.CliProcessTimeout); + } } - catch (AuthenticationException const& e) + { - EXPECT_TRUE(std::string(e.what()).find("Unsafe") != std::string::npos) << e.what(); + AzureCliCredentialOptions options; + options.TenantId = "01234567-89AB-CDEF-0123-456789ABCDEF"; + options.CliProcessTimeout = std::chrono::seconds(12345); + + AzureCliTestCredential azCliCred({}, options); + + EXPECT_EQ(azCliCred.GetTenantId(), "01234567-89AB-CDEF-0123-456789ABCDEF"); + EXPECT_EQ(azCliCred.GetCliProcessTimeout(), std::chrono::seconds(12345)); } } - // Tenant ID test via TokenRequestContext, using a specific AdditionallyAllowedTenants value. + TEST(AzureCliCredential, CmdLine) { - AzureCliCredentialOptions options; - options.AdditionallyAllowedTenants.push_back(InvalidValue); - AzureCliCredential azCliCred(options); + AzureCliTestCredential azCliCred({}); - TokenRequestContext trc; - trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); - trc.TenantId = InvalidValue; - EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + auto const cmdLineWithoutTenant + = azCliCred.GetOriginalAzCommand("https://storage.azure.com/.default", {}); - try - { - auto const token = azCliCred.GetToken(trc, {}); - } - catch (AuthenticationException const& e) - { - EXPECT_TRUE(std::string(e.what()).find("Unsafe") != std::string::npos) << e.what(); - } + auto const cmdLineWithTenant = azCliCred.GetOriginalAzCommand( + "https://storage.azure.com/.default", "01234567-89AB-CDEF-0123-456789ABCDEF"); + + EXPECT_EQ( + cmdLineWithoutTenant, + "az account get-access-token --output json --scope " + "\"https://storage.azure.com/.default\""); + + EXPECT_EQ( + cmdLineWithTenant, + "az account get-access-token --output json --scope \"https://storage.azure.com/.default\"" + " --tenant \"01234567-89AB-CDEF-0123-456789ABCDEF\""); } - // Scopes test via TokenRequestContext. + TEST(AzureCliCredential, UnsafeChars) { - AzureCliCredentialOptions options; - options.CliProcessTimeout = std::chrono::hours(24); - AzureCliCredential azCliCred(options); - - TokenRequestContext trc; - trc.Scopes.push_back(std::string("https://storage.azure.com/.default") + InvalidValue); - EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + std::string const Exploit = std::string("\" | echo OWNED | ") + InfiniteCommand + " | echo \""; - try { - auto const token = azCliCred.GetToken(trc, {}); + AzureCliCredentialOptions options; + options.TenantId = "01234567-89AB-CDEF-0123-456789ABCDEF"; + options.TenantId += Exploit; + AzureCliCredential azCliCred(options); + + TokenRequestContext trc; + trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); } - catch (AuthenticationException const& e) + { - EXPECT_TRUE(std::string(e.what()).find("Unsafe") != std::string::npos) << e.what(); - } - } -} + AzureCliCredentialOptions options; + options.CliProcessTimeout = std::chrono::hours(24); + AzureCliCredential azCliCred(options); -INSTANTIATE_TEST_SUITE_P( - AzureCliCredential, - ParameterizedTestForDisallowedChars, - ::testing::Values(" ", "|", "`", "\"", "'", ";", "&")); + TokenRequestContext trc; + trc.Scopes.push_back(std::string("https://storage.azure.com/.default") + Exploit); -class ParameterizedTestForCharDifferences : public ::testing::TestWithParam { -protected: - std::string value; -}; + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + } + } -TEST_P(ParameterizedTestForCharDifferences, ValidCharsForScopeButNotTenantId) -{ - std::string const ValidScopeButNotTenantId = GetParam(); + class ParameterizedTestForDisallowedChars : public ::testing::TestWithParam { + protected: + std::string value; + }; + TEST_P(ParameterizedTestForDisallowedChars, DisallowedCharsForScopeAndTenantId) { - AzureCliCredentialOptions options; - options.TenantId = "01234567-89AB-CDEF-0123-456789ABCDEF"; - options.TenantId += ValidScopeButNotTenantId; - AzureCliCredential azCliCred(options); + std::string const InvalidValue = GetParam(); - TokenRequestContext trc; - trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); - EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); - - try + // Tenant ID test via AzureCliCredentialOptions directly. { - auto const token = azCliCred.GetToken(trc, {}); + AzureCliCredentialOptions options; + options.TenantId = "01234567-89AB-CDEF-0123-456789ABCDEF"; + options.TenantId += InvalidValue; + AzureCliCredential azCliCred(options); + + TokenRequestContext trc; + trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + + try + { + auto const token = azCliCred.GetToken(trc, {}); + } + catch (AuthenticationException const& e) + { + EXPECT_TRUE(std::string(e.what()).find("Unsafe") != std::string::npos) << e.what(); + } } - catch (AuthenticationException const& e) + + // Tenant ID test via TokenRequestContext, using a wildcard for AdditionallyAllowedTenants. { - EXPECT_TRUE(std::string(e.what()).find("Unsafe") != std::string::npos) << e.what(); + AzureCliCredentialOptions options; + options.CliProcessTimeout = std::chrono::hours(24); + options.AdditionallyAllowedTenants.push_back("*"); + AzureCliCredential azCliCred(options); + + TokenRequestContext trc; + trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); + trc.TenantId = InvalidValue; + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + + try + { + auto const token = azCliCred.GetToken(trc, {}); + } + catch (AuthenticationException const& e) + { + EXPECT_TRUE(std::string(e.what()).find("Unsafe") != std::string::npos) << e.what(); + } } - } - { - AzureCliCredentialOptions options; - options.CliProcessTimeout = std::chrono::hours(24); - AzureCliCredential azCliCred(options); - - TokenRequestContext trc; - trc.Scopes.push_back( - std::string("https://storage.azure.com/.default") + ValidScopeButNotTenantId); - - // We expect the GetToken to fail, but not because of the unsafe chars. - try + // Tenant ID test via TokenRequestContext, using a specific AdditionallyAllowedTenants value. { - auto const token = azCliCred.GetToken(trc, {}); + AzureCliCredentialOptions options; + options.AdditionallyAllowedTenants.push_back(InvalidValue); + AzureCliCredential azCliCred(options); + + TokenRequestContext trc; + trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); + trc.TenantId = InvalidValue; + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + + try + { + auto const token = azCliCred.GetToken(trc, {}); + } + catch (AuthenticationException const& e) + { + EXPECT_TRUE(std::string(e.what()).find("Unsafe") != std::string::npos) << e.what(); + } } - catch (AuthenticationException const& e) + + // Scopes test via TokenRequestContext. { - EXPECT_TRUE(std::string(e.what()).find("Unsafe") == std::string::npos) << e.what(); + AzureCliCredentialOptions options; + options.CliProcessTimeout = std::chrono::hours(24); + AzureCliCredential azCliCred(options); + + TokenRequestContext trc; + trc.Scopes.push_back(std::string("https://storage.azure.com/.default") + InvalidValue); + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + + try + { + auto const token = azCliCred.GetToken(trc, {}); + } + catch (AuthenticationException const& e) + { + EXPECT_TRUE(std::string(e.what()).find("Unsafe") != std::string::npos) << e.what(); + } } } -} -INSTANTIATE_TEST_SUITE_P( - AzureCliCredential, - ParameterizedTestForCharDifferences, - ::testing::Values(":", "/", "_")); + INSTANTIATE_TEST_SUITE_P( + AzureCliCredential, + ParameterizedTestForDisallowedChars, + ::testing::Values(" ", "|", "`", "\"", "'", ";", "&")); -class ParameterizedTestForAllowedChars : public ::testing::TestWithParam { -protected: - std::string value; -}; - -TEST_P(ParameterizedTestForAllowedChars, ValidCharsForScopeAndTenantId) -{ - std::string const ValidChars = GetParam(); + class ParameterizedTestForCharDifferences : public ::testing::TestWithParam { + protected: + std::string value; + }; + TEST_P(ParameterizedTestForCharDifferences, ValidCharsForScopeButNotTenantId) { - AzureCliCredentialOptions options; - options.TenantId = "01234567-89AB-CDEF-0123-456789ABCDEF"; - options.TenantId += ValidChars; - AzureCliCredential azCliCred(options); - - TokenRequestContext trc; - trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); + std::string const ValidScopeButNotTenantId = GetParam(); - // We expect the GetToken to fail, but not because of the unsafe chars. - try { - auto const token = azCliCred.GetToken(trc, {}); + AzureCliCredentialOptions options; + options.TenantId = "01234567-89AB-CDEF-0123-456789ABCDEF"; + options.TenantId += ValidScopeButNotTenantId; + AzureCliCredential azCliCred(options); + + TokenRequestContext trc; + trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + + try + { + auto const token = azCliCred.GetToken(trc, {}); + } + catch (AuthenticationException const& e) + { + EXPECT_TRUE(std::string(e.what()).find("Unsafe") != std::string::npos) << e.what(); + } } - catch (AuthenticationException const& e) + { - EXPECT_TRUE(std::string(e.what()).find("Unsafe") == std::string::npos) << e.what(); + AzureCliCredentialOptions options; + options.CliProcessTimeout = std::chrono::hours(24); + AzureCliCredential azCliCred(options); + + TokenRequestContext trc; + trc.Scopes.push_back( + std::string("https://storage.azure.com/.default") + ValidScopeButNotTenantId); + + // We expect the GetToken to fail, but not because of the unsafe chars. + try + { + auto const token = azCliCred.GetToken(trc, {}); + } + catch (AuthenticationException const& e) + { + EXPECT_TRUE(std::string(e.what()).find("Unsafe") == std::string::npos) << e.what(); + } } } - { - AzureCliCredentialOptions options; - options.CliProcessTimeout = std::chrono::hours(24); - AzureCliCredential azCliCred(options); + INSTANTIATE_TEST_SUITE_P( + AzureCliCredential, + ParameterizedTestForCharDifferences, + ::testing::Values(":", "/", "_")); - TokenRequestContext trc; - trc.Scopes.push_back(std::string("https://storage.azure.com/.default") + ValidChars); + class ParameterizedTestForAllowedChars : public ::testing::TestWithParam { + protected: + std::string value; + }; + + TEST_P(ParameterizedTestForAllowedChars, ValidCharsForScopeAndTenantId) + { + std::string const ValidChars = GetParam(); - // We expect the GetToken to fail, but not because of the unsafe chars. - try { - auto const token = azCliCred.GetToken(trc, {}); + AzureCliCredentialOptions options; + options.TenantId = "01234567-89AB-CDEF-0123-456789ABCDEF"; + options.TenantId += ValidChars; + AzureCliCredential azCliCred(options); + + TokenRequestContext trc; + trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); + + // We expect the GetToken to fail, but not because of the unsafe chars. + try + { + auto const token = azCliCred.GetToken(trc, {}); + } + catch (AuthenticationException const& e) + { + EXPECT_TRUE(std::string(e.what()).find("Unsafe") == std::string::npos) << e.what(); + } } - catch (AuthenticationException const& e) + { - EXPECT_TRUE(std::string(e.what()).find("Unsafe") == std::string::npos) << e.what(); + AzureCliCredentialOptions options; + options.CliProcessTimeout = std::chrono::hours(24); + AzureCliCredential azCliCred(options); + + TokenRequestContext trc; + trc.Scopes.push_back(std::string("https://storage.azure.com/.default") + ValidChars); + + // We expect the GetToken to fail, but not because of the unsafe chars. + try + { + auto const token = azCliCred.GetToken(trc, {}); + } + catch (AuthenticationException const& e) + { + EXPECT_TRUE(std::string(e.what()).find("Unsafe") == std::string::npos) << e.what(); + } } } -} - -INSTANTIATE_TEST_SUITE_P( - AzureCliCredential, - ParameterizedTestForAllowedChars, - ::testing::Values(".", "-", "A", "9")); - -TEST(AzureCliCredential, StrictIso8601TimeFormat) -{ - constexpr auto Token = "{\"accessToken\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\"," - "\"expiresOn\":\"2022-08-24T00:43:08\"}"; // With the "T" - - 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("2022-08-24T00:43:08.000000Z", DateTime::DateFormat::Rfc3339)); -} - -TEST(AzureCliCredential, LocalTime) -{ - constexpr auto Token = "{\"accessToken\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\"," - "\"expiresOn\":\"2023-12-07 00:43:08\"}"; + INSTANTIATE_TEST_SUITE_P( + AzureCliCredential, + ParameterizedTestForAllowedChars, + ::testing::Values(".", "-", "A", "9")); + TEST(AzureCliCredential, StrictIso8601TimeFormat) { - AzureCliTestCredential azCliCred(EchoCommand(Token)); - azCliCred.SetLocalTimeToUtcDiffSeconds(-28800); // Redmond (no DST) + constexpr auto Token = "{\"accessToken\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\"," + "\"expiresOn\":\"2022-08-24T00:43:08\"}"; // With the "T" + + AzureCliTestCredential const azCliCred(EchoCommand(Token)); TokenRequestContext trc; trc.Scopes.push_back("https://storage.azure.com/.default"); @@ -641,71 +626,92 @@ TEST(AzureCliCredential, LocalTime) EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); EXPECT_EQ( - token.ExpiresOn, DateTime::Parse("2023-12-07T08:43:08Z", DateTime::DateFormat::Rfc3339)); + token.ExpiresOn, + DateTime::Parse("2022-08-24T00:43:08.000000Z", DateTime::DateFormat::Rfc3339)); } + TEST(AzureCliCredential, LocalTime) { - AzureCliTestCredential azCliCred(EchoCommand(Token)); - azCliCred.SetLocalTimeToUtcDiffSeconds(7200); // Kyiv (no DST) + constexpr auto Token = "{\"accessToken\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\"," + "\"expiresOn\":\"2023-12-07 00:43:08\"}"; - TokenRequestContext trc; - trc.Scopes.push_back("https://storage.azure.com/.default"); - auto const token = azCliCred.GetToken(trc, {}); - - EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + { + AzureCliTestCredential azCliCred(EchoCommand(Token)); + azCliCred.SetLocalTimeToUtcDiffSeconds(-28800); // Redmond (no DST) - EXPECT_EQ( - token.ExpiresOn, DateTime::Parse("2023-12-06T22:43:08Z", DateTime::DateFormat::Rfc3339)); - } -} + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); + auto const token = azCliCred.GetToken(trc, {}); -TEST(AzureCliCredential, Diagnosability) -{ - { - AzureCliTestCredential const azCliCred( - EchoCommand("az is not recognized as an internal or external command, " - "operable program or batch file.")); + EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); - TokenRequestContext trc; - trc.Scopes.push_back("https://storage.azure.com/.default"); - try - { - static_cast(azCliCred.GetToken(trc, {})); + EXPECT_EQ( + token.ExpiresOn, DateTime::Parse("2023-12-07T08:43:08Z", DateTime::DateFormat::Rfc3339)); } - catch (AuthenticationException const& e) + { - std::string const expectedMsgStart - = "AzureCliCredential didn't get the token: " - "\"az is not recognized as an internal or external command, " - "operable program or batch file."; + 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, {}); - std::string actualMsgStart = e.what(); - actualMsgStart.resize(expectedMsgStart.length()); + EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); - // It is enough to compare StartsWith() and not deal with - // the entire string due to '/n' and '/r/n' differences. - EXPECT_EQ(actualMsgStart, expectedMsgStart); + EXPECT_EQ( + token.ExpiresOn, DateTime::Parse("2023-12-06T22:43:08Z", DateTime::DateFormat::Rfc3339)); } } + TEST(AzureCliCredential, Diagnosability) { - AzureCliTestCredential const azCliCred(EchoCommand("{\"property\":\"value\"}")); - - TokenRequestContext trc; - trc.Scopes.push_back("https://storage.azure.com/.default"); - try { - static_cast(azCliCred.GetToken(trc, {})); + AzureCliTestCredential const azCliCred( + EchoCommand("az is not recognized as an internal or external command, " + "operable program or batch file.")); + + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); + try + { + static_cast(azCliCred.GetToken(trc, {})); + } + catch (AuthenticationException const& e) + { + std::string const expectedMsgStart + = "AzureCliCredential didn't get the token: " + "\"az is not recognized as an internal or external command, " + "operable program or batch file."; + + std::string actualMsgStart = e.what(); + actualMsgStart.resize(expectedMsgStart.length()); + + // It is enough to compare StartsWith() and not deal with + // the entire string due to '/n' and '/r/n' differences. + EXPECT_EQ(actualMsgStart, expectedMsgStart); + } } - catch (AuthenticationException const& e) + { - EXPECT_EQ( - e.what(), - std::string("AzureCliCredential didn't get the token: " - "\"Token JSON object: can't find or parse 'accessToken' property.\n" - "See Azure::Core::Diagnostics::Logger for details " - "(https://aka.ms/azsdk/cpp/identity/troubleshooting).\"")); + AzureCliTestCredential const azCliCred(EchoCommand("{\"property\":\"value\"}")); + + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); + try + { + static_cast(azCliCred.GetToken(trc, {})); + } + catch (AuthenticationException const& e) + { + EXPECT_EQ( + e.what(), + std::string("AzureCliCredential didn't get the token: " + "\"Token JSON object: can't find or parse 'accessToken' property.\n" + "See Azure::Core::Diagnostics::Logger for details " + "(https://aka.ms/azsdk/cpp/identity/troubleshooting).\"")); + } } } -} #endif // not UWP +}}} // namespace Azure::Identity::Test From 8178da464cdd7947cfbca4b76a1ada31ca1f73f6 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 8 Feb 2024 22:32:10 -0800 Subject: [PATCH 7/8] Remove the macro for final expansion since doxygen struggles with it. --- .../inc/azure/identity/azure_cli_credential.hpp | 6 +++++- .../azure-identity/inc/azure/identity/dll_import_export.hpp | 2 -- 2 files changed, 5 insertions(+), 3 deletions(-) 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 00f0defc39..5ff6f1ece4 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 @@ -56,7 +56,11 @@ namespace Azure { namespace Identity { * @brief Enables authentication to Microsoft Entra ID using Azure CLI to obtain an access * token. */ - class AzureCliCredential _azure_NON_FINAL_FOR_TESTS : public Core::Credentials::TokenCredential { + class AzureCliCredential +#if !defined(_azure_TESTING_BUILD) + final +#endif + : public Core::Credentials::TokenCredential { #if defined(_azure_TESTING_BUILD) friend class Azure::Identity::Test::AzureCliTestCredential; diff --git a/sdk/identity/azure-identity/inc/azure/identity/dll_import_export.hpp b/sdk/identity/azure-identity/inc/azure/identity/dll_import_export.hpp index ce74ac15a8..a4bed4310b 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/dll_import_export.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/dll_import_export.hpp @@ -39,10 +39,8 @@ #undef AZ_IDENTITY_BUILT_AS_DLL #if defined(_azure_TESTING_BUILD) -#define _azure_NON_FINAL_FOR_TESTS #define _azure_VIRTUAL_FOR_TESTS virtual #else -#define _azure_NON_FINAL_FOR_TESTS final #define _azure_VIRTUAL_FOR_TESTS #endif From ed46fe19601d7fe1137e531bbc743d2ddc281cfa Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Fri, 9 Feb 2024 14:40:12 -0800 Subject: [PATCH 8/8] Address PR feedback, add ifdef guard around _azure_VIRTUAL_FOR_TESTS. --- .../azure-identity/inc/azure/identity/dll_import_export.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sdk/identity/azure-identity/inc/azure/identity/dll_import_export.hpp b/sdk/identity/azure-identity/inc/azure/identity/dll_import_export.hpp index a4bed4310b..11f02aeffe 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/dll_import_export.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/dll_import_export.hpp @@ -39,10 +39,14 @@ #undef AZ_IDENTITY_BUILT_AS_DLL #if defined(_azure_TESTING_BUILD) +#if !defined(_azure_VIRTUAL_FOR_TESTS) #define _azure_VIRTUAL_FOR_TESTS virtual +#endif #else +#if !defined(_azure_VIRTUAL_FOR_TESTS) #define _azure_VIRTUAL_FOR_TESTS #endif +#endif /** * @brief Azure SDK abstractions.