Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE REQ] Make it so the KeyVaultCredentialPolicy makes a call to obtain an authentication challenge only in the case there is no access token in the cache or it has expired #10381

Closed
2 tasks done
vcolin7 opened this issue Apr 20, 2020 · 1 comment · Fixed by #24199
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved. KeyVault pillar-performance The issue is related to performance, one of our core engineering pillars.

Comments

@vcolin7
Copy link
Member

vcolin7 commented Apr 20, 2020

Is your feature request related to a problem? Please describe.
When authenticating calls for KeyVault Keys, the Java KeyVaultCredentialPolicy class always makes a naive call to obtain a challenge that could be used to get an access token in case there is none in the cache or the one it has is expired.

Describe the solution you'd like
The KeyVaultCredentialPolicy only makes a call for a challenge AFTER making sure there is no token in the cache or if it has expired.

Describe alternatives you've considered
Caching the challenge like the .NET SDK does and using that instead in case there is no token or it has expired.

Information Checklist

  • Description Added
  • Expected solution specified
@vcolin7 vcolin7 changed the title [FEATURE REQ] [FEATURE REQ] Make it so the KeyVaultCredentialPolicy makes a call to obtain an authentication challenge only in the case there is no access token in the cache or it has expired Apr 20, 2020
@vcolin7 vcolin7 added the feature-request This issue requires a new behavior in the product in order be resolved. label May 29, 2020
@joshfree joshfree added Client This issue points to a problem in the data-plane of the library. pillar-performance The issue is related to performance, one of our core engineering pillars. pillar-reliability The issue is related to reliability, one of our core engineering pillars. (includes stress testing) labels Jul 16, 2020
@joshfree joshfree added this to the [2020] August milestone Jul 16, 2020
@vcolin7 vcolin7 removed the pillar-reliability The issue is related to reliability, one of our core engineering pillars. (includes stress testing) label Sep 28, 2020
@vcolin7 vcolin7 modified the milestones: [2020] November, [2021] March Feb 8, 2021
@vcolin7 vcolin7 modified the milestones: [2021] March, [2021] April Mar 8, 2021
@vcolin7 vcolin7 modified the milestones: [2021] April, [2021] May Mar 26, 2021
@vcolin7 vcolin7 modified the milestones: [2021] May, [2021] June May 13, 2021
@vcolin7 vcolin7 modified the milestones: [2021] June, Backlog Jun 7, 2021
@heaths
Copy link
Member

heaths commented Sep 17, 2021

That's the old policy. Take a look at https://github.com/Azure/azure-sdk-for-net/blob/f4cbb5c5659b949de1d3cb9d82e02220003848d2/sdk/keyvault/Azure.Security.KeyVault.Shared/src/ChallengeBasedAuthenticationPolicy.cs. It's actually a little older as well and was refactored into Azure.Core for reuse with Attestation, so I think this older version is a bit more succinct. It still works the same, but if you look at the HEAD copy in main now it's base class lives in sdk/core/Azure.Core.

This is client-bound, so the same client will have a policy that will cache access tokens and refresh as needed. It's still thread-safe, but not shared across clients. There was discussion about doing so, though. Something to consider.

Still, since - at least for .NET - we recommend treating Key Vault clients - for the same endpoint - as singletons, it works out.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved. KeyVault pillar-performance The issue is related to performance, one of our core engineering pillars.
Projects
None yet
4 participants