From cf562e0d12fefd6795e31f9febd9dccb14b37d50 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 13 Aug 2024 16:26:03 -0700 Subject: [PATCH] Do not pass a client ID into the request body for MICredential within a Cloud Shell environment, but rather throw, as not supported. (#5837) * Do not pass in a client ID into the request body in a Cloud Shell environment, but rather throw, as not supported. * Address PR feedback - reword exception to avoid mention of SAI. * Address PR feedback - use param name in exception. --- sdk/identity/azure-identity/CHANGELOG.md | 2 + .../src/managed_identity_source.cpp | 23 ++-- .../src/private/managed_identity_source.hpp | 1 - .../ut/managed_identity_credential_test.cpp | 107 +++++++----------- 4 files changed, 56 insertions(+), 77 deletions(-) diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index 51bb2b3a5..8accdaa8c 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -6,6 +6,8 @@ ### Breaking Changes +- Previously, if a clientId was specified for Cloud Shell managed identity, which is not supported, the clientId was passed into the request body. Now, an exception will be thrown if a clientId is specified for Cloud Shell managed identity. + ### Bugs Fixed ### Other Changes diff --git a/sdk/identity/azure-identity/src/managed_identity_source.cpp b/sdk/identity/azure-identity/src/managed_identity_source.cpp index fccdb26dd..2f69ea3e1 100644 --- a/sdk/identity/azure-identity/src/managed_identity_source.cpp +++ b/sdk/identity/azure-identity/src/managed_identity_source.cpp @@ -250,13 +250,14 @@ std::unique_ptr AppServiceV2019ManagedIdentitySource::Cre credName, clientId, resourceId, options, "IDENTITY_ENDPOINT", "IDENTITY_HEADER", "2019"); } -// Cloud Shell doesn't support user-assigned managed identities std::unique_ptr CloudShellManagedIdentitySource::Create( std::string const& credName, std::string const& clientId, - std::string const&, + std::string const& resourceId, Azure::Core::Credentials::TokenCredentialOptions const& options) { + using Azure::Core::Credentials::AuthenticationException; + constexpr auto EndpointVarName = "MSI_ENDPOINT"; auto msiEndpoint = Environment::GetVariable(EndpointVarName); @@ -264,6 +265,13 @@ std::unique_ptr CloudShellManagedIdentitySource::Create( if (!msiEndpoint.empty()) { + if (!clientId.empty() || !resourceId.empty()) + { + throw AuthenticationException( + "User-assigned managed identities are not supported in Cloud Shell environments. Omit " + "the clientId or resourceId when constructing the ManagedIdentityCredential."); + } + return std::unique_ptr(new CloudShellManagedIdentitySource( clientId, options, ParseEndpointUrl(credName, msiEndpoint, EndpointVarName, CredSource))); } @@ -278,11 +286,6 @@ CloudShellManagedIdentitySource::CloudShellManagedIdentitySource( Azure::Core::Url endpointUrl) : ManagedIdentitySource(clientId, endpointUrl.GetHost(), options), m_url(std::move(endpointUrl)) { - using Azure::Core::Url; - if (!clientId.empty()) - { - m_body = std::string("client_id=" + Url::Encode(clientId)); - } } Azure::Core::Credentials::AccessToken CloudShellManagedIdentitySource::GetToken( @@ -312,13 +315,9 @@ Azure::Core::Credentials::AccessToken CloudShellManagedIdentitySource::GetToken( if (!scopesStr.empty()) { resource = "resource=" + scopesStr; - if (!m_body.empty()) - { - resource += "&"; - } } - auto request = std::make_unique(HttpMethod::Post, m_url, resource + m_body); + auto request = std::make_unique(HttpMethod::Post, m_url, resource); request->HttpRequest.SetHeader("Metadata", "true"); return request; diff --git a/sdk/identity/azure-identity/src/private/managed_identity_source.hpp b/sdk/identity/azure-identity/src/private/managed_identity_source.hpp index b7f7e6a82..c2bf5cf94 100644 --- a/sdk/identity/azure-identity/src/private/managed_identity_source.hpp +++ b/sdk/identity/azure-identity/src/private/managed_identity_source.hpp @@ -141,7 +141,6 @@ namespace Azure { namespace Identity { namespace _detail { class CloudShellManagedIdentitySource final : public ManagedIdentitySource { private: Core::Url m_url; - std::string m_body; explicit CloudShellManagedIdentitySource( std::string const& clientId, diff --git a/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp b/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp index e182a5ea8..3aaee6f4f 100644 --- a/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp @@ -873,7 +873,9 @@ TEST(ManagedIdentityCredential, CloudShell) TEST(ManagedIdentityCredential, CloudShellClientId) { - auto const actual = CredentialTestHelper::SimulateTokenRequest( + using Azure::Core::Credentials::AuthenticationException; + + static_cast(CredentialTestHelper::SimulateTokenRequest( [](auto transport) { TokenCredentialOptions options; options.Transport.Transport = transport; @@ -887,71 +889,49 @@ TEST(ManagedIdentityCredential, CloudShellClientId) {"IDENTITY_SERVER_THUMBPRINT", "0123456789abcdef0123456789abcdef01234567"}, }); - return std::make_unique( - "fedcba98-7654-3210-0123-456789abcdef", options); + std::unique_ptr cloudShellManagedIdentityCredential; + EXPECT_THROW( + cloudShellManagedIdentityCredential = std::make_unique( + "fedcba98-7654-3210-0123-456789abcdef", options), + AuthenticationException); + + return cloudShellManagedIdentityCredential; }, - {{"https://azure.com/.default"}, {"https://outlook.com/.default"}, {}}, - std::vector{ - "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}", - "{\"expires_in\":7200, \"access_token\":\"ACCESSTOKEN2\"}", - "{\"expires_in\":9999, \"access_token\":\"ACCESSTOKEN3\"}"}); - - EXPECT_EQ(actual.Requests.size(), 3U); - EXPECT_EQ(actual.Responses.size(), 3U); - - auto const& request0 = actual.Requests.at(0); - auto const& request1 = actual.Requests.at(1); - auto const& request2 = actual.Requests.at(2); - - auto const& response0 = actual.Responses.at(0); - auto const& response1 = actual.Responses.at(1); - auto const& response2 = actual.Responses.at(2); - - EXPECT_EQ(request0.HttpMethod, HttpMethod::Post); - EXPECT_EQ(request1.HttpMethod, HttpMethod::Post); - EXPECT_EQ(request2.HttpMethod, HttpMethod::Post); - - EXPECT_EQ(request0.AbsoluteUrl, "https://microsoft.com"); - EXPECT_EQ(request1.AbsoluteUrl, "https://microsoft.com"); - EXPECT_EQ(request2.AbsoluteUrl, "https://microsoft.com"); - - EXPECT_EQ( - request0.Body, - "resource=https%3A%2F%2Fazure.com&client_id=fedcba98-7654-3210-0123-456789abcdef"); // cspell:disable-line - - EXPECT_EQ( - request1.Body, - "resource=https%3A%2F%2Foutlook.com&client_id=fedcba98-7654-3210-0123-456789abcdef"); // cspell:disable-line - - EXPECT_EQ(request2.Body, "client_id=fedcba98-7654-3210-0123-456789abcdef"); - - { - EXPECT_NE(request0.Headers.find("Metadata"), request0.Headers.end()); - EXPECT_EQ(request0.Headers.at("Metadata"), "true"); - - EXPECT_NE(request1.Headers.find("Metadata"), request1.Headers.end()); - EXPECT_EQ(request1.Headers.at("Metadata"), "true"); - - EXPECT_NE(request2.Headers.find("Metadata"), request2.Headers.end()); - EXPECT_EQ(request2.Headers.at("Metadata"), "true"); - } - - EXPECT_EQ(response0.AccessToken.Token, "ACCESSTOKEN1"); - EXPECT_EQ(response1.AccessToken.Token, "ACCESSTOKEN2"); - EXPECT_EQ(response2.AccessToken.Token, "ACCESSTOKEN3"); - - using namespace std::chrono_literals; - EXPECT_GE(response0.AccessToken.ExpiresOn, response0.EarliestExpiration + 3600s); - EXPECT_LE(response0.AccessToken.ExpiresOn, response0.LatestExpiration + 3600s); - - EXPECT_GE(response1.AccessToken.ExpiresOn, response1.EarliestExpiration + 3600s); - EXPECT_LE(response1.AccessToken.ExpiresOn, response1.LatestExpiration + 3600s); - - EXPECT_GE(response2.AccessToken.ExpiresOn, response2.EarliestExpiration + 4999s); - EXPECT_LE(response2.AccessToken.ExpiresOn, response2.LatestExpiration + 4999s); + {}, + {"{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}"})); } TEST(ManagedIdentityCredential, CloudShellResourceId) +{ + using Azure::Core::Credentials::AuthenticationException; + + static_cast(CredentialTestHelper::SimulateTokenRequest( + [](auto transport) { + TokenCredentialOptions options; + options.Transport.Transport = transport; + + CredentialTestHelper::EnvironmentOverride const env({ + {"MSI_ENDPOINT", "https://microsoft.com/"}, + {"MSI_SECRET", ""}, + {"IDENTITY_ENDPOINT", "https://visualstudio.com/"}, + {"IMDS_ENDPOINT", "https://xbox.com/"}, + {"IDENTITY_HEADER", ""}, + {"IDENTITY_SERVER_THUMBPRINT", "0123456789abcdef0123456789abcdef01234567"}, + }); + + std::unique_ptr cloudShellManagedIdentityCredential; + EXPECT_THROW( + cloudShellManagedIdentityCredential = std::make_unique( + ResourceIdentifier("abcdef01-2345-6789-9876-543210fedcba"), options), + AuthenticationException); + + return cloudShellManagedIdentityCredential; + }, + {}, + {"{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}"})); +} + +TEST(ManagedIdentityCredential, CloudShellScope) { auto const actual = CredentialTestHelper::SimulateTokenRequest( [](auto transport) { @@ -967,8 +947,7 @@ TEST(ManagedIdentityCredential, CloudShellResourceId) {"IDENTITY_SERVER_THUMBPRINT", "0123456789abcdef0123456789abcdef01234567"}, }); - return std::make_unique( - ResourceIdentifier("abcdef01-2345-6789-9876-543210fedcba"), options); + return std::make_unique(options); }, {{"https://azure.com/.default"}, {"https://outlook.com/.default"}, {}}, std::vector{