Skip to content
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

Bugfix - Allow configuration of disableChallengeResourceVerification property of AKV SecretClient #36603

8 changes: 8 additions & 0 deletions sdk/spring/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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());
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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());
Expand Down
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();

}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
/**
*
*/
class CertificateClientBuilderFactoryTests extends
class CertificateClientBuilderFactoryTests extends
AzureHttpClientBuilderFactoryBaseTests<
CertificateClientBuilder,
CertificateClientBuilder,
AzureKeyVaultCertificateTestProperties,
CertificateClientBuilderFactoryTests.CertificateClientBuilderFactoryExt> {

Expand All @@ -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
Expand Down Expand Up @@ -88,7 +90,7 @@ protected HttpClientOptions getHttpClientOptions(CertificateClientBuilderFactory
protected List<HttpPipelinePolicy> getHttpPipelinePolicies(CertificateClientBuilderFactoryExt builderFactory) {
return builderFactory.getHttpPipelinePolicies();
}


static class CertificateClientBuilderFactoryExt extends CertificateClientBuilderFactory {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class AzureKeyVaultSecretTestProperties extends AzureHttpSdkProperties implement

private String endpoint;
private SecretServiceVersion serviceVersion;
private boolean challengeResourceVerificationEnabled = true;

@Override
public String getEndpoint() {
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down