Repository: SAP/cloud-security-xsuaa-integration
Pull request #874 (2022-04-12 17:04:57)

JwtSignatureValidator improvements
- Only identity service requires `x-zone_uuid` header for token keys retrieval
- in case of signature mismatch the result should expose the signature of the encoded JWT token

see also https://itsm.services.sap/now/workspace/agent/record/sn_customerservice_case/9f5bfe821bf9859862751f8f8b4bcbd1


Patch:
diff --git a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtSignatureValidator.java b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtSignatureValidator.java
index 472e538e..a28aafa2 100644
--- a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtSignatureValidator.java
+++ b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtSignatureValidator.java
@@ -62,10 +62,13 @@ class JwtSignatureValidator implements Validator<Token> {
 	public ValidationResult validate(Token token) {
 		String jwksUri;
 		String keyId;
+		String zoneIdForTokenKeys = null;
 
-		if (Service.IAS == configuration.getService() && !token.getIssuer().equals("" + configuration.getUrl())
-				&& token.getZoneId() == null) { // lgtm[java/dereferenced-value-may-be-null]
-			return createInvalid("Error occurred during signature validation: OIDC token must provide zone_uuid.");
+		if (Service.IAS == configuration.getService()) {
+			zoneIdForTokenKeys = token.getZoneId();
+			if (!token.getIssuer().equals("" + configuration.getUrl()) && zoneIdForTokenKeys  == null) { // lgtm[java/dereferenced-value-may-be-null]
+				return createInvalid("Error occurred during signature validation: OIDC token must provide zone_uuid.");
+			}
 		}
 		try {
 			jwksUri = getOrRequestJwksUri(token);
@@ -79,7 +82,7 @@ class JwtSignatureValidator implements Validator<Token> {
 					keyId,
 					jwksUri,
 					fallbackPublicKey,
-					token.getZoneId());
+					zoneIdForTokenKeys);
 		} catch (OAuth2ServiceException | IllegalArgumentException e) {
 			return createInvalid("Error occurred during jwks uri determination: {}", e.getMessage());
 		}
@@ -250,7 +253,7 @@ class JwtSignatureValidator implements Validator<Token> {
 					return createValid();
 				}
 				return createInvalid(
-						"Signature of Jwt Token is not valid: the identity provided by the JSON Web Token Key can not be verified.");
+						"Signature of Jwt Token is not valid: the identity provided by the JSON Web Token Key can not be verified (Signature: {}).", tokenHeaderPayloadSignature[2]);
 			} catch (Exception e) {
 				return createInvalid("Error occurred during Json Web Signature Validation: {}.", e.getMessage());
 			}


Test patch:
diff --git a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCacheTest.java b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCacheTest.java
index 9507d0bb..62ffd84f 100644
--- a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCacheTest.java
+++ b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCacheTest.java
@@ -42,7 +42,7 @@ public class OAuth2TokenKeyServiceWithCacheTest {
 	@Before
 	public void setup() throws IOException {
 		tokenKeyServiceMock = mock(OAuth2TokenKeyService.class);
-		when(tokenKeyServiceMock.retrieveTokenKeys(eq(TOKEN_KEYS_URI), any()))
+		when(tokenKeyServiceMock.retrieveTokenKeys(eq(TOKEN_KEYS_URI), isNotNull()))
 				.thenReturn(IOUtils.resourceToString("/jsonWebTokenKeys.json", StandardCharsets.UTF_8));
 
 		testCacheTicker = new TestCacheTicker();
@@ -84,10 +84,13 @@ public class OAuth2TokenKeyServiceWithCacheTest {
 
 	@Test
 	public void retrieveTokenKeys() throws OAuth2ServiceException, InvalidKeySpecException, NoSuchAlgorithmException {
-		PublicKey key = cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI, ZONE_ID);
+		PublicKey key1 = cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI, ZONE_ID);
+		PublicKey key2 = cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI, "other-zone-id");
 
-		assertThat(String.valueOf(key.getAlgorithm())).isEqualTo("RSA");
+		assertThat(String.valueOf(key1.getAlgorithm())).isEqualTo("RSA");
+		assertThat(String.valueOf(key2.getAlgorithm())).isEqualTo("RSA");
 		verify(tokenKeyServiceMock, times(1)).retrieveTokenKeys(eq(TOKEN_KEYS_URI), eq(ZONE_ID));
+		verify(tokenKeyServiceMock, times(1)).retrieveTokenKeys(eq(TOKEN_KEYS_URI), eq("other-zone-id"));
 	}
 
 	@Test
@@ -122,14 +125,27 @@ public class OAuth2TokenKeyServiceWithCacheTest {
 	@Test
 	public void retrieveTokenKeys_afterCacheWasCleared()
 			throws OAuth2ServiceException, InvalidKeySpecException, NoSuchAlgorithmException {
-		cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI, null);
+		cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI, ZONE_ID);
 		cut.clearCache();
