[Identity] Refactor Azure.Core and Azure.Identity re: Arch Board feed…

…back (#3681)

This change draws on work done by schaabs in
Azure/azure-sdk-for-net#6525 to refactor Azure.Core and Azure.Identity
to respond to architecture board feedback.  It also makes a couple of
TypeScript-specific tweaks that were discussed at the time
@azure/identity was introduced.

Fixes #3535.
Fixes #3529.

sdk/core/core-http/lib/coreHttp.ts

@@ -43,7 +43,7 @@ export {
 export { URLBuilder, URLQuery } from "./url";
 
 // Credentials
-export { TokenCredential } from "./credentials/tokenCredential";
+export { TokenCredential, GetTokenOptions, AccessToken } from "./credentials/tokenCredential";
 export { TokenCredentials } from "./credentials/tokenCredentials";
 export { BasicAuthenticationCredentials } from "./credentials/basicAuthenticationCredentials";
 export { ApiKeyCredentials, ApiKeyCredentialOptions } from "./credentials/apiKeyCredentials";

sdk/core/core-http/lib/credentials/tokenCredential.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
-import { RequestOptionsBase } from "../webResource";
+import { AbortSignalLike } from "../webResource";
 
 /**
  * Represents a credential capable of providing an authentication token.
@@ -11,11 +11,37 @@ export interface TokenCredential {
    * Gets the token provided by this credential.
    *
    * @param scopes The list of scopes for which the token will have access.
-   * @param requestOptions The RequestOptionsBase used to configure any requests
-   *                       this TokenCredential implementation might make.
+   * @param options The options used to configure any requests this
+   *                TokenCredential implementation might make.
    */
   getToken(
     scopes: string | string[],
-    requestOptions?: RequestOptionsBase
-  ): Promise<string | null>;
+    options?: GetTokenOptions
+  ): Promise<AccessToken | null>;
+}
+
+/**
+ * Defines options for TokenCredential.getToken.
+ */
+export interface GetTokenOptions {
+  /**
+   * An AbortSignalLike implementation that can be used to cancel
+   * the token request.
+   */
+  abortSignal?: AbortSignalLike;
+}
+
+/**
+ * Represents an access token with an expiration time.
+ */
+export interface AccessToken {
+  /**
+   * The access token.
+   */
+  token: string;
+
+  /**
+   * The access token's expiration date and time.
+   */
+  expiresOn: Date;
 }

sdk/core/core-http/lib/policies/bearerTokenAuthenticationPolicy.ts

@@ -1,13 +1,15 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
-import { TokenCredential } from "../credentials/tokenCredential";
+import { TokenCredential, AccessToken, GetTokenOptions } from "../credentials/tokenCredential";
 import { BaseRequestPolicy, RequestPolicy, RequestPolicyOptions, RequestPolicyFactory } from "../policies/requestPolicy";
 import { Constants } from "../util/constants";
 import { HttpOperationResponse } from "../httpOperationResponse";
 import { HttpHeaders, } from "../httpHeaders";
 import { WebResource } from "../webResource";
 
