[core-http] Rewrite bearerTokenAuthenticationPolicy

[witemple-msft]

This is a total reimplementation of bearerTokenAuthenticationPolicy. I'm proposing this because the existing implementation has some issues and relies on a class hierarchy that is unfortunately exposed through the core-http public API surface (and is therefore difficult to improve upon).

In this implementation, the architecture looks like this:

• The bearerTokenAuthenticationPolicy creates a request policy factory that uses a "token cycler" and a SimpleBearerAuthorizationPolicy to manage applying bearer authorization to requests. The policy has no smarts other than asking the cycler for a token, applying it, and then applying the rest of the request policy chain.
• The "token cycler" is a function that returns an AccessToken reliably. It is constructed from a TokenCredential and scopes.
  • It follows very simple rules:
    • If it MUST refresh (token is null, within one second of expiration, or expired), then it will await a refresh.
    • If it SHOULD refresh (token is within two minutes of expiration), then it will start a refresh.
    • Produces the cached token (which must be valid at this point).
  • It will not allow multiple refreshes to transpire at the same time, instead caching the Promise that represents the work of refreshing.
• beginRefresh creates a refresh job that takes a function producing AccessToken | null and a refresh interval (in milliseconds, default of three seconds), and continually tries to retrieve an AccessToken until it is successful, waiting for the specified interval between attempts.

This doesn't change (add or remove) any API surface.

I've also merged the tests that @sadasant helpfully provided and repaired them to work with the new default values.

[sadasant]

Beautifully done! Thank you 👏

[witemple-msft]

Check enforcer is running into some trouble this morning with events making it back from AZDO, but we're green on the merge of 75f9a1b:

[jeremymeng]

Looks good to me. Would love to have @xirzec take a look too.

[xirzec]

This is clever!

Can we mark AccessTokenRefresher and AccessTokenCache as deprecated as part of this PR?

The only other real concern I have is the case where getToken fails forever and we stubbornly never raise an error the developer can handle. Feels like there should be some limit.

[xirzec]

so this never gives up? Is there any case it could get stuck forever? What happens if I pass in a token credential with a bad endpoint or something?

[witemple-msft]

I can't think of any reason a TokenCredential would actually return null. It bothers me that the API is written in such a way that resolving with null is possible, but I think for compatibility reasons we are stuck with it. What I guess we should be doing here instead of checking for null is catching rejections.

We could just use a timer? If the refresh doesn't complete within five seconds after expiration (or within five seconds period if there is no token), we can just reject the refresher. Any requests queued up waiting for that refresh job will reject as well.

[witemple-msft]

I added the logic proposed by @sadasant below. If the existing token has already timed out, we retry one more time and then surface an exception to all awaiters if it failed again.

[sadasant]

@willmtemple @jeremymeng are we missing something here? just to make sure we're not missing anything.

[witemple-msft]

@sadasant I need to get back to this. I'll update this later today. I am wondering whether/what to do about the refreshing forever issue. Timeout seems pretty simple and low risk, and I'm wondering whether I should catch exceptions as well.

[sadasant]

@witemple-msft right when the token expires there should be at most one more refresh, and no more refreshes. After that we should assume we weren't able to refresh the token and let the network request throw naturally with the expired token.

[ghost]

Hello @witemple-msft!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.