+		PublicKey cachedKey = cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI, ZONE_ID);
+
+		assertThat(cachedKey).isNotNull();
+		verify(tokenKeyServiceMock, times(2)).retrieveTokenKeys(eq(TOKEN_KEYS_URI), eq(ZONE_ID));
+	}
+
+	@Test
+	public void getCachedTokenKeys_noZoneId() throws IOException, InvalidKeySpecException, NoSuchAlgorithmException {
+		when(tokenKeyServiceMock.retrieveTokenKeys(eq(TOKEN_KEYS_URI), isNull()))
+				.thenReturn(IOUtils.resourceToString("/jsonWebTokenKeys.json", StandardCharsets.UTF_8));
+		PublicKey key = cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI, null);
 		PublicKey cachedKey = cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI, null);
 
 		assertThat(cachedKey).isNotNull();
-		verify(tokenKeyServiceMock, times(2)).retrieveTokenKeys(eq(TOKEN_KEYS_URI), eq(null));
+		assertThat(cachedKey).isSameAs(key);
+		verify(tokenKeyServiceMock, times(1)).retrieveTokenKeys(eq(TOKEN_KEYS_URI), isNull());
 	}
 
+
 	@Test
 	public void retrieveTokenKeys_doesRequestKeysAgainAfterCacheExpired()
 			throws OAuth2ServiceException, InvalidKeySpecException, NoSuchAlgorithmException {
diff --git a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/XsuaaJwtSignatureValidatorTest.java b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/XsuaaJwtSignatureValidatorTest.java
index 3a146f08..890d638a 100644
--- a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/XsuaaJwtSignatureValidatorTest.java
+++ b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/XsuaaJwtSignatureValidatorTest.java
@@ -25,8 +25,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.when;
 
 public class XsuaaJwtSignatureValidatorTest {
@@ -59,7 +58,7 @@ public class XsuaaJwtSignatureValidatorTest {
 		tokenKeyServiceMock = Mockito.mock(OAuth2TokenKeyService.class);
 		when(tokenKeyServiceMock
 				.retrieveTokenKeys(eq(URI.create("https://authentication.stagingaws.hanavlab.ondemand.com/token_keys")),
-						any()))
+						isNull()))
 								.thenReturn(IOUtils.resourceToString("/jsonWebTokenKeys.json", UTF_8));
 
 		cut = new JwtSignatureValidator(
@@ -124,7 +123,11 @@ public class XsuaaJwtSignatureValidatorTest {
 		ValidationResult result = cut.validate(xsuaaTokenSignedWithVerificationKey);
 		assertThat(result.isErroneous(), is(true));
 		assertThat(result.getErrorDescription(),
-				containsString("Signature of Jwt Token is not valid"));
+				containsString("Signature of Jwt Token is not valid")
+		);
+		assertThat(result.getErrorDescription(),
+				containsString("(Signature: CetA62rQSNRj93S9mqaHrKJyzONKeEKcEJ9O5wObRD_")
+		);
 	}
 
 }
diff --git a/token-client/src/test/java/com/sap/cloud/security/xsuaa/client/DefaultOAuth2TokenKeyServiceTest.java b/token-client/src/test/java/com/sap/cloud/security/xsuaa/client/DefaultOAuth2TokenKeyServiceTest.java
index dbd8bb18..a0efae13 100644
--- a/token-client/src/test/java/com/sap/cloud/security/xsuaa/client/DefaultOAuth2TokenKeyServiceTest.java
+++ b/token-client/src/test/java/com/sap/cloud/security/xsuaa/client/DefaultOAuth2TokenKeyServiceTest.java
@@ -54,7 +54,7 @@ public class DefaultOAuth2TokenKeyServiceTest {
 	}
 
 	@Test
-	public void retrieveTokenKeys_responseNotOk_throwsException() throws IOException {
+	public void retrieveTokenKeysForZone_responseNotOk_throwsException() throws IOException {
 		String errorDescription = "Something went wrong";
 		CloseableHttpResponse response = HttpClientTestFactory
 				.createHttpResponse(errorDescription, HttpStatus.SC_BAD_REQUEST);
@@ -68,6 +68,20 @@ public class DefaultOAuth2TokenKeyServiceTest {
 				.hasMessageContaining("Headers [x-zone_uuid=92768714-4c2e-4b79-bc1b-009a4127ee3c]");
 	}
 
+	@Test
+	public void retrieveTokenKeys_responseNotOk_throwsException() throws IOException {
+		String errorDescription = "Something went wrong";
+		CloseableHttpResponse response = HttpClientTestFactory
+				.createHttpResponse(errorDescription, HttpStatus.SC_BAD_REQUEST);
+		when(httpClient.execute(any())).thenReturn(response);
+
+		assertThatThrownBy(() -> cut.retrieveTokenKeys(TOKEN_KEYS_ENDPOINT_URI, null))
+				.isInstanceOf(OAuth2ServiceException.class)
+				.hasMessageContaining(errorDescription)
+				.hasMessageContaining("'Something went wrong'")
+				.hasMessageContaining("Error retrieving token keys");
+	}
+
 	@Test
 	public void retrieveTokenKeys_tokenEndpointUriIsNull_throwsException() {
 		assertThatThrownBy(() -> cut.retrieveTokenKeys(null, ZONE_UUID))