Fix service plan casing and add binding name in ServiceBindingMapper (#…

…1429)

* fix service plan casing in ServiceBindingMapper and add service binding name property to configs

* keep configs grouped by uppercased plan in K8sEnvironment for backward-compatibility

* change fallback plan from "APPLICATION" to "application" in ServiceBindingMapper

* Fix K8sConstants to use K8sConstants
Update the K8sEnvironment to use the EnumMap instead of a Map

Signed-off-by: liga-oz <liga.ozolina@sap.com>


---------

Signed-off-by: liga-oz <liga.ozolina@sap.com>
Co-authored-by: liga-oz <liga.ozolina@sap.com>
Co-authored-by: Līga <72249435+liga-oz@users.noreply.github.com>

env/src/main/java/com/sap/cloud/security/config/ServiceBindingMapper.java

@@ -2,7 +2,7 @@
 
 import com.sap.cloud.environment.servicebinding.api.ServiceBinding;
 import com.sap.cloud.environment.servicebinding.api.TypedMapView;
-import com.sap.cloud.security.config.k8s.K8sConstants;
+import com.sap.cloud.security.config.cf.CFConstants;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -13,18 +13,24 @@
 import static com.sap.cloud.security.config.Service.IAS;
 import static com.sap.cloud.security.config.cf.CFConstants.IAS.DOMAIN;
 import static com.sap.cloud.security.config.cf.CFConstants.IAS.DOMAINS;
+import static com.sap.cloud.security.config.cf.CFConstants.NAME;
 import static com.sap.cloud.security.config.cf.CFConstants.SERVICE_PLAN;
 
