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

[QUERY] KeyVaultCredentialPolicy Makes Extras Requests #23556

Closed
lfgcampos opened this issue Aug 13, 2021 · 3 comments · Fixed by #24199
Closed

[QUERY] KeyVaultCredentialPolicy Makes Extras Requests #23556

lfgcampos opened this issue Aug 13, 2021 · 3 comments · Fixed by #24199
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. KeyVault question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@lfgcampos
Copy link

Query/Question
I am not really sure if this is a Question, Bug or Feature.

Whle working recently with Azure (specifically Azure Key Vault product), we noticed that we always get double requests when we want to get or set a given Secret adding a lot to the latency of the requests. Half of them are 401 responses while the other half is 200.

image

That seemed to be a quite strange behaviour and we decided to go deeper into it.
Turned out, this behaviour is on the KeyVaultCredentialPolicy which always makes an extra request. Checking code, I can see some comments about challenge based auth. This Policy (and several others) are automatically included when you build a SecretClient and there is no easy way to add/remove the policies I want.

With all that info, I would like to understand the benefits of having the "challenge based auth" and if we can avoid it by our own risk.

I managed to configure my own pipeline overriding it and providing only a BearerTokenAuthenticationPolicy. The main problem I could see is that I have to provide a hard-coded scope (https://vault.azure.net/.default in my case) while the KeyVaultCredentialPolicy extract it from the header (www-authenticate) of the first response. The other downside is that I loose all the other Policies configured before, for example UserAgentPolicy and RetryPolicy.

ex:

// Azure SDK client builders accept the credential as a parameter
return SecretClientBuilder()
    .pipeline(
        HttpPipelineBuilder()
            .policies(
                BearerTokenAuthenticationPolicy(
                    principal,
                    "https://vault.azure.net/.default"
                )
            )
            .httpClient(httpClient)
            .build()
    )
    .httpClient(httpClient) // httpclient 2
    .vaultUrl(vaultUrl)
    .credential(principal)
    .buildClient()

Why is this not a Bug or a feature Request?
I am not sure if this is a Bug, Feature Request or by design, hence I think this is primarily a question.

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Aug 13, 2021
@alzimmermsft alzimmermsft added Client This issue points to a problem in the data-plane of the library. KeyVault labels Aug 13, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 13, 2021
@alzimmermsft alzimmermsft changed the title [QUERY] [QUERY] KeyVaultCredentialPolicy Makes Extras Requests Aug 13, 2021
@alzimmermsft
Copy link
Member

@vcolin7 could you look into this

@vcolin7
Copy link
Member

vcolin7 commented Aug 13, 2021

Hi @lfgcampos, your assessment is correct, the KeyVaultCredentialPolicy does in fact make more requests than necessary. I created an issue to track this a while back and had not had the chance to work on it, but I believe I will be able to do so soon and will most likely include this in our next release of the Key Vault libraries. I will let you know once that fix is in so you don't have to manually create your own pipeline.

@lfgcampos
Copy link
Author

Hi @vcolin7, thank you very much for confirming it and we are looking forward to that.

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. customer-reported Issues that are reported by GitHub users external to the Azure organization. KeyVault question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
3 participants