-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[App Configuration] - Add audience error handling policy #53834
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
[App Configuration] - Add audience error handling policy #53834
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 improved error handling for audience configuration issues in the Azure App Configuration SDK. When users in non-public clouds fail to configure or misconfigure the ConfigurationClientOptions.Audience, they now receive clearer error messages that guide them to the appropriate documentation.
Key Changes:
- Adds
AudienceErrorHandlingPolicyto intercept AADSTS500011 authentication errors and provide context-specific guidance - Integrates the new policy into the HTTP pipeline
- Adds
Azure.Identitypackage dependency forAuthenticationFailedException
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
sdk/appconfiguration/Azure.Data.AppConfiguration/src/AudienceErrorHandlingPolicy.cs |
New pipeline policy that catches audience-related authentication failures and rethrows with improved error messages |
sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient.cs |
Integrates the new policy into the HTTP pipeline (contains critical bugs) |
sdk/appconfiguration/Azure.Data.AppConfiguration/src/Azure.Data.AppConfiguration.csproj |
Adds Azure.Identity package reference for AuthenticationFailedException |
sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient.cs
Outdated
Show resolved
Hide resolved
sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient.cs
Outdated
Show resolved
Hide resolved
be14225 to
530f325
Compare
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
sdk/appconfiguration/Azure.Data.AppConfiguration/src/AudienceErrorHandlingPolicy.cs
Outdated
Show resolved
Hide resolved
sdk/appconfiguration/Azure.Data.AppConfiguration/src/Azure.Data.AppConfiguration.csproj
Outdated
Show resolved
Hide resolved
sdk/appconfiguration/Azure.Data.AppConfiguration/src/AudienceErrorHandlingPolicy.cs
Outdated
Show resolved
Hide resolved
sdk/appconfiguration/Azure.Data.AppConfiguration/src/AudienceErrorHandlingPolicy.cs
Outdated
Show resolved
Hide resolved
sdk/appconfiguration/Azure.Data.AppConfiguration/src/AudienceErrorHandlingPolicy.cs
Outdated
Show resolved
Hide resolved
sdk/appconfiguration/Azure.Data.AppConfiguration/src/AudienceErrorHandlingPolicy.cs
Show resolved
Hide resolved
sdk/appconfiguration/Azure.Data.AppConfiguration/src/AudienceErrorHandlingPolicy.cs
Outdated
Show resolved
Hide resolved
sdk/appconfiguration/Azure.Data.AppConfiguration/src/AudienceErrorHandlingPolicy.cs
Outdated
Show resolved
Hide resolved
sdk/appconfiguration/Azure.Data.AppConfiguration/src/AudienceErrorHandlingPolicy.cs
Outdated
Show resolved
Hide resolved
avanigupta
left a comment
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.
Reminder to create the short link at https://aka.ms/appconfig/client-token-audience
|
@avanigupta done |
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
This reverts commit 414f6c4.
|
@jimmyca15 @avanigupta @mrm9084 I cannot add a live test here, because it will always fail in the playback mode. The following live test runs well in Record/Live mode. However, the test proxy won't record the request sent to Entra ID, it only records the request sent to App Config service. In the playback mode, a mocked credential will be used. So it is impossible to add a live test for the audience error. The best I can do is to add some unit tests like @mrm9084 's java SDK PR did. |

Users running in clouds other than the public cloud must correctly configure ConfigurationClientOptions.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.