diff --git a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/XsuaaJwtSignatureValidator.java b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/XsuaaJwtSignatureValidator.java index 0c27020fa7..e22bf34d05 100644 --- a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/XsuaaJwtSignatureValidator.java +++ b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/XsuaaJwtSignatureValidator.java @@ -76,13 +76,13 @@ private PublicKey fetchPublicKey(Token token, JwtSignatureAlgorithm algorithm) t throw new IllegalArgumentException("Token does not contain the mandatory " + KID_PARAMETER_NAME + " header."); } - String zidQueryParam = composeZidQueryParameter(token); + String queryParameters = composeQueryParameters(token); String jwksUri; if (jkuFactories.isEmpty()) { jwksUri = configuration.isLegacyMode() ? configuration.getUrl() + "/token_keys" - : configuration.getProperty(UAA_DOMAIN) + "/token_keys" + zidQueryParam; + : configuration.getProperty(UAA_DOMAIN) + "/token_keys" + queryParameters; } else { LOGGER.info("Loaded custom JKU factory"); jwksUri = jkuFactories.get(0).create(token.getTokenValue()); @@ -94,11 +94,21 @@ private PublicKey fetchPublicKey(Token token, JwtSignatureAlgorithm algorithm) t return tokenKeyService.getPublicKey(algorithm, keyId, uri, params); } - private String composeZidQueryParameter(Token token) { + String composeQueryParameters(Token token) { String zid = token.getAppTid(); + String clientId = token.getClientId(); + String query = ""; if (zid != null && !zid.isBlank()){ - return "?zid=" + zid; + query = "?zid=" + zid; } - return ""; + if (clientId != null && !clientId.isBlank()){ + if (query.isBlank()){ + query = "?client_id=" + clientId; + } else { + query = query + "&client_id=" + clientId; + } + } + LOGGER.debug("Composed query parameter for token keys: {}", query); + return query; } } 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 8eeb54832e..b191943374 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 @@ -14,8 +14,10 @@ import com.sap.cloud.security.xsuaa.client.OidcConfigurationService; import com.sap.cloud.security.xsuaa.http.HttpHeaders; import org.apache.commons.io.IOUtils; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.mockito.Mockito; import java.io.IOException; @@ -27,18 +29,18 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.when; public class XsuaaJwtSignatureValidatorTest { - private Token xsuaaToken; - private Token xsuaaTokenSignedWithVerificationKey; // signed with verificationkey (from configuration) + private static Token xsuaaToken; + private static Token xsuaaTokenSignedWithVerificationKey; // signed with verificationkey (from configuration) - private JwtSignatureValidator cut; - private OAuth2TokenKeyService tokenKeyServiceMock; - private OAuth2ServiceConfiguration mockConfiguration; + private static XsuaaJwtSignatureValidator cut; + private static OAuth2ServiceConfiguration mockConfiguration; - @Before - public void setup() throws IOException { + @BeforeAll + public static void setup() throws IOException { /** * Header -------- { "alg": "RS256", "jku": * "https://authentication.stagingaws.hanavlab.ondemand.com/token_keys", "kid": @@ -57,9 +59,9 @@ public void setup() throws IOException { when(mockConfiguration.getService()).thenReturn(Service.XSUAA); when(mockConfiguration.getProperty(UAA_DOMAIN)).thenReturn("authentication.stagingaws.hanavlab.ondemand.com"); - tokenKeyServiceMock = Mockito.mock(OAuth2TokenKeyService.class); + OAuth2TokenKeyService tokenKeyServiceMock = Mockito.mock(OAuth2TokenKeyService.class); when(tokenKeyServiceMock - .retrieveTokenKeys(URI.create("https://authentication.stagingaws.hanavlab.ondemand.com/token_keys?zid=uaa"), + .retrieveTokenKeys(URI.create("https://authentication.stagingaws.hanavlab.ondemand.com/token_keys?zid=uaa&client_id=sap_osb"), Map.of(HttpHeaders.X_ZID, "uaa"))) .thenReturn(IOUtils.resourceToString("/jsonWebTokenKeys.json", UTF_8)); @@ -71,12 +73,12 @@ public void setup() throws IOException { } @Test - public void xsuaa_RSASignatureMatchesJWKS() { + void xsuaa_RSASignatureMatchesJWKS() { assertThat(cut.validate(xsuaaToken).isValid(), is(true)); } @Test - public void generatedToken_SignatureMatchesVerificationkey() { + void generatedToken_SignatureMatchesVerificationkey() { when(mockConfiguration.hasProperty("verificationkey")).thenReturn(true); when(mockConfiguration.getProperty("verificationkey")).thenReturn( """ @@ -93,7 +95,7 @@ public void generatedToken_SignatureMatchesVerificationkey() { } @Test - public void validationFails_whenVerificationkeyIsInvalid() { + void validationFails_whenVerificationkeyIsInvalid() { when(mockConfiguration.hasProperty("verificationkey")).thenReturn(true); when(mockConfiguration.getProperty("verificationkey")).thenReturn("INVALIDKEY"); @@ -103,7 +105,7 @@ public void validationFails_whenVerificationkeyIsInvalid() { } @Test - public void validationFails_whenSignatureOfGeneratedTokenDoesNotMatchVerificationkey() { + void validationFails_whenSignatureOfGeneratedTokenDoesNotMatchVerificationkey() { when(mockConfiguration.hasProperty("verificationkey")).thenReturn(true); when(mockConfiguration.getProperty("verificationkey")).thenReturn( "-----BEGIN PUBLIC KEY-----MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAm1QaZzMjtEfHdimrHP3/2Yr+1z685eiOUlwybRVG9i8wsgOUh+PUGuQL8hgulLZWXU5MbwBLTECAEMQbcRTNVTolkq4i67EP6JesHJIFADbK1Ni0KuMcPuiyOLvDKiDEMnYG1XP3X3WCNfsCVT9YoU+lWIrZr/ZsIvQri8jczr4RkynbTBsPaAOygPUlipqDrpadMO1momNCbea/o6GPn38LxEw609ItfgDGhL6f/yVid5pFzZQWb+9l6mCuJww0hnhO6gt6Rv98OWDty9G0frWAPyEfuIW9B+mR/3vGhyU9IbbWpvFXiy9RVbbsM538TCjd5JF2dJvxy24addC4oQIDAQAB-----END PUBLIC KEY-----"); @@ -111,7 +113,24 @@ public void validationFails_whenSignatureOfGeneratedTokenDoesNotMatchVerificatio ValidationResult result = cut.validate(xsuaaTokenSignedWithVerificationKey); assertThat(result.isErroneous(), is(true)); assertThat(result.getErrorDescription(), containsString("Signature of Jwt Token is not valid")); - assertThat(result.getErrorDescription(), containsString("(Signature: CetA62rQSNRj93S9mqaHrKJyzONKeEKcEJ9O5wObRD_")); + assertThat(result.getErrorDescription(), + containsString("(Signature: CetA62rQSNRj93S9mqaHrKJyzONKeEKcEJ9O5wObRD_")); } + @ParameterizedTest + @CsvSource({ + "cid, tid, ?zid=tid&client_id=cid", + ", tid, ?zid=tid", + "' ', tid, ?zid=tid", + "cid, , ?client_id=cid", + "cid, ' ', ?client_id=cid", + ", , ''", + "' ', ' ', ''" + }) + void composeQueryParams(String clientId, String appTid, String query) { + Token tokenMock = Mockito.mock(Token.class); + Mockito.when(tokenMock.getClientId()).thenReturn(clientId); + Mockito.when(tokenMock.getAppTid()).thenReturn(appTid); + assertTrue(cut.composeQueryParameters(tokenMock).endsWith(query)); + } } diff --git a/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaJwtDecoder.java b/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaJwtDecoder.java index df79eb550d..3a1ba8eb62 100644 --- a/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaJwtDecoder.java +++ b/spring-xsuaa/src/main/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaJwtDecoder.java @@ -126,7 +126,7 @@ private Jwt verifyToken(JWT jwt) { try { String kid = tokenInfoExtractor.getKid(jwt); String uaaDomain = tokenInfoExtractor.getUaaDomain(jwt); - return verifyToken(jwt.getParsedString(), kid, uaaDomain, getZid(jwt)); + return verifyToken(jwt.getParsedString(), kid, uaaDomain, getZid(jwt), getClientId(jwt)); } catch (BadJwtException e) { if (e.getMessage().contains("Couldn't retrieve remote JWK set") || e.getMessage().contains("Cannot verify with online token key, uaadomain is")) { @@ -138,6 +138,19 @@ private Jwt verifyToken(JWT jwt) { } } + private static String getClientId(JWT jwt) { + String clientId; + try { + clientId = jwt.getJWTClaimsSet().getStringClaim(TokenClaims.CLAIM_CLIENT_ID); + } catch (ParseException e) { + clientId = null; + } + if (clientId != null && clientId.isBlank()){ + clientId = null; + } + return clientId; + } + @Nullable private static String getZid(JWT jwt) { String zid; @@ -154,10 +167,10 @@ private static String getZid(JWT jwt) { return zid; } - private Jwt verifyToken(String token, String kid, String uaaDomain, String zid) { + private Jwt verifyToken(String token, String kid, String uaaDomain, String zid, String clientId) { String jku; if (jkuFactories.isEmpty()) { - jku = composeJku(uaaDomain, zid); + jku = composeJku(uaaDomain, zid, clientId); } else { logger.info("Loaded custom JKU factory"); try { @@ -191,14 +204,30 @@ private void canVerifyWithKey(String kid, String uaadomain) { String.join(", ", nullParams))); } - private String composeJku(String uaaDomain, String zid) { - String zidQueryParam = zid != null ? "?zid=" + zid : ""; + String composeJku(String uaaDomain, String zid, String clientId) { + String queryParams = composeQueryParameters(zid, clientId); // uaaDomain in configuration is always without a schema, but for testing purpose http schema can be used if (uaaDomain.startsWith("http://")){ - return uaaDomain + "/token_keys" + zidQueryParam; + return uaaDomain + "/token_keys" + queryParams; + } + return "https://" + uaaDomain + "/token_keys" + queryParams; + } + + private String composeQueryParameters(String zid, String clientId) { + String query = ""; + if (zid != null && !zid.isBlank()){ + query = "?zid=" + zid; + } + if (clientId != null && !clientId.isBlank()){ + if (query.isBlank()){ + query = "?client_id=" + clientId; + } else { + query = query + "&client_id=" + clientId; + } } - return "https://" + uaaDomain + "/token_keys" + zidQueryParam; + logger.debug("Composed query parameter for token keys: {}", query); + return query; } @java.lang.SuppressWarnings("squid:S2259") diff --git a/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaJwtDecoderTest.java b/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaJwtDecoderTest.java index 5f2a881bda..a23c1b4401 100644 --- a/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaJwtDecoderTest.java +++ b/spring-xsuaa/src/test/java/com/sap/cloud/security/xsuaa/token/authentication/XsuaaJwtDecoderTest.java @@ -11,9 +11,11 @@ import com.sap.cloud.security.xsuaa.XsuaaServiceConfigurationDefault; import com.sap.cloud.security.xsuaa.token.TokenClaims; import org.apache.commons.io.IOUtils; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.ResponseEntity; @@ -23,7 +25,7 @@ import org.springframework.security.oauth2.jwt.JwtException; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.TestPropertySource; -import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.web.client.RestOperations; import java.io.IOException; @@ -31,32 +33,33 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.times; -@RunWith(SpringRunner.class) +@ExtendWith(SpringExtension.class) @TestPropertySource("/XsuaaJwtDecoderTest.properties") @ContextConfiguration(classes = XsuaaServiceConfigurationDefault.class) -public class XsuaaJwtDecoderTest { +class XsuaaJwtDecoderTest { @Autowired XsuaaServiceConfiguration configurationWithVerificationKey; XsuaaServiceConfiguration configuration = new XsuaaServiceConfigurationCustom(new XsuaaCredentials()); - private String rsaToken; - private String ccToken; - private String jwks; + private static String rsaToken; + private static String ccToken; + private static String jwks; - @Before - public void setUp() throws IOException { + @BeforeAll + static void setUp() throws IOException { rsaToken = IOUtils.resourceToString("/accessTokenRSA256WithVerificationKey.txt", StandardCharsets.UTF_8); ccToken = IOUtils.resourceToString("/token_cc.txt", StandardCharsets.UTF_8); jwks = IOUtils.resourceToString("/jwks.json", StandardCharsets.UTF_8); } @Test - public void decode_withJwks_cache_disabled() { + void decode_withJwks_cache_disabled() { RestOperations restTemplate = Mockito.mock(RestOperations.class); Mockito.when(restTemplate.exchange(any(), eq(String.class))).thenReturn(ResponseEntity.ok().body(jwks)); @@ -72,7 +75,7 @@ public void decode_withJwks_cache_disabled() { } @Test - public void decode_withJwks_cache_default() { + void decode_withJwks_cache_default() { RestOperations restTemplate = Mockito.mock(RestOperations.class); Mockito.when(restTemplate.exchange(any(), eq(String.class))).thenReturn(ResponseEntity.ok().body(jwks)); @@ -87,14 +90,14 @@ public void decode_withJwks_cache_default() { } @Test - public void decode_withFallbackVerificationKey() { + void decode_withFallbackVerificationKey() { final JwtDecoder cut = new XsuaaJwtDecoderBuilder(configurationWithVerificationKey).build(); assertThat(cut.decode(rsaToken).getClaimAsString(TokenClaims.CLAIM_CLIENT_ID)).isEqualTo("sb-clientId!t0815"); } @Test - public void decode_withInvalidFallbackVerificationKey_withoutUaaDomain() { + void decode_withInvalidFallbackVerificationKey_withoutUaaDomain() { XsuaaServiceConfiguration config = Mockito.mock(XsuaaServiceConfiguration.class); Mockito.when(config.getVerificationKey()).thenReturn( "xsuaa.verificationkey=-----BEGIN PUBLIC KEY-----MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAm1QaZzMjtEfHdimrHP3/2Yr+1z685eiOUlwybRVG9i8wsgOUh+PUGuQL8hgulLZWXU5MbwBLTECAEMQbcRTNVTolkq4i67EP6JesHJIFADbK1Ni0KuMcPuiyOLvDKiDEMnYG1XP3X3WCNfsCVT9YoU+lWIrZr/ZsIvQri8jczr4RkynbTBsPaAOygPUlipqDrpadMO1momNCbea/o6GPn38LxEw609ItfgDGhL6f/yVid5pFzZQWb+9l6mCuJww0hnhO6gt6Rv98OWDty9G0frWAPyEfuIW9B+mR/2vGhyU9IbbWpvFXiy9RVbbsM538TCjd5JF2dJvxy24addC4oQIDAQAB-----END PUBLIC KEY-----"); @@ -106,7 +109,7 @@ public void decode_withInvalidFallbackVerificationKey_withoutUaaDomain() { } @Test - public void decode_withFallbackVerificationKey_remoteKeyFetchFailed() { + void decode_withFallbackVerificationKey_remoteKeyFetchFailed() { XsuaaServiceConfiguration config = Mockito.mock(XsuaaServiceConfiguration.class); Mockito.when(config.getVerificationKey()).thenReturn( "-----BEGIN PUBLIC KEY-----MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAm1QaZzMjtEfHdimrHP3/2Yr+1z685eiOUlwybRVG9i8wsgOUh+PUGuQL8hgulLZWXU5MbwBLTECAEMQbcRTNVTolkq4i67EP6JesHJIFADbK1Ni0KuMcPuiyOLvDKiDEMnYG1XP3X3WCNfsCVT9YoU+lWIrZr/ZsIvQri8jczr4RkynbTBsPaAOygPUlipqDrpadMO1momNCbea/o6GPn38LxEw609ItfgDGhL6f/yVid5pFzZQWb+9l6mCuJww0hnhO6gt6Rv98OWDty9G0frWAPyEfuIW9B+mR/2vGhyU9IbbWpvFXiy9RVbbsM538TCjd5JF2dJvxy24addC4oQIDAQAB-----END PUBLIC KEY-----"); @@ -120,11 +123,27 @@ public void decode_withFallbackVerificationKey_remoteKeyFetchFailed() { } @Test - public void decode_withNonMatchingVerificationKey_throwsException() { + void decode_withNonMatchingVerificationKey_throwsException() { final JwtDecoder cut = new XsuaaJwtDecoderBuilder(configuration).build(); assertThatThrownBy(() -> cut.decode(ccToken)).isInstanceOf(JwtException.class) .hasMessageContaining("Cannot verify with online token key, kid, uaadomain is null"); } + @ParameterizedTest + @CsvSource({ + "uaadomain, cid, tid, https://uaadomain/token_keys?zid=tid&client_id=cid", + "uaadomain, , tid, https://uaadomain/token_keys?zid=tid", + "uaadomain, ' ', tid, https://uaadomain/token_keys?zid=tid", + "uaadomain, cid, , https://uaadomain/token_keys?client_id=cid", + "uaadomain, cid, ' ', https://uaadomain/token_keys?client_id=cid", + "uaadomain, , , https://uaadomain/token_keys", + "uaadomain, ' ', ' ', https://uaadomain/token_keys", + "http://uaadomain, ' ', ' ', http://uaadomain/token_keys" + }) + void composeJku(String uaadomain, String clientId, String appTid, String query) { + final XsuaaJwtDecoder cut = new XsuaaJwtDecoder(configuration, 60, 100, null, null); + + assertEquals(cut.composeJku(uaadomain, appTid, clientId), query); + } } \ No newline at end of file