Repository: SAP/cloud-security-xsuaa-integration
Pull request #287 (2020-05-15 17:09:45)

[java-security] SECAUTH-813: support more than 1 other configuration
SAPOfflineTokenServicesCloud and JwtValidatorBuilder should more than 1 other configuration
- OAuth2ServiceConfiguration should implement equals method

TODO:
 - [ ] wait on stakeholder feedback

nenaraab: @hassler-d 
thanks for your review. I've applied all of your review comments...
please perform another review :-)

Patch:
diff --git a/java-security/Migration_SAPJavaBuildpackProjects.md b/java-security/Migration_SAPJavaBuildpackProjects.md
index 6af15f66..400fd32b 100644
--- a/java-security/Migration_SAPJavaBuildpackProjects.md
+++ b/java-security/Migration_SAPJavaBuildpackProjects.md
@@ -1,7 +1,7 @@
-# Migration Guide for J2EE Web Applications that use SAP Java Buildpack for securing their applications
+# Migration Guide for J2EE Web Applications that use SAP Java / XSA Buildpack for securing their applications
 
 
-**This document is only applicable for J2EE web applications securing their application with SAP Java Buildpack.** The SAP Java Buildpack version `1.26.1` does not any longer provide deprecated SAP-internal security libraries and does not longer depend on Spring security. 
+**This document is only applicable for J2EE web applications securing their application with SAP Java / XSA Buildpack.** The SAP Java Buildpack version `1.26.1` does not any longer provide deprecated SAP-internal security libraries and does not longer depend on Spring security. 
 
 This migration document is a step-by-step guide explaining how to replace your dependencies to the deprecated SAP-internal security libraries with the open-sourced ones.
 
