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
Exception while fetching value of first key vault secret for the application #4924
Comments
Any updates on this one? |
Anyone have any updates on this one? |
Seems odd this is still open and there aren't more replies / participants / followers and votes. As far as i can tell this is the standard way to retrieve secrets from KeyVault. Is there something I'm missing? Can of course work around this issue and ignore exception, but seems like a foundational bug in the SDK for KeyVault. I have not actually deployed my first app to Azure yet. Is this bug only going to occur in the IDE and maybe why it's not a hot issue? |
You're not missing anything as far as I can tell. I get this also, from my App Service deployed to Azure every time it starts up. Issue opened Oct 24, 2018. No one assigned. |
The original issue report describes by-design behavior. The Key Vault client makes an unauthenticated request to AKV. This request returns a 401 with authentication info in the WWW-Authenticate header. The retry handler used by the SDK throws and catches an exception as the response isn't a 2xx value. As this isn't retriable, we get the original 401 in our code, look at the WWW-Authenticate header, then call the user-provided authentication callback, passing in the values from that header. We cache these values, so this 401 only happens on the first request. The very beginning of the issue report says they only observe the exception when they change their debug settings to stop on any exception that is thrown. That exception is going to be caught and handled by the SDK. |
"By-design" is not necessarily correct. This needs to be fixed. In my opinion, it is never acceptable practice to throw exceptions as part of normal operation, if for no other reason than that it will forever raise questions in users' minds. |
The exception never leaves the shared REST SDK code. The exception is thrown and caught in the same function. It's internal to the design of the shared REST API client and how it determines if operations can be retried. As far as the AKV code is concerned, we call HttpClient.SendAsync and then check if response.StatusCode is HttpStatusCode.Unauthorized. The only reason the exception was observable is because the user turned on the option to see exceptions when they are thrown regardless of whether they're handled. This call still needs to go through the RetryDelegatingHandler in case there is an HTTP 500 response or a timeout, but there's no way to hint to it that a 401 is an expected response, so the retry handler wraps it in an exception, evaluates it against the transient error detection strategy, doesn't retry, catches the exception, and returns the original response. The exception never makes it back to the AKV SDK or to the user's code. |
This isn't just when running under the debugger. It happens to my App Service running in Azure as well every time it accesses its key vault on startup. I see failures in my Application Map, for instance. Is there something I can do to hide it or exclude it from being logged in that case? |
The 401 response is an expected and documented part of the Key Vault API. It is used to avoid hard coding the AAD login URL and AAD resource URI. Both of these are subject to change and by designing the SDK to discover their value from the WWW-Authenticate header in the 401 response allows these to be updated without requiring all customers to update their services (for example, in Azure Government cloud, the AAD URL changed from login-us.microsoftonline.com to login.microsoftonline.us). Tools like Application Insights that record all HTTP requests will observe that 401. I have not used Application Insights, but I did find a Stack Overflow post that describes how you can treat certain requests as successful. |
In my case, I am getting a token by instantiating and using an AzureServiceTokenProvider provider object. That token is being supplied for the AuthenticationCallback class on the KeyVaultClient constructor. I think that would/should avoid this exception? But I am not setting the AzureAdInstance (We are NOT passing it into the AzureServiceTokenProvider constructor). If you look at: Microsoft.Azure.Services.AppAuthentication.AzureServiceTokenProvider, the default is: "https://login.microsoftonline.com/". Not 100% sure that's right in our case, but I think it is. I also see a constructor overload for KeyVaultClient which accepts an HttpClient. Perhaps if I retrieve a token from the client provider, create an HttpClient and pass it into the KeyVaultClient constructor that would solve the issue of the 401 exceptions showing up in AppInsights when things are starting up? This isn't something I have the capacity to attempt right now, so I'm asking - giving others ideas for a work around. I don't care for this exception based programming from MS whether it's by design or not doesn't really matter to me - seems kludge. |
You might upgrade to the Azure.Security.KeyVault.* packages which do not use exception-based flow for authentication. The var client = new SecretClient(new Uri("https://myvault.vault.azure.net"), new DefaultAzureCredential());
KeyVaultSecret secret = await client.GetSecretAsync("mysecret"); Rather than handling the authentication callbacks yourself, would this be a better option? The older Microsoft.Azure.KeyVault* code will only be receiving critical fixes. The newer libraries have a lot of improvements, some (with more to come) described in https://aka.ms/azsdkvalueprop. |
Although the exception never leaves the SDK, it has side effects:
In my case, it's happening with an HTTP 101 Switching Protocols response, which is not in any way erroneous, and the protocol I'm switching to is a streaming one that can't be read all at once. I can't get too worked up about legitimately failed requests emitting trace output, but I think the determination of what requests are failed needs to be narrowed. In particular, 1xx responses should not be treated as failed. 3xx responses probably should not either. And as for 4xx responses, well many of them are used for normal communications and none of them should really be retried that I can see, except maybe HTTP 429. In general, I think only 5xx responses (and 429 and maybe 423 Locked, but probably not) should trigger a retry. |
Thank you for raising this issue. While the initial exception for the HTTP 401 response is expected in these older Microsoft.Azure.* packages, it is something we do not do in the newer Azure.* packages. The 401 is handled without throwing and the challenge is authenticated automatically using the As for other HTTP response codes, these changes to Microsoft.Azure.* packages would be breaking behavior. Where Azure.* packages exist, we are only making critical updates to Microsoft.Azure.* packages and recommend that people upgrade. In this case, Azure.Security.KeyVault.Secrets would be what you want. We are discussing what changes we might make to these newer Azure.* packages to improve performance in error conditions for performance-critical code. While Try-style methods are nice, |
For anyone hoping that an upgrade to |
Raise this issue again. Since KeyVault has service limitation as 2000 transactions/10seconds for RSA2048, if SDK always call API twice for one client query, this will shrink service limit to 1000 transactions/10 seconds. |
The bearer token is cached if you reuse client instances, which is better for performance anyway, and unauthenticated requests against the service do not count against the quota. |
Hi,
In my application I am getting exception when my application fetches value of first key vault secret.
The exception I am only getting when I turn on "Common Language Run-time Exceptions" from exception settings.
Exception that I am getting is-
Microsoft.Rest.TransientFaultHandling.HttpRequestWithStatusException: 'Response status code indicates server error: 401 (Unauthorized).'
StackTrace:-
at Microsoft.Rest.RetryDelegatingHandler.<>c__DisplayClass11_0.<<SendAsync>b__1>d.MoveNext()
My code to fetch the key vault secret is given below-
Interface IAdAuthentication
Implementation of interface IAdAuthentication is in class AdAuthentication
Above given exception is being thrown when I fetch the value of first key vault secret for the application instance and it doesn't matter what is the value of first secret identifier. For all next key vault secret exception doesn't occur.
Along with exception value of first key vault secret is also being fetched but I want to mitigate this exception from my application.
It seems issue is around AuthenticationCallback which is passed to initialize KeyVaultClient.
Please let me know what I am missing here.
Adding details of fiddler tracing for the key vault secret request-
The text was updated successfully, but these errors were encountered: