Bugfix - Allow configuration of disableChallengeResourceVerification …

…property of AKV SecretClient (Azure#36603) * Allows configuration of disableChallengeResourceVerification property - Adds disableChallengeResourceVerification property to properties objects - Includes new property in mapping methods - Configures SecretClient in Factory when disableChallengeResourceVerification is set - Configures CertificateClient in Factory when disableChallengeResourceVerification is set - Updates/adds new tests - Updates Changelog Resolves Azure#36561 Signed-off-by: Esta Nagy <[email protected]> * Allows configuration of disableChallengeResourceVerification property - Code review fixes #1 - Renames disableChallengeResourceVerification to challengeResourceVerificationEnabled - Adds additional JavaDoc Resolves Azure#36561 Signed-off-by: Esta Nagy <[email protected]> * Allows configuration of disableChallengeResourceVerification property - Fix a missed JavaDoc Signed-off-by: Esta Nagy <[email protected]> * Improve the configuration properties javadoc, and complete the additional-spring-configuration-metadata.json * Allows configuration of disableChallengeResourceVerification property - Code review fixes Azure#3 - Simplifies factory method logic as per code review recommendation Resolves Azure#36561 Signed-off-by: Esta Nagy <[email protected]> --------- Signed-off-by: Esta Nagy <[email protected]> Co-authored-by: Azure SDK Bot <[email protected]> Co-authored-by: Xiaolu Dai <[email protected]>
kushagraThapar · Sep 2, 2023 · a0d30ff · a0d30ff
1 parent 4a864a5
commit a0d30ff
Show file tree

Hide file tree

Showing 15 changed files with 128 additions and 4 deletions.
diff --git a/sdk/spring/CHANGELOG.md b/sdk/spring/CHANGELOG.md
@@ -1,5 +1,13 @@
 # Release History
 
+## 5.6.0-beta.1 (Unreleased)
+
+### Spring Cloud Azure Autoconfigure
+This section includes changes in `spring-cloud-azure-autoconfigure` module.
+
+#### Bugs Fixed
+- Fix the issue that prevented the `disableChallengeResourceVerification` property of the AKV `SecretClient` to be configured [#36561](https://github.com/Azure/azure-sdk-for-java/issues/36561).
+
 ## 5.5.0 (2023-08-28)
 - This release is compatible with Spring Boot 3.0.0-3.1.2. (Note: 3.1.x (x>2) should be supported, but they aren't tested with this release.)
 - This release is compatible with Spring Cloud 2022.0.0-2022.0.4. (Note: 2022.0.x (x>4) should be supported, but they aren't tested with this release.)

diff --git a/...re/spring/cloud/autoconfigure/implementation/keyvault/common/AzureKeyVaultProperties.java b/...re/spring/cloud/autoconfigure/implementation/keyvault/common/AzureKeyVaultProperties.java
@@ -19,6 +19,12 @@ public class AzureKeyVaultProperties extends AbstractAzureHttpConfigurationPrope
      */
     private String endpoint;
 
+    /**
+     * Whether to enable the Azure Key Vault challenge resource verification, default: true.
+     * Calls the disableChallengeResourceVerification method of the Azure Key Vault Client Builder when set to false.
+     */
+    private boolean challengeResourceVerificationEnabled = true;
+
     /**
      *
      * @return The Azure Key Vault endpoint.
@@ -34,4 +40,21 @@ public String getEndpoint() {
     public void setEndpoint(String endpoint) {
         this.endpoint = endpoint;
     }
+
+    /**
+     *
+     * @return Whether we should keep the challenge resource verification for the Azure Key Vault Client
+     */
+    public boolean isChallengeResourceVerificationEnabled() {
+        return challengeResourceVerificationEnabled;
+    }
+
+    /**
+     *
+     * @param challengeResourceVerificationEnabled Whether we should keep Azure Key Vault challenge resource verification enabled
+     */
+    public void setChallengeResourceVerificationEnabled(
+        boolean challengeResourceVerificationEnabled) {
+        this.challengeResourceVerificationEnabled = challengeResourceVerificationEnabled;
+    }
 }
diff --git a/...d/autoconfigure/implementation/keyvault/environment/KeyVaultEnvironmentPostProcessor.java b/...d/autoconfigure/implementation/keyvault/environment/KeyVaultEnvironmentPostProcessor.java
@@ -143,6 +143,7 @@ private AzureKeyVaultSecretProperties toAzureKeyVaultSecretProperties(
         AzurePropertiesUtils.copyAzureCommonProperties(propertySourceProperties, secretProperties);
         secretProperties.setEndpoint(propertySourceProperties.getEndpoint());
         secretProperties.setServiceVersion(propertySourceProperties.getServiceVersion());
+        secretProperties.setChallengeResourceVerificationEnabled(propertySourceProperties.isChallengeResourceVerificationEnabled());
         return secretProperties;
     }
 
@@ -197,6 +198,7 @@ private AzureKeyVaultPropertySourceProperties buildMergedProperties(
         mergedProperties.setCaseSensitive(propertySourceProperties.isCaseSensitive());
         mergedProperties.setSecretKeys(propertySourceProperties.getSecretKeys());
         mergedProperties.setRefreshInterval(propertySourceProperties.getRefreshInterval());
+        mergedProperties.setChallengeResourceVerificationEnabled(propertySourceProperties.isChallengeResourceVerificationEnabled());
         return mergedProperties;
     }
 

diff --git a/...ure/implementation/keyvault/secrets/properties/AzureKeyVaultPropertySourceProperties.java b/...ure/implementation/keyvault/secrets/properties/AzureKeyVaultPropertySourceProperties.java
@@ -43,6 +43,12 @@ public class AzureKeyVaultPropertySourceProperties extends AbstractAzureHttpConf
      */
     private Duration refreshInterval = DEFAULT_REFRESH_INTERVAL;
 
+    /**
+     * Whether to enable the Azure Key Vault challenge resource verification, default: true.
+     * Calls the disableChallengeResourceVerification method of the Azure Key Vault Client Builder when set to false.
+     */
+    private boolean challengeResourceVerificationEnabled = true;
+
     /**
      *
      * @return The name of this property source.
@@ -138,4 +144,21 @@ public Duration getRefreshInterval() {
     public void setRefreshInterval(Duration refreshInterval) {
         this.refreshInterval = refreshInterval;
     }
+
+    /**
+     *
+     * @return Whether we should keep Azure Key Vault challenge resource verification enabled
+     */
+    public boolean isChallengeResourceVerificationEnabled() {
+        return challengeResourceVerificationEnabled;
+    }
+
+    /**
+     *
+     * @param challengeResourceVerificationEnabled Whether we should keep Azure Key Vault challenge resource verification enabled
+     */
+    public void setChallengeResourceVerificationEnabled(
+        boolean challengeResourceVerificationEnabled) {
+        this.challengeResourceVerificationEnabled = challengeResourceVerificationEnabled;
+    }
 }
diff --git a/...e-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/...e-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json
@@ -528,6 +528,13 @@
       "description": "Secret service version used when making API requests.",
       "sourceType": "com.azure.spring.cloud.autoconfigure.implementation.keyvault.secrets.properties.AzureKeyVaultPropertySourceProperties"
     },
+    {
+      "name": "spring.cloud.azure.keyvault.secret.property-sources[0].challenge-resource-verification-enabled",
+      "type": "java.lang.Boolean",
+      "description": "Whether to enable the Azure Key Vault challenge resource verification, default: true. Calls the disableChallengeResourceVerification method of the Azure Key Vault Client Builder when set to false.",
+      "sourceType": "com.azure.spring.cloud.autoconfigure.implementation.keyvault.secrets.properties.AzureKeyVaultPropertySourceProperties",
+      "defaultValue": true
+    },
     {
       "name": "spring.datasource.azure.credential.client-id",
       "type": "java.lang.String",

diff --git a/.../implementation/keyvault/certificates/AzureKeyVaultCertificateAutoConfigurationTests.java b/.../implementation/keyvault/certificates/AzureKeyVaultCertificateAutoConfigurationTests.java
@@ -22,6 +22,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 
 class AzureKeyVaultCertificateAutoConfigurationTests extends AbstractAzureServiceConfigurationTests<
     CertificateClientBuilderFactory, AzureKeyVaultCertificateProperties> {
@@ -139,13 +140,15 @@ void configurationPropertiesShouldBind() {
         this.contextRunner
             .withPropertyValues(
                 "spring.cloud.azure.keyvault.certificate.endpoint=" + endpoint,
-                "spring.cloud.azure.keyvault.certificate.service-version=V7_2"
+                "spring.cloud.azure.keyvault.certificate.service-version=V7_2",
+                "spring.cloud.azure.keyvault.certificate.challenge-resource-verification-enabled=false"
             )
             .run(context -> {
                 assertThat(context).hasSingleBean(AzureKeyVaultCertificateProperties.class);
                 AzureKeyVaultCertificateProperties properties = context.getBean(AzureKeyVaultCertificateProperties.class);
                 assertEquals(endpoint, properties.getEndpoint());
                 assertEquals(CertificateServiceVersion.V7_2, properties.getServiceVersion());
+                assertFalse(properties.isChallengeResourceVerificationEnabled());
             });
     }
 

diff --git a/...oconfigure/implementation/keyvault/environment/KeyVaultEnvironmentPostProcessorTests.java b/...oconfigure/implementation/keyvault/environment/KeyVaultEnvironmentPostProcessorTests.java
@@ -275,6 +275,31 @@ void specificPropertiesHasHigherPriorityThanGlobalPropertiesTest() {
         assertEquals(specificMaxRetries, properties.getRetry().getFixed().getMaxRetries());
     }
 
+    @Test
+    void challengeResourceVerificationEnabledCanBeSetAsFalseTest() {
+        environment.setProperty("spring.cloud.azure.keyvault.secret.property-source-enabled", "true");
+        environment.setProperty("spring.cloud.azure.keyvault.secret.property-sources[0].challenge-resource-verification-enabled", "false");
+        environment.setProperty("spring.cloud.azure.keyvault.secret.property-sources[0].enabled", "true");
+        environment.setProperty("spring.cloud.azure.keyvault.secret.property-sources[0].name", NAME_0);
+        environment.setProperty("spring.cloud.azure.keyvault.secret.property-sources[0].endpoint", ENDPOINT_0);
+        AzureKeyVaultSecretProperties secretProperties = processor.loadProperties(environment);
+        AzureKeyVaultPropertySourceProperties properties = secretProperties.getPropertySources().get(0);
+        assertTrue(secretProperties.isChallengeResourceVerificationEnabled());
+        assertFalse(properties.isChallengeResourceVerificationEnabled());
+    }
+
+    @Test
+    void challengeResourceVerificationEnabledIsSetByDefaultTest() {
+        environment.setProperty("spring.cloud.azure.keyvault.secret.property-source-enabled", "true");
+        environment.setProperty("spring.cloud.azure.keyvault.secret.property-sources[0].enabled", "true");
+        environment.setProperty("spring.cloud.azure.keyvault.secret.property-sources[0].name", NAME_0);
+        environment.setProperty("spring.cloud.azure.keyvault.secret.property-sources[0].endpoint", ENDPOINT_0);
+        AzureKeyVaultSecretProperties secretProperties = processor.loadProperties(environment);
+        AzureKeyVaultPropertySourceProperties properties = secretProperties.getPropertySources().get(0);
+        assertTrue(secretProperties.isChallengeResourceVerificationEnabled());
+        assertTrue(properties.isChallengeResourceVerificationEnabled());
+    }
+
     @Disabled("Disable it to unblock Azure Dev Ops pipeline: https://dev.azure.com/azure-sdk/public/_build/results?buildId=1434354&view=logs&j=c1fb1ddd-7688-52ac-4c5f-1467e51181f3")
     @Test
     void buildKeyVaultPropertySourceWithExceptionTest() {

diff --git a/...oconfigure/implementation/keyvault/secrets/AzureKeyVaultSecretAutoConfigurationTests.java b/...oconfigure/implementation/keyvault/secrets/AzureKeyVaultSecretAutoConfigurationTests.java
@@ -145,6 +145,7 @@ void configurationPropertiesShouldBind() {
             .withPropertyValues(
                 "spring.cloud.azure.keyvault.secret.endpoint=" + endpoint,
                 "spring.cloud.azure.keyvault.secret.service-version=V7_2",
+                "spring.cloud.azure.keyvault.secret.challenge-resource-verification-enabled=false",
 
                 "spring.cloud.azure.keyvault.secret.property-source-enabled=false",
                 "spring.cloud.azure.keyvault.secret.property-sources[0].endpoint=" + endpoint + "-1",
@@ -161,6 +162,7 @@ void configurationPropertiesShouldBind() {
                 assertEquals(endpoint, properties.getEndpoint());
                 assertFalse(properties.isPropertySourceEnabled());
                 assertEquals(SecretServiceVersion.V7_2, properties.getServiceVersion());
+                assertFalse(properties.isChallengeResourceVerificationEnabled());
 
                 AzureKeyVaultPropertySourceProperties propertySourceProperties = properties.getPropertySources().get(0);
                 assertEquals(endpoint + "-1", propertySourceProperties.getEndpoint());

diff --git a/.../main/java/com/azure/spring/cloud/service/implementation/keyvault/KeyVaultProperties.java b/.../main/java/com/azure/spring/cloud/service/implementation/keyvault/KeyVaultProperties.java
@@ -13,4 +13,6 @@ public interface KeyVaultProperties extends AzureProperties, RetryOptionsProvide
 
     String getEndpoint();
 
+    boolean isChallengeResourceVerificationEnabled();
+
 }
diff --git a/...g/cloud/service/implementation/keyvault/certificates/CertificateClientBuilderFactory.java b/...g/cloud/service/implementation/keyvault/certificates/CertificateClientBuilderFactory.java
@@ -88,6 +88,8 @@ protected void configureService(CertificateClientBuilder builder) {
         PropertyMapper map = new PropertyMapper();
         map.from(certificateClientProperties.getEndpoint()).to(builder::vaultUrl);
         map.from(certificateClientProperties.getServiceVersion()).to(builder::serviceVersion);
+        map.from(certificateClientProperties.isChallengeResourceVerificationEnabled())
+            .whenFalse().to(enabled -> builder.disableChallengeResourceVerification());
     }
 
     @Override

diff --git a/...zure/spring/cloud/service/implementation/keyvault/secrets/SecretClientBuilderFactory.java b/...zure/spring/cloud/service/implementation/keyvault/secrets/SecretClientBuilderFactory.java
@@ -88,6 +88,8 @@ protected void configureService(SecretClientBuilder builder) {
         PropertyMapper map = new PropertyMapper();
         map.from(secretClientProperties.getEndpoint()).to(builder::vaultUrl);
         map.from(secretClientProperties.getServiceVersion()).to(builder::serviceVersion);
+        map.from(secretClientProperties.isChallengeResourceVerificationEnabled())
+            .whenFalse().to(enabled -> builder.disableChallengeResourceVerification());
     }
 
     @Override

diff --git a/.../service/implementation/keyvault/certificates/AzureKeyVaultCertificateTestProperties.java b/.../service/implementation/keyvault/certificates/AzureKeyVaultCertificateTestProperties.java
@@ -11,6 +11,8 @@ class AzureKeyVaultCertificateTestProperties extends AzureHttpSdkProperties impl
     private String endpoint;
     private CertificateServiceVersion serviceVersion;
 
+    private boolean challengeResourceVerificationEnabled = true;
+
     @Override
     public String getEndpoint() {
         return endpoint;
@@ -28,4 +30,13 @@ public CertificateServiceVersion getServiceVersion() {
     public void setServiceVersion(CertificateServiceVersion serviceVersion) {
         this.serviceVersion = serviceVersion;
     }
+
+    @Override
+    public boolean isChallengeResourceVerificationEnabled() {
+        return challengeResourceVerificationEnabled;
+    }
+
+    public void setChallengeResourceVerificationEnabled(boolean challengeResourceVerificationEnabled) {
+        this.challengeResourceVerificationEnabled = challengeResourceVerificationEnabled;
+    }
 }
diff --git a/...ud/service/implementation/keyvault/certificates/CertificateClientBuilderFactoryTests.java b/...ud/service/implementation/keyvault/certificates/CertificateClientBuilderFactoryTests.java
@@ -23,9 +23,9 @@
 /**
  *
  */
-class CertificateClientBuilderFactoryTests extends 
+class CertificateClientBuilderFactoryTests extends
     AzureHttpClientBuilderFactoryBaseTests<
-        CertificateClientBuilder, 
+        CertificateClientBuilder,
         AzureKeyVaultCertificateTestProperties,
         CertificateClientBuilderFactoryTests.CertificateClientBuilderFactoryExt> {
 
@@ -52,11 +52,13 @@ protected void verifyServicePropertiesConfigured() {
         AzureKeyVaultCertificateTestProperties properties = new AzureKeyVaultCertificateTestProperties();
         properties.setServiceVersion(CertificateServiceVersion.V7_0);
         properties.setEndpoint(ENDPOINT);
+        properties.setChallengeResourceVerificationEnabled(false);
 
         final CertificateClientBuilderFactoryExt factoryExt = new CertificateClientBuilderFactoryExt(properties);
         final CertificateClientBuilder builder = factoryExt.build();
         verify(builder, times(1)).serviceVersion(CertificateServiceVersion.V7_0);
         verify(builder, times(1)).vaultUrl(ENDPOINT);
+        verify(builder, times(1)).disableChallengeResourceVerification();
     }
 
     @Override
@@ -88,7 +90,7 @@ protected HttpClientOptions getHttpClientOptions(CertificateClientBuilderFactory
     protected List<HttpPipelinePolicy> getHttpPipelinePolicies(CertificateClientBuilderFactoryExt builderFactory) {
         return builderFactory.getHttpPipelinePolicies();
     }
-    
+
 
     static class CertificateClientBuilderFactoryExt extends CertificateClientBuilderFactory {
 

diff --git a/...ring/cloud/service/implementation/keyvault/secrets/AzureKeyVaultSecretTestProperties.java b/...ring/cloud/service/implementation/keyvault/secrets/AzureKeyVaultSecretTestProperties.java
@@ -13,6 +13,7 @@ class AzureKeyVaultSecretTestProperties extends AzureHttpSdkProperties implement
 
     private String endpoint;
     private SecretServiceVersion serviceVersion;
+    private boolean challengeResourceVerificationEnabled = true;
 
     @Override
     public String getEndpoint() {
@@ -31,4 +32,13 @@ public SecretServiceVersion getServiceVersion() {
     public void setServiceVersion(SecretServiceVersion serviceVersion) {
         this.serviceVersion = serviceVersion;
     }
+
+    @Override
+    public boolean isChallengeResourceVerificationEnabled() {
+        return challengeResourceVerificationEnabled;
+    }
+
+    public void setChallengeResourceVerificationEnabled(boolean challengeResourceVerificationEnabled) {
+        this.challengeResourceVerificationEnabled = challengeResourceVerificationEnabled;
+    }
 }
diff --git a/...spring/cloud/service/implementation/keyvault/secrets/SecretClientBuilderFactoryTests.java b/...spring/cloud/service/implementation/keyvault/secrets/SecretClientBuilderFactoryTests.java
@@ -78,12 +78,14 @@ protected void verifyServicePropertiesConfigured() {
         AzureKeyVaultSecretTestProperties properties = new AzureKeyVaultSecretTestProperties();
         properties.setServiceVersion(SecretServiceVersion.V7_0);
         properties.setEndpoint(ENDPOINT);
+        properties.setChallengeResourceVerificationEnabled(false);
 
         final SecretClientBuilderFactoryExt factoryExt = new SecretClientBuilderFactoryExt(properties);
         final SecretClientBuilder builder = factoryExt.build();
 
         verify(builder, times(1)).vaultUrl(ENDPOINT);
         verify(builder, times(1)).serviceVersion(SecretServiceVersion.V7_0);
+        verify(builder, times(1)).disableChallengeResourceVerification();
     }
 
     static class SecretClientBuilderFactoryExt extends SecretClientBuilderFactory {
Original file line number	Diff line number	Diff line change
Expand Up		@@ -13,4 +13,6 @@ public interface KeyVaultProperties extends AzureProperties, RetryOptionsProvide

		String getEndpoint();

		boolean isChallengeResourceVerificationEnabled();

		}