+export const TokenRefreshBufferMs = 2 * 60 * 1000; // 2 Minutes
+
 /**
  * Creates a new BearerTokenAuthenticationPolicy factory.
  *
@@ -30,6 +32,8 @@ export function bearerTokenAuthenticationPolicy(credential: TokenCredential, sco
  *
  */
 export class BearerTokenAuthenticationPolicy extends BaseRequestPolicy {
+  private cachedToken: AccessToken | undefined = undefined;
+
   /**
    * Creates a new BearerTokenAuthenticationPolicy object.
    *
@@ -55,15 +59,25 @@ export class BearerTokenAuthenticationPolicy extends BaseRequestPolicy {
     webResource: WebResource
   ): Promise<HttpOperationResponse> {
     if (!webResource.headers) webResource.headers = new HttpHeaders();
-    const token = await this.credential.getToken(
-      this.scopes, {
-        abortSignal: webResource.abortSignal
-      }
-    );
+    const token = await this.getToken({
+      abortSignal: webResource.abortSignal
+    });
     webResource.headers.set(
       Constants.HeaderConstants.AUTHORIZATION,
       `Bearer ${token}`
     );
     return this._nextPolicy.sendRequest(webResource);
   }
+
+  private async getToken(options: GetTokenOptions): Promise<string | undefined> {
+    if (
+      this.cachedToken &&
+      new Date(Date.now() + TokenRefreshBufferMs) < this.cachedToken.expiresOn
+    ) {
+      return this.cachedToken.token;
+    }
+
+    this.cachedToken = (await this.credential.getToken(this.scopes, options)) || undefined;
+    return this.cachedToken ? this.cachedToken.token : undefined;
+  }
 }

sdk/core/core-http/test/policies/bearerTokenAuthenticationPolicyTests.ts

@@ -4,13 +4,13 @@
 import { assert } from "chai";
 import { fake } from "sinon";
 import { OperationSpec } from "../../lib/operationSpec";
-import { TokenCredential } from "../../lib/credentials/tokenCredential";
+import { TokenCredential, GetTokenOptions, AccessToken } from "../../lib/credentials/tokenCredential";
 import { RequestPolicy, RequestPolicyOptions, } from "../../lib/policies/requestPolicy";
 import { Constants } from "../../lib/util/constants";
 import { HttpOperationResponse } from "../../lib/httpOperationResponse";
 import { HttpHeaders, } from "../../lib/httpHeaders";
 import { WebResource } from "../../lib/webResource";
-import { BearerTokenAuthenticationPolicy } from "../../lib/policies/bearerTokenAuthenticationPolicy";
+import { BearerTokenAuthenticationPolicy, TokenRefreshBufferMs } from "../../lib/policies/bearerTokenAuthenticationPolicy";
 
 describe("BearerTokenAuthenticationPolicy", function () {
   const mockPolicy: RequestPolicy = {
@@ -26,18 +26,13 @@ describe("BearerTokenAuthenticationPolicy", function () {
   it("correctly adds an Authentication header with the Bearer token", async function () {
     const mockToken = "token";
     const tokenScopes = ["scope1", "scope2"];
-    const fakeGetToken = fake.returns(Promise.resolve(mockToken));
+    const fakeGetToken = fake.returns(Promise.resolve({ token: mockToken, expiresOn: new Date() }));
     const mockCredential: TokenCredential = {
       getToken: fakeGetToken
     };
 
-    const bearerTokenAuthPolicy = new BearerTokenAuthenticationPolicy(
-      mockPolicy,
-      new RequestPolicyOptions(),
-      mockCredential,
-      tokenScopes
-    );
     const request = createRequest();
+    const bearerTokenAuthPolicy = createBearerTokenPolicy(tokenScopes, mockCredential);
     await bearerTokenAuthPolicy.sendRequest(request);
 
     assert(fakeGetToken.calledWith(tokenScopes, { abortSignal: undefined }));
@@ -47,9 +42,57 @@ describe("BearerTokenAuthenticationPolicy", function () {
     );
   });
 
+  it("refreshes access tokens when they expire", async () => {
+    const now = Date.now();
+    const refreshCred1 = new MockRefreshAzureCredential(new Date(now));
+    const refreshCred2 = new MockRefreshAzureCredential(new Date(now + TokenRefreshBufferMs));
+    const notRefreshCred1 = new MockRefreshAzureCredential(
+      new Date(now + TokenRefreshBufferMs + 5000)
+    );
+
+    const credentialsToTest: [MockRefreshAzureCredential, number][] = [
+      [refreshCred1, 2],
+      [refreshCred2, 2],
+      [notRefreshCred1, 1]
+    ];
+
+    const request = createRequest();
+    for (const [credentialToTest, expectedCalls] of credentialsToTest) {
+      const policy = createBearerTokenPolicy("testscope", credentialToTest);
+      await policy.sendRequest(request);
+      await policy.sendRequest(request);
+      assert.strictEqual(credentialToTest.authCount, expectedCalls);
+    }
+  });
+
   function createRequest(operationSpec?: OperationSpec): WebResource {
     const request = new WebResource();
     request.operationSpec = operationSpec;
     return request;
   }
+
+  function createBearerTokenPolicy(scopes: string | string[], credential: TokenCredential) {
+    return new BearerTokenAuthenticationPolicy(
+      mockPolicy,
+      new RequestPolicyOptions(),
+      credential,
+      scopes);
+  }
 });
+
+class MockRefreshAzureCredential implements TokenCredential {
+  private _expiresOn: Date;
+  public authCount = 0;
+
+  constructor(expiresOn: Date) {
+    this._expiresOn = expiresOn;
+  }
+
+  public getToken(
+    _scopes: string | string[],
+    _options?: GetTokenOptions
+  ): Promise<AccessToken | null> {
+    this.authCount++;
+    return Promise.resolve({ token: "mocktoken", expiresOn: this._expiresOn });
+  }
+}

sdk/identity/identity/src/client/identityClient.ts

@@ -2,8 +2,7 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 import qs from "qs";
-import { AccessToken } from "../credentials/accessToken";
-import { RequestOptionsBase, ServiceClient, ServiceClientOptions } from "@azure/core-http";
+import { AccessToken, ServiceClient, ServiceClientOptions, GetTokenOptions } from "@azure/core-http";
 
 export class IdentityClient extends ServiceClient {
   private static readonly DefaultAuthorityHost = "https://login.microsoftonline.com/";
@@ -21,7 +20,7 @@ export class IdentityClient extends ServiceClient {
     clientId: string,
     clientSecret: string,
     scopes: string | string[],
-    requestOptions?: RequestOptionsBase
+    getTokenOptions?: GetTokenOptions
   ): Promise<AccessToken | null> {
     const response = await this.sendRequest({
       url: `${this.baseUri}/${tenantId}/oauth2/v2.0/token`,
@@ -37,13 +36,9 @@ export class IdentityClient extends ServiceClient {
       }),
       headers: {
         Accept: "application/json",
-        "Content-Type": "application/x-www-form-urlencoded",
-        ...(requestOptions ? requestOptions.customHeaders : {})
+        "Content-Type": "application/x-www-form-urlencoded"
       },
-      abortSignal: requestOptions && requestOptions.abortSignal,
-      timeout: requestOptions && requestOptions.timeout,
-      onUploadProgress: requestOptions && requestOptions.onUploadProgress,
-      onDownloadProgress: requestOptions && requestOptions.onDownloadProgress
+      abortSignal: getTokenOptions && getTokenOptions.abortSignal,
     });
 
     if (response.status === 200 || response.status === 201) {

sdk/identity/identity/src/credentials/aggregateCredential.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
-import { TokenCredential, RequestOptionsBase } from "@azure/core-http";
+import { AccessToken, TokenCredential, GetTokenOptions } from "@azure/core-http";
 
 export class AggregateCredential implements TokenCredential {
   private _sources: TokenCredential[] = [];
@@ -12,12 +12,12 @@ export class AggregateCredential implements TokenCredential {
 
   async getToken(
     scopes: string | string[],
-    requestOptions?: RequestOptionsBase
-  ): Promise<string | null> {
+    options?: GetTokenOptions
+  ): Promise<AccessToken | null> {
     let token = null;
 
     for (let i = 0; i < this._sources.length && token === null; i++) {
-      token = await this._sources[i].getToken(scopes, requestOptions);
+      token = await this._sources[i].getToken(scopes, options);
     }
 
     return token;

sdk/identity/identity/src/credentials/clientSecretCredential.ts

@@ -1,12 +1,11 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
-import { AccessToken } from "./accessToken";
-import { RequestOptionsBase } from "@azure/core-http";
-import { AzureCredential } from "./azureCredential";
-import { IdentityClientOptions } from "../client/identityClient";
+import { TokenCredential, GetTokenOptions, AccessToken } from "@azure/core-http";
+import { IdentityClientOptions, IdentityClient } from "../client/identityClient";
 
-export class ClientSecretCredential extends AzureCredential {
+export class ClientSecretCredential implements TokenCredential {
+  private identityClient: IdentityClient;
   private _tenantId: string;
   private _clientId: string;
   private _clientSecret: string;
@@ -17,23 +16,22 @@ export class ClientSecretCredential extends AzureCredential {
     clientSecret: string,
     options?: IdentityClientOptions
   ) {
-    super(options);
-
+    this.identityClient = new IdentityClient(options);
     this._tenantId = tenantId;
     this._clientId = clientId;
     this._clientSecret = clientSecret;
   }
 
-  protected getAccessToken(
+  public getToken(
     scopes: string | string[],
-    requestOptions?: RequestOptionsBase
+    options?: GetTokenOptions
   ): Promise<AccessToken | null> {
     return this.identityClient.authenticate(
       this._tenantId,
       this._clientId,
       this._clientSecret,
       scopes,
-      requestOptions
+      options
     );
   }
 }

sdk/identity/identity/src/credentials/environmentCredential.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
-import { TokenCredential, isNode, RequestOptionsBase } from "@azure/core-http";
+import { AccessToken, TokenCredential, isNode, GetTokenOptions } from "@azure/core-http";
 import { IdentityClientOptions } from "../client/identityClient";
 import { ClientSecretCredential } from "./clientSecretCredential";
 
@@ -22,9 +22,9 @@ export class EnvironmentCredential implements TokenCredential {
     }
   }
 
-  getToken(scopes: string | string[], requestOptions?: RequestOptionsBase): Promise<string | null> {
+  getToken(scopes: string | string[], options?: GetTokenOptions): Promise<AccessToken | null> {
     if (this._credential) {
-      return this._credential.getToken(scopes, requestOptions);
+      return this._credential.getToken(scopes, options);
     }
 
     return Promise.resolve(null);

sdk/identity/identity/src/credentials/systemCredential.ts

@@ -0,0 +1,12 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License. See License.txt in the project root for license information.
+
+import { IdentityClientOptions } from '../client/identityClient';
+import { AggregateCredential } from './aggregateCredential';
+import { EnvironmentCredential } from './environmentCredential';
+
+export class SystemCredential extends AggregateCredential {
+  constructor(identityClientOptions?: IdentityClientOptions) {
+    super(new EnvironmentCredential(identityClientOptions));
+  }
+}