Set token to be expired if response comes back as unauthorized, within BeareTokenAuthenticationPolicy.

[ahsonkhan]

Part of https://github.com/Azure/azure-sdk-for-cpp/issues/6022#issuecomment-2400588474

cc @chlowell @christothes

[azure-sdk]

API change check

API changes are not detected in this pull request.

[antkmsft]

Bigger perspective: if we have 10 clients using 1 credential, and first of them gets this new "unexpected" 401, it is going to invalidate the credential's cache. On the next request with client 1, the credential is going to fetch the new token. Client 2 will attempt to use the old token that was cached in its policy, and will also get 401, it will go ahead and invalidate not only the token that was cached in its policy, but it will invalidate the credential's cache once again, while the newly fetched value might be perfectly valid. In case of 10 clients, we will send 10 request to get fresh token, instead of 1. What if instead we would remove the cached token from the policy, but instead the policy would call token->GetToken() every single time? If token has cache, that is pretty fast. All our current credentials do have cache. But if credential does not have cache (customer implements their own?), that would be slow. Maybe we could add bool HasCache() const to TokenCredential? Then, in the constructor of the policy, we would either have the current behavior of this PR if credential->HasCache() == false, or would effectively disable the cached m_accessToken if (credential->HasCache() == true), and would be invoking credential->GetToken() every single time.

That would involve making more changes than I what I proposed above, but I think that might be the best way to do this.

[ahsonkhan]

What if instead we would remove the cached token from the policy, but instead the policy would call token->GetToken() every single time? If token has cache, that is pretty fast. All our current credentials do have cache.

That seems like a reasonable approach. Though I don't know what initially motivated us caching the token within the policy to begin with. As long as you don't see any unintended consequence in doing so (like you said, we already cache), we can consider making the change.

[ahsonkhan]

/check-enforcer override

[chlowell]

what initially motivated us caching the token within the policy to begin with

It's a performance optimization. The policy doesn't need to call GetToken for every request because it knows the scope and expiration time of the last token it received. The perf benefit is modest when the credential has a cache but can be huge when the credential does not. For example, when the credential is AzureCliCredential, the policy's cache saves it from having to run the CLI on every request.