[Identity] Cleanups after end-to-end testing and meetings (Azure#12347)

* [Identity] Cleanups after end-to-end testing and meetings

* test fixes

Author: sadasant

sdk/identity/identity/src/credentials/managedIdentityCredential/arcMsi.ts

@@ -15,16 +15,12 @@ const logger = credentialLogger("ManagedIdentityCredential - ArcMSI");
 // Azure Arc MSI doesn't have a special expiresIn parser.
 const expiresInParser = undefined;
 
-function prepareRequestOptions(resource?: string, clientId?: string): RequestPrepareOptions {
+function prepareRequestOptions(resource?: string): RequestPrepareOptions {
   const queryParameters: any = {
     resource,
     "api-version": azureArcAPIVersion
   };
 
-  if (clientId) {
-    queryParameters.client_id = clientId;
-  }
-
   return {
     // Should be similar to: http://localhost:40342/metadata/identity/oauth2/token
     url: process.env.IDENTITY_ENDPOINT,
@@ -84,12 +80,18 @@ export const arcMsi: MSI = {
   ): Promise<AccessToken | null> {
     logger.info(`Using the Azure Arc MSI to authenticate.`);
 
+    if (clientId) {
+      throw new Error(
+        "User assigned identity is not supported by the Azure Arc Managed Identity Endpoint. To authenticate with the system assigned identity omit the client id when constructing the ManagedIdentityCredential, or if authenticating with the DefaultAzureCredential ensure the AZURE_CLIENT_ID environment variable is not set."
+      );
+    }
+
     const requestOptions = {
       disableJsonStringifyOnBody: true,
       deserializationMapper: undefined,
       abortSignal: getTokenOptions.abortSignal,
       spanOptions: getTokenOptions.tracingOptions && getTokenOptions.tracingOptions.spanOptions,
-      ...prepareRequestOptions(resource, clientId)
+      ...prepareRequestOptions(resource)
     };
 
     const filePath = await filePathRequest(identityClient, requestOptions);

sdk/identity/identity/src/credentials/managedIdentityCredential/constants.ts

@@ -5,4 +5,5 @@ export const DefaultScopeSuffix = "/.default";
 
 export const imdsEndpoint = "http://169.254.169.254/metadata/identity/oauth2/token";
 export const imdsApiVersion = "2018-02-01";
-export const azureArcAPIVersion = "2019-08-15";
+export const azureArcAPIVersion = "2019-11-01";
+export const azureFabricVersion = "2019-07-01-preview";

sdk/identity/identity/src/credentials/managedIdentityCredential/fabricMsi.ts

@@ -6,6 +6,7 @@ import { MSI } from "./models";
 import { credentialLogger } from "../../util/logging";
 import { IdentityClient } from "../../client/identityClient";
 import { msiGenericGetToken } from "./utils";
+import { azureFabricVersion } from "./constants";
 
 const logger = credentialLogger("ManagedIdentityCredential - Fabric MSI");
 
@@ -17,7 +18,7 @@ function expiresInParser(requestBody: any): number {
 function prepareRequestOptions(resource: string, clientId?: string): RequestPrepareOptions {
   const queryParameters: any = {
     resource,
-    "api-version": "2019-07-01-preview"
+    "api-version": azureFabricVersion
   };
 
   if (clientId) {

sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts

@@ -17,7 +17,6 @@ import { imdsMsi } from "./imdsMsi";
 import { MSI } from "./models";
 import { appServiceMsi2017 } from "./appServiceMsi2017";
 import { arcMsi } from "./arcMsi";
-import { fabricMsi } from "./fabricMsi";
 
 const logger = credentialLogger("ManagedIdentityCredential");
 
@@ -78,7 +77,9 @@ export class ManagedIdentityCredential implements TokenCredential {
       return this.cachedMSI;
     }
 
-    const MSIs = [fabricMsi, appServiceMsi2017, cloudShellMsi, arcMsi, imdsMsi];
+    // "fabricMsi" can't be added yet because our HTTPs pipeline doesn't allow skipping the SSL verification step,
+    // which is necessary since Service Fabric only provides self-signed certificates on their Identity Endpoint.
+    const MSIs = [appServiceMsi2017, cloudShellMsi, arcMsi, imdsMsi];
 
     for (const msi of MSIs) {
       if (await msi.isAvailable(this.identityClient, resource, clientId, getTokenOptions)) {

sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts

@@ -17,7 +17,7 @@ interface AuthRequestDetails {
   token: AccessToken | null;
 }
 
-describe("ManagedIdentityCredential", function() {
+describe("ManagedIdentityCredential", function () {
   afterEach(() => {
     delete process.env.IDENTITY_ENDPOINT;
     delete process.env.IDENTITY_HEADER;
@@ -26,7 +26,7 @@ describe("ManagedIdentityCredential", function() {
     delete process.env.IDENTITY_SERVER_THUMBPRINT;
   });
 
-  it("sends an authorization request with a modified resource name", async function() {
+  it("sends an authorization request with a modified resource name", async function () {
     const authDetails = await getMsiTokenAuthRequest(["https://service/.default"], "client", {
       authResponse: [
         { status: 200 }, // Respond to IMDS isAvailable
@@ -83,7 +83,7 @@ describe("ManagedIdentityCredential", function() {
     }
   });
 
-  it("returns error when ManagedIdentityCredential authentication failed", async function() {
+  it("returns error when ManagedIdentityCredential authentication failed", async function () {
     process.env.AZURE_CLIENT_ID = "errclient";
 
     const errResponse: OAuthErrorResponse = {
@@ -211,7 +211,7 @@ describe("ManagedIdentityCredential", function() {
       [`${filePath}`]: key
     });
 
-    const authDetails = await getMsiTokenAuthRequest(["https://service/.default"], "client", {
+    const authDetails = await getMsiTokenAuthRequest(["https://service/.default"], undefined, {
       authResponse: [
         {
           status: 401,
@@ -234,7 +234,6 @@ describe("ManagedIdentityCredential", function() {
     assert.ok(validationRequest.query, "No query string parameters on request");
 
     assert.equal(validationRequest.method, "GET");
-    assert.equal(validationRequest.query!["client_id"], "client");
     assert.equal(decodeURIComponent(validationRequest.query!["resource"]), "https://service");
 
     assert.ok(
@@ -247,7 +246,6 @@ describe("ManagedIdentityCredential", function() {
     assert.ok(authRequest.query, "No query string parameters on request");
 
     assert.equal(authRequest.method, "GET");
-    assert.equal(authRequest.query!["client_id"], "client");
     assert.equal(decodeURIComponent(authRequest.query!["resource"]), "https://service");
 
     assert.ok(
@@ -259,15 +257,18 @@ describe("ManagedIdentityCredential", function() {
     if (authDetails.token) {
       // We use Date.now underneath.
       assert.equal(
-        Math.floor(authDetails.token.expiresOnTimestamp / 100000),
-        Math.floor(Date.now() / 100000)
+        Math.floor(authDetails.token.expiresOnTimestamp / 1000000),
+        Math.floor(Date.now() / 1000000)
       );
     } else {
       assert.fail("No token was returned!");
     }
   });
 
-  it("sends an authorization request correctly in an Azure Fabric environment", async () => {
+  // "fabricMsi" isn't part of the ManagedIdentityCredential MSIs yet
+  // because our HTTPs pipeline doesn't allow skipping the SSL verification step,
+  // which is necessary since Service Fabric only provides self-signed certificates on their Identity Endpoint.
+  it.skip("sends an authorization request correctly in an Azure Fabric environment", async () => {
     // Trigger App Service behavior by setting environment variables
     process.env.IDENTITY_ENDPOINT = "https://endpoint";
     process.env.IDENTITY_HEADER = "secret";