Repository: SAP/cloud-security-xsuaa-integration
Pull request #172 (2019-10-22 21:12:49)

PropertySourceFactory should support custom properties
In this PR 
- the `PropertySourceFactory` is cleaned up and allows the creation of a PropertySource for xsuaa Properties. 
- the `XsuaaServicesParser` offers a new method `parseCredentials()` that parses a VCAP_SERVICES json string and returns all credential fields as Properties.

With that CAP or others can overwrite all or some xsuaa properties with their own values, as documented in the `CustomPropertySourceFactoryTest` test class:
https://github.com/SAP/cloud-security-xsuaa-integration/blob/3fe1fa52e902a3dc4a6886eca42bc642390a140b/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/CustomPropertySourceFactoryTest.java



Patch:
diff --git a/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/XsuaaServicePropertySourceFactory.java b/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/XsuaaServicePropertySourceFactory.java
index 578c9d41..cdead096 100644
--- a/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/XsuaaServicePropertySourceFactory.java
+++ b/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/XsuaaServicePropertySourceFactory.java
@@ -1,6 +1,8 @@
 package com.sap.cloud.security.xsuaa;
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Properties;
 
 import org.slf4j.Logger;
@@ -12,8 +14,6 @@ import org.springframework.core.io.support.PropertySourceFactory;
 
 import com.sap.cloud.security.xsuaa.autoconfiguration.XsuaaAutoConfiguration;
 
