From 2658f90d63451c053a3f5e76b2147cb5b854fcc4 Mon Sep 17 00:00:00 2001 From: Markus Strehle <11627201+strehle@users.noreply.github.com> Date: Thu, 12 Oct 2023 19:14:36 +0200 Subject: [PATCH] feature: add runtime support for private_key_jwt client authentication (#2507) * feature: add runtime support for private_key_jwt client authentication More details in https://github.com/cloudfoundry/uaa/pull/2449, in #2433 as this PR include #2433. -> because to have smaller review packages Enable the validation of client_assertion as replacement for client_secret Add private_key_jwt as client_auth_method into tokens. * review * fix smells from sonar https://sonarcloud.io/summary/new_code?id=cloudfoundry-identity-parent&pullRequest=2507 --- .../uaa/oauth/token/TokenConstants.java | 1 + .../identity/uaa/util/UaaStringUtils.java | 7 ++ .../identity/uaa/util/UaaStringUtilsTest.java | 8 ++ ...tClientParametersAuthenticationFilter.java | 12 +- ...ibleTokenEndpointAuthenticationFilter.java | 6 + .../ClientDetailsAuthenticationProvider.java | 61 +++++++--- .../uaa/client/ClientJwtConfiguration.java | 13 +- .../identity/uaa/client/UaaClient.java | 12 +- .../UaaClientDetailsUserDetailsService.java | 3 +- .../identity/uaa/oauth/KeyInfoService.java | 6 + .../oauth/UaaAuthorizationRequestManager.java | 7 +- .../identity/uaa/oauth/UaaTokenServices.java | 9 +- .../oauth/jwt/JwtClientAuthentication.java | 98 ++++++++++++++- .../provider/oauth/OidcMetadataFetcher.java | 44 +++++-- .../uaa/util/UaaSecurityContextUtils.java | 8 +- ...TokenEndpointAuthenticationFilterTest.java | 41 +++++++ .../UaaClientAuthenticationProviderTest.java | 51 ++++++-- .../UaaAuthorizationRequestManagerTests.java | 6 + .../jwt/JwtClientAuthenticationTest.java | 115 ++++++++++++++++++ .../uaa/oauth/token/KeyInfoServiceTests.java | 8 ++ .../oauth/OidcMetadataFetcherTest.java | 104 +++++++++++++++- .../webapp/WEB-INF/spring/oauth-clients.xml | 1 + .../webapp/WEB-INF/spring/oauth-endpoints.xml | 7 ++ 23 files changed, 575 insertions(+), 53 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/TokenConstants.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/TokenConstants.java index 1c599a0e6e8..ef74275785e 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/TokenConstants.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/TokenConstants.java @@ -63,6 +63,7 @@ static public List getStringValues() { public static final String GRANT_TYPE_IMPLICIT = "implicit"; public static final String CLIENT_AUTH_NONE = "none"; + public static final String CLIENT_AUTH_PRIVATE_KEY_JWT = "private_key_jwt"; public static final String ID_TOKEN_HINT_PROMPT = "prompt"; public static final String ID_TOKEN_HINT_PROMPT_NONE = "none"; diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/util/UaaStringUtils.java b/model/src/main/java/org/cloudfoundry/identity/uaa/util/UaaStringUtils.java index 2790f23b0e9..696f65a9f62 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/util/UaaStringUtils.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/util/UaaStringUtils.java @@ -310,4 +310,11 @@ public static String getCleanedUserControlString(String input) { } return CTRL_PATTERN.matcher(input).replaceAll("_"); } + + public static String getSafeParameterValue(String[] value) { + if (null == value || value.length < 1) { + return EMPTY_STRING; + } + return StringUtils.hasText(value[0]) ? value[0] : EMPTY_STRING; + } } diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/util/UaaStringUtilsTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/util/UaaStringUtilsTest.java index 14929e831dc..a6156ab68b3 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/util/UaaStringUtilsTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/util/UaaStringUtilsTest.java @@ -424,6 +424,14 @@ void getMapFromProperties() { assertThat(objectMap, hasEntry("key", "value")); } + @Test + void getSafeParameterValue() { + assertEquals("test", UaaStringUtils.getSafeParameterValue(new String[] {"test"})); + assertEquals("", UaaStringUtils.getSafeParameterValue(new String[] {" "})); + assertEquals("", UaaStringUtils.getSafeParameterValue(new String[] {})); + assertEquals("", UaaStringUtils.getSafeParameterValue(null)); + } + private static void replaceZoneVariables(IdentityZone zone) { String s = "https://{zone.subdomain}.domain.com/z/{zone.id}?id={zone.id}&domain={zone.subdomain}"; String expect = String.format("https://%s.domain.com/z/%s?id=%s&domain=%s", zone.getSubdomain(), zone.getId(), zone.getId(), zone.getSubdomain()); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/AbstractClientParametersAuthenticationFilter.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/AbstractClientParametersAuthenticationFilter.java index c2a7de66538..d44dcef9d8b 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/AbstractClientParametersAuthenticationFilter.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/AbstractClientParametersAuthenticationFilter.java @@ -14,6 +14,7 @@ */ package org.cloudfoundry.identity.uaa.authentication; +import org.cloudfoundry.identity.uaa.oauth.jwt.JwtClientAuthentication; import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants; import org.cloudfoundry.identity.uaa.util.UaaStringUtils; import org.slf4j.Logger; @@ -42,6 +43,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; +import java.util.Optional; /** * Filter which processes and authenticates a client based on @@ -54,6 +56,7 @@ public abstract class AbstractClientParametersAuthenticationFilter implements Fi public static final String CLIENT_ID = "client_id"; public static final String CLIENT_SECRET = "client_secret"; + public static final String CLIENT_ASSERTION = "client_assertion"; protected final Logger logger = LoggerFactory.getLogger(getClass()); protected AuthenticationManager clientAuthenticationManager; @@ -85,6 +88,9 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha String clientId = loginInfo.get(CLIENT_ID); try { + if (clientId == null) { + clientId = Optional.ofNullable(loginInfo.get(CLIENT_ASSERTION)).map(JwtClientAuthentication::getClientId).orElse(null); + } wrapClientCredentialLogin(req, res, loginInfo, clientId); } catch (AuthenticationException ex) { logger.debug("Could not authenticate with client credentials."); @@ -152,8 +158,12 @@ private Authentication performClientAuthentication(HttpServletRequest req, Map getCredentials(HttpServletRequest request) { Map credentials = new HashMap<>(); - credentials.put(CLIENT_ID, request.getParameter(CLIENT_ID)); + String clientId = request.getParameter(CLIENT_ID); + credentials.put(CLIENT_ID, clientId); credentials.put(CLIENT_SECRET, request.getParameter(CLIENT_SECRET)); + if (clientId == null) { + credentials.put(CLIENT_ASSERTION, request.getParameter(CLIENT_ASSERTION)); + } return credentials; } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/BackwardsCompatibleTokenEndpointAuthenticationFilter.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/BackwardsCompatibleTokenEndpointAuthenticationFilter.java index 6b2db872771..92b11c32bd2 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/BackwardsCompatibleTokenEndpointAuthenticationFilter.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/BackwardsCompatibleTokenEndpointAuthenticationFilter.java @@ -13,8 +13,10 @@ package org.cloudfoundry.identity.uaa.authentication; +import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants; import org.cloudfoundry.identity.uaa.provider.oauth.ExternalOAuthAuthenticationManager; import org.cloudfoundry.identity.uaa.util.SessionUtils; +import org.cloudfoundry.identity.uaa.util.UaaSecurityContextUtils; import org.cloudfoundry.identity.uaa.util.UaaStringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -130,6 +132,10 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) if (clientAuth.isAuthenticated()) { // Ensure the OAuth2Authentication is authenticated authorizationRequest.setApproved(true); + String clientAuthentication = UaaSecurityContextUtils.getClientAuthenticationMethod(clientAuth); + if (clientAuthentication != null) { + authorizationRequest.getExtensions().put(ClaimConstants.CLIENT_AUTH_METHOD, clientAuthentication); + } } OAuth2Request storedOAuth2Request = oAuth2RequestFactory.createOAuth2Request(authorizationRequest); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ClientDetailsAuthenticationProvider.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ClientDetailsAuthenticationProvider.java index 05860983097..a2d020d62fe 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ClientDetailsAuthenticationProvider.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ClientDetailsAuthenticationProvider.java @@ -13,10 +13,11 @@ package org.cloudfoundry.identity.uaa.authentication; import org.cloudfoundry.identity.uaa.client.UaaClient; +import org.cloudfoundry.identity.uaa.oauth.jwt.JwtClientAuthentication; import org.cloudfoundry.identity.uaa.oauth.pkce.PkceValidationService; import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants; import org.cloudfoundry.identity.uaa.oauth.token.TokenConstants; -import org.cloudfoundry.identity.uaa.util.UaaStringUtils; +import org.springframework.security.authentication.AbstractAuthenticationToken; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.authentication.dao.DaoAuthenticationProvider; @@ -28,16 +29,21 @@ import java.util.Collections; import java.util.Map; +import java.util.Optional; import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_NONE; +import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_PRIVATE_KEY_JWT; +import static org.cloudfoundry.identity.uaa.util.UaaStringUtils.getSafeParameterValue; public class ClientDetailsAuthenticationProvider extends DaoAuthenticationProvider { + private final JwtClientAuthentication jwtClientAuthentication; - public ClientDetailsAuthenticationProvider(UserDetailsService userDetailsService, PasswordEncoder encoder) { + public ClientDetailsAuthenticationProvider(UserDetailsService userDetailsService, PasswordEncoder encoder, JwtClientAuthentication jwtClientAuthentication) { super(); setUserDetailsService(userDetailsService); setPasswordEncoder(encoder); + this.jwtClientAuthentication = jwtClientAuthentication; } @Override @@ -59,7 +65,13 @@ protected void additionalAuthenticationChecks(UserDetails userDetails, UsernameP if (authentication.getCredentials() == null) { if (isPublicGrantTypeUsageAllowed(authentication.getDetails()) && uaaClient.isAllowPublic()) { // in case of grant_type=authorization_code and code_verifier passed (PKCE) we check if client has option allowpublic with true and continue even if no secret is in request - ((UaaAuthenticationDetails) authentication.getDetails()).setAuthenticationMethod(CLIENT_AUTH_NONE); + setAuthenticationMethod(authentication, CLIENT_AUTH_NONE); + break; + } else if (isPrivateKeyJwt(authentication.getDetails())) { + if (!validatePrivateKeyJwt(authentication.getDetails(), uaaClient)) { + error = new BadCredentialsException("Bad client_assertion type"); + } + setAuthenticationMethod(authentication, CLIENT_AUTH_PRIVATE_KEY_JWT); break; } } @@ -79,11 +91,15 @@ protected void additionalAuthenticationChecks(UserDetails userDetails, UsernameP } } - private boolean isPublicGrantTypeUsageAllowed(Object uaaAuthenticationDetails) { - UaaAuthenticationDetails authenticationDetails = uaaAuthenticationDetails instanceof UaaAuthenticationDetails ? - (UaaAuthenticationDetails) uaaAuthenticationDetails : new UaaAuthenticationDetails(); - Map requestParameters = authenticationDetails.getParameterMap() != null ? - authenticationDetails.getParameterMap() : Collections.emptyMap(); + private static void setAuthenticationMethod(AbstractAuthenticationToken authentication, String method) { + if (authentication.getDetails() instanceof UaaAuthenticationDetails) { + ((UaaAuthenticationDetails) authentication.getDetails()).setAuthenticationMethod(method); + } + } + + private static boolean isPublicGrantTypeUsageAllowed(Object uaaAuthenticationDetails) { + UaaAuthenticationDetails authenticationDetails = getUaaAuthenticationDetails(uaaAuthenticationDetails); + Map requestParameters = getRequestParameters(authenticationDetails); return isPublicTokenRequest(authenticationDetails) && (isAuthorizationWithPkce(requestParameters) || isRefreshFlow(requestParameters)); } @@ -91,7 +107,7 @@ private static boolean isPublicTokenRequest(UaaAuthenticationDetails authenticat return !authenticationDetails.isAuthorizationSet() && "/oauth/token".equals(authenticationDetails.getRequestPath()); } - private boolean isAuthorizationWithPkce(Map requestParameters) { + private static boolean isAuthorizationWithPkce(Map requestParameters) { return PkceValidationService.isCodeVerifierParameterValid(getSafeParameterValue(requestParameters.get("code_verifier"))) && StringUtils.hasText(getSafeParameterValue(requestParameters.get("client_id"))) && StringUtils.hasText(getSafeParameterValue(requestParameters.get("code"))) && @@ -99,16 +115,31 @@ private boolean isAuthorizationWithPkce(Map requestParameters) TokenConstants.GRANT_TYPE_AUTHORIZATION_CODE.equals(getSafeParameterValue(requestParameters.get(ClaimConstants.GRANT_TYPE))); } - private boolean isRefreshFlow(Map requestParameters) { + private static boolean isRefreshFlow(Map requestParameters) { return StringUtils.hasText(getSafeParameterValue(requestParameters.get("client_id"))) && StringUtils.hasText(getSafeParameterValue(requestParameters.get("refresh_token"))) && TokenConstants.GRANT_TYPE_REFRESH_TOKEN.equals(getSafeParameterValue(requestParameters.get(ClaimConstants.GRANT_TYPE))); } - private String getSafeParameterValue(String[] value) { - if (null == value || value.length < 1) { - return UaaStringUtils.EMPTY_STRING; - } - return StringUtils.hasText(value[0]) ? value[0] : UaaStringUtils.EMPTY_STRING; + private static UaaAuthenticationDetails getUaaAuthenticationDetails(Object object) { + return object instanceof UaaAuthenticationDetails ? (UaaAuthenticationDetails) object : new UaaAuthenticationDetails(); + } + + private static Map getRequestParameters(UaaAuthenticationDetails authenticationDetails) { + return Optional.ofNullable(authenticationDetails.getParameterMap()).orElse(Collections.emptyMap()); + } + + private static boolean isPrivateKeyJwt(Object uaaAuthenticationDetails) { + UaaAuthenticationDetails authenticationDetails = getUaaAuthenticationDetails(uaaAuthenticationDetails); + Map requestParameters = getRequestParameters(authenticationDetails); + return (isPublicTokenRequest(authenticationDetails) && + !StringUtils.hasText(getSafeParameterValue(requestParameters.get("client_secret"))) && + StringUtils.hasText(getSafeParameterValue(requestParameters.get("client_assertion_type"))) && + StringUtils.hasText(getSafeParameterValue(requestParameters.get("client_assertion")))); + } + + private boolean validatePrivateKeyJwt(Object uaaAuthenticationDetails, UaaClient uaaClient) { + return jwtClientAuthentication.validateClientJwt(getRequestParameters(getUaaAuthenticationDetails(uaaAuthenticationDetails)), + uaaClient.getClientJwtConfiguration(), uaaClient.getUsername()); } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfiguration.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfiguration.java index 792499e100b..6507a27a66b 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfiguration.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfiguration.java @@ -211,7 +211,18 @@ public static ClientJwtConfiguration readValue(UaaClientDetails clientDetails) { !(clientDetails.getClientJwtConfig() instanceof String)) { return null; } - return JsonUtils.readValue(clientDetails.getClientJwtConfig(), ClientJwtConfiguration.class); + return readValue(clientDetails.getClientJwtConfig()); + } + + /** + * Creator from searialized ClientJwtConfiguration. + * + * @param clientJwtConfig + * @return + */ + @JsonIgnore + public static ClientJwtConfiguration readValue(String clientJwtConfig) { + return JsonUtils.readValue(clientJwtConfig, ClientJwtConfiguration.class); } /** diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClient.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClient.java index fbea54c125e..41a5e491b10 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClient.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClient.java @@ -17,11 +17,14 @@ public class UaaClient extends User { private transient Map additionalInformation; private final String secret; + private final String clientJwtConfig; - public UaaClient(String username, String password, Collection authorities, Map additionalInformation) { + public UaaClient(String username, String password, Collection authorities, Map additionalInformation, + String clientJwtConfig) { super(username, password == null ? "" : password, authorities); this.additionalInformation = additionalInformation; this.secret = password; + this.clientJwtConfig = clientJwtConfig; } public UaaClient(UserDetails userDetails, String secret) { @@ -29,6 +32,9 @@ public UaaClient(UserDetails userDetails, String secret) { userDetails.isCredentialsNonExpired(), userDetails.isAccountNonLocked(), userDetails.getAuthorities()); if (userDetails instanceof UaaClient) { this.additionalInformation = ((UaaClient) userDetails).getAdditionalInformation(); + this.clientJwtConfig = ((UaaClient) userDetails).clientJwtConfig; + } else { + this.clientJwtConfig = null; } this.secret = secret; } @@ -46,6 +52,10 @@ private Map getAdditionalInformation() { return this.additionalInformation; } + public ClientJwtConfiguration getClientJwtConfiguration() { + return Optional.ofNullable(clientJwtConfig).map(ClientJwtConfiguration::readValue).orElse(new ClientJwtConfiguration()); + } + /** * Allow to return a null password. Super class does not allow to omit a password, therefore use own method * diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsUserDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsUserDetailsService.java index 5f4d7776395..77c9dc3e0e8 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsUserDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsUserDetailsService.java @@ -21,7 +21,8 @@ public UserDetails loadUserByUsername(String username) throws UsernameNotFoundEx } catch (NoSuchClientException e) { throw new UsernameNotFoundException(e.getMessage(), e); } - return new UaaClient(username, clientDetails.getClientSecret(), clientDetails.getAuthorities(), clientDetails.getAdditionalInformation()); + return new UaaClient(username, clientDetails.getClientSecret(), clientDetails.getAuthorities(), clientDetails.getAdditionalInformation(), + clientDetails.getClientJwtConfig()); } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/KeyInfoService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/KeyInfoService.java index efa1774129a..3034fbaca6c 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/KeyInfoService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/KeyInfoService.java @@ -15,11 +15,13 @@ package org.cloudfoundry.identity.uaa.oauth; import org.cloudfoundry.identity.uaa.impl.config.LegacyTokenKey; +import org.cloudfoundry.identity.uaa.util.UaaTokenUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZoneConfiguration; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import org.cloudfoundry.identity.uaa.zone.TokenPolicy; import org.springframework.util.StringUtils; +import java.net.URISyntaxException; import java.util.HashMap; import java.util.Map; @@ -89,4 +91,8 @@ private String getActiveKeyId() { return activeKeyId; } + + public String getTokenEndpointUrl() throws URISyntaxException { + return UaaTokenUtils.constructTokenEndpointUrl(uaaBaseURL, IdentityZoneHolder.get()); + } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java index 604f530b47d..0859400ca64 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java @@ -13,13 +13,14 @@ package org.cloudfoundry.identity.uaa.oauth; import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; +import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants; import org.cloudfoundry.identity.uaa.oauth.token.TokenConstants; import org.cloudfoundry.identity.uaa.provider.IdentityProvider; import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; -import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.security.beans.SecurityContextAccessor; import org.cloudfoundry.identity.uaa.user.UaaUser; import org.cloudfoundry.identity.uaa.user.UaaUserDatabase; +import org.cloudfoundry.identity.uaa.util.UaaSecurityContextUtils; import org.cloudfoundry.identity.uaa.util.UaaStringUtils; import org.cloudfoundry.identity.uaa.util.UaaTokenUtils; import org.cloudfoundry.identity.uaa.zone.MultitenantClientServices; @@ -397,6 +398,10 @@ public UaaTokenRequest(Map requestParameters, String clientId, C @Override public OAuth2Request createOAuth2Request(ClientDetails client) { OAuth2Request request = super.createOAuth2Request(client); + String clientAuthentication = UaaSecurityContextUtils.getClientAuthenticationMethod(); + if (clientAuthentication != null) { + request.getExtensions().put(ClaimConstants.CLIENT_AUTH_METHOD, clientAuthentication); + } return new OAuth2Request( request.getRequestParameters(), client.getClientId(), diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java index 307a8252f44..d6037e8e7dc 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java @@ -319,13 +319,14 @@ private static String getAuthenticationMethod(OAuth2Request oAuth2Request) { } private void addAuthenticationMethod(Claims claims, Map additionalRootClaims, UserAuthenticationData authenticationData) { - if (authenticationData.clientAuth != null && CLIENT_AUTH_NONE.equals(authenticationData.clientAuth)) { + if (authenticationData.clientAuth != null) { // public refresh flow, allowed if access_token before was also without authentication (claim: client_auth_method=none) and refresh token is one time use (rotate it in refresh) - if (refreshTokenCreator.shouldRotateRefreshTokens() && CLIENT_AUTH_NONE.equals(claims.getClientAuth())) { - addRootClaimEntry(additionalRootClaims, CLIENT_AUTH_METHOD, authenticationData.clientAuth); - } else { + if (CLIENT_AUTH_NONE.equals(authenticationData.clientAuth) && // current authentication + (!CLIENT_AUTH_NONE.equals(claims.getClientAuth()) || // authentication before + !refreshTokenCreator.shouldRotateRefreshTokens())) { throw new TokenRevokedException("Refresh without client authentication not allowed."); } + addRootClaimEntry(additionalRootClaims, CLIENT_AUTH_METHOD, authenticationData.clientAuth); } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java index 3377477d8eb..8425c9bd6ba 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java @@ -1,29 +1,69 @@ package org.cloudfoundry.identity.uaa.oauth.jwt; +import com.nimbusds.jose.Algorithm; +import com.nimbusds.jose.JOSEException; +import com.nimbusds.jose.JWSAlgorithm; +import com.nimbusds.jose.jwk.JWKSet; +import com.nimbusds.jose.jwk.source.ImmutableJWKSet; +import com.nimbusds.jose.jwk.source.JWKSource; +import com.nimbusds.jose.proc.BadJOSEException; +import com.nimbusds.jose.proc.BadJWSException; +import com.nimbusds.jose.proc.JWSKeySelector; +import com.nimbusds.jose.proc.JWSVerificationKeySelector; +import com.nimbusds.jose.proc.SecurityContext; +import com.nimbusds.jwt.JWT; +import com.nimbusds.jwt.JWTClaimsSet; +import com.nimbusds.jwt.JWTParser; +import com.nimbusds.jwt.proc.BadJWTException; +import com.nimbusds.jwt.proc.ConfigurableJWTProcessor; +import com.nimbusds.jwt.proc.DefaultJWTClaimsVerifier; +import com.nimbusds.jwt.proc.DefaultJWTProcessor; +import org.cloudfoundry.identity.uaa.client.ClientJwtConfiguration; import org.cloudfoundry.identity.uaa.oauth.KeyInfo; import org.cloudfoundry.identity.uaa.oauth.KeyInfoService; +import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants; import org.cloudfoundry.identity.uaa.oauth.token.Claims; import org.cloudfoundry.identity.uaa.provider.OIDCIdentityProviderDefinition; +import org.cloudfoundry.identity.uaa.provider.oauth.OidcMetadataFetcher; +import org.cloudfoundry.identity.uaa.provider.oauth.OidcMetadataFetchingException; import org.cloudfoundry.identity.uaa.util.JsonUtils; +import org.cloudfoundry.identity.uaa.util.UaaStringUtils; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.util.MultiValueMap; +import java.net.URISyntaxException; +import java.text.ParseException; import java.time.Instant; import java.util.Arrays; import java.util.HashMap; +import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.UUID; public class JwtClientAuthentication { public static final String GRANT_TYPE = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"; + public static final String CLIENT_ASSERTION = "client_assertion"; + public static final String CLIENT_ASSERTION_TYPE = "client_assertion_type"; + + // no signature check with invalid algorithms + private static final Set NOT_SUPPORTED_ALGORITHMS = Set.of(Algorithm.NONE, JWSAlgorithm.HS256, JWSAlgorithm.HS384, JWSAlgorithm.HS512); + private static final Set JWT_REQUIRED_CLAIMS = Set.of(ClaimConstants.ISS, ClaimConstants.SUB, ClaimConstants.AUD, + ClaimConstants.EXPIRY_IN_SECONDS, ClaimConstants.JTI); private final KeyInfoService keyInfoService; + private final OidcMetadataFetcher oidcMetadataFetcher; public JwtClientAuthentication( KeyInfoService keyInfoService) { + this(keyInfoService, null); + } + + public JwtClientAuthentication(KeyInfoService keyInfoService, OidcMetadataFetcher oidcMetadataFetcher) { this.keyInfoService = keyInfoService; + this.oidcMetadataFetcher = oidcMetadataFetcher; } public String getClientAssertion(OIDCIdentityProviderDefinition config) { @@ -51,8 +91,8 @@ public MultiValueMap getClientAuthenticationParameters(MultiValu if (!params.containsKey("client_id")) { params.add("client_id", config.getRelyingPartyId()); } - params.add("client_assertion_type", GRANT_TYPE); - params.add("client_assertion", getClientAssertion(config)); + params.add(CLIENT_ASSERTION_TYPE, GRANT_TYPE); + params.add(CLIENT_ASSERTION, getClientAssertion(config)); return params; } @@ -65,4 +105,58 @@ private static HashMap getJwtClientConfigurationElements(Object } return jwtClientConfiguration; } + + public boolean validateClientJwt(Map requestParameters, ClientJwtConfiguration clientJwtConfiguration, String clientId) { + if (GRANT_TYPE.equals(UaaStringUtils.getSafeParameterValue(requestParameters.get(CLIENT_ASSERTION_TYPE)))) { + try { + String clientAssertion = UaaStringUtils.getSafeParameterValue(requestParameters.get(CLIENT_ASSERTION)); + if (!clientId.equals(getClientId(clientAssertion))) { + throw new BadCredentialsException("Wrong client_assertion"); + } + return clientId.equals(validateClientJWToken(JWTParser.parse(clientAssertion), oidcMetadataFetcher == null ? new JWKSet() : + JWKSet.parse(oidcMetadataFetcher.fetchWebKeySet(clientJwtConfiguration).getKeySetMap()), + clientId, keyInfoService.getTokenEndpointUrl()).getSubject()); + } catch (ParseException | URISyntaxException | OidcMetadataFetchingException e) { + throw new BadCredentialsException("Bad client_assertion", e); + } + } + return false; + } + + public static String getClientId(String clientAssertion) { + try { + JWTClaimsSet clientToken = clientAssertion != null ? JWTParser.parse(clientAssertion).getJWTClaimsSet() : null; + if (clientToken != null && clientToken.getSubject() != null && clientToken.getIssuer() != null && + clientToken.getSubject().equals(clientToken.getIssuer()) && clientToken.getAudience() != null && clientToken.getJWTID() != null && + clientToken.getExpirationTime() != null) { + // required claims, e.g. https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication + return clientToken.getSubject(); + } + throw new BadCredentialsException("Bad credentials"); + } catch (ParseException e) { + throw new BadCredentialsException("Bad client_assertion", e); + } + } + + private JWTClaimsSet validateClientJWToken(JWT jwtAssertion, JWKSet jwkSet, String expectedClientId, String expectedAud) { + Algorithm algorithm = jwtAssertion.getHeader().getAlgorithm(); + if (algorithm == null || NOT_SUPPORTED_ALGORITHMS.contains(algorithm) || !(algorithm instanceof JWSAlgorithm)) { + throw new BadCredentialsException("Bad client_assertion algorithm"); + } + JWKSource keySource = new ImmutableJWKSet<>(jwkSet); + JWSKeySelector keySelector = new JWSVerificationKeySelector<>((JWSAlgorithm) algorithm, keySource); + ConfigurableJWTProcessor jwtProcessor = new DefaultJWTProcessor<>(); + jwtProcessor.setJWSKeySelector(keySelector); + + JWTClaimsSet.Builder claimSetBuilder = new JWTClaimsSet.Builder().issuer(expectedClientId).subject(expectedClientId); + jwtProcessor.setJWTClaimsSetVerifier(new DefaultJWTClaimsVerifier<>(expectedAud, claimSetBuilder.build(), JWT_REQUIRED_CLAIMS)); + + try { + return jwtProcessor.process(jwtAssertion, null); + } catch (BadJWSException | BadJWTException jwtException) { // signature failed + throw new BadCredentialsException("Unauthorized client_assertion", jwtException); + } catch (BadJOSEException | JOSEException e) { // key resolution, structure of JWT failed + throw new BadCredentialsException("Untrusted client_assertion", e); + } + } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcher.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcher.java index 51a2b3e1d2f..2ac641908cd 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcher.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcher.java @@ -4,6 +4,7 @@ import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang3.StringUtils; import org.cloudfoundry.identity.uaa.cache.UrlContentCache; +import org.cloudfoundry.identity.uaa.client.ClientJwtConfiguration; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKeyHelper; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKeySet; @@ -52,16 +53,7 @@ public JsonWebKeySet fetchWebKeySet(AbstractExternalOAuthIdentityPro if (tokenKeyUrl == null || !org.springframework.util.StringUtils.hasText(tokenKeyUrl.toString())) { return new JsonWebKeySet<>(Collections.emptyList()); } - MultiValueMap headers = new LinkedMultiValueMap<>(); - headers.add("Authorization", getClientAuthHeader(config)); - headers.add("Accept", "application/json"); - HttpEntity tokenKeyRequest = new HttpEntity<>(null, headers); - byte[] rawContents; - if (config.isSkipSslValidation()) { - rawContents = contentCache.getUrlContent(tokenKeyUrl.toString(), trustingRestTemplate, HttpMethod.GET, tokenKeyRequest); - } else { - rawContents = contentCache.getUrlContent(tokenKeyUrl.toString(), nonTrustingRestTemplate, HttpMethod.GET, tokenKeyRequest); - } + byte[] rawContents = getJsonBody(tokenKeyUrl.toString(), config.isSkipSslValidation(), getClientAuthHeader(config)); if (rawContents == null || rawContents.length == 0) { throw new OidcMetadataFetchingException("Unable to fetch verification keys"); } @@ -72,7 +64,39 @@ public JsonWebKeySet fetchWebKeySet(AbstractExternalOAuthIdentityPro } } + public JsonWebKeySet fetchWebKeySet(ClientJwtConfiguration clientJwtConfiguration) throws OidcMetadataFetchingException { + if (clientJwtConfiguration.getJwkSet() != null) { + return clientJwtConfiguration.getJwkSet(); + } else if (clientJwtConfiguration.getJwksUri() != null) { + byte[] rawContents = getJsonBody(clientJwtConfiguration.getJwksUri(), false, null); + if (rawContents != null && rawContents.length > 0) { + ClientJwtConfiguration clientKeys = ClientJwtConfiguration.parse(null, new String(rawContents, StandardCharsets.UTF_8)); + if (clientKeys != null && clientKeys.getJwkSet() != null) { + return clientKeys.getJwkSet(); + } + } + } + throw new OidcMetadataFetchingException("Unable to fetch verification keys"); + } + + private byte[] getJsonBody(String uri, boolean isSkipSslValidation, String authorizationValue) { + MultiValueMap headers = new LinkedMultiValueMap<>(); + if (authorizationValue != null) { + headers.add("Authorization", authorizationValue); + } + headers.add("Accept", "application/json"); + HttpEntity tokenKeyRequest = new HttpEntity<>(null, headers); + if (isSkipSslValidation) { + return contentCache.getUrlContent(uri, trustingRestTemplate, HttpMethod.GET, tokenKeyRequest); + } else { + return contentCache.getUrlContent(uri, nonTrustingRestTemplate, HttpMethod.GET, tokenKeyRequest); + } + } + private String getClientAuthHeader(AbstractExternalOAuthIdentityProviderDefinition config) { + if (config.getRelyingPartySecret() == null) { + return null; + } String clientAuth = new String(Base64.encodeBase64((config.getRelyingPartyId() + ":" + config.getRelyingPartySecret()).getBytes())); return "Basic " + clientAuth; } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/util/UaaSecurityContextUtils.java b/server/src/main/java/org/cloudfoundry/identity/uaa/util/UaaSecurityContextUtils.java index dfe5be3947d..f2aaeedd073 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/util/UaaSecurityContextUtils.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/util/UaaSecurityContextUtils.java @@ -1,5 +1,6 @@ package org.cloudfoundry.identity.uaa.util; +import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.oauth2.provider.OAuth2Authentication; @@ -14,8 +15,13 @@ public final class UaaSecurityContextUtils { private UaaSecurityContextUtils() {} public static String getClientAuthenticationMethod() { - Authentication a = SecurityContextHolder.getContext().getAuthentication(); + return getClientAuthenticationMethod(SecurityContextHolder.getContext().getAuthentication()); + } + public static String getClientAuthenticationMethod(Authentication a) { if (!(a instanceof OAuth2Authentication)) { + if (a != null && a.isAuthenticated() && a.getDetails() instanceof UaaAuthenticationDetails) { + return ((UaaAuthenticationDetails) a.getDetails()).getAuthenticationMethod(); + } return null; } OAuth2Authentication oAuth2Authentication = (OAuth2Authentication) a; diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/BackwardsCompatibleTokenEndpointAuthenticationFilterTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/BackwardsCompatibleTokenEndpointAuthenticationFilterTest.java index dba3b19dd47..06e8f18c2ca 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/BackwardsCompatibleTokenEndpointAuthenticationFilterTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/BackwardsCompatibleTokenEndpointAuthenticationFilterTest.java @@ -16,6 +16,7 @@ package org.cloudfoundry.identity.uaa.authentication; import org.cloudfoundry.identity.uaa.oauth.TokenTestSupport; +import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants; import org.cloudfoundry.identity.uaa.provider.oauth.ExternalOAuthAuthenticationManager; import org.cloudfoundry.identity.uaa.provider.oauth.ExternalOAuthCodeToken; import org.cloudfoundry.identity.uaa.util.SessionUtils; @@ -31,16 +32,21 @@ import org.springframework.security.authentication.InsufficientAuthenticationException; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.oauth2.provider.AuthorizationRequest; +import org.springframework.security.oauth2.provider.OAuth2Authentication; +import org.springframework.security.oauth2.provider.OAuth2Request; import org.springframework.security.oauth2.provider.OAuth2RequestFactory; import org.springframework.security.saml.SAMLProcessingFilter; import org.springframework.security.web.AuthenticationEntryPoint; import javax.servlet.FilterChain; import java.util.Collections; +import java.util.Map; import static java.util.Optional.ofNullable; import static org.cloudfoundry.identity.uaa.oauth.TokenTestSupport.OPENID; import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.GRANT_TYPE; +import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_NONE; import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_JWT_BEARER; import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_SAML2_BEARER; import static org.junit.Assert.assertEquals; @@ -48,7 +54,9 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.same; +import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -124,13 +132,46 @@ public void attempt_password_authentication() throws Exception { request.addParameter(GRANT_TYPE, "password"); request.addParameter("username", "marissa"); request.addParameter("password", "koala"); + when(passwordAuthManager.authenticate(any())).thenReturn(mock(UaaAuthentication.class)); + OAuth2Authentication clientAuthentication = mock(OAuth2Authentication.class); + OAuth2Request oAuth2Request = mock(OAuth2Request.class); + AuthorizationRequest authorizationRequest = mock(AuthorizationRequest.class); + when(clientAuthentication.isAuthenticated()).thenReturn(true); + when(requestFactory.createAuthorizationRequest(anyMap())).thenReturn(authorizationRequest); + when(clientAuthentication.getOAuth2Request()).thenReturn(oAuth2Request); + when(oAuth2Request.getExtensions()).thenReturn(Map.of(ClaimConstants.CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); + SecurityContextHolder.getContext().setAuthentication(clientAuthentication); filter.doFilter(request, response, chain); + verify(clientAuthentication, times(0)).getDetails(); verify(filter, times(1)).attemptTokenAuthentication(same(request), same(response)); verify(passwordAuthManager, times(1)).authenticate(any()); + verify(oAuth2Request, times(1)).getExtensions(); verifyNoInteractions(samlAuthFilter); verifyNoInteractions(externalOAuthAuthenticationManager); } + @Test + public void attempt_password_authentication_with_details() throws Exception { + request.addParameter(GRANT_TYPE, "password"); + request.addParameter("username", "marissa"); + request.addParameter("password", "koala"); + when(passwordAuthManager.authenticate(any())).thenReturn(mock(UaaAuthentication.class)); + UaaAuthentication clientAuthentication = mock(UaaAuthentication.class); + UaaAuthenticationDetails uaaAuthenticationDetails = mock(UaaAuthenticationDetails.class); + AuthorizationRequest authorizationRequest = mock(AuthorizationRequest.class); + when(clientAuthentication.getDetails()).thenReturn(uaaAuthenticationDetails); + when(clientAuthentication.isAuthenticated()).thenReturn(true); + when((uaaAuthenticationDetails.getAuthenticationMethod())).thenReturn(CLIENT_AUTH_NONE); + when(requestFactory.createAuthorizationRequest(anyMap())).thenReturn(authorizationRequest); + SecurityContextHolder.getContext().setAuthentication(clientAuthentication); + filter.doFilter(request, response, chain); + verify(clientAuthentication, atLeast(1)).getDetails(); + verify(filter, times(1)).attemptTokenAuthentication(same(request), same(response)); + verify(passwordAuthManager, times(1)).authenticate(any()); + verify(authorizationRequest, times(1)).getExtensions(); + verifyNoInteractions(samlAuthFilter); + verifyNoInteractions(externalOAuthAuthenticationManager); + } @Test public void attempt_saml_assertion_authentication() throws Exception { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/UaaClientAuthenticationProviderTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/UaaClientAuthenticationProviderTest.java index ffdc00cab52..adbc6dfb4b5 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/UaaClientAuthenticationProviderTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/UaaClientAuthenticationProviderTest.java @@ -6,6 +6,7 @@ import org.cloudfoundry.identity.uaa.client.UaaClientDetailsUserDetailsService; import org.cloudfoundry.identity.uaa.login.util.RandomValueStringGenerator; import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; +import org.cloudfoundry.identity.uaa.oauth.jwt.JwtClientAuthentication; import org.cloudfoundry.identity.uaa.user.UaaUser; import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; @@ -35,6 +36,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -45,6 +47,7 @@ class UaaClientAuthenticationProviderTest { private MultitenantJdbcClientDetailsService jdbcClientDetailsService; private ClientDetails client; private ClientDetailsAuthenticationProvider authenticationProvider; + private JwtClientAuthentication jwtClientAuthentication; @Autowired JdbcTemplate jdbcTemplate; @@ -55,12 +58,13 @@ class UaaClientAuthenticationProviderTest { @BeforeEach void setUpForClientTests() { IdentityZoneManager mockIdentityZoneManager = mock(IdentityZoneManager.class); + jwtClientAuthentication = mock(JwtClientAuthentication.class); when(mockIdentityZoneManager.getCurrentIdentityZoneId()).thenReturn(IdentityZone.getUaaZoneId()); jdbcClientDetailsService = new MultitenantJdbcClientDetailsService(jdbcTemplate, mockIdentityZoneManager, passwordEncoder); UaaClientDetailsUserDetailsService clientDetailsService = new UaaClientDetailsUserDetailsService(jdbcClientDetailsService); client = createClient(); - authenticationProvider = new ClientDetailsAuthenticationProvider(clientDetailsService, passwordEncoder); + authenticationProvider = new ClientDetailsAuthenticationProvider(clientDetailsService, passwordEncoder, jwtClientAuthentication); } public BaseClientDetails createClient() { @@ -97,6 +101,17 @@ private UsernamePasswordAuthenticationToken getAuthenticationToken(String grant_ return getAuthenticationToken(request); } + private UsernamePasswordAuthenticationToken getAuthenticationTokenClientJwt(String grant_type) { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/oauth/token"); + request.addParameter("code_verifier","E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"); + request.addParameter("code", "1234567890"); + request.addParameter("client_assertion", "id"); + request.addParameter("client_assertion_type", "id"); + request.addParameter("redirect_uri", "http://localhost:8080/uaa"); + request.addParameter("grant_type", grant_type); + return getAuthenticationToken(request); + } + private UsernamePasswordAuthenticationToken getAuthenticationToken(HttpServletRequest request) { UsernamePasswordAuthenticationToken authentication = mock(UsernamePasswordAuthenticationToken.class); UaaAuthenticationDetails uaaAuthenticationDetails = new UaaAuthenticationDetails(request); @@ -114,7 +129,7 @@ void provider_authenticate_client_with_one_password() { void provider_authenticate_client_without_password_public_string() { client = createClient(ClientConstants.ALLOW_PUBLIC, "true"); UsernamePasswordAuthenticationToken a = getAuthenticationToken("authorization_code"); - authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation()), a); + authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation(), null), a); assertNotNull(a); } @@ -126,7 +141,7 @@ void provider_refresh_client_without_password_public_boolean() { request.addParameter("refresh_token", "1234567890"); request.addParameter("grant_type", "refresh_token"); UsernamePasswordAuthenticationToken a = getAuthenticationToken(request); - authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation()), a); + authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation(), null), a); assertNotNull(a); } @@ -139,7 +154,7 @@ void provider_refresh_client_with_password_inAuthorizationHeader_public_boolean( request.addParameter("refresh_token", "1234567890"); request.addParameter("grant_type", "refresh_token"); UsernamePasswordAuthenticationToken a = getAuthenticationToken(request); - assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation()), a)); + assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation(), null), a)); } @Test @@ -150,14 +165,14 @@ void provider_refresh_client_without_wrong_endpoint() { request.addParameter("refresh_token", "1234567890"); request.addParameter("grant_type", "refresh_token"); UsernamePasswordAuthenticationToken a = getAuthenticationToken(request); - assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation()), a)); + assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation(), null), a)); } @Test void provider_authenticate_client_without_password_public_boolean() { client = createClient(ClientConstants.ALLOW_PUBLIC, true); UsernamePasswordAuthenticationToken a = getAuthenticationToken("authorization_code"); - authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation()), a); + authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation(), null), a); assertNotNull(a); } @@ -165,7 +180,7 @@ void provider_authenticate_client_without_password_public_boolean() { void provider_authenticate_client_without_password_public_wrong_grant_type() { client = createClient(ClientConstants.ALLOW_PUBLIC, true); UsernamePasswordAuthenticationToken a = getAuthenticationToken("client_credentials"); - assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret secret2", Collections.emptyList(), client.getAdditionalInformation()), a)); + assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret secret2", Collections.emptyList(), client.getAdditionalInformation(), null), a)); } @Test @@ -181,7 +196,7 @@ void provider_authenticate_no_authenticationDetails() { client = createClient(ClientConstants.ALLOW_PUBLIC, true); UsernamePasswordAuthenticationToken a = getAuthenticationToken("authorization_code"); when(a.getDetails()).thenReturn(null); - assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret secret2", Collections.emptyList(), client.getAdditionalInformation()), a)); + assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret secret2", Collections.emptyList(), client.getAdditionalInformation(), null), a)); } @Test @@ -192,7 +207,7 @@ void provider_authenticate_client_without_password_public_missing_code() { when(a.getDetails()).thenReturn(uaaAuthenticationDetails); Map requestParameters = new HashMap<>(); when(uaaAuthenticationDetails.getParameterMap()).thenReturn(requestParameters); - assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation()), a)); + assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation(), null), a)); } @Test @@ -204,7 +219,7 @@ void provider_authenticate_client_without_secret_user_without_secret() { when(a.getDetails()).thenReturn(uaaAuthenticationDetails); Map requestParameters = new HashMap<>(); when(uaaAuthenticationDetails.getParameterMap()).thenReturn(requestParameters); - Exception e = assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", null, Collections.emptyList(), client.getAdditionalInformation()), a)); + Exception e = assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks(new UaaClient("client", null, Collections.emptyList(), client.getAdditionalInformation(), null), a)); assertEquals("Missing credentials", e.getMessage()); } @@ -234,4 +249,20 @@ void provider_authenticate_client_with_two_passwords_test_3() { jdbcClientDetailsService.addClientSecret(client.getClientId(), "secret2", IdentityZoneHolder.get().getId()); assertThrows(AuthenticationException.class, () -> testClientAuthentication(getToken(client.getClientId(), "secret3"))); } + + @Test + void clientJwt_authenticate_client_without_config() { + UsernamePasswordAuthenticationToken a = getAuthenticationTokenClientJwt("authorization_code"); + assertThrows(BadCredentialsException.class, () -> authenticationProvider.additionalAuthenticationChecks( + new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation(), null), a)); + } + + @Test + void clientJwt_authenticate_client_valid() { + when(jwtClientAuthentication.validateClientJwt(any(), any(), any())).thenReturn(true); + UsernamePasswordAuthenticationToken a = getAuthenticationTokenClientJwt("authorization_code"); + authenticationProvider.additionalAuthenticationChecks( + new UaaClient("client", "secret", Collections.emptyList(), client.getAdditionalInformation(), null), a); + assertNotNull(a); + } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManagerTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManagerTests.java index 51ef865cae9..5a8b76c69ed 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManagerTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManagerTests.java @@ -24,6 +24,7 @@ import org.springframework.security.oauth2.common.exceptions.UnauthorizedClientException; import org.springframework.security.oauth2.common.util.OAuth2Utils; import org.springframework.security.oauth2.provider.AuthorizationRequest; +import org.springframework.security.oauth2.provider.OAuth2Authentication; import org.springframework.security.oauth2.provider.OAuth2Request; import org.springframework.security.oauth2.provider.client.BaseClientDetails; import org.springframework.test.util.ReflectionTestUtils; @@ -134,8 +135,13 @@ void testTokenRequestIncludesResourceIds() { @Test void test_user_token_request() { + OAuth2Authentication oAuth2Authentication = mock(OAuth2Authentication.class); + OAuth2Request oAuth2Request = mock(OAuth2Request.class); when(mockSecurityContextAccessor.isUser()).thenReturn(true); when(mockSecurityContextAccessor.getAuthorities()).thenReturn((Collection)AuthorityUtils.commaSeparatedStringToAuthorityList("uaa.user,requested.scope")); + when(oAuth2Authentication.getOAuth2Request()).thenReturn(oAuth2Request); + when(oAuth2Request.getExtensions()).thenReturn(Map.of("client_auth_method", "none")); + SecurityContextHolder.getContext().setAuthentication(oAuth2Authentication); BaseClientDetails recipient = new BaseClientDetails("recipient", "requested", "requested.scope", "password", ""); parameters.put("scope", "requested.scope"); parameters.put("client_id", recipient.getClientId()); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthenticationTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthenticationTest.java index 430d12e829b..b6b9277532d 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthenticationTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthenticationTest.java @@ -1,13 +1,18 @@ package org.cloudfoundry.identity.uaa.oauth.jwt; import com.nimbusds.jose.JWSHeader; +import com.nimbusds.jose.jwk.JWK; import com.nimbusds.jose.util.X509CertUtils; import com.nimbusds.jwt.JWT; import com.nimbusds.jwt.JWTClaimsSet; import com.nimbusds.jwt.JWTParser; +import org.cloudfoundry.identity.uaa.client.ClientJwtConfiguration; import org.cloudfoundry.identity.uaa.oauth.KeyInfo; +import org.cloudfoundry.identity.uaa.oauth.KeyInfoBuilder; import org.cloudfoundry.identity.uaa.oauth.KeyInfoService; import org.cloudfoundry.identity.uaa.provider.OIDCIdentityProviderDefinition; +import org.cloudfoundry.identity.uaa.provider.oauth.OidcMetadataFetcher; +import org.cloudfoundry.identity.uaa.provider.oauth.OidcMetadataFetchingException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.security.authentication.BadCredentialsException; @@ -19,12 +24,14 @@ import java.text.ParseException; import java.util.Collections; import java.util.HashMap; +import java.util.Map; import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertThrowsExactly; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -34,8 +41,11 @@ class JwtClientAuthenticationTest { private static final String KEY_ID = "tokenKeyId"; + private static final String INVALID_CLIENT_JWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJqdGkiOiJjYmEyZmRlN2ZkYTg0YzMzYTdkZDQ5MWVmMzljZWY5NiIsImF1ZCI6WyJodHRwOi8vbG9jYWxob3N0OjgwODAvdWFhL29hdXRoL3Rva2VuIl0sInN1YiI6ImlkZW50aXR5IiwiaXNzIjoic29tZVRoaW5nIn0.HLXpsPJw0SvF8DcGmmifzJJLxX4hmfwILAtAFedk48c"; + private static final String INVALID_CLIENT_ALG = "eyJhbGciOiJIUzI1NiIsImtpZCI6InRva2VuS2V5SWQiLCJ0eXAiOiJKV1QifQ.eyJleHAiOjE2OTU4NDEyMDYsImp0aSI6ImRhMzdjYzFjMjkzOTQzMWE4YjUzZTI5MmZiMjYxMDZhIiwiYXVkIjpbImh0dHA6Ly9sb2NhbGhvc3Q6ODA4MC91YWEvb2F1dGgvdG9rZW4iXSwic3ViIjoiaWRlbnRpdHkiLCJpc3MiOiJpZGVudGl0eSIsImlhdCI6MTY5NTg0MDc4NiwicmV2b2NhYmxlIjpmYWxzZX0.gFYuUjzupzeKNK2Uq3Ijp0rDIcJfI80wl3Pt5MSypPM"; private OIDCIdentityProviderDefinition config; private KeyInfoService keyInfoService = mock(KeyInfoService.class); + private OidcMetadataFetcher oidcMetadataFetcher = mock(OidcMetadataFetcher.class); private JwtClientAuthentication jwtClientAuthentication; @BeforeEach @@ -189,6 +199,94 @@ void testGetClientAssertionCustomSingingKeyButNoCertificate() throws ParseExcept assertEquals("http://localhost:8080/uaa/token_key", header.getJWKURL().toString()); } + @Test + void testGetClientIdOfClientAssertion() throws ParseException { + // When + String clientAssertion = (String) jwtClientAuthentication.getClientAssertion(config); + // Then + assertEquals("identity", jwtClientAuthentication.getClientId(clientAssertion)); + } + + @Test + void testRequestInvalidateClientAssertion() throws Exception { + // Then + assertFalse(jwtClientAuthentication.validateClientJwt(getMockedRequestParameter("test", INVALID_CLIENT_JWT), getMockedClientJwtConfiguration(null), "identity")); + } + + @Test + void testWrongAssertionInvalidateClientId() { + // Given + jwtClientAuthentication = new JwtClientAuthentication(keyInfoService, oidcMetadataFetcher); + // Then + Exception exception = assertThrows(BadCredentialsException.class, () -> + jwtClientAuthentication.validateClientJwt(getMockedRequestParameter(null, jwtClientAuthentication.getClientAssertion(config)), + // pass a different client_id to the provided one from client_assertion JWT + getMockedClientJwtConfiguration(null), "wrong_client_id")); + assertEquals("Wrong client_assertion", exception.getMessage()); + } + + @Test + void testBadAlgorithmInvalidateClientId() throws Exception { + // Given + jwtClientAuthentication = new JwtClientAuthentication(keyInfoService, oidcMetadataFetcher); + ClientJwtConfiguration clientJwtConfiguration = getMockedClientJwtConfiguration(null); + when(oidcMetadataFetcher.fetchWebKeySet(clientJwtConfiguration)).thenReturn(clientJwtConfiguration.getJwkSet()); + // Then + Exception exception = assertThrows(BadCredentialsException.class, () -> + jwtClientAuthentication.validateClientJwt(getMockedRequestParameter(null, INVALID_CLIENT_ALG), + clientJwtConfiguration, "identity")); + assertEquals("Bad client_assertion algorithm", exception.getMessage()); + } + + @Test + void testOidcFetchFailed() throws Exception { + // Given + jwtClientAuthentication = new JwtClientAuthentication(keyInfoService, oidcMetadataFetcher); + ClientJwtConfiguration clientJwtConfiguration = getMockedClientJwtConfiguration(null); + when(oidcMetadataFetcher.fetchWebKeySet(clientJwtConfiguration)).thenThrow(new OidcMetadataFetchingException("")); + // Then + Exception exception = assertThrows(BadCredentialsException.class, () -> + jwtClientAuthentication.validateClientJwt(getMockedRequestParameter(null, jwtClientAuthentication.getClientAssertion(config)), + clientJwtConfiguration, "identity")); + assertEquals("Bad client_assertion", exception.getMessage()); + } + + @Test + void testUntrustedClientAssertion() throws Exception { + // Given + jwtClientAuthentication = new JwtClientAuthentication(keyInfoService, oidcMetadataFetcher); + // create client assertion with key ids which wont map to provide JWT, lead to failing validateClientJWToken check + ClientJwtConfiguration clientJwtConfiguration = getMockedClientJwtConfiguration("extId"); + when(oidcMetadataFetcher.fetchWebKeySet(clientJwtConfiguration)).thenReturn(clientJwtConfiguration.getJwkSet()); + String clientAssertion = jwtClientAuthentication.getClientAssertion(config); + // When + Exception exception = assertThrows(BadCredentialsException.class, () -> + jwtClientAuthentication.validateClientJwt(getMockedRequestParameter(null, clientAssertion), clientJwtConfiguration, "identity")); + // Then, expect key resolution error because of not maching configured keys to JWT kid + assertEquals("Untrusted client_assertion", exception.getMessage()); + } + + @Test + void testSignatureInvalidateClientAssertion() throws Exception { + // Given + jwtClientAuthentication = new JwtClientAuthentication(keyInfoService, oidcMetadataFetcher); + ClientJwtConfiguration clientJwtConfiguration = getMockedClientJwtConfiguration(null); + when(oidcMetadataFetcher.fetchWebKeySet(clientJwtConfiguration)).thenReturn(clientJwtConfiguration.getJwkSet()); + String clientAssertion = jwtClientAuthentication.getClientAssertion(config); + // When + Exception exception = assertThrows(BadCredentialsException.class, () -> + jwtClientAuthentication.validateClientJwt(getMockedRequestParameter(null, clientAssertion), clientJwtConfiguration, "identity")); + // Then, expect signature failed because mockKeyInfoService creates corrupted signature + assertEquals("Unauthorized client_assertion", exception.getMessage()); + } + + @Test + void testGetClientIdOfInvalidClientAssertion() throws ParseException { + // Then + assertThrows(BadCredentialsException.class, () -> jwtClientAuthentication.getClientId(INVALID_CLIENT_JWT)); + assertThrows(BadCredentialsException.class, () -> jwtClientAuthentication.getClientId("eyXXX")); + } + private void mockKeyInfoService(String keyId, String x509Certificate) { KeyInfo keyInfo = mock(KeyInfo.class); Signer signer = mock(Signer.class); @@ -229,4 +327,21 @@ private static void validateClientAssertionRfc7523Complaint(String clientAsserti assertEquals(iss, jwtClaimsSet.getIssuer()); assertEquals("identity", jwtClaimsSet.getSubject()); } + + private static Map getMockedRequestParameter(String type, String assertion) { + Map requestParameters = new HashMap<>(); + if (type != null) { + requestParameters.put(JwtClientAuthentication.CLIENT_ASSERTION_TYPE, new String[] { type }); + } else { + requestParameters.put(JwtClientAuthentication.CLIENT_ASSERTION_TYPE, new String[] { JwtClientAuthentication.GRANT_TYPE }); + } + requestParameters.put(JwtClientAuthentication.CLIENT_ASSERTION, new String[] { assertion} ); + return requestParameters; + } + + private static ClientJwtConfiguration getMockedClientJwtConfiguration(String keyId) throws ParseException { + KeyInfo keyInfo = KeyInfoBuilder.build(keyId != null ? keyId : "tokenKeyId", JwtHelperX5tTest.SIGNING_KEY_1, "http://localhost:8080/uaa"); + return ClientJwtConfiguration.parse(JWK.parse(keyInfo.getJwkMap()).toString()); + } + } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/KeyInfoServiceTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/KeyInfoServiceTests.java index 658efe21f53..19bd401bbee 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/KeyInfoServiceTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/KeyInfoServiceTests.java @@ -15,6 +15,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.springframework.security.oauth2.common.util.RandomValueStringGenerator; +import java.net.URISyntaxException; import java.util.Collections; import java.util.Map; @@ -140,6 +141,13 @@ void testActiveKeyFallsBackToLegacyKey() { assertEquals(keyInfoService.getActiveKey().verifierKey(), "testLegacyKey"); } + @Test + void testTokenEndpointUrl() throws URISyntaxException { + configureDefaultZoneKeys(Collections.emptyMap()); + + assertEquals("https://localhost/uaa/oauth/token", keyInfoService.getTokenEndpointUrl()); + } + private void configureDefaultZoneKeys(Map keys) { IdentityZoneHolder.clear(); IdentityZoneProvisioning provisioning = mock(IdentityZoneProvisioning.class); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcherTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcherTest.java index deaa4486721..3bde8524dfc 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcherTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcherTest.java @@ -1,6 +1,9 @@ package org.cloudfoundry.identity.uaa.provider.oauth; import org.cloudfoundry.identity.uaa.cache.UrlContentCache; +import org.cloudfoundry.identity.uaa.client.ClientJwtConfiguration; +import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey; +import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKeySet; import org.cloudfoundry.identity.uaa.provider.OIDCIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.junit.jupiter.api.BeforeEach; @@ -8,13 +11,18 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.mockito.Answers; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpMethod; import org.springframework.web.client.RestTemplate; import java.net.MalformedURLException; import java.net.URL; import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; @@ -40,7 +48,7 @@ class WithDiscoveryUrl { private OidcMetadata oidcMetadata; @BeforeEach - public void setup() throws MalformedURLException { + void setup() throws MalformedURLException { definition.setDiscoveryUrl(new URL("http://discovery.url")); oidcMetadata = new OidcMetadata(); @@ -52,7 +60,7 @@ public void setup() throws MalformedURLException { } @Test - public void shouldFavorUsingConfiguredIdentityProviderProperties() throws OidcMetadataFetchingException, MalformedURLException { + void shouldFavorUsingConfiguredIdentityProviderProperties() throws OidcMetadataFetchingException, MalformedURLException { definition.setAuthUrl(new URL("http://authz.should.not.have.been.updated")); definition.setTokenUrl(new URL("http://token.should.not.have.been.updated")); definition.setUserInfoUrl(new URL("http://userinfo.should.not.have.been.updated")); @@ -72,7 +80,7 @@ public void shouldFavorUsingConfiguredIdentityProviderProperties() throws OidcMe } @Test - public void givenConfiguredIdentityProviderPropertiesAreNotSet_shouldUseOidcMetadata() throws OidcMetadataFetchingException { + void givenConfiguredIdentityProviderPropertiesAreNotSet_shouldUseOidcMetadata() throws OidcMetadataFetchingException { when(urlContentCache.getUrlContent(anyString(), any(RestTemplate.class))) .thenReturn(JsonUtils.writeValueAsBytes(oidcMetadata)); @@ -87,7 +95,7 @@ public void givenConfiguredIdentityProviderPropertiesAreNotSet_shouldUseOidcMeta } @Test - public void shouldPerformDiscoveryUsingCache() throws OidcMetadataFetchingException, MalformedURLException { + void shouldPerformDiscoveryUsingCache() throws OidcMetadataFetchingException, MalformedURLException { definition.setAuthUrl(new URL("http://should.be.updated")); definition.setTokenUrl(new URL("http://should.be.updated")); definition.setSkipSslValidation(false); @@ -103,18 +111,35 @@ public void shouldPerformDiscoveryUsingCache() throws OidcMetadataFetchingExcept eq(definition.getDiscoveryUrl().toString()), eq(restTemplate) ); } + + @Test + void shouldPerformTokenKeyUrlUsingCache() throws OidcMetadataFetchingException, MalformedURLException { + definition.setTokenKeyUrl(new URL("http://should.be.updated")); + definition.setSkipSslValidation(false); + + when(urlContentCache.getUrlContent(anyString(), any(RestTemplate.class), any(HttpMethod.class), any(HttpEntity.class))) + .thenReturn("{\"keys\":[{\"alg\":\"RS256\",\"e\":\"e\",\"kid\":\"id\",\"kty\":\"RSA\",\"n\":\"n\"}]}".getBytes()); + + metadataDiscoverer.fetchWebKeySet(definition); + metadataDiscoverer.fetchWebKeySet(definition); + + verify(urlContentCache, times(2)) + .getUrlContent( + any(), any(), any(), any() + ); + } } @Nested class WithoutDiscoveryUrl { @BeforeEach - public void setup() { + void setup() { definition.setDiscoveryUrl(null); } @Test @DisplayName("when the idp is configured without a discovery URL then it should retain the configured OAuth/OIDC endpoints") - public void shouldNotPerformDiscovery() throws OidcMetadataFetchingException, MalformedURLException { + void shouldNotPerformDiscovery() throws OidcMetadataFetchingException, MalformedURLException { definition.setAuthUrl(new URL("http://authz.not.updated")); definition.setTokenUrl(new URL("http://token.not.updated")); definition.setTokenKeyUrl(new URL("http://jwk.not.updated")); @@ -135,4 +160,71 @@ public void shouldNotPerformDiscovery() throws OidcMetadataFetchingException, Ma } } + + @Nested + class WithErrorSituations { + @BeforeEach + void setup() throws MalformedURLException { + definition.setTokenKeyUrl(new URL("http://token_keys")); + definition.setSkipSslValidation(true); + definition.setRelyingPartyId("id"); + definition.setRelyingPartySecret("x"); + } + + @Test + void failWithEmptyContent() throws OidcMetadataFetchingException, MalformedURLException { + + when(urlContentCache.getUrlContent(anyString(), any(RestTemplate.class), any(HttpMethod.class), any(HttpEntity.class))) + .thenReturn("".getBytes()); + + assertThrows(OidcMetadataFetchingException.class, () -> metadataDiscoverer.fetchWebKeySet(definition)); + } + + @Test + void failWithInvalidContent() throws OidcMetadataFetchingException, MalformedURLException { + + when(urlContentCache.getUrlContent(anyString(), any(RestTemplate.class), any(HttpMethod.class), any(HttpEntity.class))) + .thenReturn("{x}".getBytes()); + + assertThrows(OidcMetadataFetchingException.class, () -> metadataDiscoverer.fetchWebKeySet(definition)); + } + } + + @Nested + class WithJwtClientKey { + @BeforeEach + void setup() throws MalformedURLException { + definition.setTokenKeyUrl(new URL("http://token_keys")); + definition.setSkipSslValidation(true); + definition.setRelyingPartyId("id"); + definition.setRelyingPartySecret(null); + when(urlContentCache.getUrlContent(anyString(), any(RestTemplate.class), any(HttpMethod.class), any(HttpEntity.class))) + .thenReturn("{\"keys\":[{\"alg\":\"RS256\",\"e\":\"e\",\"kid\":\"id\",\"kty\":\"RSA\",\"n\":\"n\"}]}".getBytes()); + } + + @Test + void getConfigFromJwksUri() throws OidcMetadataFetchingException, MalformedURLException { + + JsonWebKeySet keys = metadataDiscoverer.fetchWebKeySet(new ClientJwtConfiguration("http://token_keys", null)); + assertNotNull(keys); + assertEquals(1, keys.getKeys().size()); + assertEquals("id", keys.getKeys().get(0).getKid()); + } + + @Test + void getConfigFromJwks() throws OidcMetadataFetchingException, MalformedURLException { + + JsonWebKeySet keys = metadataDiscoverer.fetchWebKeySet(ClientJwtConfiguration.parse("{\"keys\":[{\"alg\":\"RS256\",\"e\":\"e\",\"kid\":\"a\",\"kty\":\"RSA\",\"n\":\"n\"}]}")); + assertNotNull(keys); + assertEquals(1, keys.getKeys().size()); + assertEquals("a", keys.getKeys().get(0).getKid()); + } + + @Test + void failWithInvalidConfig() throws OidcMetadataFetchingException, MalformedURLException { + + assertThrows(OidcMetadataFetchingException.class, () -> metadataDiscoverer.fetchWebKeySet(new ClientJwtConfiguration(null, null))); + } + } + } diff --git a/uaa/src/main/webapp/WEB-INF/spring/oauth-clients.xml b/uaa/src/main/webapp/WEB-INF/spring/oauth-clients.xml index b94fc503bce..b0f3b6efe8b 100644 --- a/uaa/src/main/webapp/WEB-INF/spring/oauth-clients.xml +++ b/uaa/src/main/webapp/WEB-INF/spring/oauth-clients.xml @@ -27,6 +27,7 @@ + diff --git a/uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml b/uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml index 97e6e7ec8c7..f49a20d9a89 100755 --- a/uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml +++ b/uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml @@ -286,10 +286,17 @@ + + + + + +