Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.47k stars 4.8k forks source link

[BUG] GetUserDelegationKeyAsync() fails TenantDiscovery challenge #30401

Open seanmcc-msft opened 2 years ago

seanmcc-msft commented 2 years ago

Library name and version

Azure.Storage.Blobs

Describe the bug

From ICM - https://portal.microsofticm.com/imp/v3/incidents/details/326322676/home

GetUserDelegationKeyAsync() fails when enabling Tenant Discovery as an option for BlobServiceClient. The fix is to call a storage method such as GetBlobsAsync() to perform the Tenant Discovery challenge and then call GetUserDelegationKeyAsync().

My assumption is that the GetUserDelegationKeyAsync() doesn't issue a discovery challenge before sending a request to Storage.

Here is the code block below:


                BlobClientOptions options = new BlobClientOptions()
                {
                    Retry = {
                        Delay = TimeSpan.FromSeconds(2),
                        MaxRetries = 5,
                        Mode = Azure.Core.RetryMode.Exponential,
                        MaxDelay = TimeSpan.FromSeconds(10)
                    },
                    EnableTenantDiscovery = true, // Enable Tenant Discovery
                };
                BlobServiceClient blobServiceClient = new BlobServiceClient(new Uri(blobEndpoint), aadID.ClientCertificateCredential, options);
                // Initialze the tenant and authentication context with the BlobServiceClient in the case that this SA is in a different tenant.
                // We cannot just call the DelegationKey without first initializing to the SA in the tenant and retrieving a token.
                await foreach(Azure.Storage.Blobs.Models.BlobContainerItem container in blobServiceClient.GetBlobContainersAsync(prefix: containerName))
                {
                    break; // Only retrieve up to 1 container and break out.
                }

// Call getuserdelegationkey now that we've already retrieved the token to the SA.

var userDelegationKey = await blobServiceClient.GetUserDelegationKeyAsync(DateTimeOffset.UtcNow, DateTimeOffset.UtcNow.AddHours(2));
jaschrep-msft commented 2 years ago

I believe the library is surfacing a service behavior here. By enabling tenant discovery, you are opting in that an initial request without authorization token will be sent in order for the service to return a challenge request pointing to the correct tenant to obtain your token from.

However, GetUserDelegationKey() is a method which makes no sense unless using oauth tokens, and so the service checks the authorization type to determine whether to carry out the request or fail it outright. In the case where GetUserDelegationKey() is your first method, and so the client has not yet obtained a token, I believe the service is interpreting the lack of any Authorization header as the wrong type of header, and so failing outright before trying to issue a challenge. The error message in the response body states Only authentication scheme Bearer is supported, indicating this is the check that is causing the failure.

I'm not yet sure what mitigation steps the client can take, if any, as I would imagine an application using tenant discovery isn't inherently capable of finding the correct tenant on its own to authenticate with. This likely requires discussion with the service team to find a solution.

jaschrep-msft commented 2 years ago

Service team has acknowledged this as a bug to be fixed. Internally tracked as item 15280470.

vyank commented 11 months ago

Is there any alternate for this? I am facing this issue in production.

github-actions[bot] commented 3 months ago

Hi @seanmcc-msft, we deeply appreciate your input into this project. Regrettably, this issue has remained unresolved for over 2 years and inactive for 30 days, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.