Add BearerTokenCredentialPolicy to azure_core

[vincenttran-msft]

This is the first-pass rough implementation that we were using in our playground

Fully functioning (in simple cases, does not have some intelligent caching / refreshing when the auth expires), but I do have some questions:

1. Are the lifetime annotations here valid? To be quite honest lifetimes are still a challenge for me, but this is what we came up with to get it in a working state
2. Is there any recommended patterns / ideas regarding the auto-refreshing token implementation in Rust? Or should we just side-step this and revisit it later. But from my cursory digging into other SDKs:
   • .NET has its own class AccessTokenCache which has some logic to make it able to refresh when necessary
   • Python added an on_request that is called on every request before sending, and has some logic checking for a currently None token or checking self._need_new_token (but not 100% sure what maintains the state of _need_new_token)
   • EDIT: Doi, maybe I should look at the code deeper than a scroll-through 😆 _need_new_token is a property that literally checks a similar condition as you stated (in Python's case, 300 seconds or 5 mins)

So we can either iterate on this PR to try to work on the auto-caching, or we can leave it on the backlog and revisit it when necessary 😄

[heaths]

Is there any recommended patterns / ideas regarding the auto-refreshing token implementation in Rust?

.NET and Python are actually doing the same thing. I was involved in the development of .NET's. It's still determining if the token needs refreshed on each request via the pipeline. It needs a trigger: there's no thread running concurrently doing this. The logic is basically: if the token for the current call is within 2 minutes of expiring or already expired, fetch a new one. The original request waits on that to complete. From what you described, sounds like Python is conceptually doing the same and I can't imagine what any language would do differently since we wouldn't want to burn a thread (green or otherwise) just to refresh tokens when they may not be needed for long after a token expires.

[heaths]

One major blocker to clean up, but some API changes requested as well.

[vincenttran-msft]

Alrighty, so at this point the code is not compiling, and here is my compiler errors:

**error[E0596]: cannot borrow `*__self` as mutable, as it is behind a `&` reference
  --> sdk\core\azure_core\src\policies\bearer_token_policy.rs:62:13
   |
62 |             self.need_new_token().await;
   |             ^^^^ `__self` is a `&` reference, so the data it refers to cannot be borrowed as mutable
   |
help: you can `clone` the `BearerTokenCredentialPolicy` value and consume it, but this might not be your desired behavior
  --> sdk\core\azure_core\src\policies\bearer_token_policy.rs:54:10
   |
54 |         &self,
   |          ^^^^
help: consider specifying this binding's type
   |
54 |         &self: &mut BearerTokenCredentialPolicy,
   |              ++++++++++++++++++++++++++++++++++

For more information about this error, try `rustc --explain E0596`.
error: could not compile `azure_core` (lib) due to 1 previous error**

After fighting with it for about an hour, I took a step back to think deeply about what is going wrong, and now an implementation question:

Currently, the way that I am trying to make the retry mechanism work is:

• Maintain a last_refresh_time and access_token on the policy itself that we can use to either check to refresh or to use the token itself
• Use a function need_new_token that gets called if the condition is met (>120 secs since last refresh), and this gets a new access token, and should update self.last_refresh_time and self.access_token.

But the issue I am running into is that because we are doing impl Policy for BearerTokenCredentialPolicy, this means we have to match the signature on send, and send only has a &self (non-mutable).
With that being said, is there any way given our current requirements, that we can implement this "caching" policy? I cannot wrap my head around how to store and update some values that live on self when we only have a non-mutable reference (something tells me-- you shouldn't be able to, that's why it's a &self and not a &mut self). But in that case, how do we go about implementing some caching?

Curious to hear your thoughts @heaths and apologies for the wall of text, but I wanted to be as thorough as possible. Thanks!

[heaths]

@vincenttran-msft are you going to be able to finish this, or would you like me to prioritize this in my queue?

[vincenttran-msft]

@vincenttran-msft are you going to be able to finish this, or would you like me to prioritize this in my queue?

I am going to try to get the interior mutability working with Mutex by EOD, otherwise I can revert this back to non-caching and we can review + prioritize this to merge without the caching capabilities.
Thanks!

[vincenttran-msft]

@heaths Hi Heath, I have gone ahead and just reverted back to the non-caching implementation. I did notice that the other policies have been moved out to typespec_client_core, so please let me know if we need to move this policy as well.

We can look to get this reviewed and merged in this state as I don't want to be blocking any other developments any longer on this item. Sorry for the delay!

[heaths]

Making the title more generic since this isn't just for Storage. Also, we have labels we generally like for this stuff because that triggers other events sometimes.

[heaths]

@heaths Hi Heath, I have gone ahead and just reverted back to the non-caching implementation. I did notice that the other policies have been moved out to typespec_client_core, so please let me know if we need to move this policy as well.

We can look to get this reviewed and merged in this state as I don't want to be blocking any other developments any longer on this item. Sorry for the delay!

When do you think you might have the caching in, or is this something you want help with? I helped a bit on https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/core/Azure.Core/src/Pipeline/BearerTokenAuthenticationPolicy.cs, which was refactored out of https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/keyvault/Azure.Security.KeyVault.Shared/src/ChallengeBasedAuthenticationPolicy.cs and we'll need to accommodate for Key Vault for Rust here as well. In fact, Key Vault will be our first service crate we end up releasing, most likely.

/cc @jhendrixMSFT

[heaths]

Also, yes, we can leave this in azure_core. I don't think it's Azure-specific, but seems other languages are leaving it in azure_core instead of their unbranded packages - at least, .NET and Go left it in Azure core.

[vincenttran-msft]

@heaths Hi Heath, I have gone ahead and just reverted back to the non-caching implementation. I did notice that the other policies have been moved out to typespec_client_core, so please let me know if we need to move this policy as well.
We can look to get this reviewed and merged in this state as I don't want to be blocking any other developments any longer on this item. Sorry for the delay!

When do you think you might have the caching in, or is this something you want help with? I helped a bit on https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/core/Azure.Core/src/Pipeline/BearerTokenAuthenticationPolicy.cs, which was refactored out of https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/keyvault/Azure.Security.KeyVault.Shared/src/ChallengeBasedAuthenticationPolicy.cs and we'll need to accommodate for Key Vault for Rust here as well. In fact, Key Vault will be our first service crate we end up releasing, most likely.

/cc @jhendrixMSFT

I think the caching is something I may need help with. I've gotten close, and I've scoured the internet for resources regarding interior mutability in a thread-safe manner (using RwLock or Mutex), but I just cannot for the life of me ever get it completely satisfying all requirements. One of the main issues with using Mutex was that we have: #[derive(Clone)], which cannot easily be auto-generated for our Mutex (I get kicked back by the compiler).

I know that you can then provide a hand-written implementation, but I think this is all still a little outside of my scope of knowledge. So I think the next best step would be if you could implement the caching, and I will probably learn more from seeing your implementation and understanding how it works rather than sinking any more time (which has been a great learning opportunity, but I also am working on some Python SDK commitments as well which are taking longer than expected)

[heaths]

Good as an MVP, but we'll definitely want to cache the access token. What we did in .NET was, if the token is already expired or within 2 minutes of expiring, is fetch it again. It should be indexed based on the authority, as you can see in https://github.com/Azure/azure-sdk-for-net/blob/125bfbb68c585771c6c60f32e0542d45bfc20cfe/sdk/keyvault/Azure.Security.KeyVault.Shared/src/ChallengeBasedAuthenticationPolicy.cs#L48. This is Key Vault's modifications to our more generic https://github.com/Azure/azure-sdk-for-net/blob/125bfbb68c585771c6c60f32e0542d45bfc20cfe/sdk/core/Azure.Core/src/Pipeline/BearerTokenAuthenticationPolicy.cs.