From dc4ad5bf356cba4ceb57faeeda0abb69b514de4e Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@pivotal.io>
Date: Tue, 10 Mar 2020 10:40:01 +0100
Subject: [PATCH] Polishing.

Strip -uri from property names to align with naming scheme. Simplify property retrieval. Reduce test to unit test. Update documentation.

Closes gh-542.
---
 .../config/EnvironmentVaultConfiguration.java | 25 ++++++------
 ...rationAzureMSIAuthenticationUnitTests.java | 38 +++++++++++++------
 .../reference/imperative-template.adoc        |  2 +
 3 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/spring-vault-core/src/main/java/org/springframework/vault/config/EnvironmentVaultConfiguration.java b/spring-vault-core/src/main/java/org/springframework/vault/config/EnvironmentVaultConfiguration.java
index 9aed67fa..0400c165 100644
--- a/spring-vault-core/src/main/java/org/springframework/vault/config/EnvironmentVaultConfiguration.java
+++ b/spring-vault-core/src/main/java/org/springframework/vault/config/EnvironmentVaultConfiguration.java
@@ -145,10 +145,10 @@ import org.springframework.web.client.RestOperations;
  * <li>Azure MSI path: {@code vault.azure-msi.azure-path} (since 2.2.1, defaults to
  * {@link AzureMsiAuthenticationOptions#DEFAULT_AZURE_AUTHENTICATION_PATH})</li>
  * <li>Role: {@code vault.azure-msi.role}</li>
- * <li>MetadataServiceUri: {@code vault.azure-msi.metadata-service-uri} (defaults to
+ * <li>MetadataServiceUri: {@code vault.azure-msi.metadata-service} (defaults to
  * {@link AzureMsiAuthenticationOptions#DEFAULT_INSTANCE_METADATA_SERVICE_URI})</li>
- * <li>IdentityTokenServiceUri: {@code vault.azure-msi.identity-token-service-uri} (defaults to
- * {@link AzureMsiAuthenticationOptions#DEFAULT_IDENTITY_TOKEN_SERVICE_URI})</li>
+ * <li>IdentityTokenServiceUri: {@code vault.azure-msi.identity-token-service} (defaults
+ * to {@link AzureMsiAuthenticationOptions#DEFAULT_IDENTITY_TOKEN_SERVICE_URI})</li>
  * </ul>
  * <li>Client Certificate authentication
  * <ul>
@@ -172,13 +172,14 @@ import org.springframework.web.client.RestOperations;
  * @author Mark Paluch
  * @author Michal Budzyn
  * @author Raoof Mohammed
+ * @author Justin Bertrand
  * @see org.springframework.core.env.Environment
  * @see org.springframework.core.env.PropertySource
  * @see VaultEndpoint
  * @see AppIdAuthentication
  * @see AppRoleAuthentication
  * @see AwsEc2Authentication
- * @See AzureMsiAuthentication
+ * @see AzureMsiAuthentication
  * @see ClientCertificateAuthentication
  * @see CubbyholeAuthentication
  * @see KubernetesAuthentication
@@ -389,16 +390,16 @@ public class EnvironmentVaultConfiguration extends AbstractVaultConfiguration
 		String role = getProperty("vault.azure-msi.role");
 		String path = getProperty("vault.azure-msi.azure-path",
 				AzureMsiAuthenticationOptions.DEFAULT_AZURE_AUTHENTICATION_PATH);
-		URI metadataServiceUri = getUri("vault.azure-msi.metadata-service-uri",
+		URI metadataServiceUri = getUri("vault.azure-msi.metadata-service",
 				AzureMsiAuthenticationOptions.DEFAULT_INSTANCE_METADATA_SERVICE_URI);
-		URI identityTokenServiceUri = getUri("vault.azure-msi.identity-token-service-uri",
+		URI identityTokenServiceUri = getUri("vault.azure-msi.identity-token-service",
 				AzureMsiAuthenticationOptions.DEFAULT_IDENTITY_TOKEN_SERVICE_URI);
 		Assert.hasText(role,
 				"Vault Azure MSI authentication: Role (vault.azure-msi.role) must not be empty");
 
 		AzureMsiAuthenticationOptionsBuilder builder = AzureMsiAuthenticationOptions
-				.builder().role(role).path(path)
-				.instanceMetadataUri(metadataServiceUri).identityTokenServiceUri(identityTokenServiceUri);
+				.builder().role(role).path(path).instanceMetadataUri(metadataServiceUri)
+				.identityTokenServiceUri(identityTokenServiceUri);
 
 		return new AzureMsiAuthentication(builder.build(), restOperations());
 	}
@@ -436,19 +437,15 @@ public class EnvironmentVaultConfiguration extends AbstractVaultConfiguration
 
 	@Nullable
 	private String getProperty(String key) {
-		return getProperty(key, null);
+		return getEnvironment().getProperty(key);
 	}
 
-	@Nullable
 	private String getProperty(String key, String defaultValue) {
 		return getEnvironment().getProperty(key, defaultValue);
 	}
 
-	@Nullable
 	private URI getUri(String key, URI defaultValue) {
-
-		String value = getProperty(key);
-		return value != null ? URI.create(value) : defaultValue;
+		return getEnvironment().getProperty(key, URI.class, defaultValue);
 	}
 
 	@Nullable
diff --git a/spring-vault-core/src/test/java/org/springframework/vault/config/EnvironmentVaultConfigurationAzureMSIAuthenticationUnitTests.java b/spring-vault-core/src/test/java/org/springframework/vault/config/EnvironmentVaultConfigurationAzureMSIAuthenticationUnitTests.java
index a8327f7d..10384f8e 100644
--- a/spring-vault-core/src/test/java/org/springframework/vault/config/EnvironmentVaultConfigurationAzureMSIAuthenticationUnitTests.java
+++ b/spring-vault-core/src/test/java/org/springframework/vault/config/EnvironmentVaultConfigurationAzureMSIAuthenticationUnitTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2020 the original author or authors.
+ * Copyright 2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,42 +15,56 @@
  */
 package org.springframework.vault.config;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import java.net.URI;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+
+import org.springframework.beans.DirectFieldAccessor;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.Import;
 import org.springframework.test.context.TestPropertySource;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
-import org.springframework.vault.authentication.AwsEc2Authentication;
 import org.springframework.vault.authentication.AzureMsiAuthentication;
+import org.springframework.vault.authentication.AzureMsiAuthenticationOptions;
 import org.springframework.vault.authentication.ClientAuthentication;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 /**
  * Unit tests for {@link EnvironmentVaultConfiguration} with AzureMSI authentication.
  *
  * @author Justin Bertrand
+ * @author Mark Paluch
  */
 @ExtendWith(SpringExtension.class)
-@TestPropertySource(properties = { "vault.uri=https://localhost:8123",
-		"vault.authentication=azure", "vault.azure-msi.role=role" })
+@TestPropertySource(properties = { "vault.uri=http://null", "vault.authentication=azure",
+		"vault.azure-msi.role=role", "vault.azure-msi.metadata-service=http://foo" })
 class EnvironmentVaultConfigurationAzureMSIAuthenticationUnitTests {
 
 	@Configuration
-	@Import(EnvironmentVaultConfiguration.class)
-	static class ApplicationConfiguration {
+	static class MyConfig {
+
 	}
 
-	@Autowired
-	EnvironmentVaultConfiguration configuration;
-
 	@Test
-	void shouldConfigureAuthentication() {
+	void shouldConfigureAuthentication(@Autowired ApplicationContext context) {
+
+		EnvironmentVaultConfiguration configuration = new EnvironmentVaultConfiguration();
+		configuration.setApplicationContext(context);
 
 		ClientAuthentication clientAuthentication = configuration.clientAuthentication();
 
 		assertThat(clientAuthentication).isInstanceOf(AzureMsiAuthentication.class);
+
+		DirectFieldAccessor accessor = new DirectFieldAccessor(clientAuthentication);
+		AzureMsiAuthenticationOptions options = (AzureMsiAuthenticationOptions) accessor
+				.getPropertyValue("options");
+
+		assertThat(options.getIdentityTokenServiceUri()).isEqualTo(
+				AzureMsiAuthenticationOptions.DEFAULT_IDENTITY_TOKEN_SERVICE_URI);
+		assertThat(options.getInstanceMetadataServiceUri())
+				.isEqualTo(URI.create("http://foo"));
 	}
 }
diff --git a/src/main/asciidoc/reference/imperative-template.adoc b/src/main/asciidoc/reference/imperative-template.adoc
index eff1a8e3..f16d0fbe 100644
--- a/src/main/asciidoc/reference/imperative-template.adoc
+++ b/src/main/asciidoc/reference/imperative-template.adoc
@@ -206,6 +206,8 @@ Any other value is used with `StaticUserId`.
 
 * Azure MSI path: `vault.azure-msi.azure-path` (defaults to `azure`)
 * Role: `vault.azure-msi.role`
+* Metadata Service URL: `vault.azure-msi.metadata-service` (defaults to `http://169.254.169.254/metadata/instance?api-version=2017-08-01`)
+* Identity TokenService URL: `vault.azure-msi.identity-token-service` (defaults to `http://169.254.169.254/metadata/identity/oauth2/token?resource=https://vault.hashicorp.com&api-version=2018-02-01`)
 
 **<<vault.authentication.clientcert>>**