Repository: SAP/cloud-security-xsuaa-integration
Pull request #710 (2021-11-09 14:36:19)

fix misuse of OAuth2ServiceConfiguration.getClientIdentity() in XsuaaServiceConfiguration
To avoid confusion when exception `java.lang.UnsupportedOperationException: getProperty method` is thrown for custom `XsuaaServiceConfiguration` implementations where the default `OAuth2ServiceConfiguration.getClientIdentity()` is not overridden, provide default `getClientIdentity()` method in `XsuaaClientConfiguration` interface that will notify user to implement the method and won't use the default from `OAuth2ServiceConfiguration` interface that calls unsupported `getProperty()` method.


Patch:
diff --git a/java-api/pom.xml b/java-api/pom.xml
index 3a7ef4f6..7998e7d6 100644
--- a/java-api/pom.xml
+++ b/java-api/pom.xml
@@ -69,11 +69,6 @@
 			<artifactId>commons-io</artifactId>
 			<scope>test</scope>
 		</dependency>
-		<dependency> <!-- check if it's still needed when slf4j-api 1.8 is available -->
-			<groupId>org.slf4j</groupId>
-			<artifactId>slf4j-simple</artifactId>
-			<scope>test</scope>
-		</dependency>
 	</dependencies>
 
 	<build>
diff --git a/java-api/src/main/java/com/sap/cloud/security/token/Token.java b/java-api/src/main/java/com/sap/cloud/security/token/Token.java
index 4afd8a5f..db062d54 100644
--- a/java-api/src/main/java/com/sap/cloud/security/token/Token.java
+++ b/java-api/src/main/java/com/sap/cloud/security/token/Token.java
@@ -14,12 +14,11 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import java.io.Serializable;
 import java.security.Principal;
+import java.security.ProviderException;
 import java.time.Instant;
 import java.util.*;
 
-import static com.sap.cloud.security.token.TokenClaims.AUDIENCE;
-import static com.sap.cloud.security.token.TokenClaims.AUTHORIZATION_PARTY;
-import static com.sap.cloud.security.token.TokenClaims.ISSUER;
+import static com.sap.cloud.security.token.TokenClaims.*;
 
 /**
  * Represents a JSON Web Token (JWT).
@@ -34,6 +33,8 @@ public interface Token extends Serializable {
 		}
 	};
 
+	String DEFAULT_TOKEN_FACTORY = "com.sap.cloud.security.servlet.HybridTokenFactory";
+
 	/**
 	 * Creates a token instance based on TokenFactory implementation.
 	 * 
@@ -45,6 +46,15 @@ public interface Token extends Serializable {
 		if (services.isEmpty()) {
 			throw new ProviderNotFoundException("No TokenFactory implementation found in the classpath");
 		}
+		if (services.size() > 2) {
+			throw new ProviderException("More than 1 Custom TokenFactory service provider found. There should be only one");
+		}
+		if (services.size() == 2) {
+			return services.stream()
+					.filter(tokenFactory -> !tokenFactory.getClass().getName()
+							.equals(DEFAULT_TOKEN_FACTORY))
+					.findFirst().get().create(jwt);
+		}
 		return services.get(0).create(jwt);
 	}
 
diff --git a/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/XsuaaServiceConfiguration.java b/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/XsuaaServiceConfiguration.java
index 9e31bf6d..5ed98540 100644
--- a/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/XsuaaServiceConfiguration.java
+++ b/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/XsuaaServiceConfiguration.java
@@ -5,6 +5,7 @@
  */
 package com.sap.cloud.security.xsuaa;
 
+import com.sap.cloud.security.config.ClientIdentity;
 import com.sap.cloud.security.config.OAuth2ServiceConfiguration;
 import com.sap.cloud.security.config.Service;
 
@@ -65,6 +66,14 @@ public interface XsuaaServiceConfiguration extends OAuth2ServiceConfiguration {
 		throw new UnsupportedOperationException("hasProperty method is not supported");
 	}
 
