Repository: SAP/cloud-security-xsuaa-integration
Pull request #221 (2020-02-06 11:12:33)

XSUserInfoAdapter.isInForeignMode()


Patch:
diff --git a/api/src/main/java/com/sap/xsa/security/container/XSUserInfo.java b/api/src/main/java/com/sap/xsa/security/container/XSUserInfo.java
index b7807916..b3d13ff0 100644
--- a/api/src/main/java/com/sap/xsa/security/container/XSUserInfo.java
+++ b/api/src/main/java/com/sap/xsa/security/container/XSUserInfo.java
@@ -313,13 +313,11 @@ public interface XSUserInfo {
 	 * Check if a token issued for another OAuth client has been forwarded to a
 	 * different client,
 	 *
-	 * @deprecated tokens issued for a foreign OAuth Client Id and/or Identity Zone is no longer supported here.
 	 *
 	 * @return true if token was forwarded
 	 * @throws XSUserInfoException
 	 *             if attribute is not available in the authentication token
 	 */
-	@Deprecated
 	public boolean isInForeignMode() throws XSUserInfoException;
 
 	/**
diff --git a/java-security/src/main/java/com/sap/cloud/security/adapter/xs/XSUserInfoAdapter.java b/java-security/src/main/java/com/sap/cloud/security/adapter/xs/XSUserInfoAdapter.java
index ac2132d7..72f65426 100644
--- a/java-security/src/main/java/com/sap/cloud/security/adapter/xs/XSUserInfoAdapter.java
+++ b/java-security/src/main/java/com/sap/cloud/security/adapter/xs/XSUserInfoAdapter.java
@@ -6,17 +6,20 @@ import com.sap.cloud.security.json.JsonObject;
 import com.sap.cloud.security.json.JsonParsingException;
 import com.sap.cloud.security.token.GrantType;
 import com.sap.cloud.security.token.Token;
+import com.sap.cloud.security.token.TokenClaims;
 import com.sap.cloud.security.token.XsuaaToken;
 import com.sap.xsa.security.container.XSTokenRequest;
 import com.sap.xsa.security.container.XSUserInfo;
 import com.sap.xsa.security.container.XSUserInfoException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.annotation.Nullable;
 import java.util.Optional;
 import java.util.function.Supplier;
 
-import static com.sap.cloud.security.token.TokenClaims.XSUAA.*;
 import static com.sap.cloud.security.token.TokenClaims.*;
+import static com.sap.cloud.security.token.TokenClaims.XSUAA.*;
 
 public class XSUserInfoAdapter implements XSUserInfo {
 
@@ -29,6 +32,7 @@ public class XSUserInfoAdapter implements XSUserInfo {
 	static final String ZDN = "zdn";
 	static final String SYSTEM = "SYSTEM";
 	static final String HDB = "HDB";
+	private static final Logger LOGGER = LoggerFactory.getLogger(XSUserInfoAdapter.class);
 	private final XsuaaToken xsuaaToken;
 	private OAuth2ServiceConfiguration configuration;
 
@@ -90,7 +94,7 @@ public class XSUserInfoAdapter implements XSUserInfo {
 
 	@Override
 	public String getIdentityZone() throws XSUserInfoException {
-		return getClaimValue(SUBACCOUNT_ID);
+		return getClaimValue(TokenClaims.XSUAA.SUBACCOUNT_ID);
 	}
 
 	@Override
@@ -106,7 +110,7 @@ public class XSUserInfoAdapter implements XSUserInfo {
 	 *     },
 	 */
 	public String getSubdomain() throws XSUserInfoException {
-		return Optional.ofNullable(getExternalAttribute(ZDN)).orElseThrow(createXSUserInfoException(ZDN));
+		return Optional.ofNullable(getExternalAttribute(ZDN)).orElse(null);
 	}
 
 	@Override
@@ -142,7 +146,6 @@ public class XSUserInfoAdapter implements XSUserInfo {
 
 	@Override
 	public String getToken(String namespace, String name) throws XSUserInfoException {
-		// TODO 22.01.20 c5295400: TODO becaues foreignMode = false this is always false
 		if (!(getGrantType().equals(GrantType.CLIENT_CREDENTIALS.toString())) && hasAttributes() && isInForeignMode()) {
 			throw new XSUserInfoException("The SecurityContext has been initialized with an access token of a\n"
 					+ "foreign OAuth Client Id and/or Identity Zone. Furthermore, the\n"
@@ -226,33 +229,39 @@ public class XSUserInfoAdapter implements XSUserInfo {
 				.orElseThrow(createXSUserInfoException(GRANT_TYPE));
 	}
 
-	@Override
 	/**
 	 * Check if a token issued for another OAuth client has been forwarded to a
 	 * different client,
 	 *
 	 * @return true if token was forwarded or if it cannot be determined.
-	 * @throws XSUserInfoException
-	 *             if attribute is not available in the authentication token
 	 */
-	public boolean isInForeignMode() throws XSUserInfoException {
-		/**
-		 * TODO make more robust return true instead of exception
-		 * TODO apply logs
-		 * TODO do not check on subdomain, as this is not provided in context of XSA
-
-
-		if(configuration == null) {
-			configuration = Environments.getCurrent().getXsuaaConfiguration();
+	@Override
+	public boolean isInForeignMode() {
+		if (configuration == null) {
+			LOGGER.info("No configuration provided -> falling back to foreignMode = true!");
+			return true; // default provide OAuth2ServiceConfiguration via constructor argument
+		}
+		String tokenClientId, tokenIdentityZone;
+		try {
+			tokenClientId = getClientId();
+			tokenIdentityZone = getIdentityZone();
+		} catch (XSUserInfoException e) {
+			LOGGER.warn("Tried to access missing attribute when checking for foreign mode", e);
+			return true;
 		}
-		if(getClientId().equals(configuration.getClientId()) &&
-			 getSubdomain().equals(configuration.getProperty("identityzone"))) {
-			return false;
-		} else if (matchesTokenClientIdToBrokerCloneAppId()) {
-			return false;
+		boolean clientIdsMatch = tokenClientId.equals(configuration.getClientId());
+		boolean identityZonesMatch = tokenIdentityZone
+				.equals(configuration.getProperty(CFConstants.XSUAA.IDENTITY_ZONE));
+		boolean isApplicationPlan = tokenClientId.contains("!t");
+		if (clientIdsMatch && (identityZonesMatch || isApplicationPlan)) {
+			return false; // no foreign mode
 		}
-		return true;*/
-		return false;
+		// in case of broker master: check trustedclientidsuffix
+		String bindingTrustedClientIdSuffix = configuration.getProperty(TRUSTED_CLIENT_ID_SUFFIX);
+		if (bindingTrustedClientIdSuffix != null && tokenClientId.endsWith(bindingTrustedClientIdSuffix)) {
+			return false; // no foreign mode
+		}
+		return true;
 	}
 
 	@Override
@@ -288,10 +297,6 @@ public class XSUserInfoAdapter implements XSUserInfo {
 				.map(claim -> claim.getAsString(attributeName)).orElse(null);
 	}
 
-	String getExternalAttribute(String attributeName) throws XSUserInfoException {
-		return getAttributeFromClaimAsString(EXTERNAL_ATTRIBUTE, attributeName);
-	}
-
 	private Supplier<XSUserInfoException> createXSUserInfoException(String attribute) {
 		return () -> new XSUserInfoException("Invalid user attribute " + attribute);
 	}
@@ -313,11 +318,8 @@ public class XSUserInfoAdapter implements XSUserInfo {
 		}
 	}
 
-	private boolean matchesTokenClientIdToBrokerCloneAppId() throws XSUserInfoException {
-		String appId = configuration.getProperty(CFConstants.XSUAA.APP_ID);
-		return appId.contains("!b") // broker plan
-				&& getClientId().contains("|")
-				&& getClientId().endsWith("|" + appId);
+	String getExternalAttribute(String attributeName) throws XSUserInfoException {
+		return getAttributeFromClaimAsString(EXTERNAL_ATTRIBUTE, attributeName);
 	}
 
 }
diff --git a/java-security/src/main/java/com/sap/cloud/security/config/cf/CFConstants.java b/java-security/src/main/java/com/sap/cloud/security/config/cf/CFConstants.java
index 25c68e36..3b056868 100644
--- a/java-security/src/main/java/com/sap/cloud/security/config/cf/CFConstants.java
+++ b/java-security/src/main/java/com/sap/cloud/security/config/cf/CFConstants.java
@@ -23,6 +23,7 @@ public class CFConstants {
 		private XSUAA() {
 		}
 
+		public static final String IDENTITY_ZONE = "identityzone";
 		public static final String UAA_DOMAIN = "uaadomain";
 		public static final String APP_ID = "xsappname";
 	}
diff --git a/java-security/src/main/java/com/sap/cloud/security/json/JsonObject.java b/java-security/src/main/java/com/sap/cloud/security/json/JsonObject.java
index 935316a4..7f154e60 100644
--- a/java-security/src/main/java/com/sap/cloud/security/json/JsonObject.java
+++ b/java-security/src/main/java/com/sap/cloud/security/json/JsonObject.java
@@ -108,8 +108,8 @@ public interface JsonObject {
 	 * 	String vcapServices = System.getenv(CFConstants.VCAP_SERVICES);
 	 * 	JsonObject serviceJsonObject = new DefaultJsonObject(vcapServices).getJsonObjects(Service.XSUAA.getCFName())
 	 * 			.get(0);
-	 * 	Map<String, String> xsuaaConfigMap = serviceJsonObject.getKeyValueMap();
-	 * 	Map<String, String> credentialsMap = serviceJsonObject.getJsonObject(CFConstants.CREDENTIALS)
+	 * 	Map&lt;String, String&gt; xsuaaConfigMap = serviceJsonObject.getKeyValueMap();
+	 * 	Map&lt;String, String&gt; credentialsMap = serviceJsonObject.getJsonObject(CFConstants.CREDENTIALS)
 	 * 			.getKeyValueMap();
 	 * }
 	 * </pre>
diff --git a/java-security/src/main/java/com/sap/cloud/security/token/TokenClaims.java b/java-security/src/main/java/com/sap/cloud/security/token/TokenClaims.java
index 9f40aa1a..98e92c8e 100644
--- a/java-security/src/main/java/com/sap/cloud/security/token/TokenClaims.java
+++ b/java-security/src/main/java/com/sap/cloud/security/token/TokenClaims.java
@@ -20,6 +20,8 @@ public final class TokenClaims {
 	public static final String EMAIL = "email";
 
 	public final class XSUAA {
+
+
 		private XSUAA() {
 		}
 
@@ -32,6 +34,8 @@ public final class TokenClaims {
 														// "my-app!t123.Display"
 		public static final String EXTERNAL_ATTRIBUTE = "ext_attr";
 		public static final String EXTERNAL_ATTRIBUTE_ENHANCER = "enhancer";
+
+		public static final String TRUSTED_CLIENT_ID_SUFFIX = "trustedclientidsuffix";
 	}
 
 	/* SAP User token


Test patch:
diff --git a/java-security/src/test/java/com/sap/cloud/security/adapter/xs/XSUserInfoAdapterTest.java b/java-security/src/test/java/com/sap/cloud/security/adapter/xs/XSUserInfoAdapterTest.java
index 35f685d5..05345860 100644
--- a/java-security/src/test/java/com/sap/cloud/security/adapter/xs/XSUserInfoAdapterTest.java
+++ b/java-security/src/test/java/com/sap/cloud/security/adapter/xs/XSUserInfoAdapterTest.java
@@ -5,19 +5,18 @@ import com.sap.cloud.security.config.OAuth2ServiceConfigurationBuilder;
 import com.sap.cloud.security.config.Service;
 import com.sap.cloud.security.config.cf.CFConstants;
 import com.sap.cloud.security.json.JsonObject;
-import com.sap.cloud.security.token.GrantType;
-import com.sap.cloud.security.token.TokenClaims;
-import com.sap.cloud.security.token.XsuaaScopeConverter;
-import com.sap.cloud.security.token.XsuaaToken;
+import com.sap.cloud.security.token.*;
 import com.sap.xsa.security.container.XSUserInfoException;
 import org.apache.commons.io.IOUtils;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
+import org.mockito.Mockito;
 
 import java.io.IOException;
 
 import static com.sap.cloud.security.adapter.xs.XSUserInfoAdapter.*;
+import static com.sap.cloud.security.config.cf.CFConstants.XSUAA.IDENTITY_ZONE;
+import static com.sap.cloud.security.token.TokenClaims.XSUAA.TRUSTED_CLIENT_ID_SUFFIX;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -37,7 +36,12 @@ public class XSUserInfoAdapterTest {
 
 	@Before
 	public void setUp() throws XSUserInfoException {
-		cut = spy(new XSUserInfoAdapter(token.withScopeConverter(new XsuaaScopeConverter(TEST_APP_ID))));
+		OAuth2ServiceConfiguration configuration = OAuth2ServiceConfigurationBuilder.forService(Service.XSUAA)
+				.withClientId("sb-clone1!b5|LR-master!b5")
+				.withProperty(CFConstants.XSUAA.APP_ID, "brokerplanmasterapp!b123")
+				.withProperty(IDENTITY_ZONE, "paas")
+				.build();
+		cut = new XSUserInfoAdapter(token.withScopeConverter(new XsuaaScopeConverter(TEST_APP_ID)), configuration);
 	}
 
 	// TODO 22.01.20 c5295400: implement external context fallback tests
@@ -129,19 +133,16 @@ public class XSUserInfoAdapterTest {
 
 	@Test
 	public void testGetDBToken() throws XSUserInfoException {
-		when(cut.isInForeignMode()).thenReturn(false);
 		assertThat(cut.getDBToken()).isEqualTo(cut.getAppToken());
 	}
 
 	@Test
 	public void testGetHdbToken() throws XSUserInfoException {
-		when(cut.isInForeignMode()).thenReturn(false);
 		assertThat(cut.getHdbToken()).isEqualTo(cut.getAppToken());
 	}
 
 	@Test
 	public void getToken_fallbackToTokenValue() throws XSUserInfoException {
-		when(cut.isInForeignMode()).thenReturn(false);
 		assertThat(cut.getToken(XSUserInfoAdapter.SYSTEM, XSUserInfoAdapter.HDB)).isEqualTo(token.getTokenValue());
 	}
 
@@ -165,7 +166,7 @@ public class XSUserInfoAdapterTest {
 		when(mockToken.getClaimAsJsonObject(XS_USER_ATTRIBUTES)).thenReturn(mock(JsonObject.class));
 
 		cut = spy(new XSUserInfoAdapter(mockToken));
-		when(cut.isInForeignMode()).thenReturn(false);
+		doReturn(false).when(cut).isInForeignMode();
 
 		assertThat(cut.getToken(XSUserInfoAdapter.SYSTEM, XSUserInfoAdapter.HDB)).isEqualTo(internalToken);
 	}
@@ -236,6 +237,12 @@ public class XSUserInfoAdapterTest {
 		assertThat(cut.getGrantType()).isEqualTo("urn:ietf:params:oauth:grant-type:saml2-bearer");
 	}
 
+	@Test
+	public void isForeignModeIsTrue_whenConfigurationIsNotAvailable() throws XSUserInfoException {
+		cut = new XSUserInfoAdapter(token);
+		assertThat(cut.isInForeignMode()).isTrue();
+	}
+
 	@Test
 	public void testGetSubaccountId() throws XSUserInfoException {
 		assertThat(cut.getSubaccountId()).isEqualTo("paas");
@@ -288,63 +295,144 @@ public class XSUserInfoAdapterTest {
 	}
 
 	@Test
-	public void isByDefaultInForeignMode() throws XSUserInfoException {
-		assertThat(cut.isInForeignMode()).isFalse();
-	}
-
-	@Test
-	public void getHdbToken() throws XSUserInfoException, IOException {
+	public void getHdbToken_AuthCodeToken_NoAttributes() throws XSUserInfoException, IOException {
 		XsuaaToken token = new XsuaaToken(IOUtils.resourceToString("/xsuaaXsaAccessTokenRSA256_signedWithVerificationKey.txt", UTF_8));
 		OAuth2ServiceConfiguration configuration = OAuth2ServiceConfigurationBuilder.forService(Service.XSUAA)
 				.withClientId("sb-java-hello-world!i1")
 				.withProperty(CFConstants.XSUAA.APP_ID, "java-hello-world!i1")
-				.withProperty("identityzone", "uaa")
+				.withProperty(IDENTITY_ZONE, "uaa")
 				.withProperty("identityzoneid", "uaa")
 				.build();
-		cut = spy(new XSUserInfoAdapter(token, configuration));
-		assertThat(cut.isInForeignMode()).isFalse();
+
+		cut = new XSUserInfoAdapter(token, configuration);
+
 		assertThat(cut.getHdbToken()).isNotNull();
 		assertThat(cut.getHdbToken()).startsWith("eyJhbGciOiAiUlMyNTYiLCJ0eXAiOiAiS");
 	}
 
 	@Test
-	@Ignore
-	public void isForeignModeFalse_WhenTokenCloneIdMatchesBrokerAppId() throws XSUserInfoException {
+	public void getHdbToken_AudCodeToken_WithAttributes() throws XSUserInfoException {
+		XsuaaToken token = mock(XsuaaToken.class);
+		String mockTokenValue = "mock token value";
+
+		when(token.getTokenValue()).thenReturn(mockTokenValue);
+		when(token.getClaimAsString(TokenClaims.XSUAA.CLIENT_ID)).thenReturn("sb-margin-assurance-ui!i1");
+		when(token.getClaimAsString(TokenClaims.XSUAA.SUBACCOUNT_ID)).thenReturn("uaa");
+		when(token.getGrantType()).thenReturn(GrantType.AUTHORIZATION_CODE);
+
+		JsonObject xsUserAttributes = mock(JsonObject.class);
+		when(xsUserAttributes.isEmpty()).thenReturn(false);
+		when(token.getClaimAsJsonObject(XS_USER_ATTRIBUTES)).thenReturn(xsUserAttributes);
+
+		OAuth2ServiceConfiguration configuration = OAuth2ServiceConfigurationBuilder.forService(Service.XSUAA)
+				.withClientId("sb-margin-assurance-ui!i1")
+				.withProperty(CFConstants.XSUAA.APP_ID, "margin-assurance-ui!i1")
+				.withProperty(IDENTITY_ZONE, "uaa")
+				.withProperty("identityzoneid", "uaa")
+				.build();
+
+
+		cut = new XSUserInfoAdapter(token, configuration);
+
+		assertThat(cut.getHdbToken()).isNotNull();
+		assertThat(cut.getHdbToken()).isEqualTo(mockTokenValue);
+	}
+
+	@Test
+	public void isForeignModeFalse_whenTrustedClientIdSuffixMatches() throws XSUserInfoException {
 		String tokenClientId = "sb-clone1!b22|brokerplanmasterapp!b123"; // cid
 		String configurationAppId = "brokerplanmasterapp!b123";
-
 		XsuaaToken token = mock(XsuaaToken.class);
 		when(token.getClaimAsString(TokenClaims.XSUAA.CLIENT_ID)).thenReturn(tokenClientId);
+		when(token.getClaimAsString(TokenClaims.XSUAA.SUBACCOUNT_ID)).thenReturn("otherIdentityZone");
 
 		OAuth2ServiceConfiguration configuration = OAuth2ServiceConfigurationBuilder.forService(Service.XSUAA)
 				.withClientId(tokenClientId)
+				.withProperty(CFConstants.CLIENT_ID, tokenClientId)
 				.withProperty(CFConstants.XSUAA.APP_ID, configurationAppId)
+				.withProperty(IDENTITY_ZONE, "uaa")
+				.withProperty(TRUSTED_CLIENT_ID_SUFFIX, "|brokerplanmasterapp!b123")
 				.build();
 
-		cut = spy(new XSUserInfoAdapter(token, configuration));
-		when(cut.getSubdomain()).thenReturn("otherSubDomain");
+		cut = new XSUserInfoAdapter(token, configuration);
+
 		assertThat(cut.isInForeignMode()).isFalse();
 	}
 
 	@Test
-	@Ignore
-	public void isForeignModeFalse_WhenClientIdAndPaasSubdomainMatches() throws XSUserInfoException {
+	public void isForeignModeFalse_WhenIdentityZoneDoesNotMatchButCliendIdIsApplicationPlan() throws XSUserInfoException {
 		String tokenClientId = "sb-application!t0123"; // cid
-		String tokenSubdomain = "brokerplanmasterapp!b123"; // ext_attr -> zdn
+		String identityZone = "brokerplanmasterapp!b123"; // ext_attr -> zdn
+		OAuth2ServiceConfiguration configuration = OAuth2ServiceConfigurationBuilder.forService(Service.XSUAA)
+				.withClientId(tokenClientId)
+				.withProperty(IDENTITY_ZONE, identityZone)
+				.build();
 
-		XsuaaToken token = mock(XsuaaToken.class);
-		when(token.getClaimAsString(TokenClaims.XSUAA.CLIENT_ID)).thenReturn(tokenClientId);
+		cut = createComponentUnderTestSpy(configuration);
+		doReturn(tokenClientId).when(cut).getClientId();
+		doReturn("otherIdentityZone").when(cut).getIdentityZone();
+
+		assertThat(cut.isInForeignMode()).isFalse();
+	}
 
+	@Test
+	public void isForeignModeFalse_WhenClientIdAndIdentityZonesMatch() throws XSUserInfoException {
+		String tokenClientId = "sb-application"; // cid
+		String identityZone = "brokerplanmasterapp!b123"; // ext_attr -> zdn
 		OAuth2ServiceConfiguration configuration = OAuth2ServiceConfigurationBuilder.forService(Service.XSUAA)
 				.withClientId(tokenClientId)
-				.withProperty("identityzone", tokenSubdomain)
+				.withProperty(IDENTITY_ZONE, identityZone)
 				.build();
 
-		cut = spy(new XSUserInfoAdapter(token, configuration));
-		when(cut.getSubdomain()).thenReturn(tokenSubdomain);
+
+		cut = createComponentUnderTestSpy(configuration);
+		doReturn(tokenClientId).when(cut).getClientId();
+		doReturn(identityZone).when(cut).getIdentityZone();
+
 		assertThat(cut.isInForeignMode()).isFalse();
 	}
 
+	@Test
+	public void isForeignModeTrue_whenClientIdDoesNotMatchIdentityZone() throws XSUserInfoException {
+		OAuth2ServiceConfiguration configuration = OAuth2ServiceConfigurationBuilder.forService(Service.XSUAA)
+				.withProperty(CFConstants.XSUAA.APP_ID, "sb-application")
+				.build();
+
+		cut = createComponentUnderTestSpy(configuration);
+		doReturn("sb-application!t0123").when(cut).getClientId();
+		doReturn("otherIdentityZone").when(cut).getIdentityZone();
+
+		assertThat(cut.isInForeignMode()).isTrue();
+	}
+
+	@Test
+	public void isForeignModeTrue_whenClientIdIsMissing() throws XSUserInfoException {
+		cut = createComponentUnderTestSpy();
+
+		doReturn("brokerplanmasterapp!b123").when(cut).getIdentityZone();
+		doThrow(new XSUserInfoException("")).when(cut).getClientId();
+
+		assertThat(cut.isInForeignMode()).isTrue();
+	}
+
+	@Test
+	public void isForeignModeTrue_whenIdentityZoneIsMissing() throws XSUserInfoException {
+		cut = createComponentUnderTestSpy();
+
+		doReturn("sb-application!t0123").when(cut).getClientId();
+		doThrow(new XSUserInfoException("")).when(cut).getIdentityZone();
+
+		assertThat(cut.isInForeignMode()).isTrue();
+	}
+
+	private XSUserInfoAdapter createComponentUnderTestSpy() throws XSUserInfoException {
+		return spy(new XSUserInfoAdapter(mock(XsuaaToken.class), mock(OAuth2ServiceConfiguration.class)));
+	}
+
+
+	private XSUserInfoAdapter createComponentUnderTestSpy(OAuth2ServiceConfiguration configuration) throws XSUserInfoException {
+		return spy(new XSUserInfoAdapter(Mockito.mock(XsuaaToken.class), configuration));
+	}
 
 	private XsuaaToken createMockToken(GrantType grantType) {
 		XsuaaToken mockToken = mock(XsuaaToken.class);