[Identity] Improvements to MSAL support (#11407)

* Add in configurable http pipeline and cache * Add in configurable http pipeline and cache * Fix port, cache failure, console.logs * Fix port, cache failure, console.logs * Merge master, remove unused dep * Update to latest msal-node * Update to latest msal-node * Address feedback * Export AuthenticationRecord * Export AuthenticationRecord * Don't expose the access token
Azure · Sep 29, 2020 · b9eb1c5 · b9eb1c5
1 parent cc8c8fc
commit b9eb1c5
Show file tree

Hide file tree

Showing 7 changed files with 173 additions and 36 deletions.
diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml
diff --git a/sdk/identity/identity/package.json b/sdk/identity/identity/package.json
@@ -82,14 +82,13 @@
     "@azure/core-http": "^1.1.6",
     "@azure/core-tracing": "1.0.0-preview.9",
     "@azure/logger": "^1.0.0",
-    "@azure/msal-node": "^1.0.0-alpha.5",
+    "@azure/msal-node": "^1.0.0-alpha.8",
     "@opentelemetry/api": "^0.10.2",
     "events": "^3.0.0",
     "jws": "^4.0.0",
     "msal": "^1.0.2",
     "open": "^7.0.0",
     "qs": "^6.7.0",
-    "axios": "^0.19.2",
     "tslib": "^2.0.0",
     "uuid": "^8.1.0"
   },

