[Identity] [core-http] Concept PR for the token refresher update (Azu…

…re#10085)

* AccessTokenRefresher concept

* formatting

* simple test

* changed to timeBetweenRefreshAttemptsInMs

* forcedRefresh if no cached token

* making timeBetweenRefreshAttemptsInMs private

* undoing some of the previous test changes

* much better approach after some tweaks

* license header and lastCalled default value to 0

* mocking instead of delaying, for the tests

* moved the undefined to another place, it looks better, and a fmt fix

* updates after fixing conflicts

* Apply suggestions from code review

Co-authored-by: Jeff Fisher <xirzec@xirzec.com>

* fixes after recent suggestions

* formatting

* requiredMillisecondsBeforeNewRefresh defaults to 30 seconds

Co-authored-by: Jeff Fisher <xirzec@xirzec.com>

sdk/core/core-http/src/credentials/accessTokenRefresher.ts

@@ -0,0 +1,52 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+import { AccessToken, TokenCredential, GetTokenOptions } from "@azure/core-auth";
+
+/**
+ * Helps the core-http token authentication policies with requesting a new token if we're not currently waiting for a new token.
+ */
+export class AccessTokenRefresher {
+  private promise: Promise<AccessToken | undefined> | undefined;
+  private lastCalled = 0;
+
+  constructor(
+    private credential: TokenCredential,
+    private scopes: string | string[],
+    private requiredMillisecondsBeforeNewRefresh: number = 30000
+  ) {}
+
+  public isReady(): boolean {
+    // We're only ready for a new refresh if the required milliseconds have passed.
+    return (
+      !this.lastCalled || Date.now() - this.lastCalled > this.requiredMillisecondsBeforeNewRefresh
+    );
+  }
+
+  /**
+   * Stores the time in which it is called,
+   * then requests a new token,
+   * then sets this.promise to undefined,
+   * then returns the token.
+   * @param options getToken options
+   */
+  private async getToken(options: GetTokenOptions): Promise<AccessToken | undefined> {
+    this.lastCalled = Date.now();
+    const token = await this.credential.getToken(this.scopes, options);
+    this.promise = undefined;
+    return token || undefined;
+  }
+
+  /**
+   * Requests a new token if we're not currently waiting for a new token.
+   * Returns null if the required time between each call hasn't been reached.
+   * @param options getToken options
+   */
+  public refresh(options: GetTokenOptions): Promise<AccessToken | undefined> {
+    if (!this.promise) {
+      this.promise = this.getToken(options);
+    }
+
+    return this.promise;
+  }
+}

sdk/core/core-http/src/nodeFetchHttpClient.ts

@@ -84,7 +84,7 @@ export class NodeFetchHttpClient extends FetchHttpClient {
 
   // eslint-disable-next-line @azure/azure-sdk/ts-apisurface-standardized-verbs
   async fetch(input: CommonRequestInfo, init?: CommonRequestInit): Promise<CommonResponse> {
-    return node_fetch(input, init) as unknown as Promise<CommonResponse>;
+    return (node_fetch(input, init) as unknown) as Promise<CommonResponse>;
   }
 
   async prepareRequest(httpRequest: WebResourceLike): Promise<Partial<RequestInit>> {

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

@@ -13,6 +13,7 @@ import { HttpOperationResponse } from "../httpOperationResponse";
 import { HttpHeaders } from "../httpHeaders";
 import { WebResourceLike } from "../webResource";
 import { AccessTokenCache, ExpiringAccessTokenCache } from "../credentials/accessTokenCache";
+import { AccessTokenRefresher } from "../credentials/accessTokenRefresher";
 
 /**
  * Creates a new BearerTokenAuthenticationPolicy factory.
@@ -38,6 +39,13 @@ export function bearerTokenAuthenticationPolicy(
   };
 }
 
+/**
+ * The automated token refresh will only start to happen at the
+ * expiration date minus the value of timeBetweenRefreshAttemptsInMs,
+ * which is by default 30 seconds.
+ */
+const timeBetweenRefreshAttemptsInMs = 30000;
+
 /**
  *
  * Provides a RequestPolicy that can request a token from a TokenCredential
@@ -46,6 +54,8 @@ export function bearerTokenAuthenticationPolicy(
  *
  */
 export class BearerTokenAuthenticationPolicy extends BaseRequestPolicy {
+  private tokenRefresher: AccessTokenRefresher;
+
   /**
    * Creates a new BearerTokenAuthenticationPolicy object.
    *
@@ -63,6 +73,11 @@ export class BearerTokenAuthenticationPolicy extends BaseRequestPolicy {
     private tokenCache: AccessTokenCache
   ) {
     super(nextPolicy, options);
+    this.tokenRefresher = new AccessTokenRefresher(
+      this.credential,
+      this.scopes,
+      timeBetweenRefreshAttemptsInMs
+    );
   }
 
   /**
@@ -81,11 +96,27 @@ export class BearerTokenAuthenticationPolicy extends BaseRequestPolicy {
     return this._nextPolicy.sendRequest(webResource);
   }
 
+  /**
+   * Attempts a token update if any other time related conditionals have been reached based on the tokenRefresher class.
+   */
+  private async updateTokenIfNeeded(options: GetTokenOptions): Promise<void> {
+    if (this.tokenRefresher.isReady()) {
+      const accessToken = await this.tokenRefresher.refresh(options);
+      this.tokenCache.setCachedToken(accessToken);
+    }
+  }
+
   private async getToken(options: GetTokenOptions): Promise<string | undefined> {
     let accessToken = this.tokenCache.getCachedToken();
     if (accessToken === undefined) {
-      accessToken = (await this.credential.getToken(this.scopes, options)) || undefined;
-      this.tokenCache.setCachedToken(accessToken);
+      // Waiting for the next refresh only if the cache is unable to retrieve the access token,
+      // which means that it has expired, or it has never been set.
+      accessToken = await this.tokenRefresher.refresh(options);
+    } else {
+      // If we still have a cached access token,
+      // And any other time related conditionals have been reached based on the tokenRefresher class,
+      // then attempt to refresh without waiting.
+      this.updateTokenIfNeeded(options);
     }
 
     return accessToken ? accessToken.token : undefined;

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

@@ -2,7 +2,7 @@
 // Licensed under the MIT license.
 
 import { assert } from "chai";
-import { fake } from "sinon";
+import { fake, createSandbox } from "sinon";
 import { OperationSpec } from "../../src/operationSpec";
 import { TokenCredential, GetTokenOptions, AccessToken } from "@azure/core-auth";
 import { RequestPolicy, RequestPolicyOptions } from "../../src/policies/requestPolicy";
@@ -15,6 +15,7 @@ import {
   ExpiringAccessTokenCache,
   TokenRefreshBufferMs
 } from "../../src/credentials/accessTokenCache";
+import { AccessTokenRefresher } from "../../src/credentials/accessTokenRefresher";
 
 describe("BearerTokenAuthenticationPolicy", function() {
   const mockPolicy: RequestPolicy = {
@@ -61,7 +62,7 @@ describe("BearerTokenAuthenticationPolicy", function() {
     const credentialsToTest: [MockRefreshAzureCredential, number][] = [
       [refreshCred1, 2],
       [refreshCred2, 2],
-      [notRefreshCred1, 1]
+      [notRefreshCred1, 2]
     ];
 
     const request = createRequest();
@@ -73,6 +74,20 @@ describe("BearerTokenAuthenticationPolicy", function() {
     }
   });
 
+  it("tests that AccessTokenRefresher is working", async function() {
+    const now = Date.now();
+    const credentialToTest = new MockRefreshAzureCredential(now);
+    const request = createRequest();
+    const policy = createBearerTokenPolicy("testscope", credentialToTest);
+    await policy.sendRequest(request);
+
+    const sandbox = createSandbox();
+    sandbox.replace(AccessTokenRefresher.prototype, "isReady", () => true);
+    await policy.sendRequest(request);
+    sandbox.restore();
+    assert.strictEqual(credentialToTest.authCount, 2);
+  });
+
   function createRequest(operationSpec?: OperationSpec): WebResource {
     const request = new WebResource();
     request.operationSpec = operationSpec;