[Identity] Fixed bug on 2.0.3 (#20444)

sadasant · witemple-msft · web-flow · commit 994ad01bb529 · 2022-02-18T13:09:53.000-05:00
* [Identity] Fixed bug on 2.0.3

2.0.3 contains a regression: Passing a client Id besides the options argument on the `ManagedIdentityCredential` would discard the options argument. We have fixed it and we have added tests.

* Update sdk/identity/identity/CHANGELOG.md

Co-authored-by: Will Temple &lt;witemple@microsoft.com&gt;

* one more test, to reflect the feedback from Will

Co-authored-by: Will Temple &lt;witemple@microsoft.com&gt;
diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md
@@ -1,14 +1,10 @@
 # Release History
 
-## 2.0.4 (Unreleased)
-
-### Features Added
-
-### Breaking Changes
+## 2.0.4 (2022-02-18)
 
 ### Bugs Fixed
 
-### Other Changes
+- Fixed a regression in version 2.0.3 in which providing an options bag, but _not_ a client ID, to the `ManagedIdentityCredential` constructor would discard the `options` parameter.
 
 ## 2.0.3 (2022-02-16)
 
diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts
@@ -62,7 +62,7 @@ export class ManagedIdentityCredential implements TokenCredential {
       _options = options;
     } else {
       // options only constructor
-      _options = options;
+      _options = clientIdOrOptions;
     }
     this.identityClient = new IdentityClient(_options);
     this.isAvailableIdentityClient = new IdentityClient({
diff --git a/sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts b/sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts
@@ -299,6 +299,62 @@ describe("ManagedIdentityCredential", function () {
     );
   });
 
+  it("IMDS MSI accepts a custom set of retries, even when client Id is passed through the first parameter", async function () {
+    const { error } = await testContext.sendCredentialRequests({
+      scopes: ["scopes"],
+      credential: new ManagedIdentityCredential("errclient", {
+        retryOptions: {
+          maxRetries: 4,
+        },
+      }),
+      insecureResponses: [
+        // Any response on the ping request is fine, since it means that the endpoint is indeed there.
+        createResponse(503, {}, { "Retry-After": "2" }),
+        // After the ping, we try to get a token from the IMDS endpoint.
+        createResponse(503, {}, { "Retry-After": "2" }),
+        createResponse(503, {}, { "Retry-After": "2" }),
+        createResponse(503, {}, { "Retry-After": "2" }),
+        createResponse(503, {}, { "Retry-After": "2" }),
+        // This is the extra one
+        createResponse(503, {}, { "Retry-After": "2" }),
+      ],
+    });
+
+    assert.ok(error?.message);
+    assert.equal(
+      error?.message.split("\n")[0],
+      "ManagedIdentityCredential authentication failed. Status code: 503"
+    );
+  });
+
+  it("IMDS MSI accepts a custom set of retries, even when client Id is not passed through the first parameter", async function () {
+    const { error } = await testContext.sendCredentialRequests({
+      scopes: ["scopes"],
+      credential: new ManagedIdentityCredential({
+        retryOptions: {
+          maxRetries: 4,
+        },
+      }),
+      insecureResponses: [
+        // Any response on the ping request is fine, since it means that the endpoint is indeed there.
+        createResponse(503, {}, { "Retry-After": "2" }),
+        // After the ping, we try to get a token from the IMDS endpoint.
+        createResponse(503, {}, { "Retry-After": "2" }),
+        createResponse(503, {}, { "Retry-After": "2" }),
+        createResponse(503, {}, { "Retry-After": "2" }),
+        createResponse(503, {}, { "Retry-After": "2" }),
+        // This is the extra one
+        createResponse(503, {}, { "Retry-After": "2" }),
+      ],
+    });
+
+    assert.ok(error?.message);
+    assert.equal(
+      error?.message.split("\n")[0],
+      "ManagedIdentityCredential authentication failed. Status code: 503"
+    );
+  });
+
   it("IMDS MSI skips verification if the AZURE_POD_IDENTITY_AUTHORITY_HOST environment variable is available", async function () {
     process.env.AZURE_POD_IDENTITY_AUTHORITY_HOST = "token URL";
 

Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ export class ManagedIdentityCredential implements TokenCredential {`
`62`	`62`	`_options = options;`
`63`	`63`	`} else {`
`64`	`64`	`// options only constructor`
`65`		`- _options = options;`
	`65`	`+ _options = clientIdOrOptions;`
`66`	`66`	`}`
`67`	`67`	`this.identityClient = new IdentityClient(_options);`
`68`	`68`	`this.isAvailableIdentityClient = new IdentityClient({`