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

@@ -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

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

@@ -250,20 +250,28 @@ std::unique_ptr<ManagedIdentitySource> AppServiceV2019ManagedIdentitySource::Cre
       credName, clientId, resourceId, options, "IDENTITY_ENDPOINT", "IDENTITY_HEADER", "2019");
 }
 
-// Cloud Shell doesn't support user-assigned managed identities
 std::unique_ptr<ManagedIdentitySource> 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);
 
   std::string const CredSource = "Cloud Shell";
 
   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<ManagedIdentitySource>(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<TokenRequest>(HttpMethod::Post, m_url, resource + m_body);
+      auto request = std::make_unique<TokenRequest>(HttpMethod::Post, m_url, resource);
       request->HttpRequest.SetHeader("Metadata", "true");
 
       return request;

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,

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<void>(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<ManagedIdentityCredential>(
-            "fedcba98-7654-3210-0123-456789abcdef", options);
-      },
-      {{"https://azure.com/.default"}, {"https://outlook.com/.default"}, {}},
-      std::vector<std::string>{
-          "{\"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");
+        std::unique_ptr<ManagedIdentityCredential const> cloudShellManagedIdentityCredential;
+        EXPECT_THROW(
+            cloudShellManagedIdentityCredential = std::make_unique<ManagedIdentityCredential>(
+                "fedcba98-7654-3210-0123-456789abcdef", options),
+            AuthenticationException);
 
-    EXPECT_NE(request1.Headers.find("Metadata"), request1.Headers.end());
-    EXPECT_EQ(request1.Headers.at("Metadata"), "true");
+        return cloudShellManagedIdentityCredential;
+      },
+      {},
+      {"{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}"}));
+}
 
-    EXPECT_NE(request2.Headers.find("Metadata"), request2.Headers.end());
-    EXPECT_EQ(request2.Headers.at("Metadata"), "true");
-  }
+TEST(ManagedIdentityCredential, CloudShellResourceId)
+{
+  using Azure::Core::Credentials::AuthenticationException;
 
-  EXPECT_EQ(response0.AccessToken.Token, "ACCESSTOKEN1");
-  EXPECT_EQ(response1.AccessToken.Token, "ACCESSTOKEN2");
-  EXPECT_EQ(response2.AccessToken.Token, "ACCESSTOKEN3");
+  static_cast<void>(CredentialTestHelper::SimulateTokenRequest(
+      [](auto transport) {
+        TokenCredentialOptions options;
+        options.Transport.Transport = transport;
 
-  using namespace std::chrono_literals;
-  EXPECT_GE(response0.AccessToken.ExpiresOn, response0.EarliestExpiration + 3600s);
-  EXPECT_LE(response0.AccessToken.ExpiresOn, response0.LatestExpiration + 3600s);
+        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"},
+        });
 
-  EXPECT_GE(response1.AccessToken.ExpiresOn, response1.EarliestExpiration + 3600s);
-  EXPECT_LE(response1.AccessToken.ExpiresOn, response1.LatestExpiration + 3600s);
+        std::unique_ptr<ManagedIdentityCredential const> cloudShellManagedIdentityCredential;
+        EXPECT_THROW(
+            cloudShellManagedIdentityCredential = std::make_unique<ManagedIdentityCredential>(
+                ResourceIdentifier("abcdef01-2345-6789-9876-543210fedcba"), options),
+            AuthenticationException);
 
-  EXPECT_GE(response2.AccessToken.ExpiresOn, response2.EarliestExpiration + 4999s);
-  EXPECT_LE(response2.AccessToken.ExpiresOn, response2.LatestExpiration + 4999s);
+        return cloudShellManagedIdentityCredential;
+      },
+      {},
+      {"{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}"}));
 }
 
-TEST(ManagedIdentityCredential, CloudShellResourceId)
+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<ManagedIdentityCredential>(
-            ResourceIdentifier("abcdef01-2345-6789-9876-543210fedcba"), options);
+        return std::make_unique<ManagedIdentityCredential>(options);
       },
       {{"https://azure.com/.default"}, {"https://outlook.com/.default"}, {}},
       std::vector<std::string>{