[Identity] Cleanups (#18532)

This PR contains several cleanups on the Identity project. These include: - Moved the `test/manual` files to `test/manual/interactive-browser-credential`. - `samples/manual` to `test/manual/authorization-code-credential`. - `TokenCredentialOptions` to its own file. - `ManagedIdentityCredential` cleanups (mostly that I removed the `msiGenericGetToken`). - Very small cleanups on the credentials (mostly comments). - MSAL common files to start with "msal" (`msalBrowserCommon`, and `msalNodeCommon`), to make them easier to find. I always had a hard time finding them since they are named differently than the other files in those folders. - Removed `any` types from the internal implementation of the `ManagedIdentityCredential`. This PR is part of the work mentioned in #17489 , but it won’t fully solve it.
Azure · Nov 6, 2021 · 746bb4f · 746bb4f
1 parent 489c703
commit 746bb4f
Show file tree

Hide file tree

Showing 66 changed files with 291 additions and 220 deletions.
diff --git a/...dentity/identity-cache-persistence/test/internal/node/clientCertificateCredential.spec.ts b/...dentity/identity-cache-persistence/test/internal/node/clientCertificateCredential.spec.ts
@@ -15,7 +15,7 @@ import {
   TokenCachePersistenceOptions
 } from "../../../../identity/src";
 import { MsalTestCleanup, msalNodeTestSetup } from "../../../../identity/test/msalTestUtils";
-import { MsalNode } from "../../../../identity/src/msal/nodeFlows/nodeCommon";
+import { MsalNode } from "../../../../identity/src/msal/nodeFlows/msalNodeCommon";
 
 import { createPersistence } from "./setup.spec";
 

diff --git a/sdk/identity/identity-cache-persistence/test/internal/node/clientSecretCredential.spec.ts b/sdk/identity/identity-cache-persistence/test/internal/node/clientSecretCredential.spec.ts
@@ -11,7 +11,7 @@ import { ConfidentialClientApplication } from "@azure/msal-node";
 
 import { ClientSecretCredential, TokenCachePersistenceOptions } from "../../../../identity/src";
 import { MsalTestCleanup, msalNodeTestSetup } from "../../../../identity/test/msalTestUtils";
-import { MsalNode } from "../../../../identity/src/msal/nodeFlows/nodeCommon";
+import { MsalNode } from "../../../../identity/src/msal/nodeFlows/msalNodeCommon";
 
 import { createPersistence } from "./setup.spec";
 

diff --git a/sdk/identity/identity-cache-persistence/test/internal/node/deviceCodeCredential.spec.ts b/sdk/identity/identity-cache-persistence/test/internal/node/deviceCodeCredential.spec.ts
@@ -11,7 +11,7 @@ import { isLiveMode } from "@azure-tools/test-recorder";
 
 import { DeviceCodeCredential, TokenCachePersistenceOptions } from "../../../../identity/src";
 import { MsalTestCleanup, msalNodeTestSetup } from "../../../../identity/test/msalTestUtils";
-import { MsalNode } from "../../../../identity/src/msal/nodeFlows/nodeCommon";
+import { MsalNode } from "../../../../identity/src/msal/nodeFlows/msalNodeCommon";
 
 import { createPersistence } from "./setup.spec";
 

diff --git a/...identity/identity-cache-persistence/test/internal/node/usernamePasswordCredential.spec.ts b/...identity/identity-cache-persistence/test/internal/node/usernamePasswordCredential.spec.ts
@@ -11,7 +11,7 @@ import { PublicClientApplication } from "@azure/msal-node";
 
 import { UsernamePasswordCredential, TokenCachePersistenceOptions } from "../../../../identity/src";
 import { MsalTestCleanup, msalNodeTestSetup } from "../../../../identity/test/msalTestUtils";
-import { MsalNode } from "../../../../identity/src/msal/nodeFlows/nodeCommon";
+import { MsalNode } from "../../../../identity/src/msal/nodeFlows/msalNodeCommon";
 
 import { createPersistence } from "./setup.spec";
 

diff --git a/sdk/identity/identity/src/client/identityClient.ts b/sdk/identity/identity/src/client/identityClient.ts
@@ -2,9 +2,9 @@
 // Licensed under the MIT license.
 
 import { INetworkModule, NetworkRequestOptions, NetworkResponse } from "@azure/msal-common";
-import { CommonClientOptions, ServiceClient } from "@azure/core-client";
 import { AccessToken, GetTokenOptions } from "@azure/core-auth";
 import { SpanStatusCode } from "@azure/core-tracing";
+import { ServiceClient } from "@azure/core-client";
 import { isNode } from "@azure/core-util";
 import {
   createHttpHeaders,
@@ -17,6 +17,7 @@ import { getIdentityTokenEndpointSuffix } from "../util/identityTokenEndpoint";
 import { DefaultAuthorityHost } from "../constants";
 import { createSpan } from "../util/tracing";
 import { logger } from "../util/logging";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 
 const noCorrelationId = "noCorrelationId";
 
@@ -36,6 +37,19 @@ export interface TokenResponse {
   refreshToken?: string;
 }
 
+/**
+ * Internal type roughly matching the raw responses of the authentication endpoints.
+ *
+ * @internal
+ */
+export interface TokenResponseParsedBody {
+  token?: string;
+  access_token?: string;
+  refresh_token?: string;
+  expires_in: number;
+  expires_on?: number | string;
+}
+
 /**
  * @internal
  */
@@ -89,23 +103,19 @@ export class IdentityClient extends ServiceClient implements INetworkModule {
 
   async sendTokenRequest(
     request: PipelineRequest,
-    expiresOnParser?: (responseBody: any) => number
+    expiresOnParser?: (responseBody: TokenResponseParsedBody) => number
   ): Promise<TokenResponse | null> {
     logger.info(`IdentityClient: sending token request to [${request.url}]`);
     const response = await this.sendRequest(request);
 
     expiresOnParser =
       expiresOnParser ||
-      ((responseBody: any) => {
+      ((responseBody: TokenResponseParsedBody) => {
         return Date.now() + responseBody.expires_in * 1000;
       });
 
     if (response.bodyAsText && (response.status === 200 || response.status === 201)) {
-      const parsedBody: {
-        token?: string;
-        access_token?: string;
-        refresh_token?: string;
-      } = JSON.parse(response.bodyAsText);
+      const parsedBody: TokenResponseParsedBody = JSON.parse(response.bodyAsText);
 
       if (!parsedBody.access_token) {
         return null;
@@ -138,7 +148,7 @@ export class IdentityClient extends ServiceClient implements INetworkModule {
     scopes: string,
     refreshToken: string | undefined,
     clientSecret: string | undefined,
-    expiresOnParser?: (responseBody: any) => number,
+    expiresOnParser?: (responseBody: TokenResponseParsedBody) => number,
     options?: GetTokenOptions
   ): Promise<TokenResponse | null> {
     if (refreshToken === undefined) {
@@ -295,16 +305,3 @@ export class IdentityClient extends ServiceClient implements INetworkModule {
     };
   }
 }
-
-/**
- * Provides options to configure how the Identity library makes authentication
- * requests to Azure Active Directory.
- */
-export interface TokenCredentialOptions extends CommonClientOptions {
-  /**
-   * The authority host to use for authentication requests.
-   * Possible values are available through {@link AzureAuthorityHosts}.
-   * The default is "https://login.microsoftonline.com".
-   */
-  authorityHost?: string;
-}
diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredential.browser.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredential.browser.ts
@@ -3,7 +3,7 @@
 
 import { TokenCredential, AccessToken } from "@azure/core-auth";
 
-import { TokenCredentialOptions } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 import { credentialLogger, formatError } from "../util/logging";
 
 const BrowserNotSupportedError = new Error(

diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts
@@ -2,7 +2,7 @@
 // Licensed under the MIT license.
 
 import { TokenCredential, GetTokenOptions, AccessToken } from "@azure/core-auth";
-import { TokenCredentialOptions } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 import { credentialLogger } from "../util/logging";
 import { checkTenantId } from "../util/checkTenantId";
 import { MsalAuthorizationCode } from "../msal/nodeFlows/msalAuthorizationCode";
@@ -101,7 +101,7 @@ export class AuthorizationCodeCredential implements TokenCredential {
       // the clientId+clientSecret constructor
       this.authorizationCode = authorizationCodeOrRedirectUri;
       this.redirectUri = redirectUriOrOptions;
-      // options okay
+      // in this case, options are good as they come
     } else {
       // clientId only
       this.authorizationCode = clientSecretOrAuthorizationCode;

diff --git a/sdk/identity/identity/src/credentials/azureApplicationCredential.browser.ts b/sdk/identity/identity/src/credentials/azureApplicationCredential.browser.ts
@@ -3,7 +3,7 @@
 
 import { AccessToken } from "@azure/core-auth";
 
-import { TokenCredentialOptions } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 import { credentialLogger, formatError } from "../util/logging";
 import { ChainedTokenCredential } from "./chainedTokenCredential";
 

diff --git a/sdk/identity/identity/src/credentials/azureApplicationCredential.ts b/sdk/identity/identity/src/credentials/azureApplicationCredential.ts
@@ -5,7 +5,7 @@
 // Licensed under the MIT license.
 
 import { TokenCredential } from "@azure/core-auth";
-import { TokenCredentialOptions } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 import { ChainedTokenCredential } from "./chainedTokenCredential";
 import { EnvironmentCredential } from "./environmentCredential";
 import { CredentialPersistenceOptions } from "./credentialPersistenceOptions";
@@ -46,8 +46,7 @@ export class AzureApplicationCredential extends ChainedTokenCredential {
    * Creates an instance of the AzureApplicationCredential class.
    *
    * The AzureApplicationCredential provides a default {@link ChainedTokenCredential} configuration that should
-   * work for most applications that use the Azure SDK.  The following credential
-   * types will be tried, in order:
+   * work for most applications deployed on Azure. The following credential types will be tried, in order:
    *
    * - {@link EnvironmentCredential}
    * - {@link ManagedIdentityCredential}

diff --git a/sdk/identity/identity/src/credentials/azureCliCredential.ts b/sdk/identity/identity/src/credentials/azureCliCredential.ts
@@ -117,7 +117,7 @@ export class AzureCliCredential implements TokenCredential {
 
     let responseData = "";
 
-    const { span } = createSpan("AzureCliCredential.getToken", options);
+    const { span } = createSpan(`${this.constructor.name}.getToken`, options);
 
     try {
       const obj = await cliCredentialInternals.getAzureCliAccessToken(resource, tenantId);

diff --git a/sdk/identity/identity/src/credentials/azureCliCredentialOptions.ts b/sdk/identity/identity/src/credentials/azureCliCredentialOptions.ts
@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { TokenCredentialOptions } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 
 /**
  * Options for the {@link AzureCliCredential}

diff --git a/sdk/identity/identity/src/credentials/azurePowerShellCredentialOptions.ts b/sdk/identity/identity/src/credentials/azurePowerShellCredentialOptions.ts
@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { TokenCredentialOptions } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 
 /**
  * Options for the {@link AzurePowerShellCredential}

diff --git a/sdk/identity/identity/src/credentials/clientCertificateCredentialOptions.ts b/sdk/identity/identity/src/credentials/clientCertificateCredentialOptions.ts
@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { TokenCredentialOptions } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 import { CredentialPersistenceOptions } from "./credentialPersistenceOptions";
 
 /**

diff --git a/sdk/identity/identity/src/credentials/clientSecretCredential.browser.ts b/sdk/identity/identity/src/credentials/clientSecretCredential.browser.ts
@@ -6,7 +6,8 @@ import { createHttpHeaders, createPipelineRequest } from "@azure/core-rest-pipel
 import { SpanStatusCode } from "@azure/core-tracing";
 import { credentialLogger, formatError, formatSuccess } from "../util/logging";
 import { getIdentityTokenEndpointSuffix } from "../util/identityTokenEndpoint";
-import { TokenCredentialOptions, IdentityClient } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
+import { IdentityClient } from "../client/identityClient";
 import { createSpan } from "../util/tracing";
 
 const logger = credentialLogger("ClientSecretCredential");
@@ -66,7 +67,7 @@ export class ClientSecretCredential implements TokenCredential {
     options?: GetTokenOptions
   ): Promise<AccessToken | null> {
     const { span, updatedOptions: newOptions } = createSpan(
-      "ClientSecretCredential.getToken",
+      `${this.constructor.name}.getToken`,
       options
     );
 

diff --git a/sdk/identity/identity/src/credentials/clientSecretCredentialOptions.ts b/sdk/identity/identity/src/credentials/clientSecretCredentialOptions.ts
@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { TokenCredentialOptions } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 import { CredentialPersistenceOptions } from "./credentialPersistenceOptions";
 
 /**

diff --git a/sdk/identity/identity/src/credentials/credentialPersistenceOptions.ts b/sdk/identity/identity/src/credentials/credentialPersistenceOptions.ts
@@ -32,7 +32,7 @@ export interface CredentialPersistenceOptions {
    * }
    *
    * main().catch((error) => {
-   *   console.error("An error occured:", error);
+   *   console.error("An error occurred:", error);
    *   process.exit(1);
    * });
    * ```

diff --git a/sdk/identity/identity/src/credentials/defaultAzureCredential.browser.ts b/sdk/identity/identity/src/credentials/defaultAzureCredential.browser.ts
@@ -3,7 +3,7 @@
 
 import { AccessToken } from "@azure/core-auth";
 
-import { TokenCredentialOptions } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 import { credentialLogger, formatError } from "../util/logging";
 import { ChainedTokenCredential } from "./chainedTokenCredential";
 

diff --git a/sdk/identity/identity/src/credentials/defaultAzureCredential.ts b/sdk/identity/identity/src/credentials/defaultAzureCredential.ts
@@ -3,7 +3,7 @@
 
 import { TokenCredential } from "@azure/core-auth";
 
-import { TokenCredentialOptions } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 
 import { ChainedTokenCredential } from "./chainedTokenCredential";
 

diff --git a/sdk/identity/identity/src/credentials/environmentCredential.ts b/sdk/identity/identity/src/credentials/environmentCredential.ts
@@ -4,7 +4,7 @@
 import { AccessToken, TokenCredential, GetTokenOptions } from "@azure/core-auth";
 
 import { credentialLogger, processEnvVars, formatSuccess, formatError } from "../util/logging";
-import { TokenCredentialOptions } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 import { ClientSecretCredential } from "./clientSecretCredential";
 import { AuthenticationError, CredentialUnavailableError } from "../errors";
 import { checkTenantId } from "../util/checkTenantId";
@@ -28,7 +28,8 @@ export const AllSupportedEnvironmentVariables = [
   "AZURE_PASSWORD"
 ];
 
-const logger = credentialLogger("EnvironmentCredential");
+const credentialName = "EnvironmentCredential";
+const logger = credentialLogger(credentialName);
 
 /**
  * Enables authentication to Azure Active Directory depending on the available environment variables.
@@ -124,16 +125,15 @@ export class EnvironmentCredential implements TokenCredential {
    * @param options - Optional parameters. See {@link GetTokenOptions}.
    */
   async getToken(scopes: string | string[], options: GetTokenOptions = {}): Promise<AccessToken> {
-    return trace("EnvironmentCredential.getToken", options, async (newOptions) => {
+    return trace(`${credentialName}.getToken`, options, async (newOptions) => {
       if (this._credential) {
         try {
           const result = await this._credential.getToken(scopes, newOptions);
           logger.getToken.info(formatSuccess(scopes));
           return result;
         } catch (err) {
           const authenticationError = new AuthenticationError(400, {
-            error:
-              "EnvironmentCredential authentication failed. To troubleshoot, visit https://aka.ms/azsdk/js/identity/environmentcredential/troubleshoot.",
+            error: `${credentialName} authentication failed. To troubleshoot, visit https://aka.ms/azsdk/js/identity/environmentcredential/troubleshoot.`,
             error_description: err.message
               .toString()
               .split("More details:")
@@ -144,7 +144,7 @@ export class EnvironmentCredential implements TokenCredential {
         }
       }
       throw new CredentialUnavailableError(
-        "EnvironmentCredential is unavailable. No underlying credential could be used. To troubleshoot, visit https://aka.ms/azsdk/js/identity/environmentcredential/troubleshoot."
+        `${credentialName} is unavailable. No underlying credential could be used. To troubleshoot, visit https://aka.ms/azsdk/js/identity/environmentcredential/troubleshoot.`
       );
     });
   }

diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.browser.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.browser.ts
@@ -8,7 +8,7 @@ import { trace } from "../util/tracing";
 import { MsalFlow } from "../msal/flows";
 import { AuthenticationRecord } from "../msal/types";
 import { MSALAuthCode } from "../msal/browserFlows/msalAuthCode";
-import { MsalBrowserFlowOptions } from "../msal/browserFlows/browserCommon";
+import { MsalBrowserFlowOptions } from "../msal/browserFlows/msalBrowserCommon";
 import {
   InteractiveBrowserCredentialInBrowserOptions,
   InteractiveBrowserCredentialNodeOptions

diff --git a/sdk/identity/identity/src/credentials/interactiveCredentialOptions.ts b/sdk/identity/identity/src/credentials/interactiveCredentialOptions.ts
@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { TokenCredentialOptions } from "../client/identityClient";
+import { TokenCredentialOptions } from "../tokenCredentialOptions";
 import { AuthenticationRecord } from "../msal/types";
 
 /**