Repository: SAP/cloud-security-xsuaa-integration
Pull request #70 (2019-04-19 10:28:24)

extractToken should support oauth and/or client credentials
With this change two kind of tokens are supported: Oauth and basic/client-credentials.

E.g. in case the "Authorization" header is not provided, and basic or client-credentials is supported it tries to return the password or client-credentials token.

nenaraab: @mwdb 
can you please do a review?

Patch:
diff --git a/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/extractor/TokenBrokerResolver.java b/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/extractor/TokenBrokerResolver.java
index 9a0da6b6..07a477f9 100644
--- a/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/extractor/TokenBrokerResolver.java
+++ b/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/extractor/TokenBrokerResolver.java
@@ -17,6 +17,7 @@ import org.springframework.cache.Cache;
 import org.springframework.security.oauth2.server.resource.web.BearerTokenResolver;
 
 import com.sap.cloud.security.xsuaa.XsuaaServiceConfiguration;
+import org.springframework.util.StringUtils;
 
 /**
  * Analyse authentication header and obtain token from UAA
@@ -105,7 +106,9 @@ public class TokenBrokerResolver implements BearerTokenResolver {
 		for (AuthenticationMethod credentialType : authenticationMethods) {
 			Enumeration<String> headers = request.getHeaders(AUTHORIZATION_HEADER);
 			String token = getBrokerToken(credentialType, headers, oauthTokenUrl);
-			return token;
+			if (!StringUtils.isEmpty(token)) {
+				return token;
+			}
 		}
 		return null;
 	}
diff --git a/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaAudienceValidator.java b/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaAudienceValidator.java
index 9f9dc55b..e347a4a7 100644
--- a/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaAudienceValidator.java
+++ b/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaAudienceValidator.java
@@ -2,16 +2,16 @@ package com.sap.cloud.security.xsuaa.token.authentication;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.stream.Collectors;
 
+import com.sap.cloud.security.xsuaa.XsuaaServiceConfiguration;
+import com.sap.cloud.security.xsuaa.token.Token;
 import org.springframework.security.oauth2.core.OAuth2Error;
 import org.springframework.security.oauth2.core.OAuth2ErrorCodes;
 import org.springframework.security.oauth2.core.OAuth2TokenValidator;
 import org.springframework.security.oauth2.core.OAuth2TokenValidatorResult;
 import org.springframework.security.oauth2.jwt.Jwt;
 
-import com.sap.cloud.security.xsuaa.XsuaaServiceConfiguration;
-import com.sap.cloud.security.xsuaa.token.Token;
-
 /**
  * Validate audience using audience field content. in case this field is empty,
  * the audience is derived from the scope field
@@ -34,7 +34,7 @@ public class XsuaaAudienceValidator implements OAuth2TokenValidator<Jwt> {
 			return OAuth2TokenValidatorResult.success();
 		} else {
 			// case 2: foreign token
-			List<String> allowedAudiences = allowedAudiences(token);
+			List<String> allowedAudiences = getAllowedAudiences(token);
 			if (allowedAudiences.contains(xsuaaServiceConfiguration.getAppId())) {
 				return OAuth2TokenValidatorResult.success();
 			} else {
@@ -51,7 +51,7 @@ public class XsuaaAudienceValidator implements OAuth2TokenValidator<Jwt> {
 	 * @param token
 	 * @return (empty) list of audiences
 	 */
-	private List<String> allowedAudiences(Jwt token) {
+	List<String> getAllowedAudiences(Jwt token) {
 		List<String> allAudiences = new ArrayList<>();
 		List<String> tokenAudiences = token.getAudience();
 
@@ -75,7 +75,7 @@ public class XsuaaAudienceValidator implements OAuth2TokenValidator<Jwt> {
 				}
 			}
 		}
