diff --git a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtIssuerValidator.java b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtIssuerValidator.java index d0225438f..1715898ac 100644 --- a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtIssuerValidator.java +++ b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/JwtIssuerValidator.java @@ -1,26 +1,27 @@ /** * SPDX-FileCopyrightText: 2018-2023 SAP SE or an SAP affiliate company and Cloud Security Client Java contributors - *

+ *

* SPDX-License-Identifier: Apache-2.0 */ package com.sap.cloud.security.token.validation.validators; -import static com.sap.cloud.security.token.validation.ValidationResults.createInvalid; -import static com.sap.cloud.security.token.validation.ValidationResults.createValid; -import static com.sap.cloud.security.xsuaa.Assertions.assertNotEmpty; - -import java.net.URI; -import java.net.URISyntaxException; -import java.util.List; - -import com.sap.cloud.security.config.Service; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import com.sap.cloud.security.config.OAuth2ServiceConfiguration; +import com.sap.cloud.security.json.JsonParsingException; import com.sap.cloud.security.token.Token; import com.sap.cloud.security.token.validation.ValidationResult; import com.sap.cloud.security.token.validation.Validator; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.net.MalformedURLException; +import java.net.URL; +import java.util.List; +import java.util.Objects; +import java.util.regex.Pattern; + +import static com.sap.cloud.security.token.validation.ValidationResults.createInvalid; +import static com.sap.cloud.security.token.validation.ValidationResults.createValid; +import static com.sap.cloud.security.xsuaa.Assertions.assertNotEmpty; /** * Validates that the jwt token is issued by a trust worthy identity provider. @@ -37,6 +38,7 @@ * These checks are a prerequisite for using the `JwtSignatureValidator`. */ class JwtIssuerValidator implements Validator { + protected static final String HTTPS_SCHEME = "https://"; private final List domains; protected final Logger logger = LoggerFactory.getLogger(getClass()); @@ -55,55 +57,34 @@ class JwtIssuerValidator implements Validator { @Override public ValidationResult validate(Token token) { - String issuer = token.getIssuer(); - if (token.getService().equals(Service.IAS) && !issuer.startsWith("http")) { - issuer = "https://" + issuer; - } - ValidationResult validationResult = validateUrl(issuer); - if (validationResult.isErroneous()) { - return validationResult; + String issuer; + + try { + issuer = token.getIssuer(); + } catch(JsonParsingException e) { + return createInvalid("Issuer validation can not be performed because token issuer claim was not a String value."); } - return matchesTokenIssuerUrl(issuer); - } - private ValidationResult matchesTokenIssuerUrl(String issuer) { - URI issuerUri = URI.create(issuer); - if (issuerUri.getQuery() == null && issuerUri.getFragment() == null && issuerUri.getHost() != null) { - for (String d : domains) { - if (issuerUri.getHost().endsWith(d)) { - return createValid(); - } - } + if (issuer == null || issuer.trim().isEmpty()) { + return createInvalid("Issuer validation can not be performed because token does not contain an issuer claim."); } - return createInvalid( - "Issuer is not trusted because issuer '{}' doesn't match any of these domains '{}' of the identity provider.", - issuer, domains); - } - private ValidationResult validateUrl(String issuer) { - URI issuerUri; + String issuerUrl = issuer.startsWith(HTTPS_SCHEME) ? issuer : HTTPS_SCHEME + issuer; try { - if (issuer == null || issuer.trim().isEmpty()) { - return createInvalid( - "Issuer validation can not be performed because Jwt token does not contain an issuer claim."); - } - if (!issuer.startsWith("http")) { - return createInvalid( - "Issuer is not trusted because issuer '{}' does not provide a valid URI (missing http scheme). Please contact your Identity Provider Administrator.", - issuer); - } - issuerUri = new URI(issuer); - if (issuerUri.getQuery() == null && issuerUri.getFragment() == null && issuerUri.getHost() != null) { + new URL(issuerUrl); + } catch (MalformedURLException e) { + return createInvalid("Issuer validation can not be performed because token issuer is not a valid URL suitable for https."); + } + + String issuerDomain = issuerUrl.substring(HTTPS_SCHEME.length()); + for(String d : domains) { + // a string that ends with . and contains 1-63 letters, digits or '-' before that for the subdomain + String validSubdomainPattern = String.format("^[a-zA-Z0-9-]{1,63}\\.%s$", Pattern.quote(d)); + if(Objects.equals(d, issuerDomain) || issuerDomain.matches(validSubdomainPattern)) { return createValid(); } - } catch (URISyntaxException e) { - logger.error( - "Error: issuer claim '{}' does not provide a valid URI: {}. Please contact your Identity Provider Administrator.", - issuer, e.getMessage(), e); } - return createInvalid( - "Issuer is not trusted because issuer does not provide a valid URI. Please contact your Identity Provider Administrator.", - issuer); - } + return createInvalid("Issuer {} was not a trusted domain or a subdomain of the trusted domains {}.", issuer, domains); + } } diff --git a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/SapIdJwtSignatureValidator.java b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/SapIdJwtSignatureValidator.java index 6e25ec7f5..86193f732 100644 --- a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/SapIdJwtSignatureValidator.java +++ b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/SapIdJwtSignatureValidator.java @@ -68,7 +68,7 @@ private URI getJwksUri(Token token) throws OAuth2ServiceException { } if (isTenantIdCheckEnabled && !domain.equals("" + configuration.getUrl()) && token.getAppTid() == null) { - throw new IllegalArgumentException("OIDC token must provide a valid " + TokenClaims.SAP_GLOBAL_APP_TID + " header when issuer has a different domain than the url from the service credentials."); + throw new IllegalArgumentException("OIDC token must provide the " + TokenClaims.SAP_GLOBAL_APP_TID + " claim for tenant validation when issuer is not the same as the url from the service credentials."); } diff --git a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/JwtIssuerValidatorTest.java b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/JwtIssuerValidatorTest.java index 3a918c7fe..59c1fbb0e 100644 --- a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/JwtIssuerValidatorTest.java +++ b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/JwtIssuerValidatorTest.java @@ -1,6 +1,6 @@ /** * SPDX-FileCopyrightText: 2018-2023 SAP SE or an SAP affiliate company and Cloud Security Client Java contributors - *

+ *

* SPDX-License-Identifier: Apache-2.0 */ package com.sap.cloud.security.token.validation.validators; @@ -8,7 +8,6 @@ import com.sap.cloud.security.config.Service; import com.sap.cloud.security.token.SapIdToken; import com.sap.cloud.security.token.Token; -import com.sap.cloud.security.token.TokenClaims; import com.sap.cloud.security.token.validation.ValidationResult; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -20,10 +19,9 @@ import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.Mockito.when; @@ -31,12 +29,13 @@ class JwtIssuerValidatorTest { private JwtIssuerValidator cut; private Token token; - private final String[] domains = new String[] { "customer.ondemand.com", "accounts400.ondemand.com" }; + private final String[] trustedDomains = new String[] { "customer.ondemand.com", "accounts400.ondemand.com" }; @BeforeEach void setup() { - cut = new JwtIssuerValidator(Arrays.asList(domains)); + cut = new JwtIssuerValidator(Arrays.asList(trustedDomains)); token = Mockito.mock(SapIdToken.class); + when(token.getService()).thenReturn(Service.IAS); } @Test @@ -48,84 +47,133 @@ void constructor_throwsOnNullValues() { .hasMessageContainingAll("JwtIssuerValidator", "domain(s)"); } - @Test - void validationFails_whenIssuerDomainDoesNotMatchIdentityProviderDomains() { - configureMock("https://otherdomain.test.ondemand.com", null); - assertThat(cut.validate(token).isValid(), is(false)); - } - @ParameterizedTest - @NullAndEmptySource - @ValueSource(strings = { " " }) - void validationIgnoresEmptyIssuer_whenIasIssuerIsGiven(String issuer) { - cut = new JwtIssuerValidator(Collections.singletonList("accounts400.ondemand.com")); - configureMock(issuer, "https://test.accounts400.ondemand.com"); + @CsvSource({ + "accounts400.ondemand.com", + "https://accounts400.ondemand.com", + "tenant.accounts400.ondemand.com", + "https://tenant.accounts400.ondemand.com", + "tenant-0815WithNumbers.accounts400.ondemand.com", + "https://tenant-0815WithNumbers.accounts400.ondemand.com" + }) + void validationSucceeds_forValidIssuers(String issuer) { + when(token.getIssuer()).thenReturn(issuer); ValidationResult validationResult = cut.validate(token); assertThat(validationResult.isValid(), is(true)); - } - - @Test - void validationSucceeds_whenIasIssuerIsEmptyOrNull() { - cut = new JwtIssuerValidator(Collections.singletonList("accounts400.ondemand.com")); - configureMock("https://test.accounts400.ondemand.com", null); - - ValidationResult validationResult = cut.validate(token); assertThat(validationResult.isErroneous(), is(false)); } @Test - void validationFails_withoutMatchingIasIssuer() { - configureMock("https://otherDomain.accounts400.ondemand.com", "https://iasDomain.accounts.ondemand.com"); + void validationFails_whenSubdomainHasMoreThan63Characters() { + for(String d : trustedDomains) { + when(token.getIssuer()).thenReturn("https://a." + d); + assertThat(cut.validate(token).isValid(), is(true)); - ValidationResult validationResult = cut.validate(token); - assertThat(validationResult.isErroneous(), is(true)); - assertThat(validationResult.getErrorDescription(), startsWith( - "Issuer is not trusted because issuer 'https://iasDomain.accounts.ondemand.com' doesn't match any of these domains '[customer.ondemand.com, accounts400.ondemand.com]' of the identity provider.")); + when(token.getIssuer()).thenReturn("https://" + "a".repeat(63) + "." + d); + assertThat(cut.validate(token).isValid(), is(true)); + + when(token.getIssuer()).thenReturn("https://" + "a".repeat(64) + "." + d); + assertThat(cut.validate(token).isValid(), is(false)); + } } - @Test - void validationIgnoresInvalidIssuer_whenIasIssuerIsGiven() { - cut = new JwtIssuerValidator(Arrays.asList(domains)); - configureMock("invalid_url", "https://otherDomain.accounts400.ondemand.com"); + @ParameterizedTest + @CsvSource({ + "https://accou\tnts400.ondemand.com", + "https://accou\nnts400.ondemand.com", + "https://accounts400.onde\tmand.com", + "https://accounts400.onde\nmand.com", + "https://tena\tnt.accounts400.ondemand.com", + "https://tena\nnt.accounts400.ondemand.com", + "https://tenant.accounts400.onde\tmand.com", + "https://tenant.accounts400.onde\nmand.com" + }) + void validationFails_whenIssuerContainsInvisibleCharacters(String issuer) { + when(token.getIssuer()).thenReturn(issuer); + assertThat(cut.validate(token).isValid(), is(false)); + assertThat(cut.validate(token).isErroneous(), is(true)); + } - ValidationResult validationResult = cut.validate(token); - assertThat(validationResult.isErroneous(), is(false)); + @ParameterizedTest + @CsvSource({ + "https://accounts400%2eondemand.com", + "https://accounts400.ondemand.com%2eattackerdomain.com", + "https://tenant%2eaccounts400.ondemand.com", + "https://attackerdomain.com%2eaccounts400.ondemand.com", + "tenant%2d0815WithNumbers.accounts400.ondemand.com", + }) + void validationFails_whenIssuerContainsEncodedCharacters(String issuer) { + when(token.getIssuer()).thenReturn(issuer); + assertThat(cut.validate(token).isValid(), is(false)); + assertThat(cut.validate(token).isErroneous(), is(true)); } @ParameterizedTest - @CsvSource({ "https://subdomain.accounts400.ondemand.com#anyFragment_keys", - "https://subdomain.accounts400.ondemand.com?a=b", - "\0://myauth.com", - "https://otherDomain.org?accounts400.ondemand.com", }) - void validationFails_iasIssuerUrl(String iasIssuer) { - cut = new JwtIssuerValidator(Arrays.asList(domains)); - configureMock("https://otherDomain.accounts400.ondemand.com", iasIssuer); + @NullAndEmptySource + @ValueSource(strings = { " " }) + void validationFails_whenIssuerIsEmpty(String issuer) { + when(token.getIssuer()).thenReturn(issuer); + assertThat(cut.validate(token).isValid(), is(false)); + assertThat(cut.validate(token).isErroneous(), is(true)); + } - ValidationResult validationResult = cut.validate(token); - assertThat(validationResult.isErroneous(), is(true)); - assertThat(validationResult.getErrorDescription(), startsWith("Issuer is not trusted because issuer ")); + @Test + void validationFails_whenIssuerIsNotAValidURL() { + when(token.getIssuer()).thenReturn("https://"); + assertThat(cut.validate(token).isValid(), is(false)); + assertThat(cut.validate(token).isErroneous(), is(true)); + + when(token.getIssuer()).thenReturn("http://"); + assertThat(cut.validate(token).isValid(), is(false)); + assertThat(cut.validate(token).isErroneous(), is(true)); + + when(token.getIssuer()).thenReturn("http://" + trustedDomains[0]); + assertThat(cut.validate(token).isValid(), is(false)); + assertThat(cut.validate(token).isErroneous(), is(true)); } @ParameterizedTest - @CsvSource({ "https://otherDomain.accounts400.ondemand.com,", - "https://paas.accounts400.ondemand.com,", - "https://nestle.com,paas.accounts400.ondemand.com,", - "subdomain.accounts400.ondemand.com,", - "https://nestle.com,https://paas.accounts400.ondemand.com," }) - void validationSucceeds(String issuer, String iasIssuer) { - cut = new JwtIssuerValidator(Arrays.asList(domains)); - configureMock(issuer, iasIssuer); - - ValidationResult validationResult = cut.validate(token); - assertThat(validationResult.isValid(), is(true)); + @CsvSource({ + "https://accounts400.ondemand.coma", + "https://accounts400.ondemand.com0", + "https://accounts400.ondemand.com/", + "https://accounts400.ondemand.com/path", + "https://accounts400.ondemand.com%2f", + "https://accounts400.ondemand.com%2fpath", + "https://accounts400.ondemand.com&", + "https://accounts400.ondemand.com%26", + "https://accounts400.ondemand.com?", + "https://accounts400.ondemand.com?foo", + "https://accounts400.ondemand.com?foo=bar", + "https://accounts400.ondemand.com%3f", + "https://accounts400.ondemand.com%3ffoo", + "https://accounts400.ondemand.com%3ffoo=bar", + "https://accounts400.ondemand.com#", + "https://accounts400.ondemand.com#foo", + "https://accounts400.ondemand.com%23", + "https://accounts400.ondemand.com%23foo", + "https://user@accounts400.ondemand.com", + "https://user%40accounts400.ondemand.com", + }) + void validationFails_whenIssuerContainsMoreThanDomain(String issuer) { + when(token.getIssuer()).thenReturn(issuer); + assertThat(cut.validate(token).isValid(), is(false)); + assertThat(cut.validate(token).isErroneous(), is(true)); } - private void configureMock(String issuer, String iasIssuer) { - when(token.getService()).thenReturn(Service.IAS); - when(token.getIssuer()).thenCallRealMethod(); - when(token.getClaimAsString(TokenClaims.ISSUER)).thenReturn(issuer); - when(token.getClaimAsString(TokenClaims.IAS_ISSUER)).thenReturn(iasIssuer); - when(token.hasClaim("ias_iss")).thenReturn(iasIssuer != null); + @ParameterizedTest + @CsvSource({ + "https://attackerdomain.com", + "https://tenant.attackerdomain.com", + "https://myaccounts400.ondemand.com", + "https://accounts400.ondemand.com.attackerDomain.com", + "https://accounts400.ondemand.com%2eattackerDomain.com", + "https://accounts400.ondemand.com%2dattackerDomain.com", + }) + void validationFails_whenIssuerIsNotASubdomainOfTrustedDomains(String issuer) { + when(token.getIssuer()).thenReturn(issuer); + assertThat(cut.validate(token).isValid(), is(false)); + assertThat(cut.validate(token).isErroneous(), is(true)); } } diff --git a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/SapIdJwtSignatureValidatorTest.java b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/SapIdJwtSignatureValidatorTest.java index 46f2c511d..dbf21ed5b 100644 --- a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/SapIdJwtSignatureValidatorTest.java +++ b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/SapIdJwtSignatureValidatorTest.java @@ -118,7 +118,7 @@ public void validationFails_WhenAppTidIsNull() { ValidationResult validationResult = cut.validate(iasPaasToken); assertTrue(validationResult.isErroneous()); assertThat(validationResult.getErrorDescription(), - containsString("OIDC token must provide a valid app_tid header when issuer has a different domain than the url from the service credentials.")); + containsString("Token signature can not be validated because: OIDC token must provide the app_tid claim for tenant validation when issuer is not the same as the url from the service credentials.")); } @Test diff --git a/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/DefaultOidcConfigurationService.java b/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/DefaultOidcConfigurationService.java index e9b7ae525..609a6f025 100644 --- a/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/DefaultOidcConfigurationService.java +++ b/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/DefaultOidcConfigurationService.java @@ -40,7 +40,7 @@ public DefaultOidcConfigurationService(CloseableHttpClient httpClient) { public static URI getDiscoveryEndpointUri(@Nonnull String issuerUri) { // to support existing IAS applications - URI uri = URI.create(issuerUri.startsWith("http") ? issuerUri : "https://" + issuerUri); + URI uri = URI.create(issuerUri.startsWith("https://") ? issuerUri : "https://" + issuerUri); return UriUtil.expandPath(uri, DISCOVERY_ENDPOINT_DEFAULT); }