-
Notifications
You must be signed in to change notification settings - Fork 953
[App Configuration] add audience error handling policy #25795
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds error handling for authentication failures caused by misconfigured audience settings when using Azure App Configuration in non-public clouds. The implementation intercepts Azure AD error code AADSTS500011 and provides user-friendly error messages with guidance based on whether the audience was configured or not.
Key changes:
- New policy to intercept and handle audience-related authentication errors
- Contextual error messages directing users to documentation
- Integration of the policy into the client pipeline
- Comprehensive test coverage for various error scenarios
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| sdk/data/azappconfig/internal/audience/policy.go | Implements the AudienceErrorHandlingPolicy that intercepts AADSTS500011 errors and provides user-friendly error messages based on whether audience was configured |
| sdk/data/azappconfig/internal/audience/policy_test.go | Comprehensive test coverage for the policy including success cases, non-audience errors, configured/unconfigured audience errors, and wrapped errors |
| sdk/data/azappconfig/client.go | Integrates the audience error handling policy into the client's request pipeline |
| cache := synctoken.NewCache() | ||
| client, err := azcore.NewClient(moduleName, moduleVersion, runtime.PipelineOptions{ | ||
| PerRetry: []policy.Policy{authPolicy, synctoken.NewPolicy(cache)}, | ||
| PerRetry: []policy.Policy{authPolicy, synctoken.NewPolicy(cache), audience.NewAudienceErrorHandlingPolicy(options.Cloud.Services != nil)}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks we are checking if Audience is present. why options.Cloud.Services != nil indicates the presence of Audience?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
discussed offline. reached out to the Go sdk team for the service name
| if p.AudienceConfigured { | ||
| return nil, errors.New("unable to authenticate to Azure App Configuration. An incorrect token audience was provided. Please set ClientOptions.Cloud to the appropriate audience for the target cloud. For details on how to configure the authentication token audience visit https://aka.ms/appconfig/client-token-audience") | ||
| } else { | ||
| return nil, errors.New("unable to authenticate to Azure App Configuration. No authentication token audience was provided. Please set ClientOptions.Cloud to the appropriate audience for the target cloud. For details on how to configure the authentication token audience visit https://aka.ms/appconfig/client-token-audience") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused by the error messages:
- the link
https://aka.ms/appconfig/client-token-audienceincludes the correct audience, but I don't see how to set it. - I'm assuming that
ClientOptions.Cloudneeds to be set to correct cloud. But I did some search, the well-known clouds likecloud.AzureGovernmentdoes not include the app config audience, how does it work? - I still don't know how to set audience, and I even don't know if it's needed since I'm told to set the ClientOptions.Cloud
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed offline
Users running in clouds other than the public cloud must correctly configure ClientOptions.Cloud.Services["AzureAppConfiguration"].Audience when using the ConfigurationClient from the Azure SDK.
There are two main ways users can misconfigure
They do not specify audience when they are running in non-public cloud
They specify audience, and the audience they specify is the wrong one for the cloud is using
In both cases we will get an error when trying to get an Entra ID token that looks like:
"Microsoft.Identity.Client.MsalServiceException: AADSTS500011: The resource principal named https://appconfig.azure.com/ was not found in the tenant named msazurecloud. This can happen if the application has not been installed by the administrator of the tenant or consented to by any user in the tenant."
We should handle this error and surface up an improved error message
Audience not provided
If we get this error and audience is not provided we should surface an error message that says audience should be configured and link to our public doc that documents the appropriate audience for each cloud
Audience provided and incorrect
If we get this error and the audience is provided but is wrong, we should surface an error message that the configured audience is wrong and link to our public doc that documents the appropriate audience for each cloud.