-		return allAudiences;
+		return allAudiences.stream().distinct().filter(value -> !value.isEmpty()).collect(Collectors.toList());
 	}
 
 	private List<String> getScopes(Jwt token) {


Test patch:
diff --git a/spring-xsuaa-it/src/test/java/com/sap/cloud/security/xsuaa/extractor/BasicCredentialExtractorTest.java b/spring-xsuaa-it/src/test/java/com/sap/cloud/security/xsuaa/extractor/BasicCredentialExtractorTest.java
index 44177c4d..fd8a31cb 100644
--- a/spring-xsuaa-it/src/test/java/com/sap/cloud/security/xsuaa/extractor/BasicCredentialExtractorTest.java
+++ b/spring-xsuaa-it/src/test/java/com/sap/cloud/security/xsuaa/extractor/BasicCredentialExtractorTest.java
@@ -40,20 +40,6 @@ public class BasicCredentialExtractorTest {
 	@Autowired
 	private AuthenticationInformationExtractor authenticationConfiguration;
 
-	/**
-	 * @throws java.lang.Exception
-	 */
-	@BeforeClass
-	public static void setUpBeforeClass() throws Exception {
-	}
-
-	/**
-	 * @throws java.lang.Exception
-	 */
-	@AfterClass
-	public static void tearDownAfterClass() throws Exception {
-	}
-
 	/**
 	 * @throws java.lang.Exception
 	 */
@@ -62,13 +48,6 @@ public class BasicCredentialExtractorTest {
 		request = new MockHttpServletRequest();
 	}
 
-	/**
-	 * @throws java.lang.Exception
-	 */
-	@After
-	public void tearDown() throws Exception {
-	}
-
 	@Test
 	public void testBasicCredentialsNoMultiTenancy() {
 		TokenBrokerResolver extractor = new TokenBrokerResolver(getXsuaaServiceConfiguration(), tokenCache, tokenBroker,
@@ -90,27 +69,6 @@ public class BasicCredentialExtractorTest {
 		assertThat(token).isEqualTo("other_token_pwd");
 	}
 
-	private XsuaaServiceConfigurationDummy getXsuaaServiceConfiguration() {
-		XsuaaServiceConfigurationDummy cfg = new XsuaaServiceConfigurationDummy();
-		cfg.appId = "a1!123";
-		cfg.clientId = "myclient!t1";
-		cfg.clientSecret = "top.secret";
-		cfg.uaaDomain = "auth.com";
-		cfg.uaaUrl = "https://mydomain.auth.com";
-		return cfg;
-	}
-
-	public AuthenticationInformationExtractor authenticationMethods(AuthenticationMethod... methods) {
-		return new DefaultAuthenticationInformationExtractor() {
-
-			@Override
-			public List<AuthenticationMethod> getAuthenticationMethods(HttpServletRequest request) {
-				return Arrays.asList(methods);
-			}
-		};
-
-	}
-
 	@Test
 	public void testClientCredentials() {
 		TokenBrokerResolver extractor = new TokenBrokerResolver(getXsuaaServiceConfiguration(), tokenCache, tokenBroker,
@@ -137,7 +95,7 @@ public class BasicCredentialExtractorTest {
 	}
 
 	@Test(expected = IllegalArgumentException.class)
-	public void invalidCombinedCredentials() {
+	public void testInvalidCombinedCredentials() {
 
 		AuthenticationInformationExtractor invalidCombination = new DefaultAuthenticationInformationExtractor() {
 
@@ -156,16 +114,47 @@ public class BasicCredentialExtractorTest {
 	}
 
 	@Test
-	public void combinedCredentials() {
+	public void testCombinedCredentials() {
 		request.addHeader("Authorization", "Bearer "
 				+ "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c");
 
 		TokenBrokerResolver extractor = new TokenBrokerResolver(getXsuaaServiceConfiguration(), tokenCache, tokenBroker,
-				authenticationMethods(AuthenticationMethod.OAUTH2, AuthenticationMethod.OAUTH2));
+				authenticationMethods(AuthenticationMethod.OAUTH2, AuthenticationMethod.BASIC));
 
 		String token = extractor.resolve(request);
 		assertThat(token).isEqualTo(
 				"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c");
 	}
 
+	@Test
+	public void testCombinedCredentials_shouldTakeBasicAsFallback() {
+		request.addHeader("Authorization", "basic " + Base64.getEncoder().encodeToString("myuser:mypass".getBytes()));
+
+		TokenBrokerResolver extractor = new TokenBrokerResolver(getXsuaaServiceConfiguration(), tokenCache, tokenBroker,
+				authenticationMethods(AuthenticationMethod.BASIC, AuthenticationMethod.OAUTH2));
+
+		String token = extractor.resolve(request);
+		assertThat(token).isEqualTo("token_pwd");
+	}
+
+	private XsuaaServiceConfigurationDummy getXsuaaServiceConfiguration() {
+		XsuaaServiceConfigurationDummy cfg = new XsuaaServiceConfigurationDummy();
+		cfg.appId = "a1!123";
+		cfg.clientId = "myclient!t1";
+		cfg.clientSecret = "top.secret";
+		cfg.uaaDomain = "auth.com";
+		cfg.uaaUrl = "https://mydomain.auth.com";
+		return cfg;
+	}
+
+	public AuthenticationInformationExtractor authenticationMethods(AuthenticationMethod... methods) {
+		return new DefaultAuthenticationInformationExtractor() {
+
+			@Override
+			public List<AuthenticationMethod> getAuthenticationMethods(HttpServletRequest request) {
+				return Arrays.asList(methods);
+			}
+		};
+
+	}
 }
diff --git a/spring-xsuaa-test/src/main/java/com/sap/cloud/security/xsuaa/test/JwtGenerator.java b/spring-xsuaa-test/src/main/java/com/sap/cloud/security/xsuaa/test/JwtGenerator.java
index f77748f3..e62055bf 100644
--- a/spring-xsuaa-test/src/main/java/com/sap/cloud/security/xsuaa/test/JwtGenerator.java
+++ b/spring-xsuaa-test/src/main/java/com/sap/cloud/security/xsuaa/test/JwtGenerator.java
@@ -198,9 +198,10 @@ public class JwtGenerator {
 		for (String scope : scopes) {
 			if (scope.contains(".")) {
 				String aud = scope.substring(0, scope.indexOf("."));
-				if (!audiences.contains(aud)) {
-					audiences.add(aud);
+				if (aud.isEmpty() || audiences.contains(aud)) {
+					break;
 				}
+				audiences.add(aud);
 			}
 		}
 		return audiences;
diff --git a/spring-xsuaa-test/src/test/java/com/sap/cloud/security/xsuaa/test/JwtGeneratorTest.java b/spring-xsuaa-test/src/test/java/com/sap/cloud/security/xsuaa/test/JwtGeneratorTest.java
index cbb6ac6a..86ac06c9 100644
--- a/spring-xsuaa-test/src/test/java/com/sap/cloud/security/xsuaa/test/JwtGeneratorTest.java
+++ b/spring-xsuaa-test/src/test/java/com/sap/cloud/security/xsuaa/test/JwtGeneratorTest.java
@@ -109,7 +109,7 @@ public class JwtGeneratorTest {
 	}
 
 	@Test
-	public void testTokenWithCustomClaims() throws IOException {
+	public void testTokenWithCustomClaims() {
 		Map<String, Object> claims = new HashMap<>();
 		claims.put("myCustomClaim", "myCustomClaimValue");
 		Jwt jwt = jwtGenerator.addCustomClaims(claims).getToken();
@@ -118,9 +118,9 @@ public class JwtGeneratorTest {
 	}
 
 	@Test
-	public void testTokenWithAudienceClaims() throws IOException {
+	public void testTokenWithDerivedAudienceClaim() {
 
-		Jwt jwt = jwtGenerator.addScopes("openid", "app1.scope", "app2.sub.scope", "app2.scope").deriveAudiences(true)
+		Jwt jwt = jwtGenerator.addScopes("openid", "app1.scope", "app2.sub.scope", "app2.scope", ".scopeWithoutAppId").deriveAudiences(true)
 				.getToken();
 
 		assertThat(jwt.getAudience().size(), equalTo(2));
diff --git a/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaAudienceValidatorTest.java b/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaAudienceValidatorTest.java
index 9d70ebfd..09e7384c 100644
--- a/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaAudienceValidatorTest.java
+++ b/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaAudienceValidatorTest.java
@@ -1,5 +1,8 @@
 package com.sap.cloud.security.xsuaa.token.authentication;
 
+import static org.hamcrest.CoreMatchers.hasItem;
+import static org.hamcrest.CoreMatchers.is;
+
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Date;
@@ -53,9 +56,18 @@ public class XsuaaAudienceValidatorTest {
 
 	@Test
 	public void testSameClientIdWithoutAudience() {
-		OAuth2TokenValidatorResult result2 = new XsuaaAudienceValidator(serviceConfigurationSameClientId)
+		OAuth2TokenValidatorResult result = new XsuaaAudienceValidator(serviceConfigurationSameClientId)
 				.validate(tokenWithoutAudience);
-		Assert.assertFalse(result2.hasErrors());
+		Assert.assertFalse(result.hasErrors());
+	}
+
+	@Test
+	public void testExtractAudiencesFromTokenScopes() {
+		Jwt token = new JwtGenerator().addScopes("test1!t1.read","test2!t1.read","test2!t1.write", ".scopeWithoutAppId").getToken();
+		List<String> audiences = new XsuaaAudienceValidator(serviceConfigurationSameClientId).getAllowedAudiences(token);
+		Assert.assertThat(audiences.size(), is(2));
+		Assert.assertThat(audiences, hasItem("test1!t1"));
+		Assert.assertThat(audiences, hasItem("test2!t1"));
 	}
 
 	@Test