diff --git a/sdk/identity/identity/review/identity.api.md b/sdk/identity/identity/review/identity.api.md
@@ -31,6 +31,15 @@ export class AuthenticationError extends Error {
 // @public
 export const AuthenticationErrorName = "AuthenticationError";
 
+// @public
+export interface AuthenticationRecord {
+    authority?: string;
+    environment: string;
+    homeAccountId: string;
+    tenantId: string;
+    username: string;
+}
+
 // @public
 export class AuthorizationCodeCredential implements TokenCredential {
     constructor(tenantId: string | "common", clientId: string, clientSecret: string, authorizationCode: string, redirectUri: string, options?: TokenCredentialOptions);
@@ -139,6 +148,13 @@ export class InteractiveBrowserCredential implements TokenCredential {
 
 // @public
 export interface InteractiveBrowserCredentialOptions extends TokenCredentialOptions {
+    authenticationRecord?: AuthenticationRecord;
+    cacheOptions?: {
+        cachePlugin?: {
+            readFromStorage: () => Promise<string>;
+            writeToStorage: (getMergedState: (oldState: string) => string) => Promise<void>;
+        };
+    };
     clientId?: string;
     loginStyle?: BrowserLoginStyle;
     postLogoutRedirectUri?: string | (() => string);

diff --git a/sdk/identity/identity/src/client/identityClient.ts b/sdk/identity/identity/src/client/identityClient.ts
@@ -10,8 +10,14 @@ import {
   RequestPrepareOptions,
   GetTokenOptions,
   createPipelineFromOptions,
-  isNode
+  isNode,
+  OperationArguments,
+  OperationSpec,
+  RawHttpHeaders,
+  HttpHeaders
 } from "@azure/core-http";
+import { INetworkModule, NetworkRequestOptions, NetworkResponse } from "@azure/msal-node";
+
 import { CanonicalCode } from "@opentelemetry/api";
 import { AuthenticationError, AuthenticationErrorName } from "./errors";
 import { createSpan } from "../util/tracing";
@@ -36,7 +42,7 @@ export interface TokenResponse {
   refreshToken?: string;
 }
 
-export class IdentityClient extends ServiceClient {
+export class IdentityClient extends ServiceClient implements INetworkModule {
   public authorityHost: string;
 
   constructor(options?: TokenCredentialOptions) {
@@ -184,6 +190,36 @@ export class IdentityClient extends ServiceClient {
     }
   }
 
+  sendGetRequestAsync<T>(
+    url: string,
+    options?: NetworkRequestOptions
+  ): Promise<NetworkResponse<T>> {
+    const webResource = new WebResource(url, "GET", options?.body, {}, options?.headers);
+
+    return this.sendRequest(webResource).then((response) => {
+      return {
+        body: response.parsedBody as T,
+        headers: response.headers.rawHeaders(),
+        status: response.status
+      };
+    });
+  }
+
+  sendPostRequestAsync<T>(
+    url: string,
+    options?: NetworkRequestOptions
+  ): Promise<NetworkResponse<T>> {
+    const webResource = new WebResource(url, "POST", options?.body, {}, options?.headers);
+
+    return this.sendRequest(webResource).then((response) => {
+      return {
+        body: response.parsedBody as T,
+        headers: response.headers.rawHeaders(),
+        status: response.status
+      };
+    });
+  }
+
   static getDefaultOptions(): TokenCredentialOptions {
     return {
       authorityHost: DefaultAuthorityHost

diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts
@@ -4,32 +4,30 @@
 /* eslint-disable @typescript-eslint/no-unused-vars */
 
 import { TokenCredential, GetTokenOptions, AccessToken } from "@azure/core-http";
-import { InteractiveBrowserCredentialOptions } from "./interactiveBrowserCredentialOptions";
+import {
+  InteractiveBrowserCredentialOptions,
+  AuthenticationRecord
+} from "./interactiveBrowserCredentialOptions";
 import { credentialLogger, formatError } from "../util/logging";
 import { TokenCredentialOptions, IdentityClient } from "../client/identityClient";
 import { DefaultTenantId, DeveloperSignOnClientId } from "../constants";
 import { Socket } from "net";
 
-const SERVER_PORT = process.env.PORT || 80;
-
 import express from "express";
-import { PublicClientApplication, TokenCache, AuthorizationCodeRequest } from "@azure/msal-node";
+import {
+  PublicClientApplication,
+  TokenCache,
+  AuthorizationCodeRequest,
+  Configuration
+} from "@azure/msal-node";
 import open from "open";
 import path from "path";
 import http from "http";
+import { CredentialUnavailable } from "../client/errors";
 
-const BrowserNotSupportedError = new Error(
-  "InteractiveBrowserCredential is not supported in Node.js."
-);
 const logger = credentialLogger("InteractiveBrowserCredential");
 
-interface AuthenticationRecord {
-  authority?: string;
-  homeAccountId: string;
-  environment: string;
-  tenantId: string;
-  username: string;
-}
+class AuthenticationRequired extends CredentialUnavailable {}
 
 /**
  * Enables authentication to Azure Active Directory inside of the web browser
@@ -46,15 +44,16 @@ export class InteractiveBrowserCredential implements TokenCredential {
   private redirectUri: string;
   private authorityHost: string;
   private authenticationRecord: AuthenticationRecord | undefined;
+  private port: number;
 
   constructor(options?: InteractiveBrowserCredentialOptions) {
     this.identityClient = new IdentityClient(options);
     this.tenantId = (options && options.tenantId) || DefaultTenantId;
     this.clientId = (options && options.clientId) || DeveloperSignOnClientId;
 
     // Future update: this is for persistent caching
-    this.persistenceEnabled = false;
-    this.authenticationRecord = undefined;
+    this.persistenceEnabled = this.persistenceEnabled = options?.cacheOptions !== undefined;
+    this.authenticationRecord = options?.authenticationRecord;
 
     if (options && options.redirectUri) {
       if (typeof options.redirectUri === "string") {
@@ -66,6 +65,12 @@ export class InteractiveBrowserCredential implements TokenCredential {
       this.redirectUri = "http://localhost";
     }
 
+    const url = new URL(this.redirectUri);
+    this.port = parseInt(url.port);
+    if (isNaN(this.port)) {
+      this.port = 80;
+    }
+
     if (options && options.authorityHost) {
       if (options.authorityHost.endsWith("/")) {
         this.authorityHost = options.authorityHost + this.tenantId;
@@ -76,13 +81,13 @@ export class InteractiveBrowserCredential implements TokenCredential {
       this.authorityHost = "https://login.microsoftonline.com/" + this.tenantId;
     }
 
-    const publicClientConfig = {
+    const publicClientConfig: Configuration = {
       auth: {
         clientId: this.clientId,
-        authority: this.authorityHost,
-        redirectUri: this.redirectUri
+        authority: this.authorityHost
       },
-      cache: undefined
+      cache: options?.cacheOptions,
+      system: { networkClient: this.identityClient }
     };
     this.pca = new PublicClientApplication(publicClientConfig);
     this.msalCacheManager = this.pca.getTokenCache();
@@ -104,7 +109,37 @@ export class InteractiveBrowserCredential implements TokenCredential {
   ): Promise<AccessToken | null> {
     const scopeArray = typeof scopes === "object" ? scopes : [scopes];
 
-    return this.acquireTokenFromBrowser(scopeArray);
+    if (this.authenticationRecord && this.persistenceEnabled) {
+      return this.acquireTokenFromCache().catch((e) => {
+        if (e instanceof AuthenticationRequired) {
+          return this.acquireTokenFromBrowser(scopeArray);
+        } else {
+          throw e;
+        }
+      });
+    } else {
+      return this.acquireTokenFromBrowser(scopeArray);
+    }
+  }
+
+  private async acquireTokenFromCache(): Promise<AccessToken | null> {
+    await this.msalCacheManager.readFromPersistence();
+
+    const silentRequest = {
+      account: this.authenticationRecord!,
+      scopes: ["https://vault.azure.net/user_impersonation", "https://vault.azure.net/.default"]
+    };
+
+    try {
+      const response = await this.pca.acquireTokenSilent(silentRequest);
+      logger.info("Successful silent token acquisition");
+      return {
+        expiresOnTimestamp: response.expiresOn.getTime(),
+        token: response.accessToken
+      };
+    } catch (e) {
+      throw new AuthenticationRequired("Could not authenticate silently using the cache");
+    }
   }
 
   private async openAuthCodeUrl(scopeArray: string[]): Promise<void> {
@@ -115,6 +150,10 @@ export class InteractiveBrowserCredential implements TokenCredential {
 
     const response = await this.pca.getAuthCodeUrl(authCodeUrlParameters);
     await open(response);
+
+    if (this.persistenceEnabled) {
+      await this.msalCacheManager.readFromPersistence();
+    }
   }
 
   private async acquireTokenFromBrowser(scopeArray: string[]): Promise<AccessToken | null> {
@@ -146,7 +185,7 @@ export class InteractiveBrowserCredential implements TokenCredential {
         try {
           const authResponse = await this.pca.acquireTokenByCode(tokenRequest);
           res.sendStatus(200);
-          logger.info(`authResponse: ${authResponse}`);
+
           if (this.persistenceEnabled) {
             this.msalCacheManager.writeToPersistence();
           }
@@ -168,8 +207,8 @@ export class InteractiveBrowserCredential implements TokenCredential {
         }
       });
 
-      listen = app.listen(SERVER_PORT, () =>
-        logger.info(`Msal Node Auth Code Sample app listening on port ${SERVER_PORT}!`)
+      listen = app.listen(this.port, () =>
+        logger.info(`Msal Node Auth Code Sample app listening on port ${this.port}!`)
       );
       listen.on("connection", (socket) => (socketToDestroy = socket));
 

diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredentialOptions.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredentialOptions.ts
@@ -13,6 +13,36 @@ import { TokenCredentialOptions } from "../client/identityClient";
  */
 export type BrowserLoginStyle = "redirect" | "popup";
 
+/**
+ * The record to use to find the cached tokens in the cache
+ */
+export interface AuthenticationRecord {
+  /**
+   * The associated authority, if used
+   */
+  authority?: string;
+
+  /**
+   * The home account Id
+   */
+  homeAccountId: string;
+
+  /**
+   * The login environment, eg "login.windows.net"
+   */
+  environment: string;
+
+  /**
+   * The associated tenant ID
+   */
+  tenantId: string;
+
+  /**
+   * The username of the logged in account
+   */
+  username: string;
+}
+
 /**
  * Defines options for the InteractiveBrowserCredential class.
  */
@@ -46,4 +76,19 @@ export interface InteractiveBrowserCredentialOptions extends TokenCredentialOpti
    * The client (application) ID of an App Registration in the tenant.
    */
   clientId?: string;
+
+  /**
+   * The cache options to use when credentials are being checked.
+   */
+  cacheOptions?: {
+    cachePlugin?: {
+      readFromStorage: () => Promise<string>;
+      writeToStorage: (getMergedState: (oldState: string) => string) => Promise<void>;
+    };
+  };
+
+  /**
+   * The authentication record to use to find existing tokens in the cache
+   */
+  authenticationRecord?: AuthenticationRecord;
 }
diff --git a/sdk/identity/identity/src/index.ts b/sdk/identity/identity/src/index.ts
@@ -19,7 +19,8 @@ export { AzureCliCredential } from "./credentials/azureCliCredential";
 
 export {
   InteractiveBrowserCredentialOptions,
-  BrowserLoginStyle
+  BrowserLoginStyle,
+  AuthenticationRecord
 } from "./credentials/interactiveBrowserCredentialOptions";
 export { ManagedIdentityCredential } from "./credentials/managedIdentityCredential";
 export {