-@Deprecated
 public class ServiceBindingMapper {
 	private static final Logger LOGGER = LoggerFactory.getLogger(ServiceBindingMapper.class);
 
-	@Deprecated
+	/**
+	 * Parses a service binding by extracting the configuration information and
+	 * passing it to a configuration builder.
+	 *
+	 * @return a new {@link OAuth2ServiceConfigurationBuilder} that is configured
+	 *         based on the given {@link ServiceBinding}.
+	 */
 	@Nullable
 	public static OAuth2ServiceConfigurationBuilder mapToOAuth2ServiceConfigurationBuilder(ServiceBinding b) {
 		if (!b.getServiceName().isPresent()) {
 			LOGGER.error("Ignores Service Binding with name {} as service name is not provided.", b.getName());
-			return null; // as of now, method is never called when service name isn't given
+			return null;
 		}
 
 		final Service service = Service.from(b.getServiceName().get());
@@ -38,7 +44,8 @@ public static OAuth2ServiceConfigurationBuilder mapToOAuth2ServiceConfigurationB
 		TypedMapView credentials = TypedMapView.ofCredentials(b);
 		OAuth2ServiceConfigurationBuilder builder = OAuth2ServiceConfigurationBuilder.forService(service)
 				.withProperties(credentials.getEntries(String.class))
-				.withProperty(SERVICE_PLAN, b.getServicePlan().orElse(K8sConstants.Plan.APPLICATION.name()).toUpperCase());
+				.withProperty(NAME, b.getName().orElse(""))
+				.withProperty(SERVICE_PLAN, b.getServicePlan().orElse(CFConstants.Plan.APPLICATION.toString()));
 
 		if (IAS.equals(service)) {
 			parseDomains(builder, credentials);

env/src/main/java/com/sap/cloud/security/config/k8s/K8sEnvironment.java

@@ -15,7 +15,6 @@
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import java.util.*;
-import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import static com.sap.cloud.security.config.cf.CFConstants.SERVICE_PLAN;
@@ -30,7 +29,7 @@ public class K8sEnvironment implements Environment {
 	private static final Logger LOGGER = LoggerFactory.getLogger(K8sEnvironment.class);
 
 	static K8sEnvironment instance;
-	private final Map<Service, Map<String, OAuth2ServiceConfiguration>> serviceConfigurations;
+	private final Map<Service, EnumMap<K8sConstants.Plan, OAuth2ServiceConfiguration>> serviceConfigurations;
 
 	private K8sEnvironment() {
 		serviceConfigurations = new EnumMap<>(Service.class);
@@ -53,20 +52,32 @@ public Type getType() {
 	private void loadAll() {
 		List<ServiceBinding> serviceBindings = DefaultServiceBindingAccessor.getInstance().getServiceBindings();
 
-		Map<String, OAuth2ServiceConfiguration> xsuaaPlans = serviceBindings.stream()
+		EnumMap<Plan, OAuth2ServiceConfiguration> xsuaaPlans = serviceBindings.stream()
 				.filter(b -> Service.XSUAA.equals(Service.from(b.getServiceName().orElse(null))))
 				.map(ServiceBindingMapper::mapToOAuth2ServiceConfigurationBuilder)
 				.filter(Objects::nonNull)
 				.map(OAuth2ServiceConfigurationBuilder::build)
-				.collect(Collectors.toMap(config -> config.getProperty(SERVICE_PLAN),
-						Function.identity()));
-		Map<String, OAuth2ServiceConfiguration> identityPlans = serviceBindings.stream()
+				.collect(Collectors.toMap(config -> Plan.from(config.getProperty(SERVICE_PLAN)),
+						config -> config,
+						(l, r) -> {
+							LOGGER.info("Found 2 service configurations of '{}' plan, taking the last one",
+									r.getProperty(SERVICE_PLAN));
+							return r;
+						},
+						() -> new EnumMap<>(Plan.class)));
+		EnumMap<Plan, OAuth2ServiceConfiguration> identityPlans = serviceBindings.stream()
 				.filter(b -> Service.IAS.equals(Service.from(b.getServiceName().orElse(null))))
 				.map(ServiceBindingMapper::mapToOAuth2ServiceConfigurationBuilder)
 				.filter(Objects::nonNull)
 				.map(OAuth2ServiceConfigurationBuilder::build)
-				.collect(Collectors.toMap(config -> config.getProperty(SERVICE_PLAN),
-						Function.identity()));
+				.collect(Collectors.toMap(config -> Plan.from(config.getProperty(SERVICE_PLAN)),
+						config -> config,
+						(l, r) -> {
+							LOGGER.info("Found 2 service configurations of '{}' plan, taking the last one",
+									r.getProperty(SERVICE_PLAN));
+							return r;
+						},
+						() -> new EnumMap<>(Plan.class)));
 		serviceConfigurations.put(Service.XSUAA, xsuaaPlans);
 		serviceConfigurations.put(Service.IAS, identityPlans);
 	}
@@ -79,27 +90,25 @@ private void loadAll() {
 	 * @return the map of all found configurations or empty map, in case there are
 	 *         no service bindings.
 	 */
-	Map<String, OAuth2ServiceConfiguration> getServiceConfigurationsOf(Service service) {
-		return serviceConfigurations.getOrDefault(service, Collections.emptyMap());
+	EnumMap<Plan, OAuth2ServiceConfiguration> getServiceConfigurationsOf(Service service) {
+		return serviceConfigurations.getOrDefault(service, new EnumMap<>(Plan.class));
 	}
 
 	@Nullable
 	@Override
 	public OAuth2ServiceConfiguration getXsuaaConfiguration() {
-		return Optional.ofNullable(getServiceConfigurationsOf(Service.XSUAA).get(Plan.APPLICATION.name()))
-				.orElse(Optional.ofNullable(getServiceConfigurationsOf(Service.XSUAA).get(Plan.BROKER.name()))
-						.orElse(Optional.ofNullable(getServiceConfigurationsOf(Service.XSUAA).get(Plan.SPACE.name()))
-								.orElse(Optional
-										.ofNullable(getServiceConfigurationsOf(Service.XSUAA).get(Plan.DEFAULT.name()))
-										.orElse(null))));
+		return Optional.ofNullable(getServiceConfigurationsOf(Service.XSUAA).get(Plan.APPLICATION))
+				.orElse(Optional.ofNullable(getServiceConfigurationsOf(Service.XSUAA).get(Plan.BROKER))
+						.orElse(Optional.ofNullable(getServiceConfigurationsOf(Service.XSUAA).get(Plan.SPACE))
+								.orElse(getServiceConfigurationsOf(Service.XSUAA).get(Plan.DEFAULT))));
 
 	}
 
 	@Nullable
 	@Override
 	public OAuth2ServiceConfiguration getXsuaaConfigurationForTokenExchange() {
 		if (getNumberOfXsuaaConfigurations() > 1) {
-			return getServiceConfigurationsOf(Service.XSUAA).get(Plan.BROKER.name());
+			return getServiceConfigurationsOf(Service.XSUAA).get(Plan.BROKER);
 		}
 		return getXsuaaConfiguration();
 	}

env/src/test/java/com/sap/cloud/security/config/k8s/K8sEnvironmentTest.java

@@ -13,17 +13,19 @@
 import com.sap.cloud.security.config.OAuth2ServiceConfiguration;
 import com.sap.cloud.security.config.Service;
 import com.sap.cloud.security.config.cf.CFConstants;
-import org.junit.jupiter.api.*;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 
 import java.nio.file.Paths;
-import java.util.*;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 
 import static com.sap.cloud.environment.servicebinding.SapServiceOperatorLayeredServiceBindingAccessor.DEFAULT_PARSING_STRATEGIES;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 
 class K8sEnvironmentTest {
 
@@ -45,11 +47,11 @@ void afterEach() {
 	void getXsuaaConfiguration() {
 		cut = K8sEnvironment.getInstance();
 		OAuth2ServiceConfiguration config = cut.getXsuaaConfiguration();
-		assertEquals("xsuaaClientId", config.getClientId());
-		assertEquals("uaadomain.org", config.getProperty(CFConstants.XSUAA.UAA_DOMAIN));
-		assertEquals("xsuaa-basic", config.getProperty(CFConstants.XSUAA.APP_ID));
-		assertEquals("xsuaaSecret", config.getClientSecret());
-		assertEquals("https://auth.sap.com", config.getUrl().toString());
+		assertEquals("xsuaaClientId2", config.getClientId());
+		assertEquals("uaadomain2.org", config.getProperty(CFConstants.XSUAA.UAA_DOMAIN));
+		assertEquals("xsuaa-basic2", config.getProperty(CFConstants.XSUAA.APP_ID));
+		assertEquals("xsuaaSecret2", config.getClientSecret());
+		assertEquals("https://auth2.sap.com", config.getUrl().toString());
 	}
 
 	@Test

env/src/test/resources/k8s/xsuaa/xsuaa-application2/clientid

@@ -0,0 +1 @@
+xsuaaClientId2

env/src/test/resources/k8s/xsuaa/xsuaa-application2/clientsecret

@@ -0,0 +1 @@
+xsuaaSecret2

env/src/test/resources/k8s/xsuaa/xsuaa-application2/plan

@@ -0,0 +1 @@
+application

env/src/test/resources/k8s/xsuaa/xsuaa-application2/uaadomain

@@ -0,0 +1 @@
+uaadomain2.org

env/src/test/resources/k8s/xsuaa/xsuaa-application2/url

@@ -0,0 +1 @@
+https://auth2.sap.com

env/src/test/resources/k8s/xsuaa/xsuaa-application2/xsappname

@@ -0,0 +1 @@
+xsuaa-basic2

java-api/src/main/java/com/sap/cloud/security/config/cf/CFConstants.java

@@ -14,6 +14,7 @@ public class CFConstants {
 	public static final String VCAP_APPLICATION = "VCAP_APPLICATION";
 	public static final String CREDENTIALS = "credentials";
 	public static final String SERVICE_PLAN = "plan";
+	public static final String NAME = "name";
 	public static final String URL = "url";
 	public static final String CLIENT_ID = "clientid";
 	public static final String CLIENT_SECRET = "clientsecret";

java-api/src/main/java/com/sap/cloud/security/config/k8s/K8sConstants.java

@@ -5,8 +5,6 @@
  */
 package com.sap.cloud.security.config.k8s;
 
-import com.sap.cloud.security.config.cf.CFConstants;
-
 /**
  * Constants that simplifies access to service configuration properties in the
  * Kubernetes environment.
@@ -24,9 +22,13 @@ private K8sConstants() {
 	public enum Plan {
 		DEFAULT, BROKER, APPLICATION, SPACE, APIACCESS, SYSTEM;
 
-		public static CFConstants.Plan from(String planAsString) {
-			return CFConstants.Plan.valueOf(planAsString.toUpperCase());
+		public static K8sConstants.Plan from(String planAsString) {
+			if (planAsString == null) {
+				return APPLICATION;
+			}
+			return K8sConstants.Plan.valueOf(planAsString.toUpperCase());
 		}
 
+
 	}
 }

spring-security/src/main/java/com/sap/cloud/security/spring/config/IdentityServicesPropertySourceFactory.java

@@ -54,10 +54,10 @@ public class IdentityServicesPropertySourceFactory implements PropertySourceFact
 			.asList(new String[] { "clientid", "clientsecret", "identityzoneid",
 					"sburl", "tenantid", "tenantmode", "uaadomain", "url", "verificationkey", "xsappname",
 					"certificate",
-					"key", "credential-type", "certurl" }));
+					"key", "credential-type", "certurl", "name", "plan" }));
 
 	private static final List<String> IAS_ATTRIBUTES = Collections.unmodifiableList(Arrays
-			.asList(new String[] { "clientid", "clientsecret", "domains", "url" }));
+			.asList(new String[] { "clientid", "clientsecret", "domains", "url", "name", "plan" }));
 
 	private Properties properties;
 

spring-security/src/main/java/com/sap/cloud/security/spring/config/OAuth2ServiceConfigurationProperties.java

@@ -206,6 +206,22 @@ public Service getService() {
 		return getConfiguration().getService();
 	}
 
+	public void setName(String name) {
+		builder.withProperty(CFConstants.NAME, name);
+	}
+
+	public String getName() {
+		return getConfiguration().getProperty(CFConstants.NAME);
+	}
+
+	public void setPlan(String plan) {
+		builder.withProperty(CFConstants.SERVICE_PLAN, plan);
+	}
+
+	public String getPlan() {
+		return getConfiguration().getProperty(CFConstants.SERVICE_PLAN);
+	}
+
 	@Override
 	public boolean isLegacyMode() {
 		return getConfiguration().isLegacyMode();