Repository: SAP/cloud-security-xsuaa-integration
Pull request #222 (2020-02-07 15:03:45)

Added more logging information for isInForeignMode
This can be helpful to trace issues in production. If not needed then this PR can be closed.


Patch:
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 94f00f0e..2bcba621 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
@@ -1,15 +1,13 @@
 package com.sap.cloud.security.adapter.xs;
 
-import com.sap.cloud.security.config.Environment;
-import com.sap.cloud.security.config.Environments;
 import com.sap.cloud.security.config.OAuth2ServiceConfiguration;
 import com.sap.cloud.security.config.cf.CFConstants;
 import com.sap.cloud.security.json.JsonObject;
 import com.sap.cloud.security.json.JsonParsingException;
-import com.sap.cloud.security.token.AccessToken;
 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;
@@ -35,26 +33,28 @@ public class XSUserInfoAdapter implements XSUserInfo {
 	static final String SYSTEM = "SYSTEM";
 	static final String HDB = "HDB";
 	private static final Logger LOGGER = LoggerFactory.getLogger(XSUserInfoAdapter.class);
-	private final AccessToken accessToken;
+	private final XsuaaToken xsuaaToken;
 	private OAuth2ServiceConfiguration configuration;
 
-	public XSUserInfoAdapter(Token accessToken) throws XSUserInfoException {
-		this(accessToken, Environments.getCurrent().getXsuaaConfiguration());
+	public XSUserInfoAdapter(Token xsuaaToken) throws XSUserInfoException {
+		if (!(xsuaaToken instanceof XsuaaToken)) {
+			throw new XSUserInfoException("token needs to be an instance of XsuaaToken.");
+		}
+		this.xsuaaToken = (XsuaaToken) xsuaaToken;
 	}
 
-	public XSUserInfoAdapter(AccessToken accessToken) throws XSUserInfoException {
-		if (accessToken == null) {
+	public XSUserInfoAdapter(XsuaaToken xsuaaToken) throws XSUserInfoException {
+		if (xsuaaToken == null) {
 			throw new XSUserInfoException("token must not be null.");
 		}
-		this.accessToken = accessToken;
+		this.xsuaaToken = xsuaaToken;
 	}
 
-	XSUserInfoAdapter(Token accessToken, OAuth2ServiceConfiguration configuration) throws XSUserInfoException {
-		if (!(accessToken instanceof AccessToken)) {
-			throw new XSUserInfoException("token is of instance " + accessToken.getClass().getName()
-					+ " but needs to be an instance of XsuaaToken.");
+	XSUserInfoAdapter(Token xsuaaToken, OAuth2ServiceConfiguration configuration) throws XSUserInfoException {
+		if (!(xsuaaToken instanceof XsuaaToken)) {
+			throw new XSUserInfoException("token needs to be an instance of XsuaaToken.");
 		}
-		this.accessToken = (AccessToken) accessToken;
+		this.xsuaaToken = (XsuaaToken) xsuaaToken;
 		this.configuration = configuration;
 	}
 
@@ -141,7 +141,7 @@ public class XSUserInfoAdapter implements XSUserInfo {
 
 	@Override
 	public String getAppToken() {
-		return accessToken.getTokenValue();
+		return xsuaaToken.getTokenValue();
 	}
 
 	@Override
@@ -158,17 +158,17 @@ public class XSUserInfoAdapter implements XSUserInfo {
 		}
 		if (name.equals(HDB)) {
 			String token;
-			if (accessToken.hasClaim(EXTERNAL_CONTEXT)) {
+			if (xsuaaToken.hasClaim(EXTERNAL_CONTEXT)) {
 				token = getAttributeFromClaimAsString(EXTERNAL_CONTEXT, HDB_NAMEDUSER_SAML);
 			} else {
-				token = accessToken.getClaimAsString(HDB_NAMEDUSER_SAML);
+				token = xsuaaToken.getClaimAsString(HDB_NAMEDUSER_SAML);
 			}
 			if (token == null) {
-				token = accessToken.getTokenValue();
+				token = xsuaaToken.getTokenValue();
 			}
 			return token;
 		} else if (name.equals("JobScheduler")) {
-			return accessToken.getTokenValue();
+			return xsuaaToken.getTokenValue();
 		} else {
 			throw new XSUserInfoException("Invalid name " + name + " for namespace " + namespace);
 		}
@@ -183,7 +183,7 @@ public class XSUserInfoAdapter implements XSUserInfo {
 	@Override
 	public boolean hasAttributes() throws XSUserInfoException {
 		checkNotGrantTypeClientCredentials("hasAttributes");
-		if (accessToken.hasClaim(EXTERNAL_CONTEXT)) {
+		if (xsuaaToken.hasClaim(EXTERNAL_CONTEXT)) {
 			JsonObject extContext = getClaimAsJsonObject(EXTERNAL_CONTEXT);
 			return extContext.contains(XS_USER_ATTRIBUTES) && !extContext.getJsonObject(EXTERNAL_CONTEXT).isEmpty();
 		} else {
@@ -198,13 +198,13 @@ public class XSUserInfoAdapter implements XSUserInfo {
 
 	@Override
 	public boolean checkScope(String scope) throws XSUserInfoException {
-		return accessToken.hasScope(scope);
+		return xsuaaToken.hasScope(scope);
 	}
 
 	@Override
 	public boolean checkLocalScope(String scope) throws XSUserInfoException {
 		try {
-			return accessToken.hasLocalScope(scope);
+			return xsuaaToken.hasLocalScope(scope);
 		} catch (IllegalArgumentException e) {
 			throw new XSUserInfoException(e.getMessage());
 		}
@@ -224,7 +224,7 @@ public class XSUserInfoAdapter implements XSUserInfo {
 
 	@Override
 	public String getGrantType() throws XSUserInfoException {
-		return Optional.ofNullable(accessToken.getGrantType())
+		return Optional.ofNullable(xsuaaToken.getGrantType())
 				.map(GrantType::toString)
 				.orElseThrow(createXSUserInfoException(GRANT_TYPE));
 	}
@@ -254,13 +254,20 @@ public class XSUserInfoAdapter implements XSUserInfo {
 				.equals(configuration.getProperty(CFConstants.XSUAA.IDENTITY_ZONE));
 		boolean isApplicationPlan = tokenClientId.contains("!t");
 		if (clientIdsMatch && (identityZonesMatch || isApplicationPlan)) {
+			LOGGER.info(
+					"Token not in foreign mode because because client ids  match and identityZonesMatch={}, isApplicationPlan={} ",
+					identityZonesMatch, isApplicationPlan);
 			return false; // no foreign mode
 		}
 		// in case of broker master: check trustedclientidsuffix
 		String bindingTrustedClientIdSuffix = configuration.getProperty(TRUSTED_CLIENT_ID_SUFFIX);
 		if (bindingTrustedClientIdSuffix != null && tokenClientId.endsWith(bindingTrustedClientIdSuffix)) {
+			LOGGER.info("Token not in foreign mode because token client id matches binding trusted client suffix");
 			return false; // no foreign mode
 		}
+		LOGGER.info(
+				"Token in foreign mode: clientIdsMatch={}, identityZonesMatch={}, isApplicationPlan={}, bindingTrustedClientIdSuffix={}",
+				clientIdsMatch, identityZonesMatch, isApplicationPlan, bindingTrustedClientIdSuffix);
 		return true;
 	}
 
@@ -284,7 +291,7 @@ public class XSUserInfoAdapter implements XSUserInfo {
 	}
 
 	private void checkNotGrantTypeClientCredentials(String methodName) throws XSUserInfoException {
-		if (GrantType.CLIENT_CREDENTIALS == accessToken.getGrantType()) {
+		if (GrantType.CLIENT_CREDENTIALS == xsuaaToken.getGrantType()) {
 			String message = String.format("Method '%s' is not supported for grant type '%s'", methodName,
 					GrantType.CLIENT_CREDENTIALS);
 			throw new XSUserInfoException(message + GrantType.CLIENT_CREDENTIALS);
@@ -302,7 +309,7 @@ public class XSUserInfoAdapter implements XSUserInfo {
 	}
 
 	private String getClaimValue(String claimname) throws XSUserInfoException {
-		String value = accessToken.getClaimAsString(claimname);
+		String value = xsuaaToken.getClaimAsString(claimname);
 		if (value == null) {
 			throw new XSUserInfoException("Invalid user attribute " + claimname);
 		}
@@ -312,7 +319,7 @@ public class XSUserInfoAdapter implements XSUserInfo {
 	@Nullable
 	private JsonObject getClaimAsJsonObject(String claimName) throws XSUserInfoException {
 		try {
-			return accessToken.getClaimAsJsonObject(claimName);
+			return xsuaaToken.getClaimAsJsonObject(claimName);
 		} catch (JsonParsingException e) {
 			throw createXSUserInfoException(claimName).get();
 		}
diff --git a/java-security/src/main/java/com/sap/cloud/security/servlet/AbstractTokenAuthenticator.java b/java-security/src/main/java/com/sap/cloud/security/servlet/AbstractTokenAuthenticator.java
index 1251a2a3..5916214b 100644
--- a/java-security/src/main/java/com/sap/cloud/security/servlet/AbstractTokenAuthenticator.java
+++ b/java-security/src/main/java/com/sap/cloud/security/servlet/AbstractTokenAuthenticator.java
@@ -98,7 +98,7 @@ public abstract class AbstractTokenAuthenticator implements TokenAuthenticator {
 	 */
 	protected abstract Token extractFromHeader(String authorizationHeader);
 
-	protected Validator<Token> getOrCreateTokenValidator() {
+	private Validator<Token> getOrCreateTokenValidator() {
 		if (tokenValidator == null) {
 			JwtValidatorBuilder jwtValidatorBuilder = JwtValidatorBuilder.getInstance(getServiceConfiguration())
 					.withHttpClient(httpClient);
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 2dfb08df..0f88cb4a 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
@@ -28,10 +28,10 @@ public class JwtAudienceValidator implements Validator<Token> {
 	private final List<String> clientIds = new ArrayList();
 
 	public JwtAudienceValidator(String clientId) {
-		configureTrustedClientId(clientId);
+		configureAnotherServiceInstance(clientId);
 	}
 
-	JwtAudienceValidator configureTrustedClientId(String clientId) {
+	public JwtAudienceValidator configureAnotherServiceInstance(String clientId) {
 		assertHasText(clientId, "JwtAudienceValidator requires a clientId.");
 		clientIds.add(clientId);
 		logger.info("configured JwtAudienceValidator with clientId {}.", clientId);
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 aceae2a5..6ce55837 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
@@ -185,7 +185,7 @@ public class JwtValidatorBuilder {
 	private JwtAudienceValidator createAudienceValidator() {
 		JwtAudienceValidator jwtAudienceValidator = new JwtAudienceValidator(configuration.getClientId());
 		if (otherConfiguration != null) {
-			jwtAudienceValidator.configureTrustedClientId(otherConfiguration.getClientId());
+			jwtAudienceValidator.configureAnotherServiceInstance(otherConfiguration.getClientId());
 		}
 		return jwtAudienceValidator;
 	}


Test patch:
diff --git a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/JwtAudienceValidatorTest.java b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/JwtAudienceValidatorTest.java
index 8c494dd8..7dcc0c21 100644
--- a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/JwtAudienceValidatorTest.java
+++ b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/JwtAudienceValidatorTest.java
@@ -32,7 +32,7 @@ public class JwtAudienceValidatorTest {
 	@Test
 	public void validate_tokenAudienceMatchesForeignClientId() {
 		ValidationResult result = new JwtAudienceValidator("any")
-				.configureTrustedClientId("foreignclient")
+				.configureAnotherServiceInstance("foreignclient")
 				.validate(token);
 
 		assertThat(result.isValid()).isTrue();
@@ -53,7 +53,7 @@ public class JwtAudienceValidatorTest {
 	@Test
 	public void validationFails_when_NoTokenAudienceMatches() {
 		ValidationResult result = new JwtAudienceValidator("any")
-				.configureTrustedClientId("anyother")
+				.configureAnotherServiceInstance("anyother")
 				.validate(token);
 
 		assertThat(result.isErroneous()).isTrue();