diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index e7b89527a93e..42714ab7c1c3 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -107,7 +107,7 @@ # ServiceOwners: @miaojiang # PRLabel: %App Configuration -/sdk/appconfiguration/ @alzimmermsft @Azure/azure-java-sdk +/sdk/appconfiguration/ @mrm9084 @rossgrambo @avanigupta @alzimmermsft @Azure/azure-java-sdk # ServiceLabel: %App Configuration # AzureSdkOwners: @mrm9084 diff --git a/sdk/appconfiguration/azure-data-appconfiguration/CHANGELOG.md b/sdk/appconfiguration/azure-data-appconfiguration/CHANGELOG.md index 67c451261084..fa8e7494f0e0 100644 --- a/sdk/appconfiguration/azure-data-appconfiguration/CHANGELOG.md +++ b/sdk/appconfiguration/azure-data-appconfiguration/CHANGELOG.md @@ -3,7 +3,9 @@ ## 1.9.0-beta.1 (Unreleased) ### Features Added + - Added a pipeline policy to handle query parameters to make sure the keys are always in lower case and in alphabetical order. +- Added audience policy to provide more meaningful error messages for Azure Active Directory authentication failures. The policy detects AAD audience-related errors and provides clear guidance on audience configuration issues. ### Breaking Changes diff --git a/sdk/appconfiguration/azure-data-appconfiguration/checkstyle-suppressions.xml b/sdk/appconfiguration/azure-data-appconfiguration/checkstyle-suppressions.xml index aa64ccaf2075..2c39234812f6 100644 --- a/sdk/appconfiguration/azure-data-appconfiguration/checkstyle-suppressions.xml +++ b/sdk/appconfiguration/azure-data-appconfiguration/checkstyle-suppressions.xml @@ -4,6 +4,7 @@ + diff --git a/sdk/appconfiguration/azure-data-appconfiguration/src/main/java/com/azure/data/appconfiguration/ConfigurationClientBuilder.java b/sdk/appconfiguration/azure-data-appconfiguration/src/main/java/com/azure/data/appconfiguration/ConfigurationClientBuilder.java index 7e209d811dd7..26a0b21942b4 100644 --- a/sdk/appconfiguration/azure-data-appconfiguration/src/main/java/com/azure/data/appconfiguration/ConfigurationClientBuilder.java +++ b/sdk/appconfiguration/azure-data-appconfiguration/src/main/java/com/azure/data/appconfiguration/ConfigurationClientBuilder.java @@ -3,6 +3,14 @@ package com.azure.data.appconfiguration; +import java.net.MalformedURLException; +import java.net.URL; +import java.time.temporal.ChronoUnit; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Objects; + import com.azure.core.annotation.ServiceClientBuilder; import com.azure.core.client.traits.ConfigurationTrait; import com.azure.core.client.traits.ConnectionStringTrait; @@ -33,30 +41,22 @@ import com.azure.core.util.ClientOptions; import com.azure.core.util.Configuration; import com.azure.core.util.CoreUtils; +import static com.azure.core.util.CoreUtils.getApplicationId; import com.azure.core.util.HttpClientOptions; import com.azure.core.util.TracingOptions; import com.azure.core.util.builder.ClientBuilderUtil; import com.azure.core.util.logging.ClientLogger; import com.azure.core.util.tracing.Tracer; import com.azure.core.util.tracing.TracerProvider; +import com.azure.data.appconfiguration.implementation.AudiencePolicy; import com.azure.data.appconfiguration.implementation.AzureAppConfigurationImpl; +import static com.azure.data.appconfiguration.implementation.ClientConstants.APP_CONFIG_TRACING_NAMESPACE_VALUE; import com.azure.data.appconfiguration.implementation.ConfigurationClientCredentials; import com.azure.data.appconfiguration.implementation.ConfigurationCredentialsPolicy; import com.azure.data.appconfiguration.implementation.QueryParamPolicy; import com.azure.data.appconfiguration.implementation.SyncTokenPolicy; import com.azure.data.appconfiguration.models.ConfigurationAudience; -import java.net.MalformedURLException; -import java.net.URL; -import java.time.temporal.ChronoUnit; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Objects; - -import static com.azure.core.util.CoreUtils.getApplicationId; -import static com.azure.data.appconfiguration.implementation.ClientConstants.APP_CONFIG_TRACING_NAMESPACE_VALUE; - /** * This class provides a fluent builder API to help aid the configuration and instantiation of * {@link ConfigurationClient ConfigurationClients} and {@link ConfigurationAsyncClient ConfigurationAsyncClients}, call @@ -289,6 +289,9 @@ private HttpPipeline createDefaultHttpPipeline(SyncTokenPolicy syncTokenPolicy, policies.add(syncTokenPolicy); policies.addAll(perRetryPolicies); + // Add policy to provide better error messages for AAD audience authentication failures + policies.add(new AudiencePolicy(audience)); + List httpHeaderList = new ArrayList<>(); localClientOptions.getHeaders() .forEach(header -> httpHeaderList.add(new HttpHeader(header.getName(), header.getValue()))); diff --git a/sdk/appconfiguration/azure-data-appconfiguration/src/main/java/com/azure/data/appconfiguration/implementation/AudiencePolicy.java b/sdk/appconfiguration/azure-data-appconfiguration/src/main/java/com/azure/data/appconfiguration/implementation/AudiencePolicy.java new file mode 100644 index 000000000000..cfde6ab443db --- /dev/null +++ b/sdk/appconfiguration/azure-data-appconfiguration/src/main/java/com/azure/data/appconfiguration/implementation/AudiencePolicy.java @@ -0,0 +1,74 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +package com.azure.data.appconfiguration.implementation; + +import com.azure.core.exception.HttpResponseException; +import com.azure.core.http.HttpPipelineCallContext; +import com.azure.core.http.HttpPipelineNextPolicy; +import com.azure.core.http.HttpPipelineNextSyncPolicy; +import com.azure.core.http.HttpResponse; +import com.azure.core.http.policy.HttpPipelinePolicy; +import com.azure.data.appconfiguration.models.ConfigurationAudience; + +import reactor.core.publisher.Mono; + +/** + * HTTP pipeline policy that handles Azure Active Directory audience-related authentication errors. + * This policy intercepts HTTP responses and provides more meaningful error messages when + * audience configuration issues occur during authentication. + */ +public class AudiencePolicy implements HttpPipelinePolicy { + + private static final String NO_AUDIENCE_ERROR_MESSAGE + = "Unable to authenticate to Azure App Configuration. No authentication token audience was provided. " + + "Please set an Audience in your ConfigurationClientBuilder for the target cloud. " + + "For details on how to configure the authentication token audience visit " + + "https://aka.ms/appconfig/client-token-audience."; + + private static final String INCORRECT_AUDIENCE_ERROR_MESSAGE + = "Unable to authenticate to Azure App Configuration. An incorrect token audience was provided. " + + "Please set the Audience in your ConfigurationClientBuilder to the appropriate audience for this cloud. " + + "For details on how to configure the authentication token audience visit " + + "https://aka.ms/appconfig/client-token-audience."; + + private static final String AAD_AUDIENCE_ERROR_CODE = "AADSTS500011"; + + private final ConfigurationAudience audience; + + /** + * Creates a new instance of AudiencePolicy. + * + * @param audience The configuration audience to use for validation. May be null. + */ + public AudiencePolicy(ConfigurationAudience audience) { + this.audience = audience; + } + + @Override + public Mono process(HttpPipelineCallContext context, HttpPipelineNextPolicy next) { + return next.process().onErrorMap(HttpResponseException.class, this::handleAudienceException); + } + + @Override + public HttpResponse processSync(HttpPipelineCallContext context, HttpPipelineNextSyncPolicy next) { + try { + return next.processSync(); + } catch (HttpResponseException ex) { + throw handleAudienceException(ex); + } + } + + /** + * Handles audience-related authentication exceptions by providing more meaningful error messages. + * + * @param ex The original HttpResponseException + * @return A new HttpResponseException with improved error message if audience-related, otherwise the original exception + */ + private HttpResponseException handleAudienceException(HttpResponseException ex) { + if (ex.getMessage() != null && ex.getMessage().contains(AAD_AUDIENCE_ERROR_CODE)) { + String message = audience == null ? NO_AUDIENCE_ERROR_MESSAGE : INCORRECT_AUDIENCE_ERROR_MESSAGE; + return new HttpResponseException(message, ex.getResponse(), ex); + } + return ex; + } +} diff --git a/sdk/appconfiguration/azure-data-appconfiguration/src/test/java/com/azure/data/appconfiguration/implementation/AudiencePolicyTest.java b/sdk/appconfiguration/azure-data-appconfiguration/src/test/java/com/azure/data/appconfiguration/implementation/AudiencePolicyTest.java new file mode 100644 index 000000000000..78ef12332b26 --- /dev/null +++ b/sdk/appconfiguration/azure-data-appconfiguration/src/test/java/com/azure/data/appconfiguration/implementation/AudiencePolicyTest.java @@ -0,0 +1,167 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.data.appconfiguration.implementation; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; + +import com.azure.core.exception.HttpResponseException; +import com.azure.core.http.HttpMethod; +import com.azure.core.http.HttpPipeline; +import com.azure.core.http.HttpPipelineBuilder; +import com.azure.core.http.HttpRequest; +import com.azure.core.http.HttpResponse; +import com.azure.core.http.policy.HttpPipelinePolicy; +import com.azure.core.test.SyncAsyncExtension; +import com.azure.core.test.annotation.SyncAsyncTest; +import com.azure.core.test.http.MockHttpResponse; +import com.azure.core.test.http.NoOpHttpClient; +import com.azure.core.util.Context; +import com.azure.data.appconfiguration.models.ConfigurationAudience; + +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +/** + * Unit tests for AudiencePolicy + */ +public class AudiencePolicyTest { + private static final String LOCAL_HOST = "http://localhost"; + private static final String AAD_AUDIENCE_ERROR_CODE = "AADSTS500011"; + private static final String NO_AUDIENCE_ERROR_MESSAGE + = "Unable to authenticate to Azure App Configuration. No authentication token audience was provided. " + + "Please set an Audience in your ConfigurationClientBuilder for the target cloud. " + + "For details on how to configure the authentication token audience visit " + + "https://aka.ms/appconfig/client-token-audience."; + + private static final String INCORRECT_AUDIENCE_ERROR_MESSAGE + = "Unable to authenticate to Azure App Configuration. An incorrect token audience was provided. " + + "Please set the Audience in your ConfigurationClientBuilder to the appropriate audience for this cloud. " + + "For details on how to configure the authentication token audience visit " + + "https://aka.ms/appconfig/client-token-audience."; + + @SyncAsyncTest + public void processWithoutException() { + AudiencePolicy audiencePolicy = new AudiencePolicy(ConfigurationAudience.AZURE_PUBLIC_CLOUD); + + HttpPipelinePolicy testPolicy = (context, next) -> { + return next.process(); + }; + + final HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(new NoOpHttpClient() { + @Override + public Mono send(HttpRequest request) { + return Mono.just(new MockHttpResponse(request, 200)); + } + }).policies(audiencePolicy, testPolicy).build(); + + SyncAsyncExtension.execute(() -> sendRequestSync(pipeline), () -> sendRequest(pipeline)); + } + + @Test + public void processWithNonAudienceException() { + AudiencePolicy audiencePolicy = new AudiencePolicy(ConfigurationAudience.AZURE_PUBLIC_CLOUD); + + HttpPipelinePolicy exceptionPolicy = (context, next) -> { + HttpResponseException ex + = new HttpResponseException("Some other error", new MockHttpResponse(context.getHttpRequest(), 401)); + return Mono.error(ex); + }; + + final HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(new NoOpHttpClient()) + .policies(audiencePolicy, exceptionPolicy) + .build(); + + StepVerifier.create(sendRequest(pipeline)) + .expectErrorMatches(throwable -> throwable instanceof HttpResponseException + && throwable.getMessage().equals("Some other error")) + .verify(); + + // Test sync version + HttpResponseException thrown = assertThrows(HttpResponseException.class, () -> sendRequestSync(pipeline)); + assertEquals("Some other error", thrown.getMessage()); + } + + @Test + public void processWithAudienceExceptionAndNullAudience() { + AudiencePolicy audiencePolicy = new AudiencePolicy(null); + + HttpPipelinePolicy exceptionPolicy = (context, next) -> { + HttpResponseException ex = new HttpResponseException("Error " + AAD_AUDIENCE_ERROR_CODE + " occurred", + new MockHttpResponse(context.getHttpRequest(), 401)); + return Mono.error(ex); + }; + + final HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(new NoOpHttpClient()) + .policies(audiencePolicy, exceptionPolicy) + .build(); + + StepVerifier.create(sendRequest(pipeline)) + .expectErrorMatches(throwable -> throwable instanceof HttpResponseException + && throwable.getMessage().equals(NO_AUDIENCE_ERROR_MESSAGE)) + .verify(); + + // Test sync version + HttpResponseException thrown = assertThrows(HttpResponseException.class, () -> sendRequestSync(pipeline)); + assertEquals(NO_AUDIENCE_ERROR_MESSAGE, thrown.getMessage()); + } + + @Test + public void processWithAudienceExceptionAndConfiguredAudience() { + AudiencePolicy audiencePolicy = new AudiencePolicy(ConfigurationAudience.AZURE_PUBLIC_CLOUD); + + HttpPipelinePolicy exceptionPolicy = (context, next) -> { + HttpResponseException ex = new HttpResponseException("Error " + AAD_AUDIENCE_ERROR_CODE + " occurred", + new MockHttpResponse(context.getHttpRequest(), 401)); + return Mono.error(ex); + }; + + final HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(new NoOpHttpClient()) + .policies(audiencePolicy, exceptionPolicy) + .build(); + + StepVerifier.create(sendRequest(pipeline)) + .expectErrorMatches(throwable -> throwable instanceof HttpResponseException + && throwable.getMessage().equals(INCORRECT_AUDIENCE_ERROR_MESSAGE)) + .verify(); + + // Test sync version + HttpResponseException thrown = assertThrows(HttpResponseException.class, () -> sendRequestSync(pipeline)); + assertEquals(INCORRECT_AUDIENCE_ERROR_MESSAGE, thrown.getMessage()); + } + + @Test + public void processWithNullMessageException() { + AudiencePolicy audiencePolicy = new AudiencePolicy(ConfigurationAudience.AZURE_PUBLIC_CLOUD); + + HttpPipelinePolicy exceptionPolicy = (context, next) -> { + HttpResponseException ex + = new HttpResponseException(null, new MockHttpResponse(context.getHttpRequest(), 401)); + return Mono.error(ex); + }; + + final HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(new NoOpHttpClient()) + .policies(audiencePolicy, exceptionPolicy) + .build(); + + StepVerifier.create(sendRequest(pipeline)) + .expectErrorMatches( + throwable -> throwable instanceof HttpResponseException && throwable.getMessage() == null) + .verify(); + + // Test sync version + HttpResponseException thrown = assertThrows(HttpResponseException.class, () -> sendRequestSync(pipeline)); + assertEquals(null, thrown.getMessage(), "Should return original exception when message is null"); + } + + private Mono sendRequest(HttpPipeline pipeline) { + return pipeline.send(new HttpRequest(HttpMethod.GET, LOCAL_HOST)); + } + + private HttpResponse sendRequestSync(HttpPipeline pipeline) { + return pipeline.sendSync(new HttpRequest(HttpMethod.GET, LOCAL_HOST), Context.NONE); + } +}