[core-http] Should TokenCredential.getToken accept a more granular type than RequestOptionsBase?

[daviwil]

This issue tracks a PR discussion which asks whether we should create a more granular type in @azure/core-http than RequestOptionsBase to pass to TokenCredential.getToken.

The easiest option is to just re-export this type from @azure/identity. Even if we make a more narrowly-scoped type for getToken to use, it'd still have to live in @azure/core-http to be used in the TokenCredential interface. RequestOptionsBase is pretty minimal and only refers to one other type (TransferProgressEvent), so it doesn't bring along any major baggage. The properties that it does contain are fairly useful for the purpose of augmenting HTTP requests that are sent through the pipeline.

/cc @schaabs @ramya-rao-a @bterlson

[schaabs]

Exporting this from @azure/identity doesn't solve the problem. No client libraries should take a dependency on @azure/identity

[schaabs]

I think the solution to this is to have it take it's own type of option bag, something like GetTokenOptions. Then in @azure/identity implementation define a new interface which is both GetTokenOptions and RequestOptionsBase, something like IdentityRequestOptions. Thoughts?

[ramya-rao-a]

Is GetTokenOptions in your proposal a function or a type?

Are we saying

getToken(scopes: string | string[], requestOptions?: RequestOptionsBase)

becomes

getToken(scopes: string | string[], tokenOptions?: any)

in TokenCredential

and in Azure Identity, all implementations will use

getToken(scopes: string | string[], tokenOptions?: RequestOptionsBase) ?

[daviwil]

Apologies, I didn't fill in the gap as to why the "export from @azure/identity" suggestion was the solution.

Here are the facts:

• We don't want end users to depend on @azure/core-http directly
• @azure/identity exports TokenCredential implementations that will be created and passed to SDK client libraries like @azure/keyvault-keys
• TokenCredential.getToken takes a RequestOptionsBase so that any HTTP requests created in that call can be configured and an aborter can be passed through
• The end user shouldn't need to call TokenCredential.getToken, but since TokenCredential implementations have a public getToken method, it makes sense to re-export the RequestOptionsBase interface through @azure/identity so that they don't have "missing type" issues

But maybe the idea behind this issue is moot: The only code that consumes TokenCredentials is @azure/core-http and since TokenCredential is an interface, the TypeScript compiler should accept an implementation of it even if the end user doesn't have direct access to the TokenCredential interface. That would make RequestOptionsBase (and TokenCredential) an internal implementation detail between @azure/identity and @azure/core-http which doesn't need to be exposed to the end user.

[ramya-rao-a]

The only code that consumes TokenCredentials is @azure/core-http

That is not true. Non http client libraries like Event Hubs and Service Hubs will need to run the getToken() function to get the token and pass it to the service for auth

And these libraries do not care about the requestOptionsBase

[daviwil]

Good point, I suppose if they implemented some concept of a "policy" it'd be outside of @azure/core-http since they don't use HTTP as their own transport.

RequestOptionsBase is still relevant for requesting the token though since that request is done over HTTP against the AAD service.

[schaabs]

As we discussed offline this should be just the aborter or possibly separate options class which for now just contains the aborter for now.