diff --git a/java-security/src/main/java/com/sap/cloud/security/config/OAuth2ServiceConfigurationBuilder.java b/java-security/src/main/java/com/sap/cloud/security/config/OAuth2ServiceConfigurationBuilder.java
index 4b78c323..2b5fe315 100644
--- a/java-security/src/main/java/com/sap/cloud/security/config/OAuth2ServiceConfigurationBuilder.java
+++ b/java-security/src/main/java/com/sap/cloud/security/config/OAuth2ServiceConfigurationBuilder.java
@@ -127,7 +127,7 @@ public class OAuth2ServiceConfigurationBuilder {
 
 		@Override
 		public URI getUrl() {
-			return URI.create(properties.get(URL));
+			return hasProperty(URL) ? URI.create(properties.get(URL)) : null;
 		}
 
 		@Override
diff --git a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtAudienceValidator.java b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtAudienceValidator.java
index dd1369f2..12240962 100644
--- a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtAudienceValidator.java
+++ b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtAudienceValidator.java
@@ -26,7 +26,7 @@ public class JwtAudienceValidator implements Validator<Token> {
 	private static final Logger logger = LoggerFactory.getLogger(JwtAudienceValidator.class);
 	private static final char DOT = '.';
 
-	private final Set<String> clientIds = new LinkedHashSet<>();
+	private final Set<String> trustedClientIds = new LinkedHashSet<>();
 
 	JwtAudienceValidator(String clientId) {
 		configureTrustedClientId(clientId);
@@ -34,7 +34,7 @@ public class JwtAudienceValidator implements Validator<Token> {
 
 	JwtAudienceValidator configureTrustedClientId(String clientId) {
 		assertHasText(clientId, "JwtAudienceValidator requires a clientId.");
-		clientIds.add(clientId);
+		trustedClientIds.add(clientId);
 		logger.info("configured JwtAudienceValidator with clientId {}.", clientId);
 
 		return this;
@@ -50,11 +50,11 @@ public class JwtAudienceValidator implements Validator<Token> {
 		}
 		return ValidationResults.createInvalid(
 				"Jwt token with audience {} is not issued for these clientIds: {}.",
-				token.getAudiences(), clientIds);
+				token.getAudiences(), trustedClientIds);
 	}
 
 	private boolean validateDefault(Set<String> allowedAudiences) {
-		for (String configuredClientId : clientIds) {
+		for (String configuredClientId : trustedClientIds) {
 			if (allowedAudiences.contains(configuredClientId)) {
 				return true;
 			}
@@ -63,7 +63,7 @@ public class JwtAudienceValidator implements Validator<Token> {
 	}
 
 	private boolean validateAudienceOfXsuaaBrokerClone(Set<String> allowedAudiences) {
-		for (String configuredClientId : clientIds) {
+		for (String configuredClientId : trustedClientIds) {
 			if (configuredClientId.contains("!b")) {
 				for (String audience : allowedAudiences) {
 					if (audience.endsWith("|" + configuredClientId)) {
@@ -85,7 +85,7 @@ public class JwtAudienceValidator implements Validator<Token> {
 		Set<String> audiences = new LinkedHashSet<>();
 
 		for (String audience : token.getAudiences()) {
-			if (audience.contains(".")) {
+			if (audience.contains("" + DOT)) {
 				// CF UAA derives the audiences from the scopes.
 				// In case the scopes contains namespaces, these needs to be removed.
 				String aud = extractAppId(audience);
@@ -117,7 +117,7 @@ public class JwtAudienceValidator implements Validator<Token> {
 	/**
 	 * In case of audiences, the namespaces are trimmed. In case of scopes, the
 	 * namespaces and the scope names are trimmed.
-	 * 
+	 *
 	 * @param scopeOrAudience
 	 * @return
 	 */
@@ -125,4 +125,7 @@ public class JwtAudienceValidator implements Validator<Token> {
 		return scopeOrAudience.substring(0, scopeOrAudience.indexOf(DOT)).trim();
 	}
 
+	public Set<String> getTrustedClientIds() {
+		return trustedClientIds;
+	}
 }
diff --git a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtValidatorBuilder.java b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtValidatorBuilder.java
index 2f20a1de..a3b8da6e 100644
--- a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtValidatorBuilder.java
+++ b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtValidatorBuilder.java
@@ -28,7 +28,7 @@ public class JwtValidatorBuilder {
 	private final Collection<Validator<Token>> validators = new ArrayList<>();
 	private final List<ValidationListener> validationListeners = new ArrayList<>();
 	private OAuth2ServiceConfiguration configuration;
-	private OAuth2ServiceConfiguration otherConfiguration;
+	private final Set<OAuth2ServiceConfiguration> otherConfigurations = new HashSet();
 	private OidcConfigurationService oidcConfigurationService = null;
 	private OAuth2TokenKeyService tokenKeyService = null;
 	private Validator<Token> customAudienceValidator;
@@ -149,8 +149,8 @@ public class JwtValidatorBuilder {
 	 */
 	public JwtValidatorBuilder configureAnotherServiceInstance(
 			@Nullable OAuth2ServiceConfiguration otherConfiguration) {
-		if (otherConfiguration != configuration) {
-			this.otherConfiguration = otherConfiguration;
+		if (Objects.nonNull(otherConfiguration)) {
+			this.otherConfigurations.add(otherConfiguration);
 		}
 		return this;
 	}
@@ -213,12 +213,12 @@ public class JwtValidatorBuilder {
 		if (configuration.hasProperty(CFConstants.XSUAA.APP_ID)) {
 			jwtAudienceValidator.configureTrustedClientId(configuration.getProperty(CFConstants.XSUAA.APP_ID));
 		}
-		if (otherConfiguration != null) {
+		otherConfigurations.forEach((otherConfiguration) -> {
 			jwtAudienceValidator.configureTrustedClientId(otherConfiguration.getClientId());
 			if (otherConfiguration.hasProperty(CFConstants.XSUAA.APP_ID)) {
 				jwtAudienceValidator.configureTrustedClientId(otherConfiguration.getProperty(CFConstants.XSUAA.APP_ID));
 			}
-		}
+		});
 		return jwtAudienceValidator;
 	}
 


Test patch:
diff --git a/java-security/src/test/java/com/sap/cloud/security/adapter/spring/SAPOfflineTokenServicesCloudTest.java b/java-security/src/test/java/com/sap/cloud/security/adapter/spring/SAPOfflineTokenServicesCloudTest.java
index 74e47080..9f102c24 100644
--- a/java-security/src/test/java/com/sap/cloud/security/adapter/spring/SAPOfflineTokenServicesCloudTest.java
+++ b/java-security/src/test/java/com/sap/cloud/security/adapter/spring/SAPOfflineTokenServicesCloudTest.java
@@ -1,6 +1,5 @@
 package com.sap.cloud.security.adapter.spring;
 
-import com.sap.cloud.security.adapter.spring.SAPOfflineTokenServicesCloud;
 import com.sap.cloud.security.config.OAuth2ServiceConfiguration;
 import com.sap.cloud.security.config.OAuth2ServiceConfigurationBuilder;
 import com.sap.cloud.security.config.Service;
@@ -25,15 +24,14 @@ import java.util.stream.Collectors;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 public class SAPOfflineTokenServicesCloudTest {
 
 	private SAPOfflineTokenServicesCloud cut;
 	private String xsuaaToken;
 	private String iasToken;
-	private JwtValidatorBuilder jwtValidatorBuilderMock;
+	private JwtValidatorBuilder jwtValidatorBuilderSpy;
 
 	public SAPOfflineTokenServicesCloudTest() throws IOException {
 		xsuaaToken = IOUtils.resourceToString("/xsuaaCCAccessTokenRSA256.txt", StandardCharsets.UTF_8);
@@ -45,15 +43,15 @@ public class SAPOfflineTokenServicesCloudTest {
 		OAuth2ServiceConfiguration configuration = OAuth2ServiceConfigurationBuilder
 				.forService(Service.XSUAA)
 				.withProperty(CFConstants.XSUAA.APP_ID, "testApp")
-				.withProperty(CFConstants.CLIENT_ID, "clientId")
+				.withProperty(CFConstants.CLIENT_ID, "clientId-offline")
 				.withProperty(CFConstants.XSUAA.UAA_DOMAIN, "localhost")
 				.build();
 
-		jwtValidatorBuilderMock = Mockito.spy(JwtValidatorBuilder.getInstance(configuration));
-		when(jwtValidatorBuilderMock.build()).thenReturn(
-				new CombiningValidator<>(token -> ValidationResults.createValid()));
+		jwtValidatorBuilderSpy = Mockito.spy(JwtValidatorBuilder.getInstance(configuration));
+		doReturn(new CombiningValidator<>(token -> ValidationResults.createValid())).when(jwtValidatorBuilderSpy)
+				.build();
 
-		cut = new SAPOfflineTokenServicesCloud(configuration, jwtValidatorBuilderMock);
+		cut = new SAPOfflineTokenServicesCloud(configuration, jwtValidatorBuilderSpy);
 		SecurityContext.clearToken();
 	}
 
@@ -81,7 +79,7 @@ public class SAPOfflineTokenServicesCloudTest {
 				.withProperty(CFConstants.CLIENT_ID, "clientId")
 				.build();
 
-		cut = new SAPOfflineTokenServicesCloud(configuration, jwtValidatorBuilderMock);
+		cut = new SAPOfflineTokenServicesCloud(configuration, jwtValidatorBuilderSpy);
 
 		cut.afterPropertiesSet();
 		OAuth2Authentication authentication = cut.loadAuthentication(iasToken);
@@ -123,7 +121,7 @@ public class SAPOfflineTokenServicesCloudTest {
 
 	@Test
 	public void loadAuthentication_tokenValidationFailed_throwsException() {
-		when(jwtValidatorBuilderMock.build()).thenCallRealMethod();
+		when(jwtValidatorBuilderSpy.build()).thenCallRealMethod();
 		cut.afterPropertiesSet();
 
 		assertThatThrownBy(() -> cut.loadAuthentication(xsuaaToken)).isInstanceOf(InvalidTokenException.class);
@@ -148,23 +146,30 @@ public class SAPOfflineTokenServicesCloudTest {
 	}
 
 	@Test
-	public void createInstanceWithAnotherConfiguration() {
+	public void createInstanceWithMultipleOtherConfigurations() {
 		OAuth2ServiceConfiguration otherConfiguration = Mockito.mock(OAuth2ServiceConfiguration.class);
-		when(otherConfiguration.getClientId()).thenReturn("clientId");
+		when(otherConfiguration.getClientId()).thenReturn("clientId1");
+
+		OAuth2ServiceConfiguration otherConfiguration2 = Mockito.mock(OAuth2ServiceConfiguration.class);
+		when(otherConfiguration2.getClientId()).thenReturn("clientId2");
 
 		cut.withAnotherServiceConfiguration(otherConfiguration);
+		cut.withAnotherServiceConfiguration(otherConfiguration2);
+
 		cut.afterPropertiesSet();
-		Mockito.verify(jwtValidatorBuilderMock,
+		Mockito.verify(jwtValidatorBuilderSpy,
 				times(1)).configureAnotherServiceInstance(otherConfiguration);
+		Mockito.verify(jwtValidatorBuilderSpy,
+				times(1)).configureAnotherServiceInstance(otherConfiguration2);
 	}
 
 	@Test
 	public void afterPropertiesSet() {
-		cut = new SAPOfflineTokenServicesCloud(Mockito.mock(OAuth2ServiceConfiguration.class), jwtValidatorBuilderMock);
+		cut = new SAPOfflineTokenServicesCloud(Mockito.mock(OAuth2ServiceConfiguration.class), jwtValidatorBuilderSpy);
 
-		Mockito.verify(jwtValidatorBuilderMock, times(0)).build();
+		Mockito.verify(jwtValidatorBuilderSpy, times(0)).build();
 		cut.afterPropertiesSet();
-		Mockito.verify(jwtValidatorBuilderMock, times(1)).build();
+		Mockito.verify(jwtValidatorBuilderSpy, times(1)).build();
 	}
 
 }
\ No newline at end of file
diff --git a/java-security/src/test/java/com/sap/cloud/security/config/OAuth2ServiceConfigurationBuilderTest.java b/java-security/src/test/java/com/sap/cloud/security/config/OAuth2ServiceConfigurationBuilderTest.java
index 9677bf71..570c2de4 100644
--- a/java-security/src/test/java/com/sap/cloud/security/config/OAuth2ServiceConfigurationBuilderTest.java
+++ b/java-security/src/test/java/com/sap/cloud/security/config/OAuth2ServiceConfigurationBuilderTest.java
@@ -78,4 +78,46 @@ public class OAuth2ServiceConfigurationBuilderTest {
 		assertThat(configuration.getUrl()).isEqualTo(URI.create(url));
 	}
 
+	@Test
+	public void equals_sameClientId() {
+		OAuth2ServiceConfiguration config = OAuth2ServiceConfigurationBuilder
+				.forService(Service.XSUAA)
+				.withClientId("client-id")
+				.build();
+
+		assertThat(cut.withClientId("client-id").build()).isEqualTo(config);
+	}
+
+	@Test
+	public void notEquals_differentClientId() {
+		OAuth2ServiceConfiguration config = OAuth2ServiceConfigurationBuilder
+				.forService(Service.XSUAA)
+				.withClientId("client-id")
+				.build();
+
+		assertThat(cut.withClientId("client-id2").build()).isNotEqualTo(config);
+	}
+
+	@Test
+	public void sameHashCode() {
+		OAuth2ServiceConfiguration config = OAuth2ServiceConfigurationBuilder
+				.forService(Service.XSUAA)
+				.withClientId("client-id").withClientSecret("secret")
+				.build();
+
+		assertThat(cut.withClientId("client-id").withClientSecret("secret").build().hashCode())
+				.isEqualTo(config.hashCode());
+	}
+
+	@Test
+	public void notSameHashCode() {
+		OAuth2ServiceConfiguration config = OAuth2ServiceConfigurationBuilder
+				.forService(Service.XSUAA)
+				.withClientId("client-id").withClientSecret("secret")
+				.build();
+
+		assertThat(cut.withClientId("client-id2").withClientSecret("secret-2").build().hashCode())
+				.isNotEqualTo(config.hashCode());
+	}
+
 }
\ No newline at end of file
diff --git a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/JwtValidatorBuilderTest.java b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/JwtValidatorBuilderTest.java
index 88cf9c0c..8c1c813b 100644
--- a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/JwtValidatorBuilderTest.java
+++ b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/JwtValidatorBuilderTest.java
@@ -5,12 +5,15 @@ import com.sap.cloud.security.config.OAuth2ServiceConfigurationBuilder;
 import com.sap.cloud.security.config.cf.CFConstants;
 import com.sap.cloud.security.token.Token;
 import com.sap.cloud.security.token.validation.*;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 
 import static com.sap.cloud.security.config.Service.IAS;
@@ -112,4 +115,34 @@ public class JwtValidatorBuilderTest {
 				.contains(tokenValidator);
 	}
 
+	@Test
+	public void configureOtherServiceInstances() {
+		Collection clientIds = new ArrayList();
+		JwtAudienceValidator audienceValidator;
+
+		OAuth2ServiceConfiguration xsuaaConfig1 = xsuaaConfigBuilder.build();
+		OAuth2ServiceConfiguration xsuaaConfig2 = OAuth2ServiceConfigurationBuilder.forService(XSUAA)
+				.withClientId("sb-test-app2!b222").build();
+		OAuth2ServiceConfiguration xsuaaConfig3 = OAuth2ServiceConfigurationBuilder.forService(XSUAA)
+				.withClientId("sb-test-app3!b333").build();
+
+		clientIds.add(xsuaaConfig1.getClientId());
+		clientIds.add(xsuaaConfig2.getClientId());
+		clientIds.add(xsuaaConfig3.getClientId());
+
+		CombiningValidator<Token> combiningValidator = JwtValidatorBuilder
+				.getInstance(xsuaaConfig1)
+				.configureAnotherServiceInstance(xsuaaConfig2)
+				.configureAnotherServiceInstance(xsuaaConfig3)
+				.build();
+
+		for (Validator validator : combiningValidator.getValidators()) {
+			if (validator instanceof JwtAudienceValidator) {
+				assertThat(((JwtAudienceValidator) validator).getTrustedClientIds()).containsAll(clientIds);
+				return;
+			}
+		}
+		Assert.fail("No JwtAudienceValidator found that contains all clientIds!"); // should never be called
+	}
+
 }
\ No newline at end of file