-import net.minidev.json.parser.ParseException;
-
 /**
  * Part of Auto Configuration {@link XsuaaAutoConfiguration}
  *
@@ -32,7 +32,8 @@ import net.minidev.json.parser.ParseException;
  *
  */
 public class XsuaaServicePropertySourceFactory implements PropertySourceFactory {
-	private final Logger logger = LoggerFactory.getLogger(getClass());
+	private static final Logger logger = LoggerFactory.getLogger(XsuaaServicePropertySourceFactory.class);
+
 	protected static final String XSUAA_PREFIX = "xsuaa.";
 	private static final String XSUAA_PROPERTIES_KEY = "xsuaa";
 	public static final String CLIENT_ID = "xsuaa.clientid";
@@ -40,40 +41,46 @@ public class XsuaaServicePropertySourceFactory implements PropertySourceFactory
 	public static final String URL = "xsuaa.url";
 	public static final String UAA_DOMAIN = "xsuaa.uaadomain";
 
-	private static final String[] XSUAA_ATTRIBUTES = new String[] { "clientid", "clientsecret", "identityzoneid",
-			"sburl", "tenantid", "tenantmode", "uaadomain", "url", "verificationkey", "xsappname" };
-
-	private Properties xsuaaProperties = null;
+	private static final List<String> XSUAA_ATTRIBUTES = Arrays
+			.asList(new String[] { "clientid", "clientsecret", "identityzoneid",
+					"sburl", "tenantid", "tenantmode", "uaadomain", "url", "verificationkey", "xsappname" });
 
 	public XsuaaServicePropertySourceFactory() {
 	}
 
 	@Override
 	public PropertySource<?> createPropertySource(String name, EncodedResource resource) throws IOException {
+		Properties properties = null;
 		XsuaaServicesParser vcapServicesParser = null;
-		if (xsuaaProperties == null) {
-			if (resource != null && resource.getResource().getFilename() != null
-					&& !resource.getResource().getFilename().isEmpty()) {
-				vcapServicesParser = new XsuaaServicesParser(resource.getResource().getInputStream());
-			} else {
-				vcapServicesParser = new XsuaaServicesParser();
-			}
-			xsuaaProperties = getConfigurationProperties(vcapServicesParser);
+		if (resource != null && resource.getResource().getFilename() != null
+				&& !resource.getResource().getFilename().isEmpty()) {
+			vcapServicesParser = new XsuaaServicesParser(resource.getResource().getInputStream());
+		} else {
+			vcapServicesParser = new XsuaaServicesParser();
 		}
-		return new PropertiesPropertySource(XSUAA_PROPERTIES_KEY, xsuaaProperties);
+		properties = vcapServicesParser.parseCredentials();
+		logger.info("Parsed {} XSUAA properties.", properties.size());
+		return create(XSUAA_PROPERTIES_KEY, properties);
 	}
 
-	protected Properties getConfigurationProperties(XsuaaServicesParser vcapServicesParser) throws IOException {
-		try {
-			Properties xsuaaProperties = new Properties();
-			for (String attributeName : XSUAA_ATTRIBUTES) {
-				vcapServicesParser.getAttribute(attributeName).ifPresent(
-						attributeValue -> xsuaaProperties.put(XSUAA_PREFIX + attributeName, attributeValue));
+	/**
+	 * Creates a PropertySource object for a map of xsuaa properties.
+	 *
+	 * @param name
+	 *            of the propertySource. Use only "xsuaa" as name in case you like
+	 *            to overwrite/set all properties.
+	 * @param properties
+	 *            map of xsuaa properties.
+	 * @return created @Code{PropertySource}
+	 */
+	public static PropertySource create(String name, Properties properties) {
+		for (final String property : properties.stringPropertyNames()) {
+			if (XSUAA_ATTRIBUTES.contains(property)) {
+				properties.setProperty(XSUAA_PREFIX + property, properties.remove(property).toString());
+			} else {
+				logger.info("Property {} is not considered as part of PropertySource.", property);
 			}
-			logger.info("Extracted {} XSUAA properties", xsuaaProperties.size());
-			return xsuaaProperties;
-		} catch (ParseException ex) {
-			throw new IOException(ex);
 		}
+		return new PropertiesPropertySource(name, properties);
 	}
 }
\ No newline at end of file
diff --git a/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/XsuaaServicesParser.java b/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/XsuaaServicesParser.java
index 7aa72bd7..50ff54a9 100644
--- a/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/XsuaaServicesParser.java
+++ b/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/XsuaaServicesParser.java
@@ -1,15 +1,20 @@
 package com.sap.cloud.security.xsuaa;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import javax.annotation.Nullable;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Properties;
+import java.util.Set;
 
 import org.apache.commons.io.Charsets;
 import org.apache.commons.io.IOUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.springframework.util.StringUtils;
 
 import net.minidev.json.JSONArray;
 import net.minidev.json.JSONObject;
@@ -18,61 +23,106 @@ import net.minidev.json.parser.ParseException;
 
 public class XsuaaServicesParser {
 
-	private final Logger logger = LoggerFactory.getLogger(XsuaaServicesParser.class);
+	private static final Logger logger = LoggerFactory.getLogger(XsuaaServicesParser.class);
 
 	private static final String TAGS = "tags";
 	private static final String CREDENTIALS = "credentials";
 	private static final String VCAP_SERVICES = "VCAP_SERVICES";
 	private static final String XSUAA_TAG = "xsuaa";
 
-	private static String vcapServices;
+	private final String vcapServices;
+	private JSONObject credentialsJSON;
 
 	public XsuaaServicesParser() {
 		vcapServices = System.getenv().get(VCAP_SERVICES);
-		if (StringUtils.isEmpty(vcapServices)) {
-			logger.warn("Cannot find {} environment variable.", VCAP_SERVICES);
+		if (vcapServices == null || vcapServices.isEmpty()) {
+			logger.warn("Cannot extract XSUAA properties from VCAP_SERVICES environment variable.");
 		}
 	}
 
-	public XsuaaServicesParser(InputStream is) throws IOException {
-		vcapServices = IOUtils.toString(is, Charsets.toCharset("utf-8"));
-		if (StringUtils.isEmpty(vcapServices)) {
+	public XsuaaServicesParser(InputStream inputStream) throws IOException {
+		vcapServices = IOUtils.toString(inputStream, Charsets.toCharset(UTF_8.name()));
+		if (vcapServices == null || vcapServices.isEmpty()) {
 			logger.warn("Cannot parse inputStream to extract XSUAA properties.");
 		}
 	}
 
+	public XsuaaServicesParser(String vcapServicesJson) {
+		vcapServices = vcapServicesJson;
+		if (vcapServicesJson == null || vcapServicesJson.isEmpty()) {
+			logger.warn("Cannot extract XSUAA properties from passed vcapServicesJson.");
+		}
+	}
+
 	/**
+	 * Parses the VCAP_SERVICES for xsuaa tag and returns a requested attribute/property from
+	 * credentials.
+	 * 
 	 * @param name
 	 *            the attribute name
-	 * @return associated value to given tag name or null if attribute not found
-	 * @throws ParseException
-	 *             in case of configuration errors
+	 * @return associated value to given tag name or null if attribute/property not found
+	 * @throws IOException
+	 *             in case of parse errors
+	 * @deprecated in favor of {@link #parseCredentials()}. Will be deleted with version 3.0.0.
 	 */
-	public Optional<String> getAttribute(String name) throws ParseException {
-
-		if (StringUtils.isEmpty(vcapServices)) {
-			logger.warn("VCAP_SERVICES could not be load.");
-			return Optional.empty();
+	@Deprecated
+	public Optional<String> getAttribute(String name) throws IOException {
+		if (credentialsJSON == null) {
+			credentialsJSON = parseCredentials(vcapServices);
 		}
-
-		JSONObject vcap = (JSONObject) new JSONParser(JSONParser.MODE_PERMISSIVE).parse(vcapServices);
-		JSONObject xsuaaBinding = searchXSuaaBinding(vcap);
-
-		if (Objects.nonNull(xsuaaBinding) && xsuaaBinding.containsKey(CREDENTIALS)) {
-			JSONObject credentials = (JSONObject) xsuaaBinding.get(CREDENTIALS);
-			Optional<String> attributeString = Optional.ofNullable(credentials.getAsString(name));
+		if (credentialsJSON != null) {
+			Optional<String> attributeString = Optional.ofNullable(credentialsJSON.getAsString(name));
 			if (!attributeString.isPresent()) {
 				logger.info("XSUAA VCAP_SERVICES has no attribute with name '{}'.", name);
 			}
 			return attributeString;
 		}
-
 		return Optional.empty();
 	}
 
-	private JSONObject searchXSuaaBinding(final JSONObject jsonObject) {
-		for (String tag : jsonObject.keySet()) {
-			JSONObject foundObject = getJSONObjectFromTag((JSONArray) jsonObject.get(tag));
+	/**
+	 * Parses the VCAP_SERVICES for xsuaa tag and returns all credential properties.
+	 *
+	 * @return Properties that contains all properties that belong to the xsuaa credentials object.
+	 * @throws IOException
+	 *             in case of parse errors.
+	 *
+	 */
+	public Properties parseCredentials() throws IOException {
+		Properties properties = new Properties();
+		JSONObject credentialsJsonObject = parseCredentials(vcapServices);
+		if (credentialsJsonObject != null) {
+			Set<String> keys = credentialsJsonObject.keySet();
+			for (String key : keys) {
+				properties.put(key, credentialsJsonObject.get(key).toString());
+			}
+		}
+		return properties;
+	}
+
+	@Nullable
+	private static JSONObject parseCredentials(String vcapServices) throws IOException {
+		if (vcapServices == null || vcapServices.isEmpty()) {
+			logger.warn("VCAP_SERVICES could not be load.");
+			return null;
+		}
+		try {
+			JSONObject vcapServicesJSON = (JSONObject) new JSONParser(JSONParser.MODE_PERMISSIVE).parse(vcapServices);
+			JSONObject xsuaaBinding = searchXsuaaBinding(vcapServicesJSON);
+
+			if (Objects.nonNull(xsuaaBinding) && xsuaaBinding.containsKey(CREDENTIALS)) {
+				return (JSONObject) xsuaaBinding.get(CREDENTIALS);
+			}
+		} catch (ParseException ex) {
+			throw new IOException("Error while parsing XSUAA credentials from VCAP_SERVICES: {}.", ex);
+		}
+		return null;
+	}
+
+	@Nullable
+	private static JSONObject searchXsuaaBinding(final JSONObject jsonObject) {
+		for (String key : jsonObject.keySet()) {
+			JSONObject foundObject = getJSONObjectFromTag((JSONArray) jsonObject.get(key), XSUAA_TAG);
 			if (foundObject != null) {
 				return foundObject;
 			}
@@ -80,18 +130,19 @@ public class XsuaaServicesParser {
 		return null;
 	}
 
-	private JSONObject getJSONObjectFromTag(final JSONArray jsonArray) {
+	private static JSONObject getJSONObjectFromTag(final JSONArray jsonArray, String tag) {
 		JSONObject xsuaaBinding = null;
 		for (int i = 0; i < jsonArray.size(); i++) {
 			JSONObject binding = (JSONObject) jsonArray.get(i);
 			JSONArray tags = (JSONArray) binding.get(TAGS);
 
 			for (int j = 0; j < tags.size(); j++) {
-				if (tags.get(j).equals(XSUAA_TAG)) {
+				if (tags.get(j).equals(tag)) {
 					if (xsuaaBinding == null) {
 						xsuaaBinding = binding;
 					} else {
-						throw new RuntimeException("Found more than one xsuaa binding. There can only be one.");
+						throw new IllegalStateException(
+								"Found more than one xsuaa binding. Please consider unified broker plan.");
 					}
 				}
 			}


Test patch:
diff --git a/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/CustomPropertySourceFactoryTest.java b/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/CustomPropertySourceFactoryTest.java
new file mode 100644
index 00000000..0ffb5703
--- /dev/null
+++ b/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/CustomPropertySourceFactoryTest.java
@@ -0,0 +1,91 @@
+package com.sap.cloud.security.xsuaa;
+
+import java.io.IOException;
+import java.util.Properties;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.PropertySource;
+import org.springframework.core.io.support.EncodedResource;
+import org.springframework.core.io.support.PropertySourceFactory;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringRunner;
+
+@RunWith(SpringRunner.class)
+@ContextConfiguration(classes = { CustomConfiguration.class, XsuaaServiceConfigurationDefault.class })
+public class CustomPropertySourceFactoryTest {
+
+	@Autowired
+	CustomConfiguration customConfiguration;
+
+	@Autowired
+	XsuaaServiceConfigurationDefault serviceConfiguration;
+
+	@Test
+	public void testXsuaaServiceConfiguration() {
+		Assert.assertEquals("https://auth.com", serviceConfiguration.getUaaUrl()); // vcap.json
+		Assert.assertEquals("overwriteUaaDomain", serviceConfiguration.getUaaDomain()); // vcap.json
+	}
+
+	@Test
+	public void testOverwrittenXsuaaServiceConfiguration() {
+		Assert.assertEquals("customClientId", serviceConfiguration.getClientId());
+		Assert.assertEquals("customClientSecret", serviceConfiguration.getClientSecret());
+		Assert.assertEquals("customAppId!t2344", serviceConfiguration.getAppId());
+	}
+
+	@Test
+	public void testInjectedPropertyValue() {
+		Assert.assertEquals("https://auth.com", customConfiguration.xsuaaUrl); // vcap.json
+		Assert.assertEquals("overwriteUaaDomain", customConfiguration.xsuaaDomain); // vcap.json
+	}
+
+	@Test
+	public void testOverwrittenInjectedPropertyValue() {
+		Assert.assertEquals("customClientId", customConfiguration.xsuaaClientId);
+		Assert.assertEquals("customClientSecret", customConfiguration.xsuaaClientSecret);
+		Assert.assertEquals("customAppId!t2344", customConfiguration.xsappId);
+	}
+}
+
+@Configuration
+@PropertySource(factory = XsuaaServicePropertySourceFactory.class, value = "classpath:/vcap.json")
+@PropertySource(factory = CustomPropertySourceFactory.class, value = "")
+class CustomConfiguration {
+
+	@Value("${xsuaa.url:}")
+	public String xsuaaUrl;
+
+	@Value("${xsuaa.uaadomain:}")
+	public String xsuaaDomain;
+
+	@Value("${xsuaa.clientid:}")
+	public String xsuaaClientId;
+
+	@Value("${xsuaa.clientsecret:}")
+	public String xsuaaClientSecret;
+
+	@Value("${xsuaa.xsappname:}")
+	public String xsappId;
+}
+
+class CustomPropertySourceFactory implements PropertySourceFactory {
+	private String vcapJsonString = "{\"xsuaa\":[{\"credentials\":{\"xsappname\":\"customAppId!t2344\"},\"tags\":[\"xsuaa\"]}]}";
+
+	@Override
+	public org.springframework.core.env.PropertySource<?> createPropertySource(String s,
+			EncodedResource encodedResource) throws IOException {
+
+		Properties properties = new XsuaaServicesParser(vcapJsonString).parseCredentials();
+
+		properties.put("clientid", "customClientId");
+		properties.put("clientsecret", "customClientSecret");
+		properties.put("uaadomain", "overwriteUaaDomain");
+
+		return XsuaaServicePropertySourceFactory.create("custom", properties);
+	}
+}
\ No newline at end of file
diff --git a/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/XsuaaServicePropertySourceFactoryTest.java b/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/XsuaaServicePropertySourceFactoryTest.java
index 340f77d8..ea33948f 100644
--- a/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/XsuaaServicePropertySourceFactoryTest.java
+++ b/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/XsuaaServicePropertySourceFactoryTest.java
@@ -20,18 +20,23 @@ public class XsuaaServicePropertySourceFactoryTest {
 	@Autowired
 	XsuaaServiceConfigurationDefault serviceConfiguration;
 
+	@Test
+	public void testXsuaaServiceConfiguration() {
+		Assert.assertEquals("xs2.usertoken", serviceConfiguration.getClientId());
+		Assert.assertEquals("secret", serviceConfiguration.getClientSecret());
+		Assert.assertEquals("https://auth.com", serviceConfiguration.getUaaUrl());
+		Assert.assertEquals("auth.com", serviceConfiguration.getUaaDomain());
+	}
+
 	@Test
 	public void testInjectedPropertyValue() {
-		Assert.assertEquals("https://auth.com", testConfiguration.xsuaaUrl);
 		Assert.assertEquals("xs2.usertoken", testConfiguration.xsuaaClientId);
 		Assert.assertEquals("secret", testConfiguration.xsuaaClientSecret);
+		Assert.assertEquals("https://auth.com", testConfiguration.xsuaaUrl);
 		Assert.assertEquals("auth.com", testConfiguration.xsuaaDomain);
-
-		Assert.assertEquals("https://auth.com", serviceConfiguration.getUaaUrl());
-		Assert.assertEquals("xs2.usertoken", serviceConfiguration.getClientId());
-		Assert.assertEquals("secret", serviceConfiguration.getClientSecret());
-		Assert.assertEquals("auth.com", serviceConfiguration.getUaaDomain());
+		Assert.assertEquals("", testConfiguration.unknown);
 	}
+
 }
 
 @Configuration
@@ -51,5 +56,5 @@ class TestConfiguration {
 	public String xsuaaClientSecret;
 
 	@Value("${xsuaa.unknown:}")
-	private String unknown;
+	public String unknown;
 }
\ No newline at end of file