+	@Override
+	default ClientIdentity getClientIdentity() {
+		throw new UnsupportedOperationException(
+				"This default method needs to be overridden to be used! Default method from " +
+						"com.sap.cloud.security.config.OAuth2ServiceConfiguration#getClientIdentity() " +
+						"is not compatible with XsuaaServiceConfiguration interface");
+	}
+
 	@Override
 	default Service getService() {
 		throw new UnsupportedOperationException("getService method is not supported");
diff --git a/token-client/pom.xml b/token-client/pom.xml
index 0a9b8b28..211b4f43 100644
--- a/token-client/pom.xml
+++ b/token-client/pom.xml
@@ -97,11 +97,6 @@
 			<artifactId>spring-security-oauth2-jose</artifactId>
 			<scope>test</scope>
 		</dependency>
-		<dependency> <!-- check if it's still needed when slf4j-api 1.8 is available -->
-			<groupId>org.slf4j</groupId>
-			<artifactId>slf4j-simple</artifactId>
-			<scope>test</scope>
-		</dependency>
 	</dependencies>
 
 	<build>
diff --git a/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java b/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java
index 17b55f27..79bb944e 100644
--- a/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java
+++ b/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java
@@ -25,6 +25,7 @@ public class DefaultHttpClientFactory implements HttpClientFactory {
 
 	private static final Logger LOGGER = LoggerFactory.getLogger(DefaultHttpClientFactory.class);
 
+	@Override
 	public CloseableHttpClient createClient(ClientIdentity clientIdentity) throws HttpClientException {
 		LOGGER.warn("In productive environment provide well configured HttpClientFactory service");
 		if (clientIdentity != null && clientIdentity.isCertificateBased()) {
diff --git a/token-client/src/main/java/com/sap/cloud/security/client/HttpClientFactory.java b/token-client/src/main/java/com/sap/cloud/security/client/HttpClientFactory.java
index 57681c1e..f8d37a0b 100644
--- a/token-client/src/main/java/com/sap/cloud/security/client/HttpClientFactory.java
+++ b/token-client/src/main/java/com/sap/cloud/security/client/HttpClientFactory.java
@@ -11,6 +11,7 @@ import com.sap.cloud.security.token.ProviderNotFoundException;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.slf4j.LoggerFactory;
 
+import java.security.ProviderException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.ServiceLoader;
@@ -19,6 +20,7 @@ import java.util.ServiceLoader;
  * Represents a {@link CloseableHttpClient} creation interface.
  */
 public interface HttpClientFactory {
+
 	@SuppressWarnings("unchecked")
 	List<HttpClientFactory> services = new ArrayList() {
 		{
@@ -28,6 +30,9 @@ public interface HttpClientFactory {
 		}
 	};
 
+	String DEFAULT_HTTP_CLIENT_FACTORY = "com.sap.cloud.security.client.DefaultHttpClientFactory";
+
+
 	/**
 	 * Provides CloseableHttpClient based on ClientIdentity details. For
 	 * ClientIdentity that is certificate based it will resolve https client using
@@ -48,6 +53,16 @@ public interface HttpClientFactory {
 		if (services.isEmpty()) {
 			throw new ProviderNotFoundException("No HttpClientFactory service could be found in the classpath");
 		}
+		if (services.size() > 2) {
+			throw new ProviderException(
+					"More than 1 Custom HttpClientFactory service provider found. There should be only one");
+		}
+		if (services.size() == 2) {
+			return services.stream()
+					.filter(httpClientFactory -> !httpClientFactory.getClass().getName()
+							.equals(DEFAULT_HTTP_CLIENT_FACTORY))
+					.findFirst().get().createClient(clientIdentity);
+		}
 		return services.get(0).createClient(clientIdentity);
 	}
 


Test patch:
diff --git a/java-api/src/test/java/com/sap/cloud/security/servlet/HybridTokenFactory.java b/java-api/src/test/java/com/sap/cloud/security/servlet/HybridTokenFactory.java
new file mode 100644
index 00000000..b33357cf
--- /dev/null
+++ b/java-api/src/test/java/com/sap/cloud/security/servlet/HybridTokenFactory.java
@@ -0,0 +1,19 @@
+/**
+ * SPDX-FileCopyrightText: 2018-2021 SAP SE or an SAP affiliate company and Cloud Security Client Java contributors
+ * <p>
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package com.sap.cloud.security.servlet;
+
+import com.sap.cloud.security.token.AccessToken;
+import com.sap.cloud.security.token.Token;
+import com.sap.cloud.security.token.TokenFactory;
+import org.mockito.Mockito;
+
+public class HybridTokenFactory implements TokenFactory {
+
+	@Override
+	public Token create(String jwtToken) {
+		return Mockito.mock(AccessToken.class);
+	}
+}
diff --git a/java-api/src/test/java/com/sap/cloud/security/token/TokenTest.java b/java-api/src/test/java/com/sap/cloud/security/token/TokenTest.java
index 8aecb404..7265e60f 100644
--- a/java-api/src/test/java/com/sap/cloud/security/token/TokenTest.java
+++ b/java-api/src/test/java/com/sap/cloud/security/token/TokenTest.java
@@ -7,6 +7,7 @@ package com.sap.cloud.security.token;
 
 import org.junit.Test;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 
 public class TokenTest {
@@ -18,5 +19,10 @@ public class TokenTest {
 
 		cut = Token.create("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9");
 		assertNotNull(cut);
+
+		// Assert that custom Token factory has a priority over default
+		// com.sap.cloud.security.servlet.HybridTokenFactory
+		assertFalse(cut.getClass().getName().contains("AccessToken"));
 	}
+
 }
diff --git a/java-api/src/test/resources/META-INF/services/com.sap.cloud.security.token.TokenFactory b/java-api/src/test/resources/META-INF/services/com.sap.cloud.security.token.TokenFactory
index c2cb9383..70fc90c0 100644
--- a/java-api/src/test/resources/META-INF/services/com.sap.cloud.security.token.TokenFactory
+++ b/java-api/src/test/resources/META-INF/services/com.sap.cloud.security.token.TokenFactory
@@ -1,1 +1,2 @@
+com.sap.cloud.security.servlet.HybridTokenFactory
 com.sap.cloud.security.token.test.CustomTokenFactory
\ No newline at end of file
diff --git a/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/DummyXsuaaServiceConfiguration.java b/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/DummyXsuaaServiceConfiguration.java
index 6a33f4f6..8c9f8d3a 100644
--- a/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/DummyXsuaaServiceConfiguration.java
+++ b/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/DummyXsuaaServiceConfiguration.java
@@ -5,6 +5,8 @@
  */
 package com.sap.cloud.security.xsuaa;
 
+import com.sap.cloud.security.config.ClientCredentials;
+import com.sap.cloud.security.config.ClientIdentity;
 import com.sap.cloud.security.config.CredentialType;
 
 public class DummyXsuaaServiceConfiguration implements XsuaaServiceConfiguration {
@@ -56,9 +58,9 @@ public class DummyXsuaaServiceConfiguration implements XsuaaServiceConfiguration
 		return null;
 	}
 
-	// TODO remove once credential-type is available in IAS configuration
 	@Override
-	public String getProperty(String name) {
-		return null;
+	public ClientIdentity getClientIdentity() {
+		return new ClientCredentials(getClientId(), getClientSecret());
 	}
+
 }
diff --git a/token-client/src/test/java/com/sap/cloud/security/client/HttpClientFactoryTest.java b/token-client/src/test/java/com/sap/cloud/security/client/HttpClientFactoryTest.java
new file mode 100644
index 00000000..9313a1b6
--- /dev/null
+++ b/token-client/src/test/java/com/sap/cloud/security/client/HttpClientFactoryTest.java
@@ -0,0 +1,27 @@
+/**
+ * SPDX-FileCopyrightText: 2018-2021 SAP SE or an SAP affiliate company and Cloud Security Client Java contributors
+ * <p>
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package com.sap.cloud.security.client;
+
+import com.sap.cloud.security.config.ClientCredentials;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class HttpClientFactoryTest {
+
+	@Test
+	public void create() {
+		CloseableHttpClient cut = HttpClientFactory.create(new ClientCredentials("clientId", "secret"));
+		assertNotNull(cut);
+
+		// Assert that custom HttpClientFactory factory has a priority over default
+		// com.sap.cloud.security.client.DefaultHttpClientFactory
+		assertFalse(cut.getClass().getName().contains("InternalHttpClient"));
+		assertTrue(cut.getClass().getName().contains("CloseableHttpClient$MockitoMock"));
+	}
+
+}
\ No newline at end of file
diff --git a/token-client/src/test/java/com/sap/cloud/security/client/TestHttpClientFactory.java b/token-client/src/test/java/com/sap/cloud/security/client/TestHttpClientFactory.java
new file mode 100644
index 00000000..ee2d7f66
--- /dev/null
+++ b/token-client/src/test/java/com/sap/cloud/security/client/TestHttpClientFactory.java
@@ -0,0 +1,18 @@
+/**
+ * SPDX-FileCopyrightText: 2018-2021 SAP SE or an SAP affiliate company and Cloud Security Client Java contributors
+ * <p>
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package com.sap.cloud.security.client;
+
+import com.sap.cloud.security.config.ClientIdentity;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.mockito.Mockito;
+
+public class TestHttpClientFactory implements HttpClientFactory {
+
+	@Override
+	public CloseableHttpClient createClient(ClientIdentity clientIdentity) throws HttpClientException {
+		return Mockito.mock(CloseableHttpClient.class);
+	}
+}
diff --git a/token-client/src/test/resources/META-INF/services/com.sap.cloud.security.client.HttpClientFactory b/token-client/src/test/resources/META-INF/services/com.sap.cloud.security.client.HttpClientFactory
new file mode 100644
index 00000000..6b945a92
--- /dev/null
+++ b/token-client/src/test/resources/META-INF/services/com.sap.cloud.security.client.HttpClientFactory
@@ -0,0 +1,2 @@
+com.sap.cloud.security.client.DefaultHttpClientFactory
+com.sap.cloud.security.client.TestHttpClientFactory
\ No newline at end of file