From 5f72461cd3176e2f2049630d86f04b89b2bb2106 Mon Sep 17 00:00:00 2001 From: strehle Date: Mon, 7 Aug 2023 18:18:08 +0200 Subject: [PATCH 01/22] refactor: prepare for private_key_jwt in oauth_client_details BaseClientDetails from spring security oauth2 cannot be changed, therefore more to UaaClientDetails for client details load --- .../identity/uaa/client/UaaClientDetails.java | 15 +++++++++++++++ .../UaaClientDetailsUserDetailsService.java | 5 ++--- .../zone/MultitenantJdbcClientDetailsService.java | 6 +++--- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java index 633c965d15e..448ba9a2e16 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java @@ -1,5 +1,8 @@ package org.cloudfoundry.identity.uaa.client; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; +import org.springframework.security.core.SpringSecurityCoreVersion; import org.springframework.security.oauth2.provider.ClientDetails; import org.springframework.security.oauth2.provider.client.BaseClientDetails; @@ -8,12 +11,24 @@ import java.util.Set; import java.util.stream.Collectors; +@JsonInclude(JsonInclude.Include.NON_DEFAULT) +@JsonIgnoreProperties(ignoreUnknown = true) public class UaaClientDetails extends BaseClientDetails { + + private static final long serialVersionUID = SpringSecurityCoreVersion.SERIAL_VERSION_UID; + UaaClientDetails(ClientDetails prototype) { super(prototype); this.setAdditionalInformation(prototype.getAdditionalInformation()); } + public UaaClientDetails(String clientId, String clientSecret, String resourceIds, + String scopes, String grantTypes, String authorities, String redirectUris) { + super(clientId, resourceIds, scopes, grantTypes, authorities, redirectUris); + setClientSecret(clientSecret); + } + + @Override public void setScope(Collection scope) { Set sanitized = scope.stream() .flatMap(s -> Arrays.stream(s.split(","))) 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 193b43b1c63..5824a0ad360 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 @@ -4,7 +4,6 @@ import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.security.crypto.password.PasswordEncoder; -import org.springframework.security.oauth2.provider.ClientDetails; import org.springframework.security.oauth2.provider.ClientDetailsService; import org.springframework.security.oauth2.provider.NoSuchClientException; @@ -25,9 +24,9 @@ public void setPasswordEncoder(PasswordEncoder passwordEncoder) { } public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException { - ClientDetails clientDetails; + UaaClientDetails clientDetails; try { - clientDetails = clientDetailsService.loadClientByClientId(username); + clientDetails = (UaaClientDetails) clientDetailsService.loadClientByClientId(username); } catch (NoSuchClientException e) { throw new UsernameNotFoundException(e.getMessage(), e); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java index bc7e3922807..ca00b5ee7c5 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java @@ -2,6 +2,7 @@ import org.cloudfoundry.identity.uaa.audit.event.SystemDeletable; import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal; +import org.cloudfoundry.identity.uaa.client.UaaClientDetails; import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.resources.ResourceMonitor; import org.cloudfoundry.identity.uaa.security.ContextSensitiveOAuth2SecurityExpressionMethods; @@ -25,7 +26,6 @@ import org.springframework.security.oauth2.provider.ClientAlreadyExistsException; import org.springframework.security.oauth2.provider.ClientDetails; import org.springframework.security.oauth2.provider.NoSuchClientException; -import org.springframework.security.oauth2.provider.client.BaseClientDetails; import org.springframework.stereotype.Component; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -284,15 +284,15 @@ public void deleteClientSecret(String clientId, String zoneId) throws NoSuchClie */ private static class ClientDetailsRowMapper implements RowMapper { public ClientDetails mapRow(ResultSet rs, int rowNum) throws SQLException { - BaseClientDetails details = new BaseClientDetails( + UaaClientDetails details = new UaaClientDetails( rs.getString(1), + rs.getString(2), rs.getString(3), rs.getString(4), rs.getString(5), rs.getString(7), rs.getString(6) ); - details.setClientSecret(rs.getString(2)); if (rs.getObject(8) != null) { details.setAccessTokenValiditySeconds(rs.getInt(8)); } From bf61c5d49e6fd3f709c8b28558e6532968146c76 Mon Sep 17 00:00:00 2001 From: strehle Date: Tue, 22 Aug 2023 18:18:40 +0200 Subject: [PATCH 02/22] feature: add persistence support for private_key_jwt Allow to setup jwks_uri and jwks, similar to OIDC proxy mode with tokenKeyUrl and tokenKey. The private_key_jwt metadata is stored in additional_information (could be switched to own column) The setup can be done from REST and yaml. --- .../uaa/oauth/client/ClientConstants.java | 1 + .../oauth/client/ClientDetailsCreation.java | 22 ++ .../oauth/client/PrivateKeyChangeRequest.java | 93 ++++++ .../uaa/client/ClientAdminBootstrap.java | 21 +- .../uaa/client/ClientAdminEndpoints.java | 48 +++ .../client/ClientAdminEndpointsValidator.java | 18 +- .../client/PrivateKeyJwtConfiguration.java | 315 ++++++++++++++++++ .../uaa/zone/MultitenantClientServices.java | 4 + .../MultitenantJdbcClientDetailsService.java | 38 +++ .../uaa/client/ClientAdminEndpointsTests.java | 46 +++ .../PrivateKeyJwtConfigurationTest.java | 204 ++++++++++++ .../InMemoryMultitenantClientServices.java | 10 + 12 files changed, 817 insertions(+), 3 deletions(-) create mode 100644 model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequest.java create mode 100644 server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java create mode 100644 server/src/test/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfigurationTest.java diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientConstants.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientConstants.java index 79d333b8da5..6cc4dde5cc9 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientConstants.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientConstants.java @@ -21,5 +21,6 @@ public class ClientConstants { public static final String APPROVALS_DELETED = "approvals_deleted"; public static final String TOKEN_SALT = "token_salt"; public static final String REQUIRED_USER_GROUPS = "required_user_groups"; + public static final String PRIVATE_KEY_CONFIG = "private_key_config"; public static final String LAST_MODIFIED = "lastModified"; } diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientDetailsCreation.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientDetailsCreation.java index 5e0d6d249b2..a44697e6a5a 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientDetailsCreation.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientDetailsCreation.java @@ -13,6 +13,12 @@ public class ClientDetailsCreation extends BaseClientDetails { @JsonProperty("secondary_client_secret") private String secondaryClientSecret; + @JsonProperty("private_key_url") + private String privateKeyUrl; + + @JsonProperty("private_key_set") + private String privateKeySet; + @JsonIgnore public String getSecondaryClientSecret() { return secondaryClientSecret; @@ -21,4 +27,20 @@ public String getSecondaryClientSecret() { public void setSecondaryClientSecret(final String secondaryClientSecret) { this.secondaryClientSecret = secondaryClientSecret; } + + public String getPrivateKeyUrl() { + return privateKeyUrl; + } + + public void setPrivateKeyUrl(String privateKeyUrl) { + this.privateKeyUrl = privateKeyUrl; + } + + public String getPrivateKeySet() { + return privateKeySet; + } + + public void setPrivateKeySet(String privateKeySet) { + this.privateKeySet = privateKeySet; + } } diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequest.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequest.java new file mode 100644 index 00000000000..86103b8fdaf --- /dev/null +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequest.java @@ -0,0 +1,93 @@ +/******************************************************************************* + * Cloud Foundry + * Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved. + * + * This product is licensed to you under the Apache License, Version 2.0 (the "License"). + * You may not use this product except in compliance with the License. + * + * This product includes a number of subcomponents with + * separate copyright notices and license terms. Your use of these + * subcomponents is subject to the terms and conditions of the + * subcomponent's license, as noted in the LICENSE file. + *******************************************************************************/ +package org.cloudfoundry.identity.uaa.oauth.client; + + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; + +import static org.cloudfoundry.identity.uaa.oauth.client.PrivateKeyChangeRequest.ChangeMode.ADD; + +/** + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +@JsonIgnoreProperties(ignoreUnknown = true) +public class PrivateKeyChangeRequest { + + public enum ChangeMode { + UPDATE, + ADD, + DELETE + } + @JsonProperty("kid") + private String keyId; + @JsonProperty("jwks_uri") + private String keyUrl; + @JsonProperty("jwks") + private String keyConfig; + @JsonProperty("client_id") + private String clientId; + private ChangeMode changeMode = ADD; + + public PrivateKeyChangeRequest() { + } + + public PrivateKeyChangeRequest(String clientId, String keyUrl, String keyConfig) { + this.keyUrl = keyUrl; + this.keyConfig = keyConfig; + this.clientId = clientId; + } + + public String getKeyUrl() { + return keyUrl; + } + + public void setKeyUrl(String keyUrl) { + this.keyUrl = keyUrl; + } + + public String getKeyConfig() { + return keyConfig; + } + + public void setKeyConfig(String keyConfig) { + this.keyConfig = keyConfig; + } + + public String getClientId() { + return clientId; + } + + public void setClientId(String clientId) { + this.clientId = clientId; + } + + public ChangeMode getChangeMode() { + return changeMode; + } + + public void setChangeMode(ChangeMode changeMode) { + this.changeMode = changeMode; + } + + public String getKeyId() { return keyId;} + + public void setKeyId(String keyId) { + this.keyId = keyId; + } + + public String getKey() { + return keyUrl != null ? keyUrl : keyConfig; + } +} diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java index e1bbc625833..029f83f7dc7 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java @@ -6,6 +6,7 @@ import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_REFRESH_TOKEN; import java.net.MalformedURLException; +import java.net.URI; import java.net.URL; import java.util.Arrays; import java.util.Collection; @@ -19,6 +20,7 @@ import org.cloudfoundry.identity.uaa.authentication.SystemAuthentication; import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.user.UaaAuthority; +import org.cloudfoundry.identity.uaa.util.UaaUrlUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import org.cloudfoundry.identity.uaa.zone.MultitenantClientServices; @@ -204,7 +206,7 @@ private void addNewClients() { } for (String key : Arrays.asList("resource-ids", "scope", "authorized-grant-types", "authorities", "redirect-uri", "secret", "id", "override", "access-token-validity", - "refresh-token-validity", "show-on-homepage", "app-launch-url", "app-icon")) { + "refresh-token-validity", "show-on-homepage", "app-launch-url", "app-icon", "jwks", "jwks_uri")) { info.remove(key); } @@ -235,6 +237,23 @@ private void addNewClients() { } } + if (map.get("jwks_uri") instanceof String) { + String jwksUri = (String) map.get("jwks_uri"); + URI jwksUriObject = URI.create(jwksUri); + if (jwksUriObject.isAbsolute() && ("https".startsWith(jwksUriObject.getScheme()) || ("http".startsWith(jwksUriObject.getScheme()) && jwksUriObject.getHost().contains("localhost")))) { + PrivateKeyJwtConfiguration keyConfig = PrivateKeyJwtConfiguration.parse(UaaUrlUtils.normalizeUri(jwksUri), null); + if (keyConfig != null && keyConfig.getCleanString() != null) { + clientRegistrationService.addClientKeyConfig(clientId, keyConfig.getPrivateKeyJwtUrl(), IdentityZone.getUaaZoneId(), override); + } + } + } else if (map.get("jwks") instanceof String) { + String jwks = (String) map.get("jwks"); + PrivateKeyJwtConfiguration keyConfig = PrivateKeyJwtConfiguration.parse(null, jwks); + if (keyConfig != null && keyConfig.getCleanString() != null) { + clientRegistrationService.addClientKeyConfig(clientId, keyConfig.getCleanString(), IdentityZone.getUaaZoneId(), override); + } + } + ClientMetadata clientMetadata = buildClientMetadata(map, clientId); clientMetadataProvisioning.update(clientMetadata, IdentityZone.getUaaZoneId()); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java index cf98e995a61..ae1d22e5156 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java @@ -20,6 +20,7 @@ import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsCreation; import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsModification; +import org.cloudfoundry.identity.uaa.oauth.client.PrivateKeyChangeRequest; import org.cloudfoundry.identity.uaa.oauth.client.SecretChangeRequest; import org.cloudfoundry.identity.uaa.resources.ActionResult; import org.cloudfoundry.identity.uaa.resources.AttributeNameMapper; @@ -535,6 +536,53 @@ public ActionResult changeSecret(@PathVariable String client_id, @RequestBody Se return result; } + @RequestMapping(value = "/oauth/clients/{client_id}/privatekey", method = RequestMethod.PUT) + @ResponseBody + public ActionResult changePrivateKey(@PathVariable String client_id, @RequestBody PrivateKeyChangeRequest change) { + + ClientDetails clientDetails; + try { + clientDetails = clientDetailsService.retrieve(client_id, IdentityZoneHolder.get().getId()); + } catch (InvalidClientException e) { + throw new NoSuchClientException("No such client: " + client_id); + } + + try { + checkPasswordChangeIsAllowed(clientDetails, ""); + } catch (IllegalStateException e) { + throw new InvalidClientDetailsException(e.getMessage()); + } + + PrivateKeyJwtConfiguration clientKeyConfig = PrivateKeyJwtConfiguration.createFromClientDetails(clientDetails); + + ActionResult result; + switch (change.getChangeMode()){ + case ADD : + if (change.getKey() != null) { + clientRegistrationService.addClientKeyConfig(client_id, change.getKey(), IdentityZoneHolder.get().getId(), false); + result = new ActionResult("ok", "Private key is added"); + } else { + result = new ActionResult("ok", "No key added"); + } + break; + + case DELETE : + String deleteString = change.getKeyId() == null ? change.getKey() : change.getKeyId(); + if (clientKeyConfig != null && deleteString != null) { + clientRegistrationService.deleteClientKeyConfig(client_id, deleteString, IdentityZoneHolder.get().getId()); + } + result = new ActionResult("ok", "Private key is deleted"); + break; + + default: + clientRegistrationService.addClientKeyConfig(client_id, change.getKey(), IdentityZoneHolder.get().getId(), true); + result = new ActionResult("ok", "Private key updated"); + } + clientSecretChanges.incrementAndGet(); + + return result; + } + private boolean validateCurrentClientSecretAdd(String clientSecret) { return clientSecret == null || clientSecret.split(" ").length == 1; } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java index 804dd5bfc3e..5787dc9fe46 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java @@ -12,14 +12,15 @@ *******************************************************************************/ package org.cloudfoundry.identity.uaa.client; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.cloudfoundry.identity.uaa.constants.OriginKeys; +import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsCreation; import org.cloudfoundry.identity.uaa.resources.QueryableResourceManager; import org.cloudfoundry.identity.uaa.security.beans.SecurityContextAccessor; import org.cloudfoundry.identity.uaa.util.UaaUrlUtils; import org.cloudfoundry.identity.uaa.zone.ClientSecretValidator; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.InitializingBean; import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.oauth2.provider.ClientDetails; @@ -245,6 +246,19 @@ public ClientDetails validate(ClientDetails prototype, boolean create, boolean c } clientSecretValidator.validate(client.getClientSecret()); } + + if (prototype instanceof ClientDetailsCreation) { + ClientDetailsCreation clientDetailsCreation = (ClientDetailsCreation) prototype; + if (StringUtils.hasText(clientDetailsCreation.getPrivateKeyUrl()) || StringUtils.hasText(clientDetailsCreation.getPrivateKeySet())) { + PrivateKeyJwtConfiguration privateKeyJwtConfiguration = PrivateKeyJwtConfiguration.parse(clientDetailsCreation.getPrivateKeyUrl(), + clientDetailsCreation.getPrivateKeySet()); + if (privateKeyJwtConfiguration != null) { + privateKeyJwtConfiguration.persistToClientDetail(client); + } else { + logger.warn("Client configuration with private_key_jwt not valid"); + } + } + } } return client; diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java new file mode 100644 index 00000000000..cd06e974905 --- /dev/null +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java @@ -0,0 +1,315 @@ +package org.cloudfoundry.identity.uaa.client; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.nimbusds.jose.jwk.JWK; +import com.nimbusds.jose.jwk.JWKSet; +import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey; +import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKeyHelper; +import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKeySet; +import org.cloudfoundry.identity.uaa.util.JsonUtils; +import org.cloudfoundry.identity.uaa.util.UaaUrlUtils; +import org.springframework.security.oauth2.provider.ClientDetails; +import org.springframework.security.oauth2.provider.client.BaseClientDetails; +import org.springframework.util.ObjectUtils; +import org.springframework.util.StringUtils; + +import java.net.URI; +import java.text.ParseException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +import static org.cloudfoundry.identity.uaa.oauth.client.ClientConstants.PRIVATE_KEY_CONFIG; + +@JsonInclude(JsonInclude.Include.NON_NULL) +@JsonIgnoreProperties(ignoreUnknown = true) +public class PrivateKeyJwtConfiguration { + + @JsonIgnore + private static final int MAX_KEY_SIZE = 10; + + @JsonProperty("jwks_uri") + private String privateKeyJwtUrl; + + @JsonProperty("jwks") + private JsonWebKeySet privateKeyJwt; + + public PrivateKeyJwtConfiguration() { + } + + public PrivateKeyJwtConfiguration(final String privateKeyJwtUrl, final JsonWebKeySet webKeySet) { + this.privateKeyJwtUrl = privateKeyJwtUrl; + privateKeyJwt = webKeySet; + if (privateKeyJwt != null) { + validateJwkSet(); + } + } + + public String getPrivateKeyJwtUrl() { + return this.privateKeyJwtUrl; + } + + public void setPrivateKeyJwtUrl(final String privateKeyJwtUrl) { + this.privateKeyJwtUrl = privateKeyJwtUrl; + } + + public JsonWebKeySet getPrivateKeyJwt() { + return this.privateKeyJwt; + } + + public void setPrivateKeyJwt(final JsonWebKeySet privateKeyJwt) { + this.privateKeyJwt = privateKeyJwt; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + return configEquals(o); + } + + @Override + public int hashCode() { + int result = super.hashCode(); + + result = 31 * result + (privateKeyJwtUrl != null ? privateKeyJwtUrl.hashCode() : 0); + result = 31 * result + (privateKeyJwt != null ? privateKeyJwt.hashCode() : 0); + return result; + } + + public boolean configEquals(Object o) { + if (o instanceof PrivateKeyJwtConfiguration) { + PrivateKeyJwtConfiguration that = (PrivateKeyJwtConfiguration) o; + if (!Objects.equals(privateKeyJwtUrl, that.privateKeyJwtUrl)) return false; + if (privateKeyJwt != null && that.privateKeyJwt != null) { + return privateKeyJwt.getKeys().equals(that.privateKeyJwt.getKeys()); + } else { + return Objects.equals(privateKeyJwt, that.privateKeyJwt); + } + } + return false; + } + + @JsonIgnore + public String getCleanString() { + try { + if (UaaUrlUtils.isUrl(this.privateKeyJwtUrl)) { + return this.privateKeyJwtUrl; + } else if (this.privateKeyJwt != null && !ObjectUtils.isEmpty(this.privateKeyJwt.getKeySetMap())) { + return JWKSet.parse(this.privateKeyJwt.getKeySetMap()).toString(true); + } + } catch (IllegalStateException | JsonUtils.JsonUtilException | ParseException e) { + throw new InvalidClientDetailsException("Private key configuration fails ", e); + } + return null; + } + + @JsonIgnore + public static PrivateKeyJwtConfiguration parse(String privateKeyConfig) { + if (UaaUrlUtils.isUrl(privateKeyConfig)) { + return parse(privateKeyConfig, null); + } else { + return parse(null, privateKeyConfig); + } + } + + @JsonIgnore + public static PrivateKeyJwtConfiguration parse(String privateKeyUrl, String privateKeyJwt) { + PrivateKeyJwtConfiguration privateKeyJwtConfiguration = null; + if (privateKeyUrl != null) { + privateKeyJwtConfiguration = new PrivateKeyJwtConfiguration(privateKeyUrl, null); + privateKeyJwtConfiguration.validateJwksUri(); + } else if (privateKeyJwt != null && privateKeyJwt.contains("{") && privateKeyJwt.contains("}")) { + HashMap jsonMap = JsonUtils.readValue(privateKeyJwt, HashMap.class); + String cleanJwtString; + try { + if (jsonMap.containsKey("keys")) { + cleanJwtString = JWKSet.parse(jsonMap).toString(true); + } else { + cleanJwtString = JWK.parse(jsonMap).toPublicJWK().toString(); + } + privateKeyJwtConfiguration = new PrivateKeyJwtConfiguration(null, JsonWebKeyHelper.deserialize(cleanJwtString)); + privateKeyJwtConfiguration.validateJwkSet(); + } catch (ParseException e) { + throw new InvalidClientDetailsException("Private key cannot be parsed", e); + } + } + return privateKeyJwtConfiguration; + } + + private boolean validateJwkSet() { + List keyList = privateKeyJwt.getKeys(); + Set keyId = new HashSet<>(); + if (keyList.isEmpty() || keyList.size() > MAX_KEY_SIZE) { + throw new InvalidClientDetailsException("Invalid private_key_jwt: jwk set is empty of exceeds to maximum of keys. max: + " + MAX_KEY_SIZE); + } + keyList.forEach(key -> { + if (!StringUtils.hasText(key.getKid())) { + throw new InvalidClientDetailsException("Invalid private_key_jwt: kid is required attribute"); + } + keyId.add(key.getKid()); + }); + if (keyId.size() != keyList.size()) { + throw new InvalidClientDetailsException("Invalid private_key_jwt: duplicate kid in JWKSet not allowed"); + } + return true; + } + + private boolean validateJwksUri() { + URI jwksUri; + try { + jwksUri = URI.create(privateKeyJwtUrl); + } catch (IllegalArgumentException e) { + throw new InvalidClientDetailsException("Invalid private_key_jwt: jwks_uri must be URI complaint", e); + } + if (!jwksUri.isAbsolute()) { + throw new InvalidClientDetailsException("Invalid private_key_jwt: jwks_uri must be an absolute URL"); + } + if (!"https".equals(jwksUri.getScheme()) && !"http".equals(jwksUri.getScheme())) { + throw new InvalidClientDetailsException("Invalid private_key_jwt: jwks_uri must be either using https or http"); + } + if ("http".equals(jwksUri.getScheme()) && !jwksUri.getHost().endsWith("localhost")) { + throw new InvalidClientDetailsException("Invalid private_key_jwt: jwks_uri with http is not on localhost"); + } + return true; + } + + /** + * Creator from ClientDetails. Should abstract the persistence. + * Use currently the additional information entry + * + * @param clientDetails + * @return + */ + @JsonIgnore + public static PrivateKeyJwtConfiguration createFromClientDetails(ClientDetails clientDetails) { + if (clientDetails == null || + clientDetails.getAdditionalInformation() == null || + !(clientDetails.getAdditionalInformation().get(PRIVATE_KEY_CONFIG) instanceof String)) { + return null; + } + return JsonUtils.readValue((String) clientDetails.getAdditionalInformation().get(PRIVATE_KEY_CONFIG), PrivateKeyJwtConfiguration.class); + } + + /** + * Creator from ClientDetails. Should abstract the persistence. + * Use currently the additional information entry + * + * @param clientDetails + * @return + */ + @JsonIgnore + public void persistToClientDetail(ClientDetails clientDetails) { + if (clientDetails instanceof BaseClientDetails) { + BaseClientDetails baseClientDetails = (BaseClientDetails) clientDetails; + HashMap additionalInformation = Optional.ofNullable(baseClientDetails.getAdditionalInformation()).map(HashMap::new).orElse(new HashMap<>()); + additionalInformation.put(PRIVATE_KEY_CONFIG, JsonUtils.writeValueAsString(this)); + baseClientDetails.setAdditionalInformation(additionalInformation); + } + } + + /** + * Cleanup configuration in ClientDetails. Should abstract the persistence. + * Use currently the additional information entry + * + * @param clientDetails + * @return + */ + @JsonIgnore + public static void cleanClientDetail(ClientDetails clientDetails) { + if (clientDetails instanceof BaseClientDetails) { + BaseClientDetails baseClientDetails = (BaseClientDetails) clientDetails; + HashMap additionalInformation = Optional.ofNullable(baseClientDetails.getAdditionalInformation()).map(HashMap::new).orElse(new HashMap<>()); + additionalInformation.remove(PRIVATE_KEY_CONFIG); + baseClientDetails.setAdditionalInformation(additionalInformation); + } + } + + @JsonIgnore + public static PrivateKeyJwtConfiguration merge(PrivateKeyJwtConfiguration existingConfig, PrivateKeyJwtConfiguration newConfig, boolean overwrite) { + if (existingConfig == null) { + return newConfig; + } + if (newConfig == null) { + return existingConfig; + } + PrivateKeyJwtConfiguration result = null; + if (newConfig.privateKeyJwtUrl != null) { + if (overwrite) { + result = new PrivateKeyJwtConfiguration(newConfig.privateKeyJwtUrl, null); + } else { + result = existingConfig; + } + } + if (newConfig.privateKeyJwt != null) { + if (existingConfig.privateKeyJwt == null) { + if (overwrite) { + result = new PrivateKeyJwtConfiguration(null, newConfig.privateKeyJwt); + } else { + result = existingConfig; + } + } else { + JsonWebKeySet existingKeySet = existingConfig.privateKeyJwt; + List existingKeys = new ArrayList<>(existingKeySet.getKeys()); + List newKeys = new ArrayList<>(); + newConfig.getPrivateKeyJwt().getKeys().forEach(key -> { + if (existingKeys.contains(key)) { + if (overwrite) { + existingKeys.remove(key); + newKeys.add(key); + } + } else { + newKeys.add(key); + } + }); + existingKeys.addAll(newKeys); + result = new PrivateKeyJwtConfiguration(null, new JsonWebKeySet<>(existingKeys)); + } + } + return result; + } + + @JsonIgnore + public static PrivateKeyJwtConfiguration delete(PrivateKeyJwtConfiguration existingConfig, PrivateKeyJwtConfiguration tobeDeleted) { + if (existingConfig == null) { + return null; + } + if (tobeDeleted == null) { + return existingConfig; + } + PrivateKeyJwtConfiguration result = null; + if (existingConfig.privateKeyJwt != null && tobeDeleted.privateKeyJwtUrl != null) { + JsonWebKeySet existingKeySet = existingConfig.privateKeyJwt; + List keys = existingKeySet.getKeys().stream().filter(k -> !tobeDeleted.privateKeyJwtUrl.equals(k.getKid())).collect(Collectors.toList()); + if (keys.isEmpty()) { + result = null; + } else { + result = new PrivateKeyJwtConfiguration(null, new JsonWebKeySet<>(keys)); + } + } else if (existingConfig.privateKeyJwt != null && tobeDeleted.privateKeyJwt != null) { + List existingKeys = new ArrayList<>(existingConfig.getPrivateKeyJwt().getKeys()); + existingKeys.removeAll(tobeDeleted.privateKeyJwt.getKeys()); + if (existingKeys.isEmpty()) { + result = null; + } else { + result = new PrivateKeyJwtConfiguration(null, new JsonWebKeySet<>(existingKeys)); + } + } else if (existingConfig.privateKeyJwtUrl != null && tobeDeleted.privateKeyJwtUrl != null) { + if ("*".equals(tobeDeleted.privateKeyJwtUrl) || existingConfig.privateKeyJwtUrl.equals(tobeDeleted.privateKeyJwtUrl)) { + result = null; + } else { + result = existingConfig; + } + } + return result; + } +} diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java index 528ba225c7e..931e709f756 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java @@ -30,6 +30,10 @@ interface MultitenantClientSecretService { void addClientSecret(String clientId, String newSecret, String zoneId) throws NoSuchClientException; void deleteClientSecret(String clientId, String zoneId) throws NoSuchClientException; + + void addClientKeyConfig(String clientId, String keyConfig, String zoneId, boolean overwrite) throws NoSuchClientException; + + void deleteClientKeyConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException; } public abstract class MultitenantClientServices implements diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java index bc7e3922807..ac1973964a7 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java @@ -2,10 +2,12 @@ import org.cloudfoundry.identity.uaa.audit.event.SystemDeletable; import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal; +import org.cloudfoundry.identity.uaa.client.PrivateKeyJwtConfiguration; import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.resources.ResourceMonitor; import org.cloudfoundry.identity.uaa.security.ContextSensitiveOAuth2SecurityExpressionMethods; import org.cloudfoundry.identity.uaa.util.JsonUtils; +import org.cloudfoundry.identity.uaa.util.UaaUrlUtils; import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -276,6 +278,42 @@ public void deleteClientSecret(String clientId, String zoneId) throws NoSuchClie } } + @Override + public void addClientKeyConfig(String clientId, String keyConfig, String zoneId, boolean overwrite) throws NoSuchClientException { + PrivateKeyJwtConfiguration privateKeyJwtConfiguration = PrivateKeyJwtConfiguration.parse(keyConfig); + if (privateKeyJwtConfiguration != null) { + BaseClientDetails clientDetails = (BaseClientDetails) loadClientByClientId(clientId, zoneId); + PrivateKeyJwtConfiguration existingConfig = PrivateKeyJwtConfiguration.createFromClientDetails(clientDetails); + if (existingConfig != null) { + PrivateKeyJwtConfiguration result = + PrivateKeyJwtConfiguration.merge(existingConfig, privateKeyJwtConfiguration, overwrite); + if (result != null) { + result.persistToClientDetail(clientDetails); + } + updateClientDetails(clientDetails, zoneId); + } + } + } + + @Override + public void deleteClientKeyConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException { + PrivateKeyJwtConfiguration privateKeyJwtConfiguration; + if(UaaUrlUtils.isUrl(keyConfig)) { + privateKeyJwtConfiguration = PrivateKeyJwtConfiguration.parse(keyConfig); + } else { + privateKeyJwtConfiguration = new PrivateKeyJwtConfiguration(keyConfig, null); + } + if (privateKeyJwtConfiguration != null) { + BaseClientDetails clientDetails = (BaseClientDetails) loadClientByClientId(clientId, zoneId); + PrivateKeyJwtConfiguration result = PrivateKeyJwtConfiguration.delete(PrivateKeyJwtConfiguration.createFromClientDetails(clientDetails), privateKeyJwtConfiguration); + if (result != null) { + result.persistToClientDetail(clientDetails); + } else { + PrivateKeyJwtConfiguration.cleanClientDetail(clientDetails); + } + updateClientDetails(clientDetails, zoneId); + } + } /** * Row mapper for ClientDetails. diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java index 415db58534a..3998d84cc57 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java @@ -1056,6 +1056,52 @@ void testUpdateClientWithAutoapproveScopesTrue() throws Exception { assertTrue(updated.isAutoApprove("foo.write")); } + @Test + void testCreateClientWithPrivateKeyUri() { + // https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata, see jwks_uri + String jwksUri = "https://any.domain.net/openid/jwks-uri"; + when(clientDetailsService.retrieve(anyString(), anyString())).thenReturn(input); + when(mockSecurityContextAccessor.getClientId()).thenReturn(detail.getClientId()); + when(mockSecurityContextAccessor.isClient()).thenReturn(true); + + input.setClientSecret("secret"); + detail.setAuthorizedGrantTypes(input.getAuthorizedGrantTypes()); + ClientDetailsCreation createRequest = createClientDetailsCreation(input); + createRequest.setPrivateKeyUrl(jwksUri); + ClientDetails result = endpoints.createClientDetails(createRequest); + assertNull(result.getClientSecret()); + ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); + verify(clientDetailsService).create(clientCaptor.capture(), anyString()); + BaseClientDetails created = clientCaptor.getValue(); + assertTrue(PrivateKeyJwtConfiguration.createFromClientDetails(created).configEquals(PrivateKeyJwtConfiguration.parse(jwksUri))); + } + + @Test + void testCreateClientWithPrivateKeySet() { + // Example JWK, a key is bound to a kid, means assumption is, a key is the same if kid is the same + String jsonJwk = "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}"; + String jsonJwk2 = "{\"kty\":\"RSA\",\"e\":\"\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"\"}"; + String jsonJwk3 = "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"key-2\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}"; + String jsonJwkSet = "{\"keys\":[{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}]}"; + when(clientDetailsService.retrieve(anyString(), anyString())).thenReturn(input); + when(mockSecurityContextAccessor.getClientId()).thenReturn(detail.getClientId()); + when(mockSecurityContextAccessor.isClient()).thenReturn(true); + + input.setClientSecret("secret"); + detail.setAuthorizedGrantTypes(input.getAuthorizedGrantTypes()); + ClientDetailsCreation createRequest = createClientDetailsCreation(input); + createRequest.setPrivateKeySet(jsonJwk); + ClientDetails result = endpoints.createClientDetails(createRequest); + assertNull(result.getClientSecret()); + ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); + verify(clientDetailsService).create(clientCaptor.capture(), anyString()); + BaseClientDetails created = clientCaptor.getValue(); + assertTrue(PrivateKeyJwtConfiguration.createFromClientDetails(created).configEquals(PrivateKeyJwtConfiguration.parse(jsonJwk))); + assertTrue(PrivateKeyJwtConfiguration.createFromClientDetails(created).configEquals(PrivateKeyJwtConfiguration.parse(jsonJwk2))); + assertTrue(PrivateKeyJwtConfiguration.createFromClientDetails(created).configEquals(PrivateKeyJwtConfiguration.parse(jsonJwkSet))); + assertFalse(PrivateKeyJwtConfiguration.createFromClientDetails(created).configEquals(PrivateKeyJwtConfiguration.parse(jsonJwk3))); + } + private ClientDetailsCreation createClientDetailsCreation(BaseClientDetails baseClientDetails) { final var clientDetails = new ClientDetailsCreation(); clientDetails.setClientId(baseClientDetails.getClientId()); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfigurationTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfigurationTest.java new file mode 100644 index 00000000000..827ad0ae8a0 --- /dev/null +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfigurationTest.java @@ -0,0 +1,204 @@ +package org.cloudfoundry.identity.uaa.client; + +import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey; +import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKeySet; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.text.ParseException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +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.Mockito.mock; +import static org.mockito.Mockito.when; + +class PrivateKeyJwtConfigurationTest { + + private final String nValue = "u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q"; + private final String jsonWebKey = "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}"; + private final String jsonWebKeyDifferentValue = "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"new\"}"; + private final String jsonWebKey2 = "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"key-2\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}"; + private final String jsonWebKeyNoId = "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}"; + private final String jsonJwkSet = "{\"keys\":[{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}]}"; + private final String jsonJwkSetEmtpy = "{\"keys\":[]}"; + private PrivateKeyJwtConfiguration privateKeyJwtConfiguration; + + @BeforeEach + void setUp() { + } + + @Test + void testJwksValidity() { + assertNotNull(PrivateKeyJwtConfiguration.parse("https://any.domain.net/openid/jwks-uri")); + assertNotNull(PrivateKeyJwtConfiguration.parse("http://any.localhost/openid/jwks-uri")); + } + + @Test + void testJwksInvalid() { + assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse("custom://any.domain.net/openid/jwks-uri", null)); + assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse("test", null)); + assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse("http://any.domain.net/openid/jwks-uri")); + assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse("https://")); + assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse("ftp://any.domain.net/openid/jwks-uri")); + } + + @Test + void testJwkSetValidity() { + assertNotNull(PrivateKeyJwtConfiguration.parse(jsonWebKey)); + assertNotNull(PrivateKeyJwtConfiguration.parse(jsonJwkSet)); + } + + @Test + void testJwkSetInvalid() { + assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse(jsonJwkSetEmtpy)); + assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse(jsonWebKeyNoId)); + assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse("{\"keys\": \"x\"}")); + } + + @Test + void testJwkSetInvalidSize() throws ParseException { + assertThrows(InvalidClientDetailsException.class, () -> new PrivateKeyJwtConfiguration(null, new JsonWebKeySet(Collections.emptyList()))); + } + + @Test + void testGetCleanConfig() { + assertNotNull(PrivateKeyJwtConfiguration.parse("https://any.domain.net/openid/jwks-uri").getCleanString()); + assertNotNull(PrivateKeyJwtConfiguration.parse(jsonWebKey).getCleanString()); + } + + @Test + void testGetCleanConfigInvalid() { + JsonWebKeySet mockedKey = mock(JsonWebKeySet.class); + List keyList = PrivateKeyJwtConfiguration.parse(jsonJwkSet).getPrivateKeyJwt().getKeys(); + when(mockedKey.getKeys()).thenReturn(keyList); + PrivateKeyJwtConfiguration privateKey = new PrivateKeyJwtConfiguration(null, mockedKey); + when(mockedKey.getKeySetMap()).thenThrow(new IllegalStateException("error")); + assertThrows(InvalidClientDetailsException.class, () -> privateKey.getCleanString()); + PrivateKeyJwtConfiguration privateKey2 = new PrivateKeyJwtConfiguration("hello", null); + assertNull(privateKey2.getCleanString()); + } + + @Test + void testJwtSetValidate() { + JsonWebKeySet mockedKey = mock(JsonWebKeySet.class); + List keyList = PrivateKeyJwtConfiguration.parse(jsonJwkSet).getPrivateKeyJwt().getKeys(); + when(mockedKey.getKeys()).thenReturn(Arrays.asList(keyList.get(0), keyList.get(0))); + assertThrows(InvalidClientDetailsException.class, () -> new PrivateKeyJwtConfiguration(null, mockedKey)); + } + + @Test + void testConfigMerge() { + PrivateKeyJwtConfiguration configuration = PrivateKeyJwtConfiguration.parse(jsonJwkSet); + assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + PrivateKeyJwtConfiguration addKey = PrivateKeyJwtConfiguration.parse(jsonWebKey2); + configuration = PrivateKeyJwtConfiguration.merge(configuration, addKey, false); + assertEquals(2, configuration.getPrivateKeyJwt().getKeys().size()); + assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); + assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(1).getKeyProperties().get("n")); + + configuration = PrivateKeyJwtConfiguration.merge(configuration, addKey, true); + assertEquals(2, configuration.getPrivateKeyJwt().getKeys().size()); + + configuration = PrivateKeyJwtConfiguration.parse(jsonJwkSet); + assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); + + configuration = PrivateKeyJwtConfiguration.merge(PrivateKeyJwtConfiguration.parse(jsonJwkSet), PrivateKeyJwtConfiguration.parse(jsonWebKeyDifferentValue), true); + assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + assertEquals("new", configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); + + configuration = PrivateKeyJwtConfiguration.merge(PrivateKeyJwtConfiguration.parse(jsonJwkSet), PrivateKeyJwtConfiguration.parse(jsonWebKeyDifferentValue), false); + assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); + } + + @Test + void testConfigMergeDifferentType() { + PrivateKeyJwtConfiguration configuration = PrivateKeyJwtConfiguration.parse(jsonJwkSet); + assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + assertNull(configuration.getPrivateKeyJwtUrl()); + configuration = PrivateKeyJwtConfiguration.merge(configuration, PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), false); + assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + assertNull(configuration.getPrivateKeyJwtUrl()); + + configuration = PrivateKeyJwtConfiguration.merge(configuration, PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), true); + assertNull(configuration.getPrivateKeyJwt()); + assertNotNull(configuration.getPrivateKeyJwtUrl()); + + configuration = PrivateKeyJwtConfiguration.merge(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), PrivateKeyJwtConfiguration.parse("https://new/jwks-uri"), false); + assertNull(configuration.getPrivateKeyJwt()); + assertEquals("https://any/jwks-uri", configuration.getPrivateKeyJwtUrl()); + + configuration = PrivateKeyJwtConfiguration.merge(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), PrivateKeyJwtConfiguration.parse("https://new/jwks-uri"), true); + assertNull(configuration.getPrivateKeyJwt()); + assertEquals("https://new/jwks-uri", configuration.getPrivateKeyJwtUrl()); + + configuration = PrivateKeyJwtConfiguration.merge(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), PrivateKeyJwtConfiguration.parse(jsonJwkSet), false); + assertNull(configuration.getPrivateKeyJwt()); + assertEquals("https://any/jwks-uri", configuration.getPrivateKeyJwtUrl()); + + configuration = PrivateKeyJwtConfiguration.merge(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), PrivateKeyJwtConfiguration.parse(jsonJwkSet), true); + assertNull(configuration.getPrivateKeyJwtUrl()); + assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); + } + + @Test + void testConfigMergeNulls() { + PrivateKeyJwtConfiguration configuration = PrivateKeyJwtConfiguration.parse(jsonJwkSet); + PrivateKeyJwtConfiguration existingKeyConfig = PrivateKeyJwtConfiguration.merge(configuration, null, true); + assertTrue(configuration.equals(existingKeyConfig)); + assertEquals(configuration, existingKeyConfig); + + PrivateKeyJwtConfiguration newKeyConfig = PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"); + configuration = PrivateKeyJwtConfiguration.merge(null, newKeyConfig, true); + assertTrue(configuration.equals(newKeyConfig)); + assertTrue(configuration.equals(newKeyConfig)); + } + + @Test + void testConfigDelete() { + PrivateKeyJwtConfiguration configuration = PrivateKeyJwtConfiguration.parse(jsonJwkSet); + assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + assertNull(configuration.getPrivateKeyJwtUrl()); + PrivateKeyJwtConfiguration addKey = PrivateKeyJwtConfiguration.parse(jsonWebKey2); + configuration = PrivateKeyJwtConfiguration.merge(configuration, addKey, false); + assertEquals(2, configuration.getPrivateKeyJwt().getKeys().size()); + configuration = PrivateKeyJwtConfiguration.delete(configuration, addKey); + assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + configuration = PrivateKeyJwtConfiguration.delete(configuration, addKey); + configuration = PrivateKeyJwtConfiguration.delete(configuration, addKey); + assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + configuration = PrivateKeyJwtConfiguration.merge(configuration, addKey, false); + configuration = PrivateKeyJwtConfiguration.delete(configuration, addKey); + assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + configuration = PrivateKeyJwtConfiguration.merge(configuration, addKey, false); + configuration = PrivateKeyJwtConfiguration.delete(configuration, new PrivateKeyJwtConfiguration("key-2", null)); + configuration = PrivateKeyJwtConfiguration.delete(configuration, new PrivateKeyJwtConfiguration("key-1", null)); + assertNull(configuration); + configuration = PrivateKeyJwtConfiguration.delete(PrivateKeyJwtConfiguration.parse(jsonJwkSet), PrivateKeyJwtConfiguration.parse(jsonWebKey)); + assertNull(configuration); + + configuration = PrivateKeyJwtConfiguration.delete(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), PrivateKeyJwtConfiguration.parse("https://any/jwks-uri")); + assertNull(configuration); + configuration = PrivateKeyJwtConfiguration.delete(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), PrivateKeyJwtConfiguration.parse("https://other/jwks-uri")); + assertNotNull(configuration); + } + @Test + void testConfigDeleteNull() { + assertNull(PrivateKeyJwtConfiguration.delete(null, PrivateKeyJwtConfiguration.parse("https://other/jwks-uri"))); + assertNotNull(PrivateKeyJwtConfiguration.delete(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), null)); + } + + @Test + void xxx() { + PrivateKeyJwtConfiguration xxx = PrivateKeyJwtConfiguration.parse("test-1"); + xxx = new PrivateKeyJwtConfiguration("test-1", null); + } +} \ No newline at end of file diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java index c46478fafe5..700f9959027 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java @@ -48,6 +48,16 @@ public void deleteClientSecret(String clientId, String zoneId) throws NoSuchClie throw new UnsupportedOperationException(); } + @Override + public void addClientKeyConfig(String clientId, String keyConfig, String zoneId, boolean overwrite) throws NoSuchClientException { + throw new UnsupportedOperationException(); + } + + @Override + public void deleteClientKeyConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException { + throw new UnsupportedOperationException(); + } + @Override public void addClientDetails(ClientDetails clientDetails, String zoneId) throws ClientAlreadyExistsException { getInMemoryService(zoneId).put(clientDetails.getClientId(), (BaseClientDetails) clientDetails); From e72194a8b23c15fc4cd76268a784defdf11aa05b Mon Sep 17 00:00:00 2001 From: strehle Date: Thu, 24 Aug 2023 00:33:07 +0200 Subject: [PATCH 03/22] more tests --- .../MultitenantJdbcClientDetailsService.java | 11 +-- .../uaa/client/ClientAdminBootstrapTests.java | 30 ++++++- .../uaa/client/ClientAdminEndpointsTests.java | 79 +++++++++++++++++++ 3 files changed, 112 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java index ac1973964a7..816313abeb6 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java @@ -284,14 +284,11 @@ public void addClientKeyConfig(String clientId, String keyConfig, String zoneId, if (privateKeyJwtConfiguration != null) { BaseClientDetails clientDetails = (BaseClientDetails) loadClientByClientId(clientId, zoneId); PrivateKeyJwtConfiguration existingConfig = PrivateKeyJwtConfiguration.createFromClientDetails(clientDetails); - if (existingConfig != null) { - PrivateKeyJwtConfiguration result = - PrivateKeyJwtConfiguration.merge(existingConfig, privateKeyJwtConfiguration, overwrite); - if (result != null) { - result.persistToClientDetail(clientDetails); - } - updateClientDetails(clientDetails, zoneId); + PrivateKeyJwtConfiguration result = PrivateKeyJwtConfiguration.merge(existingConfig, privateKeyJwtConfiguration, overwrite); + if (result != null) { + result.persistToClientDetail(clientDetails); } + updateClientDetails(clientDetails, zoneId); } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java index 8336a6bace1..7e77098a1c3 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java @@ -252,6 +252,34 @@ void simpleAddClientWithSignupSuccessRedirectUrl() throws Exception { assertTrue(clientDetails.getRegisteredRedirectUri().contains("callback_url")); } + @Test + void simpleAddClientWithJwksUri() throws Exception { + Map map = new HashMap<>(); + map.put("id", "foo"); + map.put("secret", "bar"); + map.put("scope", "openid"); + map.put("authorized-grant-types", GRANT_TYPE_AUTHORIZATION_CODE); + map.put("authorities", "uaa.none"); + map.put("redirect-uri", "http://localhost/callback"); + map.put("jwks_uri", "https://localhost:8080/uaa"); + ClientDetails clientDetails = doSimpleTest(map, clientAdminBootstrap, multitenantJdbcClientDetailsService, clients); + assertNotNull(clientDetails.getAdditionalInformation().get(ClientConstants.PRIVATE_KEY_CONFIG)); + } + + @Test + void simpleAddClientWithJwkSet() throws Exception { + Map map = new HashMap<>(); + map.put("id", "foo"); + map.put("secret", "bar"); + map.put("scope", "openid"); + map.put("authorized-grant-types", GRANT_TYPE_AUTHORIZATION_CODE); + map.put("authorities", "uaa.none"); + map.put("redirect-uri", "http://localhost/callback"); + map.put("jwks", "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}"); + ClientDetails clientDetails = doSimpleTest(map, clientAdminBootstrap, multitenantJdbcClientDetailsService, clients); + assertNotNull(clientDetails.getAdditionalInformation().get(ClientConstants.PRIVATE_KEY_CONFIG)); + } + @Test void clientMetadata_getsBootstrapped() { Map map = new HashMap<>(); @@ -592,7 +620,7 @@ static ClientDetails doSimpleTest( for (String key : Arrays.asList("resource-ids", "scope", "authorized-grant-types", "authorities", "redirect-uri", "secret", "id", "override", "access-token-validity", - "refresh-token-validity")) { + "refresh-token-validity", "jwks", "jwks_uri")) { info.remove(key); } for (Map.Entry entry : info.entrySet()) { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java index 3998d84cc57..be87e42d1fc 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java @@ -9,7 +9,9 @@ import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsCreation; import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsModification; +import org.cloudfoundry.identity.uaa.oauth.client.PrivateKeyChangeRequest; import org.cloudfoundry.identity.uaa.oauth.client.SecretChangeRequest; +import org.cloudfoundry.identity.uaa.resources.ActionResult; import org.cloudfoundry.identity.uaa.resources.QueryableResourceManager; import org.cloudfoundry.identity.uaa.resources.ResourceMonitor; import org.cloudfoundry.identity.uaa.resources.SearchResults; @@ -73,6 +75,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.Mockito.withSettings; @@ -1076,6 +1079,82 @@ void testCreateClientWithPrivateKeyUri() { assertTrue(PrivateKeyJwtConfiguration.createFromClientDetails(created).configEquals(PrivateKeyJwtConfiguration.parse(jwksUri))); } + @Test + void testCreateClientWithPrivateKeyUriInvalid() { + // invalid jwks_uri + String jwksUri = "http://myhost/openid/jwks-uri"; + when(clientDetailsService.retrieve(anyString(), anyString())).thenReturn(input); + when(mockSecurityContextAccessor.getClientId()).thenReturn(detail.getClientId()); + when(mockSecurityContextAccessor.isClient()).thenReturn(true); + + input.setClientSecret("secret"); + detail.setAuthorizedGrantTypes(input.getAuthorizedGrantTypes()); + ClientDetailsCreation createRequest = createClientDetailsCreation(input); + createRequest.setPrivateKeySet(jwksUri); + ClientDetails result = endpoints.createClientDetails(createRequest); + assertNull(result.getClientSecret()); + ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); + verify(clientDetailsService).create(clientCaptor.capture(), anyString()); + BaseClientDetails created = clientCaptor.getValue(); + assertNull(PrivateKeyJwtConfiguration.createFromClientDetails(created)); + } + + @Test + void testAddPrivateKeyJwtConfigUri() { + when(mockSecurityContextAccessor.getClientId()).thenReturn("bar"); + when(mockSecurityContextAccessor.isClient()).thenReturn(true); + when(mockSecurityContextAccessor.isAdmin()).thenReturn(true); + + when(clientDetailsService.retrieve(detail.getClientId(), IdentityZoneHolder.get().getId())).thenReturn(detail); + + PrivateKeyChangeRequest change = new PrivateKeyChangeRequest(); + // https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata, see jwks_uri + String jwksUri = "https://any.domain.net/openid/jwks-uri"; + change.setKeyUrl(jwksUri); + change.setChangeMode(PrivateKeyChangeRequest.ChangeMode.ADD); + + ActionResult result = endpoints.changePrivateKey(detail.getClientId(), change); + assertEquals("Private key is added", result.getMessage()); + verify(clientRegistrationService, times(1)).addClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), false); + + change.setKeyUrl(null); + result = endpoints.changePrivateKey(detail.getClientId(), change); + assertEquals("No key added", result.getMessage()); + } + + @Test + void testChangeDeletePrivateKeyJwtConfigUri() { + when(mockSecurityContextAccessor.getClientId()).thenReturn("bar"); + when(mockSecurityContextAccessor.isClient()).thenReturn(true); + when(mockSecurityContextAccessor.isAdmin()).thenReturn(true); + + when(clientDetailsService.retrieve(detail.getClientId(), IdentityZoneHolder.get().getId())).thenReturn(detail); + + PrivateKeyChangeRequest change = new PrivateKeyChangeRequest(); + // https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata, see jwks_uri + String jwksUri = "https://any.domain.net/openid/jwks-uri"; + change.setKeyUrl(jwksUri); + change.setChangeMode(PrivateKeyChangeRequest.ChangeMode.ADD); + + ActionResult result = endpoints.changePrivateKey(detail.getClientId(), change); + assertEquals("Private key is added", result.getMessage()); + verify(clientRegistrationService, times(1)).addClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), false); + + jwksUri = "https://any.new.domain.net/openid/jwks-uri"; + change.setChangeMode(PrivateKeyChangeRequest.ChangeMode.UPDATE); + change.setKeyUrl(jwksUri); + result = endpoints.changePrivateKey(detail.getClientId(), change); + assertEquals("Private key updated", result.getMessage()); + verify(clientRegistrationService, times(1)).addClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), true); + + PrivateKeyJwtConfiguration.parse(jwksUri).persistToClientDetail(detail); + change.setChangeMode(PrivateKeyChangeRequest.ChangeMode.DELETE); + change.setKeyUrl(jwksUri); + result = endpoints.changePrivateKey(detail.getClientId(), change); + assertEquals("Private key is deleted", result.getMessage()); + verify(clientRegistrationService, times(1)).deleteClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId()); + } + @Test void testCreateClientWithPrivateKeySet() { // Example JWK, a key is bound to a kid, means assumption is, a key is the same if kid is the same From 154589ca2750d0867887cd2d4c6295bbe53623d1 Mon Sep 17 00:00:00 2001 From: strehle Date: Thu, 24 Aug 2023 09:10:02 +0200 Subject: [PATCH 04/22] refactorings --- .../uaa/client/ClientAdminEndpoints.java | 2 +- .../client/ClientAdminEndpointsValidator.java | 2 +- .../client/PrivateKeyJwtConfiguration.java | 37 +++++------ .../MultitenantJdbcClientDetailsService.java | 10 +-- .../uaa/client/ClientAdminEndpointsTests.java | 15 +++-- .../PrivateKeyJwtConfigurationTest.java | 64 +++++++++++++++---- 6 files changed, 87 insertions(+), 43 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java index ae1d22e5156..6fd34919e66 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java @@ -553,7 +553,7 @@ public ActionResult changePrivateKey(@PathVariable String client_id, @RequestBod throw new InvalidClientDetailsException(e.getMessage()); } - PrivateKeyJwtConfiguration clientKeyConfig = PrivateKeyJwtConfiguration.createFromClientDetails(clientDetails); + PrivateKeyJwtConfiguration clientKeyConfig = PrivateKeyJwtConfiguration.readValue(clientDetails); ActionResult result; switch (change.getChangeMode()){ diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java index 5787dc9fe46..ce5a1f7335e 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java @@ -253,7 +253,7 @@ public ClientDetails validate(ClientDetails prototype, boolean create, boolean c PrivateKeyJwtConfiguration privateKeyJwtConfiguration = PrivateKeyJwtConfiguration.parse(clientDetailsCreation.getPrivateKeyUrl(), clientDetailsCreation.getPrivateKeySet()); if (privateKeyJwtConfiguration != null) { - privateKeyJwtConfiguration.persistToClientDetail(client); + privateKeyJwtConfiguration.writeValue(client); } else { logger.warn("Client configuration with private_key_jwt not valid"); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java index cd06e974905..9b84fc8e8af 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java @@ -31,7 +31,7 @@ @JsonInclude(JsonInclude.Include.NON_NULL) @JsonIgnoreProperties(ignoreUnknown = true) -public class PrivateKeyJwtConfiguration { +public class PrivateKeyJwtConfiguration implements Cloneable{ @JsonIgnore private static final int MAX_KEY_SIZE = 10; @@ -73,20 +73,7 @@ public void setPrivateKeyJwt(final JsonWebKeySet privateKeyJwt) { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - if (!super.equals(o)) return false; - return configEquals(o); - } - - @Override - public int hashCode() { - int result = super.hashCode(); - - result = 31 * result + (privateKeyJwtUrl != null ? privateKeyJwtUrl.hashCode() : 0); - result = 31 * result + (privateKeyJwt != null ? privateKeyJwt.hashCode() : 0); - return result; - } - public boolean configEquals(Object o) { if (o instanceof PrivateKeyJwtConfiguration) { PrivateKeyJwtConfiguration that = (PrivateKeyJwtConfiguration) o; if (!Objects.equals(privateKeyJwtUrl, that.privateKeyJwtUrl)) return false; @@ -99,6 +86,20 @@ public boolean configEquals(Object o) { return false; } + @Override + public int hashCode() { + int result = super.hashCode(); + + result = 31 * result + (privateKeyJwtUrl != null ? privateKeyJwtUrl.hashCode() : 0); + result = 31 * result + (privateKeyJwt != null ? privateKeyJwt.hashCode() : 0); + return result; + } + + @Override + public Object clone() throws CloneNotSupportedException { + return super.clone(); + } + @JsonIgnore public String getCleanString() { try { @@ -148,10 +149,10 @@ public static PrivateKeyJwtConfiguration parse(String privateKeyUrl, String priv private boolean validateJwkSet() { List keyList = privateKeyJwt.getKeys(); - Set keyId = new HashSet<>(); if (keyList.isEmpty() || keyList.size() > MAX_KEY_SIZE) { throw new InvalidClientDetailsException("Invalid private_key_jwt: jwk set is empty of exceeds to maximum of keys. max: + " + MAX_KEY_SIZE); } + Set keyId = new HashSet<>(); keyList.forEach(key -> { if (!StringUtils.hasText(key.getKid())) { throw new InvalidClientDetailsException("Invalid private_key_jwt: kid is required attribute"); @@ -191,7 +192,7 @@ private boolean validateJwksUri() { * @return */ @JsonIgnore - public static PrivateKeyJwtConfiguration createFromClientDetails(ClientDetails clientDetails) { + public static PrivateKeyJwtConfiguration readValue(ClientDetails clientDetails) { if (clientDetails == null || clientDetails.getAdditionalInformation() == null || !(clientDetails.getAdditionalInformation().get(PRIVATE_KEY_CONFIG) instanceof String)) { @@ -208,7 +209,7 @@ public static PrivateKeyJwtConfiguration createFromClientDetails(ClientDetails c * @return */ @JsonIgnore - public void persistToClientDetail(ClientDetails clientDetails) { + public void writeValue(ClientDetails clientDetails) { if (clientDetails instanceof BaseClientDetails) { BaseClientDetails baseClientDetails = (BaseClientDetails) clientDetails; HashMap additionalInformation = Optional.ofNullable(baseClientDetails.getAdditionalInformation()).map(HashMap::new).orElse(new HashMap<>()); @@ -225,7 +226,7 @@ public void persistToClientDetail(ClientDetails clientDetails) { * @return */ @JsonIgnore - public static void cleanClientDetail(ClientDetails clientDetails) { + public static void resetConfiguration(ClientDetails clientDetails) { if (clientDetails instanceof BaseClientDetails) { BaseClientDetails baseClientDetails = (BaseClientDetails) clientDetails; HashMap additionalInformation = Optional.ofNullable(baseClientDetails.getAdditionalInformation()).map(HashMap::new).orElse(new HashMap<>()); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java index 816313abeb6..a17fdbc1471 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java @@ -283,10 +283,10 @@ public void addClientKeyConfig(String clientId, String keyConfig, String zoneId, PrivateKeyJwtConfiguration privateKeyJwtConfiguration = PrivateKeyJwtConfiguration.parse(keyConfig); if (privateKeyJwtConfiguration != null) { BaseClientDetails clientDetails = (BaseClientDetails) loadClientByClientId(clientId, zoneId); - PrivateKeyJwtConfiguration existingConfig = PrivateKeyJwtConfiguration.createFromClientDetails(clientDetails); + PrivateKeyJwtConfiguration existingConfig = PrivateKeyJwtConfiguration.readValue(clientDetails); PrivateKeyJwtConfiguration result = PrivateKeyJwtConfiguration.merge(existingConfig, privateKeyJwtConfiguration, overwrite); if (result != null) { - result.persistToClientDetail(clientDetails); + result.writeValue(clientDetails); } updateClientDetails(clientDetails, zoneId); } @@ -302,11 +302,11 @@ public void deleteClientKeyConfig(String clientId, String keyConfig, String zone } if (privateKeyJwtConfiguration != null) { BaseClientDetails clientDetails = (BaseClientDetails) loadClientByClientId(clientId, zoneId); - PrivateKeyJwtConfiguration result = PrivateKeyJwtConfiguration.delete(PrivateKeyJwtConfiguration.createFromClientDetails(clientDetails), privateKeyJwtConfiguration); + PrivateKeyJwtConfiguration result = PrivateKeyJwtConfiguration.delete(PrivateKeyJwtConfiguration.readValue(clientDetails), privateKeyJwtConfiguration); if (result != null) { - result.persistToClientDetail(clientDetails); + result.writeValue(clientDetails); } else { - PrivateKeyJwtConfiguration.cleanClientDetail(clientDetails); + PrivateKeyJwtConfiguration.resetConfiguration(clientDetails); } updateClientDetails(clientDetails, zoneId); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java index be87e42d1fc..a896095b42e 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java @@ -61,6 +61,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -1076,7 +1077,7 @@ void testCreateClientWithPrivateKeyUri() { ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); verify(clientDetailsService).create(clientCaptor.capture(), anyString()); BaseClientDetails created = clientCaptor.getValue(); - assertTrue(PrivateKeyJwtConfiguration.createFromClientDetails(created).configEquals(PrivateKeyJwtConfiguration.parse(jwksUri))); + assertEquals(PrivateKeyJwtConfiguration.readValue(created), PrivateKeyJwtConfiguration.parse(jwksUri)); } @Test @@ -1096,7 +1097,7 @@ void testCreateClientWithPrivateKeyUriInvalid() { ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); verify(clientDetailsService).create(clientCaptor.capture(), anyString()); BaseClientDetails created = clientCaptor.getValue(); - assertNull(PrivateKeyJwtConfiguration.createFromClientDetails(created)); + assertNull(PrivateKeyJwtConfiguration.readValue(created)); } @Test @@ -1147,7 +1148,7 @@ void testChangeDeletePrivateKeyJwtConfigUri() { assertEquals("Private key updated", result.getMessage()); verify(clientRegistrationService, times(1)).addClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), true); - PrivateKeyJwtConfiguration.parse(jwksUri).persistToClientDetail(detail); + PrivateKeyJwtConfiguration.parse(jwksUri).writeValue(detail); change.setChangeMode(PrivateKeyChangeRequest.ChangeMode.DELETE); change.setKeyUrl(jwksUri); result = endpoints.changePrivateKey(detail.getClientId(), change); @@ -1175,10 +1176,10 @@ void testCreateClientWithPrivateKeySet() { ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); verify(clientDetailsService).create(clientCaptor.capture(), anyString()); BaseClientDetails created = clientCaptor.getValue(); - assertTrue(PrivateKeyJwtConfiguration.createFromClientDetails(created).configEquals(PrivateKeyJwtConfiguration.parse(jsonJwk))); - assertTrue(PrivateKeyJwtConfiguration.createFromClientDetails(created).configEquals(PrivateKeyJwtConfiguration.parse(jsonJwk2))); - assertTrue(PrivateKeyJwtConfiguration.createFromClientDetails(created).configEquals(PrivateKeyJwtConfiguration.parse(jsonJwkSet))); - assertFalse(PrivateKeyJwtConfiguration.createFromClientDetails(created).configEquals(PrivateKeyJwtConfiguration.parse(jsonJwk3))); + assertEquals(PrivateKeyJwtConfiguration.readValue(created), PrivateKeyJwtConfiguration.parse(jsonJwk)); + assertEquals(PrivateKeyJwtConfiguration.readValue(created), PrivateKeyJwtConfiguration.parse(jsonJwk2)); + assertEquals(PrivateKeyJwtConfiguration.readValue(created), PrivateKeyJwtConfiguration.parse(jsonJwkSet)); + assertNotEquals(PrivateKeyJwtConfiguration.readValue(created), PrivateKeyJwtConfiguration.parse(jsonJwk3)); } private ClientDetailsCreation createClientDetailsCreation(BaseClientDetails baseClientDetails) { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfigurationTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfigurationTest.java index 827ad0ae8a0..d50c9497d7c 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfigurationTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfigurationTest.java @@ -1,18 +1,22 @@ package org.cloudfoundry.identity.uaa.client; +import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKeySet; -import org.junit.jupiter.api.BeforeEach; +import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.junit.jupiter.api.Test; +import org.springframework.security.oauth2.provider.client.BaseClientDetails; import java.text.ParseException; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertNotEquals; 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.assertTrue; import static org.mockito.Mockito.mock; @@ -27,11 +31,8 @@ class PrivateKeyJwtConfigurationTest { private final String jsonWebKeyNoId = "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}"; private final String jsonJwkSet = "{\"keys\":[{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}]}"; private final String jsonJwkSetEmtpy = "{\"keys\":[]}"; - private PrivateKeyJwtConfiguration privateKeyJwtConfiguration; - - @BeforeEach - void setUp() { - } + private final String defaultJsonUri = "{\"jwks_uri\":\"http://localhost:8080/uaa\"} "; + private final String defaultJsonKey = "{\"jwks\":{\"keys\":[{\"kty\":\"RSA\",\"e\":\"AQAB\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\",\"kid\":\"key-1\"}]}}"; @Test void testJwksValidity() { @@ -197,8 +198,49 @@ void testConfigDeleteNull() { } @Test - void xxx() { - PrivateKeyJwtConfiguration xxx = PrivateKeyJwtConfiguration.parse("test-1"); - xxx = new PrivateKeyJwtConfiguration("test-1", null); + void testHashCode() { + PrivateKeyJwtConfiguration key1 = PrivateKeyJwtConfiguration.parse("http://localhost:8080/uaa"); + PrivateKeyJwtConfiguration key2 = PrivateKeyJwtConfiguration.parse("http://localhost:8080/uaa"); + assertNotEquals(key1.hashCode(), key2.hashCode()); + assertEquals(key1.hashCode(), key1.hashCode()); + assertEquals(key2.hashCode(), key2.hashCode()); + } + + @Test + void testEquals() throws CloneNotSupportedException { + PrivateKeyJwtConfiguration key1 = PrivateKeyJwtConfiguration.parse("http://localhost:8080/uaa"); + PrivateKeyJwtConfiguration key2 = (PrivateKeyJwtConfiguration) key1.clone(); + assertEquals(key1, key2); + } + + @Test + void testSerializableObjectCalls() throws CloneNotSupportedException { + PrivateKeyJwtConfiguration key1 = JsonUtils.readValue(defaultJsonUri, PrivateKeyJwtConfiguration.class); + PrivateKeyJwtConfiguration key2 = (PrivateKeyJwtConfiguration) key1.clone(); + assertEquals(key1, key2); + + key1 = JsonUtils.readValue(defaultJsonKey, PrivateKeyJwtConfiguration.class); + key2 = (PrivateKeyJwtConfiguration) key1.clone(); + assertEquals(key1, key2); + } + + @Test + void testConfiguration() { + PrivateKeyJwtConfiguration configUri = JsonUtils.readValue(defaultJsonUri, PrivateKeyJwtConfiguration.class); + PrivateKeyJwtConfiguration configKey = JsonUtils.readValue(defaultJsonKey, PrivateKeyJwtConfiguration.class); + BaseClientDetails baseClientDetails = new BaseClientDetails(); + HashMap additionalInformation = new HashMap<>(); + additionalInformation.put(ClientConstants.PRIVATE_KEY_CONFIG, configUri); + baseClientDetails.setAdditionalInformation(additionalInformation); + + configUri.writeValue(baseClientDetails); + PrivateKeyJwtConfiguration readUriConfig = PrivateKeyJwtConfiguration.readValue(baseClientDetails); + assertEquals(configUri, readUriConfig); + + PrivateKeyJwtConfiguration.resetConfiguration(baseClientDetails); + assertNull(PrivateKeyJwtConfiguration.readValue(baseClientDetails)); + configKey.writeValue(baseClientDetails); + PrivateKeyJwtConfiguration readKeyConfig = PrivateKeyJwtConfiguration.readValue(baseClientDetails); + assertEquals(configKey, readKeyConfig); } -} \ No newline at end of file +} From 5376079012c9d56743f618a18ddcacd93330a79b Mon Sep 17 00:00:00 2001 From: strehle Date: Thu, 24 Aug 2023 09:57:30 +0200 Subject: [PATCH 05/22] add tests --- .../oauth/client/PrivateKeyChangeRequest.java | 12 ---------- .../client/PrivateKeyChangeRequestTest.java | 23 +++++++++++++++++++ .../uaa/client/ClientAdminBootstrap.java | 8 ++----- 3 files changed, 25 insertions(+), 18 deletions(-) create mode 100644 model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequestTest.java diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequest.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequest.java index 86103b8fdaf..03b3f476eae 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequest.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequest.java @@ -1,15 +1,3 @@ -/******************************************************************************* - * Cloud Foundry - * Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved. - * - * This product is licensed to you under the Apache License, Version 2.0 (the "License"). - * You may not use this product except in compliance with the License. - * - * This product includes a number of subcomponents with - * separate copyright notices and license terms. Your use of these - * subcomponents is subject to the terms and conditions of the - * subcomponent's license, as noted in the LICENSE file. - *******************************************************************************/ package org.cloudfoundry.identity.uaa.oauth.client; diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequestTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequestTest.java new file mode 100644 index 00000000000..8fbd24e5dae --- /dev/null +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequestTest.java @@ -0,0 +1,23 @@ +package org.cloudfoundry.identity.uaa.oauth.client; + +import org.cloudfoundry.identity.uaa.util.JsonUtils; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +class PrivateKeyChangeRequestTest { + + @Test + void testRequestSerialization() { + PrivateKeyChangeRequest def = new PrivateKeyChangeRequest(null, null, null); + def.setKeyId("key-1"); + def.setChangeMode(PrivateKeyChangeRequest.ChangeMode.DELETE); + def.setKeyUrl("http://localhost:8080/uaa/token_key"); + def.setKeyConfig("{}"); + def.setClientId("admin"); + String jsonRequest = JsonUtils.writeValueAsString(def); + PrivateKeyChangeRequest request = JsonUtils.readValue(jsonRequest, PrivateKeyChangeRequest.class); + assertNotEquals(def, request); + } + +} diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java index 029f83f7dc7..4c5dac5e792 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java @@ -6,7 +6,6 @@ import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_REFRESH_TOKEN; import java.net.MalformedURLException; -import java.net.URI; import java.net.URL; import java.util.Arrays; import java.util.Collection; @@ -239,12 +238,9 @@ private void addNewClients() { if (map.get("jwks_uri") instanceof String) { String jwksUri = (String) map.get("jwks_uri"); - URI jwksUriObject = URI.create(jwksUri); - if (jwksUriObject.isAbsolute() && ("https".startsWith(jwksUriObject.getScheme()) || ("http".startsWith(jwksUriObject.getScheme()) && jwksUriObject.getHost().contains("localhost")))) { - PrivateKeyJwtConfiguration keyConfig = PrivateKeyJwtConfiguration.parse(UaaUrlUtils.normalizeUri(jwksUri), null); - if (keyConfig != null && keyConfig.getCleanString() != null) { + PrivateKeyJwtConfiguration keyConfig = PrivateKeyJwtConfiguration.parse(UaaUrlUtils.normalizeUri(jwksUri), null); + if (keyConfig != null && keyConfig.getCleanString() != null) { clientRegistrationService.addClientKeyConfig(clientId, keyConfig.getPrivateKeyJwtUrl(), IdentityZone.getUaaZoneId(), override); - } } } else if (map.get("jwks") instanceof String) { String jwks = (String) map.get("jwks"); From a9cba87a83bc7281619d3aa105240e7dfbd22058 Mon Sep 17 00:00:00 2001 From: strehle Date: Thu, 24 Aug 2023 10:25:19 +0200 Subject: [PATCH 06/22] Renamed --- ...quest.java => ClientJwtChangeRequest.java} | 8 ++--- ...t.java => ClientJwtChangeRequestTest.java} | 8 ++--- .../uaa/client/ClientAdminEndpoints.java | 12 +++---- .../client/PrivateKeyJwtConfiguration.java | 4 +-- .../uaa/client/ClientAdminEndpointsTests.java | 32 +++++++++---------- 5 files changed, 32 insertions(+), 32 deletions(-) rename model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/{PrivateKeyChangeRequest.java => ClientJwtChangeRequest.java} (86%) rename model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/{PrivateKeyChangeRequestTest.java => ClientJwtChangeRequestTest.java} (63%) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequest.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java similarity index 86% rename from model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequest.java rename to model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java index 03b3f476eae..b0058153665 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequest.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java @@ -5,13 +5,13 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; -import static org.cloudfoundry.identity.uaa.oauth.client.PrivateKeyChangeRequest.ChangeMode.ADD; +import static org.cloudfoundry.identity.uaa.oauth.client.ClientJwtChangeRequest.ChangeMode.ADD; /** */ @JsonInclude(JsonInclude.Include.NON_NULL) @JsonIgnoreProperties(ignoreUnknown = true) -public class PrivateKeyChangeRequest { +public class ClientJwtChangeRequest { public enum ChangeMode { UPDATE, @@ -28,10 +28,10 @@ public enum ChangeMode { private String clientId; private ChangeMode changeMode = ADD; - public PrivateKeyChangeRequest() { + public ClientJwtChangeRequest() { } - public PrivateKeyChangeRequest(String clientId, String keyUrl, String keyConfig) { + public ClientJwtChangeRequest(String clientId, String keyUrl, String keyConfig) { this.keyUrl = keyUrl; this.keyConfig = keyConfig; this.clientId = clientId; diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequestTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequestTest.java similarity index 63% rename from model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequestTest.java rename to model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequestTest.java index 8fbd24e5dae..36da456fb42 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/PrivateKeyChangeRequestTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequestTest.java @@ -5,18 +5,18 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; -class PrivateKeyChangeRequestTest { +class ClientJwtChangeRequestTest { @Test void testRequestSerialization() { - PrivateKeyChangeRequest def = new PrivateKeyChangeRequest(null, null, null); + ClientJwtChangeRequest def = new ClientJwtChangeRequest(null, null, null); def.setKeyId("key-1"); - def.setChangeMode(PrivateKeyChangeRequest.ChangeMode.DELETE); + def.setChangeMode(ClientJwtChangeRequest.ChangeMode.DELETE); def.setKeyUrl("http://localhost:8080/uaa/token_key"); def.setKeyConfig("{}"); def.setClientId("admin"); String jsonRequest = JsonUtils.writeValueAsString(def); - PrivateKeyChangeRequest request = JsonUtils.readValue(jsonRequest, PrivateKeyChangeRequest.class); + ClientJwtChangeRequest request = JsonUtils.readValue(jsonRequest, ClientJwtChangeRequest.class); assertNotEquals(def, request); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java index 6fd34919e66..74bb3f67def 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java @@ -20,7 +20,7 @@ import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsCreation; import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsModification; -import org.cloudfoundry.identity.uaa.oauth.client.PrivateKeyChangeRequest; +import org.cloudfoundry.identity.uaa.oauth.client.ClientJwtChangeRequest; import org.cloudfoundry.identity.uaa.oauth.client.SecretChangeRequest; import org.cloudfoundry.identity.uaa.resources.ActionResult; import org.cloudfoundry.identity.uaa.resources.AttributeNameMapper; @@ -536,9 +536,9 @@ public ActionResult changeSecret(@PathVariable String client_id, @RequestBody Se return result; } - @RequestMapping(value = "/oauth/clients/{client_id}/privatekey", method = RequestMethod.PUT) + @RequestMapping(value = "/oauth/clients/{client_id}/clientjwt", method = RequestMethod.PUT) @ResponseBody - public ActionResult changePrivateKey(@PathVariable String client_id, @RequestBody PrivateKeyChangeRequest change) { + public ActionResult changeClientJwt(@PathVariable String client_id, @RequestBody ClientJwtChangeRequest change) { ClientDetails clientDetails; try { @@ -560,7 +560,7 @@ public ActionResult changePrivateKey(@PathVariable String client_id, @RequestBod case ADD : if (change.getKey() != null) { clientRegistrationService.addClientKeyConfig(client_id, change.getKey(), IdentityZoneHolder.get().getId(), false); - result = new ActionResult("ok", "Private key is added"); + result = new ActionResult("ok", "Client jwt configuration is added"); } else { result = new ActionResult("ok", "No key added"); } @@ -571,12 +571,12 @@ public ActionResult changePrivateKey(@PathVariable String client_id, @RequestBod if (clientKeyConfig != null && deleteString != null) { clientRegistrationService.deleteClientKeyConfig(client_id, deleteString, IdentityZoneHolder.get().getId()); } - result = new ActionResult("ok", "Private key is deleted"); + result = new ActionResult("ok", "Client jwt configuration is deleted"); break; default: clientRegistrationService.addClientKeyConfig(client_id, change.getKey(), IdentityZoneHolder.get().getId(), true); - result = new ActionResult("ok", "Private key updated"); + result = new ActionResult("ok", "Client jwt configuration updated"); } clientSecretChanges.incrementAndGet(); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java index 9b84fc8e8af..6034d49d394 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java @@ -109,7 +109,7 @@ public String getCleanString() { return JWKSet.parse(this.privateKeyJwt.getKeySetMap()).toString(true); } } catch (IllegalStateException | JsonUtils.JsonUtilException | ParseException e) { - throw new InvalidClientDetailsException("Private key configuration fails ", e); + throw new InvalidClientDetailsException("Client jwt configuration configuration fails ", e); } return null; } @@ -141,7 +141,7 @@ public static PrivateKeyJwtConfiguration parse(String privateKeyUrl, String priv privateKeyJwtConfiguration = new PrivateKeyJwtConfiguration(null, JsonWebKeyHelper.deserialize(cleanJwtString)); privateKeyJwtConfiguration.validateJwkSet(); } catch (ParseException e) { - throw new InvalidClientDetailsException("Private key cannot be parsed", e); + throw new InvalidClientDetailsException("Client jwt configuration cannot be parsed", e); } } return privateKeyJwtConfiguration; diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java index a896095b42e..ac434921955 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java @@ -9,7 +9,7 @@ import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsCreation; import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsModification; -import org.cloudfoundry.identity.uaa.oauth.client.PrivateKeyChangeRequest; +import org.cloudfoundry.identity.uaa.oauth.client.ClientJwtChangeRequest; import org.cloudfoundry.identity.uaa.oauth.client.SecretChangeRequest; import org.cloudfoundry.identity.uaa.resources.ActionResult; import org.cloudfoundry.identity.uaa.resources.QueryableResourceManager; @@ -1108,18 +1108,18 @@ void testAddPrivateKeyJwtConfigUri() { when(clientDetailsService.retrieve(detail.getClientId(), IdentityZoneHolder.get().getId())).thenReturn(detail); - PrivateKeyChangeRequest change = new PrivateKeyChangeRequest(); + ClientJwtChangeRequest change = new ClientJwtChangeRequest(); // https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata, see jwks_uri String jwksUri = "https://any.domain.net/openid/jwks-uri"; change.setKeyUrl(jwksUri); - change.setChangeMode(PrivateKeyChangeRequest.ChangeMode.ADD); + change.setChangeMode(ClientJwtChangeRequest.ChangeMode.ADD); - ActionResult result = endpoints.changePrivateKey(detail.getClientId(), change); - assertEquals("Private key is added", result.getMessage()); + ActionResult result = endpoints.changeClientJwt(detail.getClientId(), change); + assertEquals("Client jwt configuration is added", result.getMessage()); verify(clientRegistrationService, times(1)).addClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), false); change.setKeyUrl(null); - result = endpoints.changePrivateKey(detail.getClientId(), change); + result = endpoints.changeClientJwt(detail.getClientId(), change); assertEquals("No key added", result.getMessage()); } @@ -1131,28 +1131,28 @@ void testChangeDeletePrivateKeyJwtConfigUri() { when(clientDetailsService.retrieve(detail.getClientId(), IdentityZoneHolder.get().getId())).thenReturn(detail); - PrivateKeyChangeRequest change = new PrivateKeyChangeRequest(); + ClientJwtChangeRequest change = new ClientJwtChangeRequest(); // https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata, see jwks_uri String jwksUri = "https://any.domain.net/openid/jwks-uri"; change.setKeyUrl(jwksUri); - change.setChangeMode(PrivateKeyChangeRequest.ChangeMode.ADD); + change.setChangeMode(ClientJwtChangeRequest.ChangeMode.ADD); - ActionResult result = endpoints.changePrivateKey(detail.getClientId(), change); - assertEquals("Private key is added", result.getMessage()); + ActionResult result = endpoints.changeClientJwt(detail.getClientId(), change); + assertEquals("Client jwt configuration is added", result.getMessage()); verify(clientRegistrationService, times(1)).addClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), false); jwksUri = "https://any.new.domain.net/openid/jwks-uri"; - change.setChangeMode(PrivateKeyChangeRequest.ChangeMode.UPDATE); + change.setChangeMode(ClientJwtChangeRequest.ChangeMode.UPDATE); change.setKeyUrl(jwksUri); - result = endpoints.changePrivateKey(detail.getClientId(), change); - assertEquals("Private key updated", result.getMessage()); + result = endpoints.changeClientJwt(detail.getClientId(), change); + assertEquals("Client jwt configuration updated", result.getMessage()); verify(clientRegistrationService, times(1)).addClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), true); PrivateKeyJwtConfiguration.parse(jwksUri).writeValue(detail); - change.setChangeMode(PrivateKeyChangeRequest.ChangeMode.DELETE); + change.setChangeMode(ClientJwtChangeRequest.ChangeMode.DELETE); change.setKeyUrl(jwksUri); - result = endpoints.changePrivateKey(detail.getClientId(), change); - assertEquals("Private key is deleted", result.getMessage()); + result = endpoints.changeClientJwt(detail.getClientId(), change); + assertEquals("Client jwt configuration is deleted", result.getMessage()); verify(clientRegistrationService, times(1)).deleteClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId()); } From c69b748f802df9477cca4a1e37ffca2a4359dab7 Mon Sep 17 00:00:00 2001 From: strehle Date: Thu, 24 Aug 2023 10:27:13 +0200 Subject: [PATCH 07/22] Renamed --- .../uaa/client/ClientAdminBootstrap.java | 4 +- .../uaa/client/ClientAdminEndpoints.java | 2 +- .../client/ClientAdminEndpointsValidator.java | 6 +- ...ation.java => ClientJwtConfiguration.java} | 48 +++--- .../MultitenantJdbcClientDetailsService.java | 22 +-- .../uaa/client/ClientAdminEndpointsTests.java | 14 +- ...t.java => ClientJwtConfigurationTest.java} | 138 +++++++++--------- 7 files changed, 117 insertions(+), 117 deletions(-) rename server/src/main/java/org/cloudfoundry/identity/uaa/client/{PrivateKeyJwtConfiguration.java => ClientJwtConfiguration.java} (84%) rename server/src/test/java/org/cloudfoundry/identity/uaa/client/{PrivateKeyJwtConfigurationTest.java => ClientJwtConfigurationTest.java} (50%) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java index 4c5dac5e792..7e180f9443f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java @@ -238,13 +238,13 @@ private void addNewClients() { if (map.get("jwks_uri") instanceof String) { String jwksUri = (String) map.get("jwks_uri"); - PrivateKeyJwtConfiguration keyConfig = PrivateKeyJwtConfiguration.parse(UaaUrlUtils.normalizeUri(jwksUri), null); + ClientJwtConfiguration keyConfig = ClientJwtConfiguration.parse(UaaUrlUtils.normalizeUri(jwksUri), null); if (keyConfig != null && keyConfig.getCleanString() != null) { clientRegistrationService.addClientKeyConfig(clientId, keyConfig.getPrivateKeyJwtUrl(), IdentityZone.getUaaZoneId(), override); } } else if (map.get("jwks") instanceof String) { String jwks = (String) map.get("jwks"); - PrivateKeyJwtConfiguration keyConfig = PrivateKeyJwtConfiguration.parse(null, jwks); + ClientJwtConfiguration keyConfig = ClientJwtConfiguration.parse(null, jwks); if (keyConfig != null && keyConfig.getCleanString() != null) { clientRegistrationService.addClientKeyConfig(clientId, keyConfig.getCleanString(), IdentityZone.getUaaZoneId(), override); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java index 74bb3f67def..34d7d1a7e9f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java @@ -553,7 +553,7 @@ public ActionResult changeClientJwt(@PathVariable String client_id, @RequestBody throw new InvalidClientDetailsException(e.getMessage()); } - PrivateKeyJwtConfiguration clientKeyConfig = PrivateKeyJwtConfiguration.readValue(clientDetails); + ClientJwtConfiguration clientKeyConfig = ClientJwtConfiguration.readValue(clientDetails); ActionResult result; switch (change.getChangeMode()){ diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java index ce5a1f7335e..eec117c68a4 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java @@ -250,10 +250,10 @@ public ClientDetails validate(ClientDetails prototype, boolean create, boolean c if (prototype instanceof ClientDetailsCreation) { ClientDetailsCreation clientDetailsCreation = (ClientDetailsCreation) prototype; if (StringUtils.hasText(clientDetailsCreation.getPrivateKeyUrl()) || StringUtils.hasText(clientDetailsCreation.getPrivateKeySet())) { - PrivateKeyJwtConfiguration privateKeyJwtConfiguration = PrivateKeyJwtConfiguration.parse(clientDetailsCreation.getPrivateKeyUrl(), + ClientJwtConfiguration clientJwtConfiguration = ClientJwtConfiguration.parse(clientDetailsCreation.getPrivateKeyUrl(), clientDetailsCreation.getPrivateKeySet()); - if (privateKeyJwtConfiguration != null) { - privateKeyJwtConfiguration.writeValue(client); + if (clientJwtConfiguration != null) { + clientJwtConfiguration.writeValue(client); } else { logger.warn("Client configuration with private_key_jwt not valid"); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfiguration.java similarity index 84% rename from server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java rename to server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfiguration.java index 6034d49d394..e3747319147 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfiguration.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfiguration.java @@ -31,7 +31,7 @@ @JsonInclude(JsonInclude.Include.NON_NULL) @JsonIgnoreProperties(ignoreUnknown = true) -public class PrivateKeyJwtConfiguration implements Cloneable{ +public class ClientJwtConfiguration implements Cloneable{ @JsonIgnore private static final int MAX_KEY_SIZE = 10; @@ -42,10 +42,10 @@ public class PrivateKeyJwtConfiguration implements Cloneable{ @JsonProperty("jwks") private JsonWebKeySet privateKeyJwt; - public PrivateKeyJwtConfiguration() { + public ClientJwtConfiguration() { } - public PrivateKeyJwtConfiguration(final String privateKeyJwtUrl, final JsonWebKeySet webKeySet) { + public ClientJwtConfiguration(final String privateKeyJwtUrl, final JsonWebKeySet webKeySet) { this.privateKeyJwtUrl = privateKeyJwtUrl; privateKeyJwt = webKeySet; if (privateKeyJwt != null) { @@ -74,8 +74,8 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - if (o instanceof PrivateKeyJwtConfiguration) { - PrivateKeyJwtConfiguration that = (PrivateKeyJwtConfiguration) o; + if (o instanceof ClientJwtConfiguration) { + ClientJwtConfiguration that = (ClientJwtConfiguration) o; if (!Objects.equals(privateKeyJwtUrl, that.privateKeyJwtUrl)) return false; if (privateKeyJwt != null && that.privateKeyJwt != null) { return privateKeyJwt.getKeys().equals(that.privateKeyJwt.getKeys()); @@ -115,7 +115,7 @@ public String getCleanString() { } @JsonIgnore - public static PrivateKeyJwtConfiguration parse(String privateKeyConfig) { + public static ClientJwtConfiguration parse(String privateKeyConfig) { if (UaaUrlUtils.isUrl(privateKeyConfig)) { return parse(privateKeyConfig, null); } else { @@ -124,11 +124,11 @@ public static PrivateKeyJwtConfiguration parse(String privateKeyConfig) { } @JsonIgnore - public static PrivateKeyJwtConfiguration parse(String privateKeyUrl, String privateKeyJwt) { - PrivateKeyJwtConfiguration privateKeyJwtConfiguration = null; + public static ClientJwtConfiguration parse(String privateKeyUrl, String privateKeyJwt) { + ClientJwtConfiguration clientJwtConfiguration = null; if (privateKeyUrl != null) { - privateKeyJwtConfiguration = new PrivateKeyJwtConfiguration(privateKeyUrl, null); - privateKeyJwtConfiguration.validateJwksUri(); + clientJwtConfiguration = new ClientJwtConfiguration(privateKeyUrl, null); + clientJwtConfiguration.validateJwksUri(); } else if (privateKeyJwt != null && privateKeyJwt.contains("{") && privateKeyJwt.contains("}")) { HashMap jsonMap = JsonUtils.readValue(privateKeyJwt, HashMap.class); String cleanJwtString; @@ -138,13 +138,13 @@ public static PrivateKeyJwtConfiguration parse(String privateKeyUrl, String priv } else { cleanJwtString = JWK.parse(jsonMap).toPublicJWK().toString(); } - privateKeyJwtConfiguration = new PrivateKeyJwtConfiguration(null, JsonWebKeyHelper.deserialize(cleanJwtString)); - privateKeyJwtConfiguration.validateJwkSet(); + clientJwtConfiguration = new ClientJwtConfiguration(null, JsonWebKeyHelper.deserialize(cleanJwtString)); + clientJwtConfiguration.validateJwkSet(); } catch (ParseException e) { throw new InvalidClientDetailsException("Client jwt configuration cannot be parsed", e); } } - return privateKeyJwtConfiguration; + return clientJwtConfiguration; } private boolean validateJwkSet() { @@ -192,13 +192,13 @@ private boolean validateJwksUri() { * @return */ @JsonIgnore - public static PrivateKeyJwtConfiguration readValue(ClientDetails clientDetails) { + public static ClientJwtConfiguration readValue(ClientDetails clientDetails) { if (clientDetails == null || clientDetails.getAdditionalInformation() == null || !(clientDetails.getAdditionalInformation().get(PRIVATE_KEY_CONFIG) instanceof String)) { return null; } - return JsonUtils.readValue((String) clientDetails.getAdditionalInformation().get(PRIVATE_KEY_CONFIG), PrivateKeyJwtConfiguration.class); + return JsonUtils.readValue((String) clientDetails.getAdditionalInformation().get(PRIVATE_KEY_CONFIG), ClientJwtConfiguration.class); } /** @@ -236,17 +236,17 @@ public static void resetConfiguration(ClientDetails clientDetails) { } @JsonIgnore - public static PrivateKeyJwtConfiguration merge(PrivateKeyJwtConfiguration existingConfig, PrivateKeyJwtConfiguration newConfig, boolean overwrite) { + public static ClientJwtConfiguration merge(ClientJwtConfiguration existingConfig, ClientJwtConfiguration newConfig, boolean overwrite) { if (existingConfig == null) { return newConfig; } if (newConfig == null) { return existingConfig; } - PrivateKeyJwtConfiguration result = null; + ClientJwtConfiguration result = null; if (newConfig.privateKeyJwtUrl != null) { if (overwrite) { - result = new PrivateKeyJwtConfiguration(newConfig.privateKeyJwtUrl, null); + result = new ClientJwtConfiguration(newConfig.privateKeyJwtUrl, null); } else { result = existingConfig; } @@ -254,7 +254,7 @@ public static PrivateKeyJwtConfiguration merge(PrivateKeyJwtConfiguration existi if (newConfig.privateKeyJwt != null) { if (existingConfig.privateKeyJwt == null) { if (overwrite) { - result = new PrivateKeyJwtConfiguration(null, newConfig.privateKeyJwt); + result = new ClientJwtConfiguration(null, newConfig.privateKeyJwt); } else { result = existingConfig; } @@ -273,28 +273,28 @@ public static PrivateKeyJwtConfiguration merge(PrivateKeyJwtConfiguration existi } }); existingKeys.addAll(newKeys); - result = new PrivateKeyJwtConfiguration(null, new JsonWebKeySet<>(existingKeys)); + result = new ClientJwtConfiguration(null, new JsonWebKeySet<>(existingKeys)); } } return result; } @JsonIgnore - public static PrivateKeyJwtConfiguration delete(PrivateKeyJwtConfiguration existingConfig, PrivateKeyJwtConfiguration tobeDeleted) { + public static ClientJwtConfiguration delete(ClientJwtConfiguration existingConfig, ClientJwtConfiguration tobeDeleted) { if (existingConfig == null) { return null; } if (tobeDeleted == null) { return existingConfig; } - PrivateKeyJwtConfiguration result = null; + ClientJwtConfiguration result = null; if (existingConfig.privateKeyJwt != null && tobeDeleted.privateKeyJwtUrl != null) { JsonWebKeySet existingKeySet = existingConfig.privateKeyJwt; List keys = existingKeySet.getKeys().stream().filter(k -> !tobeDeleted.privateKeyJwtUrl.equals(k.getKid())).collect(Collectors.toList()); if (keys.isEmpty()) { result = null; } else { - result = new PrivateKeyJwtConfiguration(null, new JsonWebKeySet<>(keys)); + result = new ClientJwtConfiguration(null, new JsonWebKeySet<>(keys)); } } else if (existingConfig.privateKeyJwt != null && tobeDeleted.privateKeyJwt != null) { List existingKeys = new ArrayList<>(existingConfig.getPrivateKeyJwt().getKeys()); @@ -302,7 +302,7 @@ public static PrivateKeyJwtConfiguration delete(PrivateKeyJwtConfiguration exist if (existingKeys.isEmpty()) { result = null; } else { - result = new PrivateKeyJwtConfiguration(null, new JsonWebKeySet<>(existingKeys)); + result = new ClientJwtConfiguration(null, new JsonWebKeySet<>(existingKeys)); } } else if (existingConfig.privateKeyJwtUrl != null && tobeDeleted.privateKeyJwtUrl != null) { if ("*".equals(tobeDeleted.privateKeyJwtUrl) || existingConfig.privateKeyJwtUrl.equals(tobeDeleted.privateKeyJwtUrl)) { diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java index a17fdbc1471..0960f16f5e0 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java @@ -2,7 +2,7 @@ import org.cloudfoundry.identity.uaa.audit.event.SystemDeletable; import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal; -import org.cloudfoundry.identity.uaa.client.PrivateKeyJwtConfiguration; +import org.cloudfoundry.identity.uaa.client.ClientJwtConfiguration; import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.resources.ResourceMonitor; import org.cloudfoundry.identity.uaa.security.ContextSensitiveOAuth2SecurityExpressionMethods; @@ -280,11 +280,11 @@ public void deleteClientSecret(String clientId, String zoneId) throws NoSuchClie @Override public void addClientKeyConfig(String clientId, String keyConfig, String zoneId, boolean overwrite) throws NoSuchClientException { - PrivateKeyJwtConfiguration privateKeyJwtConfiguration = PrivateKeyJwtConfiguration.parse(keyConfig); - if (privateKeyJwtConfiguration != null) { + ClientJwtConfiguration clientJwtConfiguration = ClientJwtConfiguration.parse(keyConfig); + if (clientJwtConfiguration != null) { BaseClientDetails clientDetails = (BaseClientDetails) loadClientByClientId(clientId, zoneId); - PrivateKeyJwtConfiguration existingConfig = PrivateKeyJwtConfiguration.readValue(clientDetails); - PrivateKeyJwtConfiguration result = PrivateKeyJwtConfiguration.merge(existingConfig, privateKeyJwtConfiguration, overwrite); + ClientJwtConfiguration existingConfig = ClientJwtConfiguration.readValue(clientDetails); + ClientJwtConfiguration result = ClientJwtConfiguration.merge(existingConfig, clientJwtConfiguration, overwrite); if (result != null) { result.writeValue(clientDetails); } @@ -294,19 +294,19 @@ public void addClientKeyConfig(String clientId, String keyConfig, String zoneId, @Override public void deleteClientKeyConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException { - PrivateKeyJwtConfiguration privateKeyJwtConfiguration; + ClientJwtConfiguration clientJwtConfiguration; if(UaaUrlUtils.isUrl(keyConfig)) { - privateKeyJwtConfiguration = PrivateKeyJwtConfiguration.parse(keyConfig); + clientJwtConfiguration = ClientJwtConfiguration.parse(keyConfig); } else { - privateKeyJwtConfiguration = new PrivateKeyJwtConfiguration(keyConfig, null); + clientJwtConfiguration = new ClientJwtConfiguration(keyConfig, null); } - if (privateKeyJwtConfiguration != null) { + if (clientJwtConfiguration != null) { BaseClientDetails clientDetails = (BaseClientDetails) loadClientByClientId(clientId, zoneId); - PrivateKeyJwtConfiguration result = PrivateKeyJwtConfiguration.delete(PrivateKeyJwtConfiguration.readValue(clientDetails), privateKeyJwtConfiguration); + ClientJwtConfiguration result = ClientJwtConfiguration.delete(ClientJwtConfiguration.readValue(clientDetails), clientJwtConfiguration); if (result != null) { result.writeValue(clientDetails); } else { - PrivateKeyJwtConfiguration.resetConfiguration(clientDetails); + ClientJwtConfiguration.resetConfiguration(clientDetails); } updateClientDetails(clientDetails, zoneId); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java index ac434921955..04c85b0ca88 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java @@ -1077,7 +1077,7 @@ void testCreateClientWithPrivateKeyUri() { ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); verify(clientDetailsService).create(clientCaptor.capture(), anyString()); BaseClientDetails created = clientCaptor.getValue(); - assertEquals(PrivateKeyJwtConfiguration.readValue(created), PrivateKeyJwtConfiguration.parse(jwksUri)); + assertEquals(ClientJwtConfiguration.readValue(created), ClientJwtConfiguration.parse(jwksUri)); } @Test @@ -1097,7 +1097,7 @@ void testCreateClientWithPrivateKeyUriInvalid() { ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); verify(clientDetailsService).create(clientCaptor.capture(), anyString()); BaseClientDetails created = clientCaptor.getValue(); - assertNull(PrivateKeyJwtConfiguration.readValue(created)); + assertNull(ClientJwtConfiguration.readValue(created)); } @Test @@ -1148,7 +1148,7 @@ void testChangeDeletePrivateKeyJwtConfigUri() { assertEquals("Client jwt configuration updated", result.getMessage()); verify(clientRegistrationService, times(1)).addClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), true); - PrivateKeyJwtConfiguration.parse(jwksUri).writeValue(detail); + ClientJwtConfiguration.parse(jwksUri).writeValue(detail); change.setChangeMode(ClientJwtChangeRequest.ChangeMode.DELETE); change.setKeyUrl(jwksUri); result = endpoints.changeClientJwt(detail.getClientId(), change); @@ -1176,10 +1176,10 @@ void testCreateClientWithPrivateKeySet() { ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); verify(clientDetailsService).create(clientCaptor.capture(), anyString()); BaseClientDetails created = clientCaptor.getValue(); - assertEquals(PrivateKeyJwtConfiguration.readValue(created), PrivateKeyJwtConfiguration.parse(jsonJwk)); - assertEquals(PrivateKeyJwtConfiguration.readValue(created), PrivateKeyJwtConfiguration.parse(jsonJwk2)); - assertEquals(PrivateKeyJwtConfiguration.readValue(created), PrivateKeyJwtConfiguration.parse(jsonJwkSet)); - assertNotEquals(PrivateKeyJwtConfiguration.readValue(created), PrivateKeyJwtConfiguration.parse(jsonJwk3)); + assertEquals(ClientJwtConfiguration.readValue(created), ClientJwtConfiguration.parse(jsonJwk)); + assertEquals(ClientJwtConfiguration.readValue(created), ClientJwtConfiguration.parse(jsonJwk2)); + assertEquals(ClientJwtConfiguration.readValue(created), ClientJwtConfiguration.parse(jsonJwkSet)); + assertNotEquals(ClientJwtConfiguration.readValue(created), ClientJwtConfiguration.parse(jsonJwk3)); } private ClientDetailsCreation createClientDetailsCreation(BaseClientDetails baseClientDetails) { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfigurationTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfigurationTest.java similarity index 50% rename from server/src/test/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfigurationTest.java rename to server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfigurationTest.java index d50c9497d7c..f7a22c77df7 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/PrivateKeyJwtConfigurationTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfigurationTest.java @@ -22,7 +22,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -class PrivateKeyJwtConfigurationTest { +class ClientJwtConfigurationTest { private final String nValue = "u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q"; private final String jsonWebKey = "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}"; @@ -36,115 +36,115 @@ class PrivateKeyJwtConfigurationTest { @Test void testJwksValidity() { - assertNotNull(PrivateKeyJwtConfiguration.parse("https://any.domain.net/openid/jwks-uri")); - assertNotNull(PrivateKeyJwtConfiguration.parse("http://any.localhost/openid/jwks-uri")); + assertNotNull(ClientJwtConfiguration.parse("https://any.domain.net/openid/jwks-uri")); + assertNotNull(ClientJwtConfiguration.parse("http://any.localhost/openid/jwks-uri")); } @Test void testJwksInvalid() { - assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse("custom://any.domain.net/openid/jwks-uri", null)); - assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse("test", null)); - assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse("http://any.domain.net/openid/jwks-uri")); - assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse("https://")); - assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse("ftp://any.domain.net/openid/jwks-uri")); + assertThrows(InvalidClientDetailsException.class, () -> ClientJwtConfiguration.parse("custom://any.domain.net/openid/jwks-uri", null)); + assertThrows(InvalidClientDetailsException.class, () -> ClientJwtConfiguration.parse("test", null)); + assertThrows(InvalidClientDetailsException.class, () -> ClientJwtConfiguration.parse("http://any.domain.net/openid/jwks-uri")); + assertThrows(InvalidClientDetailsException.class, () -> ClientJwtConfiguration.parse("https://")); + assertThrows(InvalidClientDetailsException.class, () -> ClientJwtConfiguration.parse("ftp://any.domain.net/openid/jwks-uri")); } @Test void testJwkSetValidity() { - assertNotNull(PrivateKeyJwtConfiguration.parse(jsonWebKey)); - assertNotNull(PrivateKeyJwtConfiguration.parse(jsonJwkSet)); + assertNotNull(ClientJwtConfiguration.parse(jsonWebKey)); + assertNotNull(ClientJwtConfiguration.parse(jsonJwkSet)); } @Test void testJwkSetInvalid() { - assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse(jsonJwkSetEmtpy)); - assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse(jsonWebKeyNoId)); - assertThrows(InvalidClientDetailsException.class, () -> PrivateKeyJwtConfiguration.parse("{\"keys\": \"x\"}")); + assertThrows(InvalidClientDetailsException.class, () -> ClientJwtConfiguration.parse(jsonJwkSetEmtpy)); + assertThrows(InvalidClientDetailsException.class, () -> ClientJwtConfiguration.parse(jsonWebKeyNoId)); + assertThrows(InvalidClientDetailsException.class, () -> ClientJwtConfiguration.parse("{\"keys\": \"x\"}")); } @Test void testJwkSetInvalidSize() throws ParseException { - assertThrows(InvalidClientDetailsException.class, () -> new PrivateKeyJwtConfiguration(null, new JsonWebKeySet(Collections.emptyList()))); + assertThrows(InvalidClientDetailsException.class, () -> new ClientJwtConfiguration(null, new JsonWebKeySet(Collections.emptyList()))); } @Test void testGetCleanConfig() { - assertNotNull(PrivateKeyJwtConfiguration.parse("https://any.domain.net/openid/jwks-uri").getCleanString()); - assertNotNull(PrivateKeyJwtConfiguration.parse(jsonWebKey).getCleanString()); + assertNotNull(ClientJwtConfiguration.parse("https://any.domain.net/openid/jwks-uri").getCleanString()); + assertNotNull(ClientJwtConfiguration.parse(jsonWebKey).getCleanString()); } @Test void testGetCleanConfigInvalid() { JsonWebKeySet mockedKey = mock(JsonWebKeySet.class); - List keyList = PrivateKeyJwtConfiguration.parse(jsonJwkSet).getPrivateKeyJwt().getKeys(); + List keyList = ClientJwtConfiguration.parse(jsonJwkSet).getPrivateKeyJwt().getKeys(); when(mockedKey.getKeys()).thenReturn(keyList); - PrivateKeyJwtConfiguration privateKey = new PrivateKeyJwtConfiguration(null, mockedKey); + ClientJwtConfiguration privateKey = new ClientJwtConfiguration(null, mockedKey); when(mockedKey.getKeySetMap()).thenThrow(new IllegalStateException("error")); assertThrows(InvalidClientDetailsException.class, () -> privateKey.getCleanString()); - PrivateKeyJwtConfiguration privateKey2 = new PrivateKeyJwtConfiguration("hello", null); + ClientJwtConfiguration privateKey2 = new ClientJwtConfiguration("hello", null); assertNull(privateKey2.getCleanString()); } @Test void testJwtSetValidate() { JsonWebKeySet mockedKey = mock(JsonWebKeySet.class); - List keyList = PrivateKeyJwtConfiguration.parse(jsonJwkSet).getPrivateKeyJwt().getKeys(); + List keyList = ClientJwtConfiguration.parse(jsonJwkSet).getPrivateKeyJwt().getKeys(); when(mockedKey.getKeys()).thenReturn(Arrays.asList(keyList.get(0), keyList.get(0))); - assertThrows(InvalidClientDetailsException.class, () -> new PrivateKeyJwtConfiguration(null, mockedKey)); + assertThrows(InvalidClientDetailsException.class, () -> new ClientJwtConfiguration(null, mockedKey)); } @Test void testConfigMerge() { - PrivateKeyJwtConfiguration configuration = PrivateKeyJwtConfiguration.parse(jsonJwkSet); + ClientJwtConfiguration configuration = ClientJwtConfiguration.parse(jsonJwkSet); assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); - PrivateKeyJwtConfiguration addKey = PrivateKeyJwtConfiguration.parse(jsonWebKey2); - configuration = PrivateKeyJwtConfiguration.merge(configuration, addKey, false); + ClientJwtConfiguration addKey = ClientJwtConfiguration.parse(jsonWebKey2); + configuration = ClientJwtConfiguration.merge(configuration, addKey, false); assertEquals(2, configuration.getPrivateKeyJwt().getKeys().size()); assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(1).getKeyProperties().get("n")); - configuration = PrivateKeyJwtConfiguration.merge(configuration, addKey, true); + configuration = ClientJwtConfiguration.merge(configuration, addKey, true); assertEquals(2, configuration.getPrivateKeyJwt().getKeys().size()); - configuration = PrivateKeyJwtConfiguration.parse(jsonJwkSet); + configuration = ClientJwtConfiguration.parse(jsonJwkSet); assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); - configuration = PrivateKeyJwtConfiguration.merge(PrivateKeyJwtConfiguration.parse(jsonJwkSet), PrivateKeyJwtConfiguration.parse(jsonWebKeyDifferentValue), true); + configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse(jsonJwkSet), ClientJwtConfiguration.parse(jsonWebKeyDifferentValue), true); assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); assertEquals("new", configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); - configuration = PrivateKeyJwtConfiguration.merge(PrivateKeyJwtConfiguration.parse(jsonJwkSet), PrivateKeyJwtConfiguration.parse(jsonWebKeyDifferentValue), false); + configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse(jsonJwkSet), ClientJwtConfiguration.parse(jsonWebKeyDifferentValue), false); assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); } @Test void testConfigMergeDifferentType() { - PrivateKeyJwtConfiguration configuration = PrivateKeyJwtConfiguration.parse(jsonJwkSet); + ClientJwtConfiguration configuration = ClientJwtConfiguration.parse(jsonJwkSet); assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); assertNull(configuration.getPrivateKeyJwtUrl()); - configuration = PrivateKeyJwtConfiguration.merge(configuration, PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), false); + configuration = ClientJwtConfiguration.merge(configuration, ClientJwtConfiguration.parse("https://any/jwks-uri"), false); assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); assertNull(configuration.getPrivateKeyJwtUrl()); - configuration = PrivateKeyJwtConfiguration.merge(configuration, PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), true); + configuration = ClientJwtConfiguration.merge(configuration, ClientJwtConfiguration.parse("https://any/jwks-uri"), true); assertNull(configuration.getPrivateKeyJwt()); assertNotNull(configuration.getPrivateKeyJwtUrl()); - configuration = PrivateKeyJwtConfiguration.merge(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), PrivateKeyJwtConfiguration.parse("https://new/jwks-uri"), false); + configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse("https://any/jwks-uri"), ClientJwtConfiguration.parse("https://new/jwks-uri"), false); assertNull(configuration.getPrivateKeyJwt()); assertEquals("https://any/jwks-uri", configuration.getPrivateKeyJwtUrl()); - configuration = PrivateKeyJwtConfiguration.merge(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), PrivateKeyJwtConfiguration.parse("https://new/jwks-uri"), true); + configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse("https://any/jwks-uri"), ClientJwtConfiguration.parse("https://new/jwks-uri"), true); assertNull(configuration.getPrivateKeyJwt()); assertEquals("https://new/jwks-uri", configuration.getPrivateKeyJwtUrl()); - configuration = PrivateKeyJwtConfiguration.merge(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), PrivateKeyJwtConfiguration.parse(jsonJwkSet), false); + configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse("https://any/jwks-uri"), ClientJwtConfiguration.parse(jsonJwkSet), false); assertNull(configuration.getPrivateKeyJwt()); assertEquals("https://any/jwks-uri", configuration.getPrivateKeyJwtUrl()); - configuration = PrivateKeyJwtConfiguration.merge(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), PrivateKeyJwtConfiguration.parse(jsonJwkSet), true); + configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse("https://any/jwks-uri"), ClientJwtConfiguration.parse(jsonJwkSet), true); assertNull(configuration.getPrivateKeyJwtUrl()); assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); @@ -152,55 +152,55 @@ void testConfigMergeDifferentType() { @Test void testConfigMergeNulls() { - PrivateKeyJwtConfiguration configuration = PrivateKeyJwtConfiguration.parse(jsonJwkSet); - PrivateKeyJwtConfiguration existingKeyConfig = PrivateKeyJwtConfiguration.merge(configuration, null, true); + ClientJwtConfiguration configuration = ClientJwtConfiguration.parse(jsonJwkSet); + ClientJwtConfiguration existingKeyConfig = ClientJwtConfiguration.merge(configuration, null, true); assertTrue(configuration.equals(existingKeyConfig)); assertEquals(configuration, existingKeyConfig); - PrivateKeyJwtConfiguration newKeyConfig = PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"); - configuration = PrivateKeyJwtConfiguration.merge(null, newKeyConfig, true); + ClientJwtConfiguration newKeyConfig = ClientJwtConfiguration.parse("https://any/jwks-uri"); + configuration = ClientJwtConfiguration.merge(null, newKeyConfig, true); assertTrue(configuration.equals(newKeyConfig)); assertTrue(configuration.equals(newKeyConfig)); } @Test void testConfigDelete() { - PrivateKeyJwtConfiguration configuration = PrivateKeyJwtConfiguration.parse(jsonJwkSet); + ClientJwtConfiguration configuration = ClientJwtConfiguration.parse(jsonJwkSet); assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); assertNull(configuration.getPrivateKeyJwtUrl()); - PrivateKeyJwtConfiguration addKey = PrivateKeyJwtConfiguration.parse(jsonWebKey2); - configuration = PrivateKeyJwtConfiguration.merge(configuration, addKey, false); + ClientJwtConfiguration addKey = ClientJwtConfiguration.parse(jsonWebKey2); + configuration = ClientJwtConfiguration.merge(configuration, addKey, false); assertEquals(2, configuration.getPrivateKeyJwt().getKeys().size()); - configuration = PrivateKeyJwtConfiguration.delete(configuration, addKey); + configuration = ClientJwtConfiguration.delete(configuration, addKey); assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); - configuration = PrivateKeyJwtConfiguration.delete(configuration, addKey); - configuration = PrivateKeyJwtConfiguration.delete(configuration, addKey); + configuration = ClientJwtConfiguration.delete(configuration, addKey); + configuration = ClientJwtConfiguration.delete(configuration, addKey); assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); - configuration = PrivateKeyJwtConfiguration.merge(configuration, addKey, false); - configuration = PrivateKeyJwtConfiguration.delete(configuration, addKey); + configuration = ClientJwtConfiguration.merge(configuration, addKey, false); + configuration = ClientJwtConfiguration.delete(configuration, addKey); assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); - configuration = PrivateKeyJwtConfiguration.merge(configuration, addKey, false); - configuration = PrivateKeyJwtConfiguration.delete(configuration, new PrivateKeyJwtConfiguration("key-2", null)); - configuration = PrivateKeyJwtConfiguration.delete(configuration, new PrivateKeyJwtConfiguration("key-1", null)); + configuration = ClientJwtConfiguration.merge(configuration, addKey, false); + configuration = ClientJwtConfiguration.delete(configuration, new ClientJwtConfiguration("key-2", null)); + configuration = ClientJwtConfiguration.delete(configuration, new ClientJwtConfiguration("key-1", null)); assertNull(configuration); - configuration = PrivateKeyJwtConfiguration.delete(PrivateKeyJwtConfiguration.parse(jsonJwkSet), PrivateKeyJwtConfiguration.parse(jsonWebKey)); + configuration = ClientJwtConfiguration.delete(ClientJwtConfiguration.parse(jsonJwkSet), ClientJwtConfiguration.parse(jsonWebKey)); assertNull(configuration); - configuration = PrivateKeyJwtConfiguration.delete(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), PrivateKeyJwtConfiguration.parse("https://any/jwks-uri")); + configuration = ClientJwtConfiguration.delete(ClientJwtConfiguration.parse("https://any/jwks-uri"), ClientJwtConfiguration.parse("https://any/jwks-uri")); assertNull(configuration); - configuration = PrivateKeyJwtConfiguration.delete(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), PrivateKeyJwtConfiguration.parse("https://other/jwks-uri")); + configuration = ClientJwtConfiguration.delete(ClientJwtConfiguration.parse("https://any/jwks-uri"), ClientJwtConfiguration.parse("https://other/jwks-uri")); assertNotNull(configuration); } @Test void testConfigDeleteNull() { - assertNull(PrivateKeyJwtConfiguration.delete(null, PrivateKeyJwtConfiguration.parse("https://other/jwks-uri"))); - assertNotNull(PrivateKeyJwtConfiguration.delete(PrivateKeyJwtConfiguration.parse("https://any/jwks-uri"), null)); + assertNull(ClientJwtConfiguration.delete(null, ClientJwtConfiguration.parse("https://other/jwks-uri"))); + assertNotNull(ClientJwtConfiguration.delete(ClientJwtConfiguration.parse("https://any/jwks-uri"), null)); } @Test void testHashCode() { - PrivateKeyJwtConfiguration key1 = PrivateKeyJwtConfiguration.parse("http://localhost:8080/uaa"); - PrivateKeyJwtConfiguration key2 = PrivateKeyJwtConfiguration.parse("http://localhost:8080/uaa"); + ClientJwtConfiguration key1 = ClientJwtConfiguration.parse("http://localhost:8080/uaa"); + ClientJwtConfiguration key2 = ClientJwtConfiguration.parse("http://localhost:8080/uaa"); assertNotEquals(key1.hashCode(), key2.hashCode()); assertEquals(key1.hashCode(), key1.hashCode()); assertEquals(key2.hashCode(), key2.hashCode()); @@ -208,39 +208,39 @@ void testHashCode() { @Test void testEquals() throws CloneNotSupportedException { - PrivateKeyJwtConfiguration key1 = PrivateKeyJwtConfiguration.parse("http://localhost:8080/uaa"); - PrivateKeyJwtConfiguration key2 = (PrivateKeyJwtConfiguration) key1.clone(); + ClientJwtConfiguration key1 = ClientJwtConfiguration.parse("http://localhost:8080/uaa"); + ClientJwtConfiguration key2 = (ClientJwtConfiguration) key1.clone(); assertEquals(key1, key2); } @Test void testSerializableObjectCalls() throws CloneNotSupportedException { - PrivateKeyJwtConfiguration key1 = JsonUtils.readValue(defaultJsonUri, PrivateKeyJwtConfiguration.class); - PrivateKeyJwtConfiguration key2 = (PrivateKeyJwtConfiguration) key1.clone(); + ClientJwtConfiguration key1 = JsonUtils.readValue(defaultJsonUri, ClientJwtConfiguration.class); + ClientJwtConfiguration key2 = (ClientJwtConfiguration) key1.clone(); assertEquals(key1, key2); - key1 = JsonUtils.readValue(defaultJsonKey, PrivateKeyJwtConfiguration.class); - key2 = (PrivateKeyJwtConfiguration) key1.clone(); + key1 = JsonUtils.readValue(defaultJsonKey, ClientJwtConfiguration.class); + key2 = (ClientJwtConfiguration) key1.clone(); assertEquals(key1, key2); } @Test void testConfiguration() { - PrivateKeyJwtConfiguration configUri = JsonUtils.readValue(defaultJsonUri, PrivateKeyJwtConfiguration.class); - PrivateKeyJwtConfiguration configKey = JsonUtils.readValue(defaultJsonKey, PrivateKeyJwtConfiguration.class); + ClientJwtConfiguration configUri = JsonUtils.readValue(defaultJsonUri, ClientJwtConfiguration.class); + ClientJwtConfiguration configKey = JsonUtils.readValue(defaultJsonKey, ClientJwtConfiguration.class); BaseClientDetails baseClientDetails = new BaseClientDetails(); HashMap additionalInformation = new HashMap<>(); additionalInformation.put(ClientConstants.PRIVATE_KEY_CONFIG, configUri); baseClientDetails.setAdditionalInformation(additionalInformation); configUri.writeValue(baseClientDetails); - PrivateKeyJwtConfiguration readUriConfig = PrivateKeyJwtConfiguration.readValue(baseClientDetails); + ClientJwtConfiguration readUriConfig = ClientJwtConfiguration.readValue(baseClientDetails); assertEquals(configUri, readUriConfig); - PrivateKeyJwtConfiguration.resetConfiguration(baseClientDetails); - assertNull(PrivateKeyJwtConfiguration.readValue(baseClientDetails)); + ClientJwtConfiguration.resetConfiguration(baseClientDetails); + assertNull(ClientJwtConfiguration.readValue(baseClientDetails)); configKey.writeValue(baseClientDetails); - PrivateKeyJwtConfiguration readKeyConfig = PrivateKeyJwtConfiguration.readValue(baseClientDetails); + ClientJwtConfiguration readKeyConfig = ClientJwtConfiguration.readValue(baseClientDetails); assertEquals(configKey, readKeyConfig); } } From 97c06aa15b61a620569f6dd3304e72e753bf5c84 Mon Sep 17 00:00:00 2001 From: strehle Date: Thu, 24 Aug 2023 10:45:59 +0200 Subject: [PATCH 08/22] Renamed --- .../oauth/client/ClientDetailsCreation.java | 24 +++--- .../oauth/client/ClientJwtChangeRequest.java | 28 +++---- .../client/ClientJwtChangeRequestTest.java | 4 +- .../uaa/client/ClientAdminBootstrap.java | 4 +- .../uaa/client/ClientAdminEndpoints.java | 6 +- .../client/ClientAdminEndpointsValidator.java | 8 +- .../uaa/client/ClientJwtConfiguration.java | 82 +++++++++---------- .../uaa/zone/MultitenantClientServices.java | 4 +- .../MultitenantJdbcClientDetailsService.java | 4 +- .../uaa/client/ClientAdminEndpointsTests.java | 34 ++++---- .../client/ClientJwtConfigurationTest.java | 68 +++++++-------- .../InMemoryMultitenantClientServices.java | 4 +- 12 files changed, 135 insertions(+), 135 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientDetailsCreation.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientDetailsCreation.java index a44697e6a5a..b8f213744f1 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientDetailsCreation.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientDetailsCreation.java @@ -13,11 +13,11 @@ public class ClientDetailsCreation extends BaseClientDetails { @JsonProperty("secondary_client_secret") private String secondaryClientSecret; - @JsonProperty("private_key_url") - private String privateKeyUrl; + @JsonProperty("jwks_uri") + private String jsonWebKeyUri; - @JsonProperty("private_key_set") - private String privateKeySet; + @JsonProperty("jwks") + private String jsonWebKeySet; @JsonIgnore public String getSecondaryClientSecret() { @@ -28,19 +28,19 @@ public void setSecondaryClientSecret(final String secondaryClientSecret) { this.secondaryClientSecret = secondaryClientSecret; } - public String getPrivateKeyUrl() { - return privateKeyUrl; + public String getJsonWebKeyUri() { + return jsonWebKeyUri; } - public void setPrivateKeyUrl(String privateKeyUrl) { - this.privateKeyUrl = privateKeyUrl; + public void setJsonWebKeyUri(String jsonWebKeyUri) { + this.jsonWebKeyUri = jsonWebKeyUri; } - public String getPrivateKeySet() { - return privateKeySet; + public String getJsonWebKeySet() { + return jsonWebKeySet; } - public void setPrivateKeySet(String privateKeySet) { - this.privateKeySet = privateKeySet; + public void setJsonWebKeySet(String jsonWebKeySet) { + this.jsonWebKeySet = jsonWebKeySet; } } diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java index b0058153665..2e19627b2c6 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java @@ -21,9 +21,9 @@ public enum ChangeMode { @JsonProperty("kid") private String keyId; @JsonProperty("jwks_uri") - private String keyUrl; + private String jsonWebKeyUri; @JsonProperty("jwks") - private String keyConfig; + private String jsonWebKeySet; @JsonProperty("client_id") private String clientId; private ChangeMode changeMode = ADD; @@ -31,26 +31,26 @@ public enum ChangeMode { public ClientJwtChangeRequest() { } - public ClientJwtChangeRequest(String clientId, String keyUrl, String keyConfig) { - this.keyUrl = keyUrl; - this.keyConfig = keyConfig; + public ClientJwtChangeRequest(String clientId, String jsonWebKeyUri, String jsonWebKeySet) { + this.jsonWebKeyUri = jsonWebKeyUri; + this.jsonWebKeySet = jsonWebKeySet; this.clientId = clientId; } - public String getKeyUrl() { - return keyUrl; + public String getJsonWebKeyUri() { + return jsonWebKeyUri; } - public void setKeyUrl(String keyUrl) { - this.keyUrl = keyUrl; + public void setJsonWebKeyUri(String jsonWebKeyUri) { + this.jsonWebKeyUri = jsonWebKeyUri; } - public String getKeyConfig() { - return keyConfig; + public String getJsonWebKeySet() { + return jsonWebKeySet; } - public void setKeyConfig(String keyConfig) { - this.keyConfig = keyConfig; + public void setJsonWebKeySet(String jsonWebKeySet) { + this.jsonWebKeySet = jsonWebKeySet; } public String getClientId() { @@ -76,6 +76,6 @@ public void setKeyId(String keyId) { } public String getKey() { - return keyUrl != null ? keyUrl : keyConfig; + return jsonWebKeyUri != null ? jsonWebKeyUri : jsonWebKeySet; } } diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequestTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequestTest.java index 36da456fb42..67734f5200c 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequestTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequestTest.java @@ -12,8 +12,8 @@ void testRequestSerialization() { ClientJwtChangeRequest def = new ClientJwtChangeRequest(null, null, null); def.setKeyId("key-1"); def.setChangeMode(ClientJwtChangeRequest.ChangeMode.DELETE); - def.setKeyUrl("http://localhost:8080/uaa/token_key"); - def.setKeyConfig("{}"); + def.setJsonWebKeyUri("http://localhost:8080/uaa/token_key"); + def.setJsonWebKeySet("{}"); def.setClientId("admin"); String jsonRequest = JsonUtils.writeValueAsString(def); ClientJwtChangeRequest request = JsonUtils.readValue(jsonRequest, ClientJwtChangeRequest.class); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java index 7e180f9443f..598bd38845c 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java @@ -240,13 +240,13 @@ private void addNewClients() { String jwksUri = (String) map.get("jwks_uri"); ClientJwtConfiguration keyConfig = ClientJwtConfiguration.parse(UaaUrlUtils.normalizeUri(jwksUri), null); if (keyConfig != null && keyConfig.getCleanString() != null) { - clientRegistrationService.addClientKeyConfig(clientId, keyConfig.getPrivateKeyJwtUrl(), IdentityZone.getUaaZoneId(), override); + clientRegistrationService.addClientJwtConfig(clientId, keyConfig.getJwksUri(), IdentityZone.getUaaZoneId(), override); } } else if (map.get("jwks") instanceof String) { String jwks = (String) map.get("jwks"); ClientJwtConfiguration keyConfig = ClientJwtConfiguration.parse(null, jwks); if (keyConfig != null && keyConfig.getCleanString() != null) { - clientRegistrationService.addClientKeyConfig(clientId, keyConfig.getCleanString(), IdentityZone.getUaaZoneId(), override); + clientRegistrationService.addClientJwtConfig(clientId, keyConfig.getCleanString(), IdentityZone.getUaaZoneId(), override); } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java index 34d7d1a7e9f..9b3d8b14062 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java @@ -559,7 +559,7 @@ public ActionResult changeClientJwt(@PathVariable String client_id, @RequestBody switch (change.getChangeMode()){ case ADD : if (change.getKey() != null) { - clientRegistrationService.addClientKeyConfig(client_id, change.getKey(), IdentityZoneHolder.get().getId(), false); + clientRegistrationService.addClientJwtConfig(client_id, change.getKey(), IdentityZoneHolder.get().getId(), false); result = new ActionResult("ok", "Client jwt configuration is added"); } else { result = new ActionResult("ok", "No key added"); @@ -569,13 +569,13 @@ public ActionResult changeClientJwt(@PathVariable String client_id, @RequestBody case DELETE : String deleteString = change.getKeyId() == null ? change.getKey() : change.getKeyId(); if (clientKeyConfig != null && deleteString != null) { - clientRegistrationService.deleteClientKeyConfig(client_id, deleteString, IdentityZoneHolder.get().getId()); + clientRegistrationService.deleteClientJwtConfig(client_id, deleteString, IdentityZoneHolder.get().getId()); } result = new ActionResult("ok", "Client jwt configuration is deleted"); break; default: - clientRegistrationService.addClientKeyConfig(client_id, change.getKey(), IdentityZoneHolder.get().getId(), true); + clientRegistrationService.addClientJwtConfig(client_id, change.getKey(), IdentityZoneHolder.get().getId(), true); result = new ActionResult("ok", "Client jwt configuration updated"); } clientSecretChanges.incrementAndGet(); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java index eec117c68a4..6c1879656b6 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java @@ -249,13 +249,13 @@ public ClientDetails validate(ClientDetails prototype, boolean create, boolean c if (prototype instanceof ClientDetailsCreation) { ClientDetailsCreation clientDetailsCreation = (ClientDetailsCreation) prototype; - if (StringUtils.hasText(clientDetailsCreation.getPrivateKeyUrl()) || StringUtils.hasText(clientDetailsCreation.getPrivateKeySet())) { - ClientJwtConfiguration clientJwtConfiguration = ClientJwtConfiguration.parse(clientDetailsCreation.getPrivateKeyUrl(), - clientDetailsCreation.getPrivateKeySet()); + if (StringUtils.hasText(clientDetailsCreation.getJsonWebKeyUri()) || StringUtils.hasText(clientDetailsCreation.getJsonWebKeySet())) { + ClientJwtConfiguration clientJwtConfiguration = ClientJwtConfiguration.parse(clientDetailsCreation.getJsonWebKeyUri(), + clientDetailsCreation.getJsonWebKeySet()); if (clientJwtConfiguration != null) { clientJwtConfiguration.writeValue(client); } else { - logger.warn("Client configuration with private_key_jwt not valid"); + logger.warn("Client with client jwt configuration not valid"); } } } 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 e3747319147..dd664c8b842 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 @@ -37,36 +37,36 @@ public class ClientJwtConfiguration implements Cloneable{ private static final int MAX_KEY_SIZE = 10; @JsonProperty("jwks_uri") - private String privateKeyJwtUrl; + private String jwksUri; @JsonProperty("jwks") - private JsonWebKeySet privateKeyJwt; + private JsonWebKeySet jwkSet; public ClientJwtConfiguration() { } - public ClientJwtConfiguration(final String privateKeyJwtUrl, final JsonWebKeySet webKeySet) { - this.privateKeyJwtUrl = privateKeyJwtUrl; - privateKeyJwt = webKeySet; - if (privateKeyJwt != null) { + public ClientJwtConfiguration(final String jwksUri, final JsonWebKeySet webKeySet) { + this.jwksUri = jwksUri; + jwkSet = webKeySet; + if (jwkSet != null) { validateJwkSet(); } } - public String getPrivateKeyJwtUrl() { - return this.privateKeyJwtUrl; + public String getJwksUri() { + return this.jwksUri; } - public void setPrivateKeyJwtUrl(final String privateKeyJwtUrl) { - this.privateKeyJwtUrl = privateKeyJwtUrl; + public void setJwksUri(final String jwksUri) { + this.jwksUri = jwksUri; } - public JsonWebKeySet getPrivateKeyJwt() { - return this.privateKeyJwt; + public JsonWebKeySet getJwkSet() { + return this.jwkSet; } - public void setPrivateKeyJwt(final JsonWebKeySet privateKeyJwt) { - this.privateKeyJwt = privateKeyJwt; + public void setJwkSet(final JsonWebKeySet jwkSet) { + this.jwkSet = jwkSet; } @Override @@ -76,11 +76,11 @@ public boolean equals(Object o) { if (o instanceof ClientJwtConfiguration) { ClientJwtConfiguration that = (ClientJwtConfiguration) o; - if (!Objects.equals(privateKeyJwtUrl, that.privateKeyJwtUrl)) return false; - if (privateKeyJwt != null && that.privateKeyJwt != null) { - return privateKeyJwt.getKeys().equals(that.privateKeyJwt.getKeys()); + if (!Objects.equals(jwksUri, that.jwksUri)) return false; + if (jwkSet != null && that.jwkSet != null) { + return jwkSet.getKeys().equals(that.jwkSet.getKeys()); } else { - return Objects.equals(privateKeyJwt, that.privateKeyJwt); + return Objects.equals(jwkSet, that.jwkSet); } } return false; @@ -90,8 +90,8 @@ public boolean equals(Object o) { public int hashCode() { int result = super.hashCode(); - result = 31 * result + (privateKeyJwtUrl != null ? privateKeyJwtUrl.hashCode() : 0); - result = 31 * result + (privateKeyJwt != null ? privateKeyJwt.hashCode() : 0); + result = 31 * result + (jwksUri != null ? jwksUri.hashCode() : 0); + result = 31 * result + (jwkSet != null ? jwkSet.hashCode() : 0); return result; } @@ -103,10 +103,10 @@ public Object clone() throws CloneNotSupportedException { @JsonIgnore public String getCleanString() { try { - if (UaaUrlUtils.isUrl(this.privateKeyJwtUrl)) { - return this.privateKeyJwtUrl; - } else if (this.privateKeyJwt != null && !ObjectUtils.isEmpty(this.privateKeyJwt.getKeySetMap())) { - return JWKSet.parse(this.privateKeyJwt.getKeySetMap()).toString(true); + if (UaaUrlUtils.isUrl(this.jwksUri)) { + return this.jwksUri; + } else if (this.jwkSet != null && !ObjectUtils.isEmpty(this.jwkSet.getKeySetMap())) { + return JWKSet.parse(this.jwkSet.getKeySetMap()).toString(true); } } catch (IllegalStateException | JsonUtils.JsonUtilException | ParseException e) { throw new InvalidClientDetailsException("Client jwt configuration configuration fails ", e); @@ -148,7 +148,7 @@ public static ClientJwtConfiguration parse(String privateKeyUrl, String privateK } private boolean validateJwkSet() { - List keyList = privateKeyJwt.getKeys(); + List keyList = jwkSet.getKeys(); if (keyList.isEmpty() || keyList.size() > MAX_KEY_SIZE) { throw new InvalidClientDetailsException("Invalid private_key_jwt: jwk set is empty of exceeds to maximum of keys. max: + " + MAX_KEY_SIZE); } @@ -168,7 +168,7 @@ private boolean validateJwkSet() { private boolean validateJwksUri() { URI jwksUri; try { - jwksUri = URI.create(privateKeyJwtUrl); + jwksUri = URI.create(this.jwksUri); } catch (IllegalArgumentException e) { throw new InvalidClientDetailsException("Invalid private_key_jwt: jwks_uri must be URI complaint", e); } @@ -244,25 +244,25 @@ public static ClientJwtConfiguration merge(ClientJwtConfiguration existingConfig return existingConfig; } ClientJwtConfiguration result = null; - if (newConfig.privateKeyJwtUrl != null) { + if (newConfig.jwksUri != null) { if (overwrite) { - result = new ClientJwtConfiguration(newConfig.privateKeyJwtUrl, null); + result = new ClientJwtConfiguration(newConfig.jwksUri, null); } else { result = existingConfig; } } - if (newConfig.privateKeyJwt != null) { - if (existingConfig.privateKeyJwt == null) { + if (newConfig.jwkSet != null) { + if (existingConfig.jwkSet == null) { if (overwrite) { - result = new ClientJwtConfiguration(null, newConfig.privateKeyJwt); + result = new ClientJwtConfiguration(null, newConfig.jwkSet); } else { result = existingConfig; } } else { - JsonWebKeySet existingKeySet = existingConfig.privateKeyJwt; + JsonWebKeySet existingKeySet = existingConfig.jwkSet; List existingKeys = new ArrayList<>(existingKeySet.getKeys()); List newKeys = new ArrayList<>(); - newConfig.getPrivateKeyJwt().getKeys().forEach(key -> { + newConfig.getJwkSet().getKeys().forEach(key -> { if (existingKeys.contains(key)) { if (overwrite) { existingKeys.remove(key); @@ -288,24 +288,24 @@ public static ClientJwtConfiguration delete(ClientJwtConfiguration existingConfi return existingConfig; } ClientJwtConfiguration result = null; - if (existingConfig.privateKeyJwt != null && tobeDeleted.privateKeyJwtUrl != null) { - JsonWebKeySet existingKeySet = existingConfig.privateKeyJwt; - List keys = existingKeySet.getKeys().stream().filter(k -> !tobeDeleted.privateKeyJwtUrl.equals(k.getKid())).collect(Collectors.toList()); + if (existingConfig.jwkSet != null && tobeDeleted.jwksUri != null) { + JsonWebKeySet existingKeySet = existingConfig.jwkSet; + List keys = existingKeySet.getKeys().stream().filter(k -> !tobeDeleted.jwksUri.equals(k.getKid())).collect(Collectors.toList()); if (keys.isEmpty()) { result = null; } else { result = new ClientJwtConfiguration(null, new JsonWebKeySet<>(keys)); } - } else if (existingConfig.privateKeyJwt != null && tobeDeleted.privateKeyJwt != null) { - List existingKeys = new ArrayList<>(existingConfig.getPrivateKeyJwt().getKeys()); - existingKeys.removeAll(tobeDeleted.privateKeyJwt.getKeys()); + } else if (existingConfig.jwkSet != null && tobeDeleted.jwkSet != null) { + List existingKeys = new ArrayList<>(existingConfig.getJwkSet().getKeys()); + existingKeys.removeAll(tobeDeleted.jwkSet.getKeys()); if (existingKeys.isEmpty()) { result = null; } else { result = new ClientJwtConfiguration(null, new JsonWebKeySet<>(existingKeys)); } - } else if (existingConfig.privateKeyJwtUrl != null && tobeDeleted.privateKeyJwtUrl != null) { - if ("*".equals(tobeDeleted.privateKeyJwtUrl) || existingConfig.privateKeyJwtUrl.equals(tobeDeleted.privateKeyJwtUrl)) { + } else if (existingConfig.jwksUri != null && tobeDeleted.jwksUri != null) { + if ("*".equals(tobeDeleted.jwksUri) || existingConfig.jwksUri.equals(tobeDeleted.jwksUri)) { result = null; } else { result = existingConfig; diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java index 931e709f756..f795071a058 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java @@ -31,9 +31,9 @@ interface MultitenantClientSecretService { void deleteClientSecret(String clientId, String zoneId) throws NoSuchClientException; - void addClientKeyConfig(String clientId, String keyConfig, String zoneId, boolean overwrite) throws NoSuchClientException; + void addClientJwtConfig(String clientId, String keyConfig, String zoneId, boolean overwrite) throws NoSuchClientException; - void deleteClientKeyConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException; + void deleteClientJwtConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException; } public abstract class MultitenantClientServices implements diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java index 0960f16f5e0..b718398b52a 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java @@ -279,7 +279,7 @@ public void deleteClientSecret(String clientId, String zoneId) throws NoSuchClie } @Override - public void addClientKeyConfig(String clientId, String keyConfig, String zoneId, boolean overwrite) throws NoSuchClientException { + public void addClientJwtConfig(String clientId, String keyConfig, String zoneId, boolean overwrite) throws NoSuchClientException { ClientJwtConfiguration clientJwtConfiguration = ClientJwtConfiguration.parse(keyConfig); if (clientJwtConfiguration != null) { BaseClientDetails clientDetails = (BaseClientDetails) loadClientByClientId(clientId, zoneId); @@ -293,7 +293,7 @@ public void addClientKeyConfig(String clientId, String keyConfig, String zoneId, } @Override - public void deleteClientKeyConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException { + public void deleteClientJwtConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException { ClientJwtConfiguration clientJwtConfiguration; if(UaaUrlUtils.isUrl(keyConfig)) { clientJwtConfiguration = ClientJwtConfiguration.parse(keyConfig); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java index 04c85b0ca88..7099db2a3e6 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java @@ -1061,7 +1061,7 @@ void testUpdateClientWithAutoapproveScopesTrue() throws Exception { } @Test - void testCreateClientWithPrivateKeyUri() { + void testCreateClientWithJsonWebKeyUri() { // https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata, see jwks_uri String jwksUri = "https://any.domain.net/openid/jwks-uri"; when(clientDetailsService.retrieve(anyString(), anyString())).thenReturn(input); @@ -1071,7 +1071,7 @@ void testCreateClientWithPrivateKeyUri() { input.setClientSecret("secret"); detail.setAuthorizedGrantTypes(input.getAuthorizedGrantTypes()); ClientDetailsCreation createRequest = createClientDetailsCreation(input); - createRequest.setPrivateKeyUrl(jwksUri); + createRequest.setJsonWebKeyUri(jwksUri); ClientDetails result = endpoints.createClientDetails(createRequest); assertNull(result.getClientSecret()); ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); @@ -1081,7 +1081,7 @@ void testCreateClientWithPrivateKeyUri() { } @Test - void testCreateClientWithPrivateKeyUriInvalid() { + void testCreateClientWithJsonWebKeyUriInvalid() { // invalid jwks_uri String jwksUri = "http://myhost/openid/jwks-uri"; when(clientDetailsService.retrieve(anyString(), anyString())).thenReturn(input); @@ -1091,7 +1091,7 @@ void testCreateClientWithPrivateKeyUriInvalid() { input.setClientSecret("secret"); detail.setAuthorizedGrantTypes(input.getAuthorizedGrantTypes()); ClientDetailsCreation createRequest = createClientDetailsCreation(input); - createRequest.setPrivateKeySet(jwksUri); + createRequest.setJsonWebKeySet(jwksUri); ClientDetails result = endpoints.createClientDetails(createRequest); assertNull(result.getClientSecret()); ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); @@ -1101,7 +1101,7 @@ void testCreateClientWithPrivateKeyUriInvalid() { } @Test - void testAddPrivateKeyJwtConfigUri() { + void testAddClientJwtConfigUri() { when(mockSecurityContextAccessor.getClientId()).thenReturn("bar"); when(mockSecurityContextAccessor.isClient()).thenReturn(true); when(mockSecurityContextAccessor.isAdmin()).thenReturn(true); @@ -1111,20 +1111,20 @@ void testAddPrivateKeyJwtConfigUri() { ClientJwtChangeRequest change = new ClientJwtChangeRequest(); // https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata, see jwks_uri String jwksUri = "https://any.domain.net/openid/jwks-uri"; - change.setKeyUrl(jwksUri); + change.setJsonWebKeyUri(jwksUri); change.setChangeMode(ClientJwtChangeRequest.ChangeMode.ADD); ActionResult result = endpoints.changeClientJwt(detail.getClientId(), change); assertEquals("Client jwt configuration is added", result.getMessage()); - verify(clientRegistrationService, times(1)).addClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), false); + verify(clientRegistrationService, times(1)).addClientJwtConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), false); - change.setKeyUrl(null); + change.setJsonWebKeyUri(null); result = endpoints.changeClientJwt(detail.getClientId(), change); assertEquals("No key added", result.getMessage()); } @Test - void testChangeDeletePrivateKeyJwtConfigUri() { + void testChangeDeleteClientJwtConfigUri() { when(mockSecurityContextAccessor.getClientId()).thenReturn("bar"); when(mockSecurityContextAccessor.isClient()).thenReturn(true); when(mockSecurityContextAccessor.isAdmin()).thenReturn(true); @@ -1134,30 +1134,30 @@ void testChangeDeletePrivateKeyJwtConfigUri() { ClientJwtChangeRequest change = new ClientJwtChangeRequest(); // https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata, see jwks_uri String jwksUri = "https://any.domain.net/openid/jwks-uri"; - change.setKeyUrl(jwksUri); + change.setJsonWebKeyUri(jwksUri); change.setChangeMode(ClientJwtChangeRequest.ChangeMode.ADD); ActionResult result = endpoints.changeClientJwt(detail.getClientId(), change); assertEquals("Client jwt configuration is added", result.getMessage()); - verify(clientRegistrationService, times(1)).addClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), false); + verify(clientRegistrationService, times(1)).addClientJwtConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), false); jwksUri = "https://any.new.domain.net/openid/jwks-uri"; change.setChangeMode(ClientJwtChangeRequest.ChangeMode.UPDATE); - change.setKeyUrl(jwksUri); + change.setJsonWebKeyUri(jwksUri); result = endpoints.changeClientJwt(detail.getClientId(), change); assertEquals("Client jwt configuration updated", result.getMessage()); - verify(clientRegistrationService, times(1)).addClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), true); + verify(clientRegistrationService, times(1)).addClientJwtConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId(), true); ClientJwtConfiguration.parse(jwksUri).writeValue(detail); change.setChangeMode(ClientJwtChangeRequest.ChangeMode.DELETE); - change.setKeyUrl(jwksUri); + change.setJsonWebKeyUri(jwksUri); result = endpoints.changeClientJwt(detail.getClientId(), change); assertEquals("Client jwt configuration is deleted", result.getMessage()); - verify(clientRegistrationService, times(1)).deleteClientKeyConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId()); + verify(clientRegistrationService, times(1)).deleteClientJwtConfig(detail.getClientId(), jwksUri, IdentityZoneHolder.get().getId()); } @Test - void testCreateClientWithPrivateKeySet() { + void testCreateClientWithJsonKeyWebSet() { // Example JWK, a key is bound to a kid, means assumption is, a key is the same if kid is the same String jsonJwk = "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}"; String jsonJwk2 = "{\"kty\":\"RSA\",\"e\":\"\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"\"}"; @@ -1170,7 +1170,7 @@ void testCreateClientWithPrivateKeySet() { input.setClientSecret("secret"); detail.setAuthorizedGrantTypes(input.getAuthorizedGrantTypes()); ClientDetailsCreation createRequest = createClientDetailsCreation(input); - createRequest.setPrivateKeySet(jsonJwk); + createRequest.setJsonWebKeySet(jsonJwk); ClientDetails result = endpoints.createClientDetails(createRequest); assertNull(result.getClientSecret()); ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfigurationTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfigurationTest.java index f7a22c77df7..8c61ef6c535 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfigurationTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfigurationTest.java @@ -76,7 +76,7 @@ void testGetCleanConfig() { @Test void testGetCleanConfigInvalid() { JsonWebKeySet mockedKey = mock(JsonWebKeySet.class); - List keyList = ClientJwtConfiguration.parse(jsonJwkSet).getPrivateKeyJwt().getKeys(); + List keyList = ClientJwtConfiguration.parse(jsonJwkSet).getJwkSet().getKeys(); when(mockedKey.getKeys()).thenReturn(keyList); ClientJwtConfiguration privateKey = new ClientJwtConfiguration(null, mockedKey); when(mockedKey.getKeySetMap()).thenThrow(new IllegalStateException("error")); @@ -88,7 +88,7 @@ void testGetCleanConfigInvalid() { @Test void testJwtSetValidate() { JsonWebKeySet mockedKey = mock(JsonWebKeySet.class); - List keyList = ClientJwtConfiguration.parse(jsonJwkSet).getPrivateKeyJwt().getKeys(); + List keyList = ClientJwtConfiguration.parse(jsonJwkSet).getJwkSet().getKeys(); when(mockedKey.getKeys()).thenReturn(Arrays.asList(keyList.get(0), keyList.get(0))); assertThrows(InvalidClientDetailsException.class, () -> new ClientJwtConfiguration(null, mockedKey)); } @@ -96,58 +96,58 @@ void testJwtSetValidate() { @Test void testConfigMerge() { ClientJwtConfiguration configuration = ClientJwtConfiguration.parse(jsonJwkSet); - assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + assertEquals(1, configuration.getJwkSet().getKeys().size()); ClientJwtConfiguration addKey = ClientJwtConfiguration.parse(jsonWebKey2); configuration = ClientJwtConfiguration.merge(configuration, addKey, false); - assertEquals(2, configuration.getPrivateKeyJwt().getKeys().size()); - assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); - assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(1).getKeyProperties().get("n")); + assertEquals(2, configuration.getJwkSet().getKeys().size()); + assertEquals(nValue, configuration.getJwkSet().getKeys().get(0).getKeyProperties().get("n")); + assertEquals(nValue, configuration.getJwkSet().getKeys().get(1).getKeyProperties().get("n")); configuration = ClientJwtConfiguration.merge(configuration, addKey, true); - assertEquals(2, configuration.getPrivateKeyJwt().getKeys().size()); + assertEquals(2, configuration.getJwkSet().getKeys().size()); configuration = ClientJwtConfiguration.parse(jsonJwkSet); - assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); - assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); + assertEquals(1, configuration.getJwkSet().getKeys().size()); + assertEquals(nValue, configuration.getJwkSet().getKeys().get(0).getKeyProperties().get("n")); configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse(jsonJwkSet), ClientJwtConfiguration.parse(jsonWebKeyDifferentValue), true); - assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); - assertEquals("new", configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); + assertEquals(1, configuration.getJwkSet().getKeys().size()); + assertEquals("new", configuration.getJwkSet().getKeys().get(0).getKeyProperties().get("n")); configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse(jsonJwkSet), ClientJwtConfiguration.parse(jsonWebKeyDifferentValue), false); - assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); - assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); + assertEquals(1, configuration.getJwkSet().getKeys().size()); + assertEquals(nValue, configuration.getJwkSet().getKeys().get(0).getKeyProperties().get("n")); } @Test void testConfigMergeDifferentType() { ClientJwtConfiguration configuration = ClientJwtConfiguration.parse(jsonJwkSet); - assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); - assertNull(configuration.getPrivateKeyJwtUrl()); + assertEquals(1, configuration.getJwkSet().getKeys().size()); + assertNull(configuration.getJwksUri()); configuration = ClientJwtConfiguration.merge(configuration, ClientJwtConfiguration.parse("https://any/jwks-uri"), false); - assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); - assertNull(configuration.getPrivateKeyJwtUrl()); + assertEquals(1, configuration.getJwkSet().getKeys().size()); + assertNull(configuration.getJwksUri()); configuration = ClientJwtConfiguration.merge(configuration, ClientJwtConfiguration.parse("https://any/jwks-uri"), true); - assertNull(configuration.getPrivateKeyJwt()); - assertNotNull(configuration.getPrivateKeyJwtUrl()); + assertNull(configuration.getJwkSet()); + assertNotNull(configuration.getJwksUri()); configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse("https://any/jwks-uri"), ClientJwtConfiguration.parse("https://new/jwks-uri"), false); - assertNull(configuration.getPrivateKeyJwt()); - assertEquals("https://any/jwks-uri", configuration.getPrivateKeyJwtUrl()); + assertNull(configuration.getJwkSet()); + assertEquals("https://any/jwks-uri", configuration.getJwksUri()); configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse("https://any/jwks-uri"), ClientJwtConfiguration.parse("https://new/jwks-uri"), true); - assertNull(configuration.getPrivateKeyJwt()); - assertEquals("https://new/jwks-uri", configuration.getPrivateKeyJwtUrl()); + assertNull(configuration.getJwkSet()); + assertEquals("https://new/jwks-uri", configuration.getJwksUri()); configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse("https://any/jwks-uri"), ClientJwtConfiguration.parse(jsonJwkSet), false); - assertNull(configuration.getPrivateKeyJwt()); - assertEquals("https://any/jwks-uri", configuration.getPrivateKeyJwtUrl()); + assertNull(configuration.getJwkSet()); + assertEquals("https://any/jwks-uri", configuration.getJwksUri()); configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse("https://any/jwks-uri"), ClientJwtConfiguration.parse(jsonJwkSet), true); - assertNull(configuration.getPrivateKeyJwtUrl()); - assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); - assertEquals(nValue, configuration.getPrivateKeyJwt().getKeys().get(0).getKeyProperties().get("n")); + assertNull(configuration.getJwksUri()); + assertEquals(1, configuration.getJwkSet().getKeys().size()); + assertEquals(nValue, configuration.getJwkSet().getKeys().get(0).getKeyProperties().get("n")); } @Test @@ -166,19 +166,19 @@ void testConfigMergeNulls() { @Test void testConfigDelete() { ClientJwtConfiguration configuration = ClientJwtConfiguration.parse(jsonJwkSet); - assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); - assertNull(configuration.getPrivateKeyJwtUrl()); + assertEquals(1, configuration.getJwkSet().getKeys().size()); + assertNull(configuration.getJwksUri()); ClientJwtConfiguration addKey = ClientJwtConfiguration.parse(jsonWebKey2); configuration = ClientJwtConfiguration.merge(configuration, addKey, false); - assertEquals(2, configuration.getPrivateKeyJwt().getKeys().size()); + assertEquals(2, configuration.getJwkSet().getKeys().size()); configuration = ClientJwtConfiguration.delete(configuration, addKey); - assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + assertEquals(1, configuration.getJwkSet().getKeys().size()); configuration = ClientJwtConfiguration.delete(configuration, addKey); configuration = ClientJwtConfiguration.delete(configuration, addKey); - assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + assertEquals(1, configuration.getJwkSet().getKeys().size()); configuration = ClientJwtConfiguration.merge(configuration, addKey, false); configuration = ClientJwtConfiguration.delete(configuration, addKey); - assertEquals(1, configuration.getPrivateKeyJwt().getKeys().size()); + assertEquals(1, configuration.getJwkSet().getKeys().size()); configuration = ClientJwtConfiguration.merge(configuration, addKey, false); configuration = ClientJwtConfiguration.delete(configuration, new ClientJwtConfiguration("key-2", null)); configuration = ClientJwtConfiguration.delete(configuration, new ClientJwtConfiguration("key-1", null)); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java index 700f9959027..d7cbd7e2ba7 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java @@ -49,12 +49,12 @@ public void deleteClientSecret(String clientId, String zoneId) throws NoSuchClie } @Override - public void addClientKeyConfig(String clientId, String keyConfig, String zoneId, boolean overwrite) throws NoSuchClientException { + public void addClientJwtConfig(String clientId, String keyConfig, String zoneId, boolean overwrite) throws NoSuchClientException { throw new UnsupportedOperationException(); } @Override - public void deleteClientKeyConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException { + public void deleteClientJwtConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException { throw new UnsupportedOperationException(); } From 187dbb93aaf667b785efa8ae2a9a93d7d3745436 Mon Sep 17 00:00:00 2001 From: strehle Date: Thu, 24 Aug 2023 15:11:05 +0200 Subject: [PATCH 09/22] Add column client_jwt_config and do some refactoring for UaaClientDetails usage --- .../identity/uaa/client/UaaClientDetails.java | 42 ++++++++++++++- .../uaa/zone/MultitenantClientServices.java | 5 ++ .../MultitenantJdbcClientDetailsService.java | 52 +++++++++++++------ ...lientJwtConfig_To_Oauth_Client_Details.sql | 1 + ...lientJwtConfig_To_Oauth_Client_Details.sql | 1 + ...lientJwtConfig_To_Oauth_Client_Details.sql | 1 + .../InMemoryMultitenantClientServices.java | 11 ++++ ...titenantJdbcClientDetailsServiceTests.java | 35 +++++++++++-- 8 files changed, 127 insertions(+), 21 deletions(-) create mode 100644 server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_104__Add_ClientJwtConfig_To_Oauth_Client_Details.sql create mode 100644 server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_104__Add_ClientJwtConfig_To_Oauth_Client_Details.sql create mode 100644 server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_104__Add_ClientJwtConfig_To_Oauth_Client_Details.sql diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java index 448ba9a2e16..07917c9fe53 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java @@ -2,12 +2,14 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; import org.springframework.security.core.SpringSecurityCoreVersion; import org.springframework.security.oauth2.provider.ClientDetails; import org.springframework.security.oauth2.provider.client.BaseClientDetails; import java.util.Arrays; import java.util.Collection; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -17,15 +19,22 @@ public class UaaClientDetails extends BaseClientDetails { private static final long serialVersionUID = SpringSecurityCoreVersion.SERIAL_VERSION_UID; + @JsonProperty("client_jwt_config") + private String clientJwtConfig; + + public UaaClientDetails() { + } + UaaClientDetails(ClientDetails prototype) { super(prototype); this.setAdditionalInformation(prototype.getAdditionalInformation()); } - public UaaClientDetails(String clientId, String clientSecret, String resourceIds, + public UaaClientDetails(String clientId, String clientSecret, String clientJwtConfig, String resourceIds, String scopes, String grantTypes, String authorities, String redirectUris) { super(clientId, resourceIds, scopes, grantTypes, authorities, redirectUris); setClientSecret(clientSecret); + this.clientJwtConfig = clientJwtConfig; } @Override @@ -35,4 +44,35 @@ public void setScope(Collection scope) { .collect(Collectors.toSet()); super.setScope(sanitized); } + + public String getClientJwtConfig() { + return clientJwtConfig; + } + + public void setClientJwtConfig(String clientJwtConfig) { + this.clientJwtConfig = clientJwtConfig; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (o instanceof UaaClientDetails) { + UaaClientDetails uaaClientDetails = (UaaClientDetails) o; + return Objects.equals(clientJwtConfig, uaaClientDetails.clientJwtConfig); + } + return false; + } + + @Override + public int hashCode() { + int result = super.hashCode(); + + result = 31 * result + (clientJwtConfig != null ? clientJwtConfig.hashCode() : 0); + return result; + } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java index 528ba225c7e..e1411b38df4 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java @@ -1,5 +1,6 @@ package org.cloudfoundry.identity.uaa.zone; +import org.cloudfoundry.identity.uaa.client.UaaClientDetails; import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; import org.springframework.security.oauth2.provider.*; @@ -11,8 +12,12 @@ interface MultitenantClientRegistrationService extends ClientRegistrationService void updateClientDetails(ClientDetails clientDetails, String zoneId) throws NoSuchClientException; + void addUaaClientDetails(UaaClientDetails uaaClientDetails, String zoneId) throws ClientAlreadyExistsException; + void updateClientSecret(String clientId, String secret, String zoneId) throws NoSuchClientException; + void updateClientJwtConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException; + void removeClientDetails(String clientId, String zoneId) throws NoSuchClientException; List listClientDetails(String zoneId); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java index ca00b5ee7c5..0118205e529 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java @@ -64,7 +64,7 @@ public class MultitenantJdbcClientDetailsService extends MultitenantClientServic "authorized_grant_types, web_server_redirect_uri, authorities, access_token_validity, " + "refresh_token_validity, additional_information, autoapprove, lastmodified, required_user_groups"; - private static final String CLIENT_FIELDS = "client_secret, " + CLIENT_FIELDS_FOR_UPDATE; + private static final String CLIENT_FIELDS = "client_secret, client_jwt_config, " + CLIENT_FIELDS_FOR_UPDATE; private static final String BASE_FIND_STATEMENT = "select client_id, " + CLIENT_FIELDS + " from oauth_client_details"; @@ -80,7 +80,7 @@ public class MultitenantJdbcClientDetailsService extends MultitenantClientServic private static final String DEFAULT_INSERT_STATEMENT = "insert into oauth_client_details (" + CLIENT_FIELDS - + ", client_id, identity_zone_id, created_by) values (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)"; + + ", client_id, identity_zone_id, created_by) values (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)"; private static final String DEFAULT_UPDATE_STATEMENT = "update oauth_client_details " + "set " @@ -90,6 +90,10 @@ public class MultitenantJdbcClientDetailsService extends MultitenantClientServic "update oauth_client_details " + "set client_secret = ? where client_id = ? and identity_zone_id = ?"; + private static final String DEFAULT_UPDATE_CLIENT_JWT_CONFIG_STATEMENT = + "update oauth_client_details " + + "set client_jwt_config = ? where client_id = ? and identity_zone_id = ?"; + static final String DEFAULT_DELETE_STATEMENT = "delete from oauth_client_details where client_id = ? and identity_zone_id = ?"; @@ -153,6 +157,14 @@ public void updateClientDetails(ClientDetails clientDetails, String zoneId) thro } } + @Override + public void addUaaClientDetails(UaaClientDetails uaaClientDetails, String zoneId) throws ClientAlreadyExistsException { + if (exists(uaaClientDetails.getClientId(), zoneId)) { + throw new ClientAlreadyExistsException("Client already exists: " + uaaClientDetails.getClientId()); + } + jdbcTemplate.update(DEFAULT_INSERT_STATEMENT, getInsertClientDetailsFields(uaaClientDetails, zoneId)); + } + @Override public void updateClientSecret(String clientId, String secret, String zoneId) throws NoSuchClientException { int count = jdbcTemplate.update(DEFAULT_UPDATE_SECRET_STATEMENT, passwordEncoder.encode(secret), clientId, zoneId); @@ -161,6 +173,14 @@ public void updateClientSecret(String clientId, String secret, String zoneId) th } } + @Override + public void updateClientJwtConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException { + int count = jdbcTemplate.update(DEFAULT_UPDATE_CLIENT_JWT_CONFIG_STATEMENT, keyConfig, clientId, zoneId); + if (count != 1) { + throw new NoSuchClientException("No client found with id = " + clientId); + } + } + @Override public void removeClientDetails(String clientId, String zoneId) throws NoSuchClientException { deleteByClient(clientId, zoneId); @@ -172,12 +192,13 @@ public List listClientDetails(String zoneId) { private Object[] getInsertClientDetailsFields(ClientDetails clientDetails, String zoneId) { Object[] fieldsForUpdate = getFieldsForUpdate(clientDetails, zoneId); - Object[] clientDetailFieldsForUpdate = new Object[fieldsForUpdate.length + 2]; - System.arraycopy(fieldsForUpdate, 0, clientDetailFieldsForUpdate, 1, fieldsForUpdate.length); + Object[] clientDetailFieldsForUpdate = new Object[fieldsForUpdate.length + 3]; + System.arraycopy(fieldsForUpdate, 0, clientDetailFieldsForUpdate, 2, fieldsForUpdate.length); clientDetailFieldsForUpdate[0] = clientDetails.getClientSecret() != null ? passwordEncoder.encode(clientDetails.getClientSecret()) : null; + clientDetailFieldsForUpdate[1] = (clientDetails instanceof UaaClientDetails) ? ((UaaClientDetails) clientDetails).getClientJwtConfig() : null; clientDetailFieldsForUpdate[clientDetailFieldsForUpdate.length - 1] = getUserId(); return clientDetailFieldsForUpdate; } @@ -290,17 +311,18 @@ public ClientDetails mapRow(ResultSet rs, int rowNum) throws SQLException { rs.getString(3), rs.getString(4), rs.getString(5), - rs.getString(7), - rs.getString(6) + rs.getString(6), + rs.getString(8), + rs.getString(7) ); - if (rs.getObject(8) != null) { - details.setAccessTokenValiditySeconds(rs.getInt(8)); - } if (rs.getObject(9) != null) { - details.setRefreshTokenValiditySeconds(rs.getInt(9)); + details.setAccessTokenValiditySeconds(rs.getInt(9)); + } + if (rs.getObject(10) != null) { + details.setRefreshTokenValiditySeconds(rs.getInt(10)); } - String json = rs.getString(10); - String scopes = rs.getString(11); + String json = rs.getString(11); + String scopes = rs.getString(12); Set autoApproveScopes = new HashSet<>(); if (scopes != null) { autoApproveScopes = commaDelimitedListToSet(scopes); @@ -330,12 +352,12 @@ public ClientDetails mapRow(ResultSet rs, int rowNum) throws SQLException { // lastModified - if (rs.getObject(12) != null) { - details.addAdditionalInformation("lastModified", rs.getTimestamp(12)); + if (rs.getObject(13) != null) { + details.addAdditionalInformation("lastModified", rs.getTimestamp(13)); } //required_user_groups - String requiredUserGroups = rs.getString(13); + String requiredUserGroups = rs.getString(14); if (StringUtils.isEmpty(requiredUserGroups)) { details.addAdditionalInformation(REQUIRED_USER_GROUPS, emptySet()); } else { diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_104__Add_ClientJwtConfig_To_Oauth_Client_Details.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_104__Add_ClientJwtConfig_To_Oauth_Client_Details.sql new file mode 100644 index 00000000000..a44ddb41de9 --- /dev/null +++ b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_104__Add_ClientJwtConfig_To_Oauth_Client_Details.sql @@ -0,0 +1 @@ +ALTER TABLE oauth_client_details ADD COLUMN client_jwt_config CLOB DEFAULT NULL; \ No newline at end of file diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_104__Add_ClientJwtConfig_To_Oauth_Client_Details.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_104__Add_ClientJwtConfig_To_Oauth_Client_Details.sql new file mode 100644 index 00000000000..c86c41044d3 --- /dev/null +++ b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_104__Add_ClientJwtConfig_To_Oauth_Client_Details.sql @@ -0,0 +1 @@ +ALTER TABLE oauth_client_details ADD COLUMN client_jwt_config TEXT DEFAULT NULL; \ No newline at end of file diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_104__Add_ClientJwtConfig_To_Oauth_Client_Details.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_104__Add_ClientJwtConfig_To_Oauth_Client_Details.sql new file mode 100644 index 00000000000..c86c41044d3 --- /dev/null +++ b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_104__Add_ClientJwtConfig_To_Oauth_Client_Details.sql @@ -0,0 +1 @@ +ALTER TABLE oauth_client_details ADD COLUMN client_jwt_config TEXT DEFAULT NULL; \ No newline at end of file diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java index c46478fafe5..1cdd7cd47e3 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java @@ -1,5 +1,6 @@ package org.cloudfoundry.identity.uaa.zone; +import org.cloudfoundry.identity.uaa.client.UaaClientDetails; import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; import org.springframework.security.oauth2.provider.ClientAlreadyExistsException; import org.springframework.security.oauth2.provider.ClientDetails; @@ -58,6 +59,11 @@ public void updateClientDetails(ClientDetails clientDetails, String zoneId) thro addClientDetails(clientDetails, zoneId); } + @Override + public void addUaaClientDetails(UaaClientDetails uaaClientDetails, String zoneId) throws ClientAlreadyExistsException { + getInMemoryService(zoneId).put(uaaClientDetails.getClientId(), (BaseClientDetails) uaaClientDetails); + } + @Override public void updateClientSecret(String clientId, String secret, String zoneId) throws NoSuchClientException { ofNullable((BaseClientDetails) loadClientByClientId(clientId, zoneId)).ifPresent(client -> @@ -65,6 +71,11 @@ public void updateClientSecret(String clientId, String secret, String zoneId) th ); } + @Override + public void updateClientJwtConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException { + throw new UnsupportedOperationException(); + } + @Override public void removeClientDetails(String clientId, String zoneId) throws NoSuchClientException { getInMemoryService(zoneId).remove(clientId); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java index a34d7b8bb7e..67bd965e06d 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java @@ -4,6 +4,7 @@ import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent; import org.cloudfoundry.identity.uaa.authentication.UaaAuthentication; import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationTestFactory; +import org.cloudfoundry.identity.uaa.client.UaaClientDetails; import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.login.util.RandomValueStringGenerator; import org.cloudfoundry.identity.uaa.oauth.UaaOauth2Authentication; @@ -67,9 +68,9 @@ class MultitenantJdbcClientDetailsServiceTests { private MultitenantJdbcClientDetailsService service; - private static final String SELECT_SQL = "select client_id, client_secret, resource_ids, scope, authorized_grant_types, web_server_redirect_uri, authorities, access_token_validity, refresh_token_validity, lastmodified, required_user_groups from oauth_client_details where client_id=?"; + private static final String SELECT_SQL = "select client_id, client_secret, client_jwt_config, resource_ids, scope, authorized_grant_types, web_server_redirect_uri, authorities, access_token_validity, refresh_token_validity, lastmodified, required_user_groups from oauth_client_details where client_id=?"; - private static final String INSERT_SQL = "insert into oauth_client_details (client_id, client_secret, resource_ids, scope, authorized_grant_types, web_server_redirect_uri, authorities, access_token_validity, refresh_token_validity, autoapprove, identity_zone_id, lastmodified, required_user_groups) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,?)"; + private static final String INSERT_SQL = "insert into oauth_client_details (client_id, client_secret, client_jwt_config, resource_ids, scope, authorized_grant_types, web_server_redirect_uri, authorities, access_token_validity, refresh_token_validity, autoapprove, identity_zone_id, lastmodified, required_user_groups) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,?,?)"; private RandomValueStringGenerator randomValueStringGenerator; @@ -211,7 +212,7 @@ void loadingClientIdWithNoDetails() { int rowsInserted = jdbcTemplate.update(INSERT_SQL, "clientIdWithNoDetails", null, null, null, null, null, null, null, null, null, - currentZoneId, + null, currentZoneId, new Timestamp(System.currentTimeMillis()), dbRequestedUserGroups ); @@ -243,7 +244,7 @@ void loadingClientIdWithAdditionalInformation() { jdbcTemplate.update(INSERT_SQL, "clientIdWithAddInfo", null, null, null, null, null, null, null, null, null, - currentZoneId, lastModifiedDate, + null, currentZoneId, lastModifiedDate, dbRequestedUserGroups); jdbcTemplate .update("update oauth_client_details set additional_information=? where client_id=?", @@ -269,7 +270,7 @@ void autoApproveOnlyReturnedInField_andNotInAdditionalInfo() { String clientId = "client-with-autoapprove"; jdbcTemplate.update(INSERT_SQL, clientId, null, null, - null, null, null, null, null, null, "foo.read", currentZoneId, lastModifiedDate, dbRequestedUserGroups); + null, null, null, null, null, null, null, "foo.read", currentZoneId, lastModifiedDate, dbRequestedUserGroups); jdbcTemplate .update("update oauth_client_details set additional_information=? where client_id=?", "{\"autoapprove\":[\"bar.read\"]}", clientId); @@ -294,6 +295,7 @@ void loadingClientIdWithSingleDetails() { jdbcTemplate.update(INSERT_SQL, "clientIdWithSingleDetails", "mySecret", + "myClientJwtConfig", "myResource", "myScope", "myAuthorizedGrantType", @@ -336,6 +338,7 @@ void loadGroupsGeneratesEmptyCollection() { jdbcTemplate.update(INSERT_SQL, clientId, "mySecret", + "myClientJwtConfig", "myResource", "myScope", "myAuthorizedGrantType", @@ -387,6 +390,7 @@ void loadingClientIdWithMultipleDetails() { jdbcTemplate.update(INSERT_SQL, "clientIdWithMultipleDetails", "mySecret", + "myClientJwtConfig", "myResource1,myResource2", "myScope1,myScope2", "myAuthorizedGrantType1,myAuthorizedGrantType2", @@ -534,6 +538,27 @@ void deleteClientSecretForInvalidClient() { containsString("No client with requested id: invalid_client_id")); } + @Test + void updateClientJwtConfig() { + UaaClientDetails clientDetails = new UaaClientDetails(); + clientDetails.setClientId("newClientIdWithClientJwtConfig"); + clientDetails.setClientJwtConfig("small"); + service.addUaaClientDetails(clientDetails, mockIdentityZoneManager.getCurrentIdentityZoneId()); + + Map map = jdbcTemplate.queryForMap(SELECT_SQL, + "newClientIdWithClientJwtConfig"); + assertEquals("small", (String) map.get("client_jwt_config")); + + service.updateClientJwtConfig(clientDetails.getClientId(), "any json web key config", mockIdentityZoneManager.getCurrentIdentityZoneId()); + + map = jdbcTemplate.queryForMap(SELECT_SQL, + "newClientIdWithClientJwtConfig"); + + assertEquals("newClientIdWithClientJwtConfig", map.get("client_id")); + assertTrue(map.containsKey("client_jwt_config")); + assertEquals("any json web key config", (String) map.get("client_jwt_config")); + } + @Test void updateClientRedirectURI() { From 82a5857407d01686cac4cf7b70677ed3df42a90b Mon Sep 17 00:00:00 2001 From: strehle Date: Thu, 24 Aug 2023 15:44:21 +0200 Subject: [PATCH 10/22] Sonar findings --- .../identity/uaa/client/UaaClientDetails.java | 5 ++- .../MultitenantJdbcClientDetailsService.java | 4 +-- .../uaa/client/UaaClientDetailsTest.java | 34 ++++++++++++++++++- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java index 07917c9fe53..d2b62db8403 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java @@ -27,13 +27,12 @@ public UaaClientDetails() { UaaClientDetails(ClientDetails prototype) { super(prototype); - this.setAdditionalInformation(prototype.getAdditionalInformation()); + setAdditionalInformation(prototype.getAdditionalInformation()); } - public UaaClientDetails(String clientId, String clientSecret, String clientJwtConfig, String resourceIds, + public UaaClientDetails(String clientId, String resourceIds, String scopes, String grantTypes, String authorities, String redirectUris) { super(clientId, resourceIds, scopes, grantTypes, authorities, redirectUris); - setClientSecret(clientSecret); this.clientJwtConfig = clientJwtConfig; } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java index 0118205e529..9d3cdb85f44 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java @@ -307,14 +307,14 @@ private static class ClientDetailsRowMapper implements RowMapper public ClientDetails mapRow(ResultSet rs, int rowNum) throws SQLException { UaaClientDetails details = new UaaClientDetails( rs.getString(1), - rs.getString(2), - rs.getString(3), rs.getString(4), rs.getString(5), rs.getString(6), rs.getString(8), rs.getString(7) ); + details.setClientSecret(rs.getString(2)); + details.setClientJwtConfig(rs.getString(3)); if (rs.getObject(9) != null) { details.setAccessTokenValiditySeconds(rs.getInt(9)); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsTest.java index 66b6864256b..bcd561d9ee1 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsTest.java @@ -19,9 +19,12 @@ import static org.hamcrest.collection.IsIterableContainingInOrder.contains; import static org.hamcrest.collection.IsMapContaining.hasEntry; import static org.hamcrest.collection.IsMapWithSize.aMapWithSize; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; class UaaClientDetailsTest { - @Nested + + @Nested class Creation { private BaseClientDetails testClient; @@ -63,6 +66,35 @@ void copiesAdditionalInformation() { .withAdditionalInformation(allOf(aMapWithSize(1), hasEntry("key", "value"))) )); } + + @Test + void testClientJwtConfig() { + UaaClientDetails copy = new UaaClientDetails(testClient); + copy.setClientJwtConfig("test"); + assertEquals("test", copy.getClientJwtConfig()); + } + + @Test + void testEquals() { + UaaClientDetails copy = new UaaClientDetails(testClient); + UaaClientDetails copy2 = new UaaClientDetails(testClient); + copy.setClientJwtConfig("test"); + assertNotEquals(copy, copy2); + assertNotEquals(copy, new UaaClientDetails()); + copy.setClientJwtConfig(null); + assertEquals(copy, copy2); + } + + @Test + void testHashCode() { + UaaClientDetails copy = new UaaClientDetails(testClient); + UaaClientDetails copy2 = new UaaClientDetails(testClient.getClientId(), "", + "test.none", "", "test.admin", null); + copy.setClientJwtConfig("test"); + assertNotEquals(copy.hashCode(), copy2.hashCode()); + copy.setClientJwtConfig(null); + assertEquals(copy.hashCode(), copy2.hashCode()); + } } @Nested From bf6f7b2356d8952aed3aea7b035d24e666c4ee21 Mon Sep 17 00:00:00 2001 From: strehle Date: Thu, 24 Aug 2023 15:50:21 +0200 Subject: [PATCH 11/22] cleanup --- .../org/cloudfoundry/identity/uaa/client/UaaClientDetails.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java index d2b62db8403..708ac46949d 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java @@ -27,13 +27,12 @@ public UaaClientDetails() { UaaClientDetails(ClientDetails prototype) { super(prototype); - setAdditionalInformation(prototype.getAdditionalInformation()); + this.setAdditionalInformation(prototype.getAdditionalInformation()); } public UaaClientDetails(String clientId, String resourceIds, String scopes, String grantTypes, String authorities, String redirectUris) { super(clientId, resourceIds, scopes, grantTypes, authorities, redirectUris); - this.clientJwtConfig = clientJwtConfig; } @Override From 415983bb9385d5c90bce4a605162e225c946149b Mon Sep 17 00:00:00 2001 From: strehle Date: Thu, 24 Aug 2023 17:07:33 +0200 Subject: [PATCH 12/22] Refactoring because of usage of client_jwt_config now from oauth_client_details additional_information is not used anymore --- .../uaa/oauth/client/ClientConstants.java | 1 - .../uaa/client/ClientAdminBootstrap.java | 31 +++++++++-------- .../uaa/client/ClientAdminEndpoints.java | 8 ++--- .../uaa/client/ClientJwtConfiguration.java | 34 +++++++------------ .../MultitenantJdbcClientDetailsService.java | 18 +++++----- .../uaa/client/ClientAdminBootstrapTests.java | 26 +++++++------- .../uaa/client/ClientAdminEndpointsTests.java | 12 +++---- .../client/ClientJwtConfigurationTest.java | 21 +++++------- 8 files changed, 69 insertions(+), 82 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientConstants.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientConstants.java index 6cc4dde5cc9..79d333b8da5 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientConstants.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientConstants.java @@ -21,6 +21,5 @@ public class ClientConstants { public static final String APPROVALS_DELETED = "approvals_deleted"; public static final String TOKEN_SALT = "token_salt"; public static final String REQUIRED_USER_GROUPS = "required_user_groups"; - public static final String PRIVATE_KEY_CONFIG = "private_key_config"; public static final String LAST_MODIFIED = "lastModified"; } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java index 598bd38845c..dcd5783c363 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java @@ -159,7 +159,7 @@ private void addNewClients() { if (map.get("authorized-grant-types") == null) { throw new InvalidClientDetailsException("Client must have at least one authorized-grant-type. client ID: " + clientId); } - BaseClientDetails client = new BaseClientDetails(clientId, (String) map.get("resource-ids"), + UaaClientDetails client = new UaaClientDetails(clientId, (String) map.get("resource-ids"), (String) map.get("scope"), (String) map.get("authorized-grant-types"), (String) map.get("authorities"), getRedirectUris(map)); @@ -210,6 +210,21 @@ private void addNewClients() { } client.setAdditionalInformation(info); + + if (map.get("jwks_uri") instanceof String) { + String jwksUri = (String) map.get("jwks_uri"); + ClientJwtConfiguration keyConfig = ClientJwtConfiguration.parse(UaaUrlUtils.normalizeUri(jwksUri), null); + if (keyConfig != null && keyConfig.getCleanString() != null) { + keyConfig.writeValue(client); + } + } else if (map.get("jwks") instanceof String) { + String jwks = (String) map.get("jwks"); + ClientJwtConfiguration keyConfig = ClientJwtConfiguration.parse(null, jwks); + if (keyConfig != null && keyConfig.getCleanString() != null) { + keyConfig.writeValue(client); + } + } + try { clientRegistrationService.addClientDetails(client, IdentityZone.getUaaZoneId()); if (secondSecret != null) { @@ -236,20 +251,6 @@ private void addNewClients() { } } - if (map.get("jwks_uri") instanceof String) { - String jwksUri = (String) map.get("jwks_uri"); - ClientJwtConfiguration keyConfig = ClientJwtConfiguration.parse(UaaUrlUtils.normalizeUri(jwksUri), null); - if (keyConfig != null && keyConfig.getCleanString() != null) { - clientRegistrationService.addClientJwtConfig(clientId, keyConfig.getJwksUri(), IdentityZone.getUaaZoneId(), override); - } - } else if (map.get("jwks") instanceof String) { - String jwks = (String) map.get("jwks"); - ClientJwtConfiguration keyConfig = ClientJwtConfiguration.parse(null, jwks); - if (keyConfig != null && keyConfig.getCleanString() != null) { - clientRegistrationService.addClientJwtConfig(clientId, keyConfig.getCleanString(), IdentityZone.getUaaZoneId(), override); - } - } - ClientMetadata clientMetadata = buildClientMetadata(map, clientId); clientMetadataProvisioning.update(clientMetadata, IdentityZone.getUaaZoneId()); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java index 9b3d8b14062..6d52740e88a 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java @@ -540,20 +540,20 @@ public ActionResult changeSecret(@PathVariable String client_id, @RequestBody Se @ResponseBody public ActionResult changeClientJwt(@PathVariable String client_id, @RequestBody ClientJwtChangeRequest change) { - ClientDetails clientDetails; + UaaClientDetails uaaClientDetails; try { - clientDetails = clientDetailsService.retrieve(client_id, IdentityZoneHolder.get().getId()); + uaaClientDetails = (UaaClientDetails) clientDetailsService.retrieve(client_id, IdentityZoneHolder.get().getId()); } catch (InvalidClientException e) { throw new NoSuchClientException("No such client: " + client_id); } try { - checkPasswordChangeIsAllowed(clientDetails, ""); + checkPasswordChangeIsAllowed(uaaClientDetails, ""); } catch (IllegalStateException e) { throw new InvalidClientDetailsException(e.getMessage()); } - ClientJwtConfiguration clientKeyConfig = ClientJwtConfiguration.readValue(clientDetails); + ClientJwtConfiguration clientKeyConfig = ClientJwtConfiguration.readValue(uaaClientDetails); ActionResult result; switch (change.getChangeMode()){ 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 dd664c8b842..78e8c318737 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 @@ -12,7 +12,6 @@ import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.util.UaaUrlUtils; import org.springframework.security.oauth2.provider.ClientDetails; -import org.springframework.security.oauth2.provider.client.BaseClientDetails; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; @@ -23,12 +22,9 @@ import java.util.HashSet; import java.util.List; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; -import static org.cloudfoundry.identity.uaa.oauth.client.ClientConstants.PRIVATE_KEY_CONFIG; - @JsonInclude(JsonInclude.Include.NON_NULL) @JsonIgnoreProperties(ignoreUnknown = true) public class ClientJwtConfiguration implements Cloneable{ @@ -186,52 +182,48 @@ private boolean validateJwksUri() { /** * Creator from ClientDetails. Should abstract the persistence. - * Use currently the additional information entry + * Use currently the client_jwt_config in UaaClientDetails * * @param clientDetails * @return */ @JsonIgnore - public static ClientJwtConfiguration readValue(ClientDetails clientDetails) { + public static ClientJwtConfiguration readValue(UaaClientDetails clientDetails) { if (clientDetails == null || - clientDetails.getAdditionalInformation() == null || - !(clientDetails.getAdditionalInformation().get(PRIVATE_KEY_CONFIG) instanceof String)) { + clientDetails.getClientJwtConfig() == null || + !(clientDetails.getClientJwtConfig() instanceof String)) { return null; } - return JsonUtils.readValue((String) clientDetails.getAdditionalInformation().get(PRIVATE_KEY_CONFIG), ClientJwtConfiguration.class); + return JsonUtils.readValue((String) clientDetails.getClientJwtConfig(), ClientJwtConfiguration.class); } /** * Creator from ClientDetails. Should abstract the persistence. - * Use currently the additional information entry + * Use currently the client_jwt_config in UaaClientDetails * * @param clientDetails * @return */ @JsonIgnore public void writeValue(ClientDetails clientDetails) { - if (clientDetails instanceof BaseClientDetails) { - BaseClientDetails baseClientDetails = (BaseClientDetails) clientDetails; - HashMap additionalInformation = Optional.ofNullable(baseClientDetails.getAdditionalInformation()).map(HashMap::new).orElse(new HashMap<>()); - additionalInformation.put(PRIVATE_KEY_CONFIG, JsonUtils.writeValueAsString(this)); - baseClientDetails.setAdditionalInformation(additionalInformation); + if (clientDetails instanceof UaaClientDetails) { + UaaClientDetails uaaClientDetails = (UaaClientDetails) clientDetails; + uaaClientDetails.setClientJwtConfig(JsonUtils.writeValueAsString(this)); } } /** * Cleanup configuration in ClientDetails. Should abstract the persistence. - * Use currently the additional information entry + * Use currently the client_jwt_config in UaaClientDetails * * @param clientDetails * @return */ @JsonIgnore public static void resetConfiguration(ClientDetails clientDetails) { - if (clientDetails instanceof BaseClientDetails) { - BaseClientDetails baseClientDetails = (BaseClientDetails) clientDetails; - HashMap additionalInformation = Optional.ofNullable(baseClientDetails.getAdditionalInformation()).map(HashMap::new).orElse(new HashMap<>()); - additionalInformation.remove(PRIVATE_KEY_CONFIG); - baseClientDetails.setAdditionalInformation(additionalInformation); + if (clientDetails instanceof UaaClientDetails) { + UaaClientDetails uaaClientDetails = (UaaClientDetails) clientDetails; + uaaClientDetails.setClientJwtConfig(null); } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java index 1869b094a3d..ed3fe82ebe6 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java @@ -303,13 +303,13 @@ public void deleteClientSecret(String clientId, String zoneId) throws NoSuchClie public void addClientJwtConfig(String clientId, String keyConfig, String zoneId, boolean overwrite) throws NoSuchClientException { ClientJwtConfiguration clientJwtConfiguration = ClientJwtConfiguration.parse(keyConfig); if (clientJwtConfiguration != null) { - BaseClientDetails clientDetails = (BaseClientDetails) loadClientByClientId(clientId, zoneId); - ClientJwtConfiguration existingConfig = ClientJwtConfiguration.readValue(clientDetails); + UaaClientDetails uaaClientDetails = (UaaClientDetails) loadClientByClientId(clientId, zoneId); + ClientJwtConfiguration existingConfig = ClientJwtConfiguration.readValue(uaaClientDetails); ClientJwtConfiguration result = ClientJwtConfiguration.merge(existingConfig, clientJwtConfiguration, overwrite); if (result != null) { - result.writeValue(clientDetails); + result.writeValue(uaaClientDetails); } - updateClientDetails(clientDetails, zoneId); + updateClientDetails(uaaClientDetails, zoneId); } } @@ -322,14 +322,14 @@ public void deleteClientJwtConfig(String clientId, String keyConfig, String zone clientJwtConfiguration = new ClientJwtConfiguration(keyConfig, null); } if (clientJwtConfiguration != null) { - BaseClientDetails clientDetails = (BaseClientDetails) loadClientByClientId(clientId, zoneId); - ClientJwtConfiguration result = ClientJwtConfiguration.delete(ClientJwtConfiguration.readValue(clientDetails), clientJwtConfiguration); + UaaClientDetails uaaClientDetails = (UaaClientDetails) loadClientByClientId(clientId, zoneId); + ClientJwtConfiguration result = ClientJwtConfiguration.delete(ClientJwtConfiguration.readValue(uaaClientDetails), clientJwtConfiguration); if (result != null) { - result.writeValue(clientDetails); + result.writeValue(uaaClientDetails); } else { - ClientJwtConfiguration.resetConfiguration(clientDetails); + ClientJwtConfiguration.resetConfiguration(uaaClientDetails); } - updateClientDetails(clientDetails, zoneId); + updateClientDetails(uaaClientDetails, zoneId); } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java index 7e77098a1c3..fb2c063dca4 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java @@ -255,29 +255,29 @@ void simpleAddClientWithSignupSuccessRedirectUrl() throws Exception { @Test void simpleAddClientWithJwksUri() throws Exception { Map map = new HashMap<>(); - map.put("id", "foo"); + map.put("id", "foo-jwks-uri"); map.put("secret", "bar"); map.put("scope", "openid"); map.put("authorized-grant-types", GRANT_TYPE_AUTHORIZATION_CODE); map.put("authorities", "uaa.none"); map.put("redirect-uri", "http://localhost/callback"); map.put("jwks_uri", "https://localhost:8080/uaa"); - ClientDetails clientDetails = doSimpleTest(map, clientAdminBootstrap, multitenantJdbcClientDetailsService, clients); - assertNotNull(clientDetails.getAdditionalInformation().get(ClientConstants.PRIVATE_KEY_CONFIG)); + UaaClientDetails clientDetails = (UaaClientDetails) doSimpleTest(map, clientAdminBootstrap, multitenantJdbcClientDetailsService, clients); + assertNotNull(clientDetails.getClientJwtConfig()); } @Test void simpleAddClientWithJwkSet() throws Exception { Map map = new HashMap<>(); - map.put("id", "foo"); + map.put("id", "foo-jwks"); map.put("secret", "bar"); map.put("scope", "openid"); map.put("authorized-grant-types", GRANT_TYPE_AUTHORIZATION_CODE); map.put("authorities", "uaa.none"); map.put("redirect-uri", "http://localhost/callback"); map.put("jwks", "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"u_A1S-WoVAnHlNQ_1HJmOPBVxIdy1uSNsp5JUF5N4KtOjir9EgG9HhCFRwz48ykEukrgaK4ofyy_wRXSUJKW7Q\"}"); - ClientDetails clientDetails = doSimpleTest(map, clientAdminBootstrap, multitenantJdbcClientDetailsService, clients); - assertNotNull(clientDetails.getAdditionalInformation().get(ClientConstants.PRIVATE_KEY_CONFIG)); + UaaClientDetails clientDetails = (UaaClientDetails) doSimpleTest(map, clientAdminBootstrap, multitenantJdbcClientDetailsService, clients); + assertNotNull(clientDetails.getClientJwtConfig()); } @Test @@ -355,17 +355,17 @@ void setUp() { @Test void simpleAddClientWithAutoApprove() { Map map = createClientMap(autoApproveId); - BaseClientDetails output = new BaseClientDetails(autoApproveId, "none", "openid", "authorization_code,refresh_token", "uaa.none", "http://localhost/callback"); + UaaClientDetails output = new UaaClientDetails(autoApproveId, "none", "openid", "authorization_code,refresh_token", "uaa.none", "http://localhost/callback"); output.setClientSecret("bar"); doReturn(output).when(multitenantJdbcClientDetailsService).loadClientByClientId(eq(autoApproveId), anyString()); clients.put((String) map.get("id"), map); - BaseClientDetails expectedAdd = new BaseClientDetails(output); + UaaClientDetails expectedAdd = new UaaClientDetails(output); clientAdminBootstrap.afterPropertiesSet(); verify(multitenantJdbcClientDetailsService).addClientDetails(expectedAdd, "uaa"); - BaseClientDetails expectedUpdate = new BaseClientDetails(expectedAdd); + UaaClientDetails expectedUpdate = new UaaClientDetails(expectedAdd); expectedUpdate.setAdditionalInformation(Collections.singletonMap(ClientConstants.AUTO_APPROVE, true)); verify(multitenantJdbcClientDetailsService).updateClientDetails(expectedUpdate, "uaa"); } @@ -373,16 +373,16 @@ void simpleAddClientWithAutoApprove() { @Test void simpleAddClientWithAllowPublic() { Map map = createClientMap(allowPublicId); - BaseClientDetails output = new BaseClientDetails(allowPublicId, "none", "openid", "authorization_code,refresh_token", "uaa.none", "http://localhost/callback"); + UaaClientDetails output = new UaaClientDetails(allowPublicId, "none", "openid", "authorization_code,refresh_token", "uaa.none", "http://localhost/callback"); output.setClientSecret("bar"); doReturn(output).when(multitenantJdbcClientDetailsService).loadClientByClientId(eq(allowPublicId), anyString()); clients.put((String) map.get("id"), map); - BaseClientDetails expectedAdd = new BaseClientDetails(output); + UaaClientDetails expectedAdd = new UaaClientDetails(output); clientAdminBootstrap.afterPropertiesSet(); - BaseClientDetails expectedUpdate = new BaseClientDetails(expectedAdd); + UaaClientDetails expectedUpdate = new UaaClientDetails(expectedAdd); expectedUpdate.setAdditionalInformation(Collections.singletonMap(ClientConstants.ALLOW_PUBLIC, true)); verify(multitenantJdbcClientDetailsService, times(1)).updateClientDetails(expectedUpdate, "uaa"); } @@ -390,7 +390,7 @@ void simpleAddClientWithAllowPublic() { @Test void simpleAddClientWithAllowPublicNoClient() { Map map = createClientMap(allowPublicId); - BaseClientDetails output = new BaseClientDetails(allowPublicId, "none", "openid", "authorization_code,refresh_token", "uaa.none", "http://localhost/callback"); + UaaClientDetails output = new UaaClientDetails(allowPublicId, "none", "openid", "authorization_code,refresh_token", "uaa.none", "http://localhost/callback"); output.setClientSecret("bar"); doThrow(new NoSuchClientException(allowPublicId)).when(multitenantJdbcClientDetailsService).loadClientByClientId(eq(allowPublicId), anyString()); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java index 7099db2a3e6..e9966583362 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java @@ -1074,9 +1074,9 @@ void testCreateClientWithJsonWebKeyUri() { createRequest.setJsonWebKeyUri(jwksUri); ClientDetails result = endpoints.createClientDetails(createRequest); assertNull(result.getClientSecret()); - ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); + ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(UaaClientDetails.class); verify(clientDetailsService).create(clientCaptor.capture(), anyString()); - BaseClientDetails created = clientCaptor.getValue(); + UaaClientDetails created = clientCaptor.getValue(); assertEquals(ClientJwtConfiguration.readValue(created), ClientJwtConfiguration.parse(jwksUri)); } @@ -1094,9 +1094,9 @@ void testCreateClientWithJsonWebKeyUriInvalid() { createRequest.setJsonWebKeySet(jwksUri); ClientDetails result = endpoints.createClientDetails(createRequest); assertNull(result.getClientSecret()); - ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); + ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(UaaClientDetails.class); verify(clientDetailsService).create(clientCaptor.capture(), anyString()); - BaseClientDetails created = clientCaptor.getValue(); + UaaClientDetails created = clientCaptor.getValue(); assertNull(ClientJwtConfiguration.readValue(created)); } @@ -1173,9 +1173,9 @@ void testCreateClientWithJsonKeyWebSet() { createRequest.setJsonWebKeySet(jsonJwk); ClientDetails result = endpoints.createClientDetails(createRequest); assertNull(result.getClientSecret()); - ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class); + ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(UaaClientDetails.class); verify(clientDetailsService).create(clientCaptor.capture(), anyString()); - BaseClientDetails created = clientCaptor.getValue(); + UaaClientDetails created = clientCaptor.getValue(); assertEquals(ClientJwtConfiguration.readValue(created), ClientJwtConfiguration.parse(jsonJwk)); assertEquals(ClientJwtConfiguration.readValue(created), ClientJwtConfiguration.parse(jsonJwk2)); assertEquals(ClientJwtConfiguration.readValue(created), ClientJwtConfiguration.parse(jsonJwkSet)); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfigurationTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfigurationTest.java index 8c61ef6c535..25612e47d72 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfigurationTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfigurationTest.java @@ -1,16 +1,13 @@ package org.cloudfoundry.identity.uaa.client; -import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKeySet; import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.junit.jupiter.api.Test; -import org.springframework.security.oauth2.provider.client.BaseClientDetails; import java.text.ParseException; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -228,19 +225,17 @@ void testSerializableObjectCalls() throws CloneNotSupportedException { void testConfiguration() { ClientJwtConfiguration configUri = JsonUtils.readValue(defaultJsonUri, ClientJwtConfiguration.class); ClientJwtConfiguration configKey = JsonUtils.readValue(defaultJsonKey, ClientJwtConfiguration.class); - BaseClientDetails baseClientDetails = new BaseClientDetails(); - HashMap additionalInformation = new HashMap<>(); - additionalInformation.put(ClientConstants.PRIVATE_KEY_CONFIG, configUri); - baseClientDetails.setAdditionalInformation(additionalInformation); + UaaClientDetails uaaClientDetails = new UaaClientDetails(); + uaaClientDetails.setClientJwtConfig(JsonUtils.writeValueAsString(configUri)); - configUri.writeValue(baseClientDetails); - ClientJwtConfiguration readUriConfig = ClientJwtConfiguration.readValue(baseClientDetails); + configUri.writeValue(uaaClientDetails); + ClientJwtConfiguration readUriConfig = ClientJwtConfiguration.readValue(uaaClientDetails); assertEquals(configUri, readUriConfig); - ClientJwtConfiguration.resetConfiguration(baseClientDetails); - assertNull(ClientJwtConfiguration.readValue(baseClientDetails)); - configKey.writeValue(baseClientDetails); - ClientJwtConfiguration readKeyConfig = ClientJwtConfiguration.readValue(baseClientDetails); + ClientJwtConfiguration.resetConfiguration(uaaClientDetails); + assertNull(ClientJwtConfiguration.readValue(uaaClientDetails)); + configKey.writeValue(uaaClientDetails); + ClientJwtConfiguration readKeyConfig = ClientJwtConfiguration.readValue(uaaClientDetails); assertEquals(configKey, readKeyConfig); } } From aace8e0bd120de75f8e6b1504b3b9de7d3d0ebc5 Mon Sep 17 00:00:00 2001 From: strehle Date: Fri, 25 Aug 2023 13:38:34 +0200 Subject: [PATCH 13/22] remove not needed method. Even if UaaClientDetails is used the addClientDetails method can be used and therefore it does not make sense to have 2 add methods --- .../identity/uaa/zone/MultitenantClientServices.java | 3 --- .../uaa/zone/MultitenantJdbcClientDetailsService.java | 8 -------- .../uaa/zone/InMemoryMultitenantClientServices.java | 6 ------ .../zone/MultitenantJdbcClientDetailsServiceTests.java | 2 +- 4 files changed, 1 insertion(+), 18 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java index e1411b38df4..c5fbdfa1706 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantClientServices.java @@ -1,6 +1,5 @@ package org.cloudfoundry.identity.uaa.zone; -import org.cloudfoundry.identity.uaa.client.UaaClientDetails; import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; import org.springframework.security.oauth2.provider.*; @@ -12,8 +11,6 @@ interface MultitenantClientRegistrationService extends ClientRegistrationService void updateClientDetails(ClientDetails clientDetails, String zoneId) throws NoSuchClientException; - void addUaaClientDetails(UaaClientDetails uaaClientDetails, String zoneId) throws ClientAlreadyExistsException; - void updateClientSecret(String clientId, String secret, String zoneId) throws NoSuchClientException; void updateClientJwtConfig(String clientId, String keyConfig, String zoneId) throws NoSuchClientException; diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java index 9d3cdb85f44..c5ce7ee7466 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java @@ -157,14 +157,6 @@ public void updateClientDetails(ClientDetails clientDetails, String zoneId) thro } } - @Override - public void addUaaClientDetails(UaaClientDetails uaaClientDetails, String zoneId) throws ClientAlreadyExistsException { - if (exists(uaaClientDetails.getClientId(), zoneId)) { - throw new ClientAlreadyExistsException("Client already exists: " + uaaClientDetails.getClientId()); - } - jdbcTemplate.update(DEFAULT_INSERT_STATEMENT, getInsertClientDetailsFields(uaaClientDetails, zoneId)); - } - @Override public void updateClientSecret(String clientId, String secret, String zoneId) throws NoSuchClientException { int count = jdbcTemplate.update(DEFAULT_UPDATE_SECRET_STATEMENT, passwordEncoder.encode(secret), clientId, zoneId); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java index 1cdd7cd47e3..a9fe483bcfd 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/InMemoryMultitenantClientServices.java @@ -1,6 +1,5 @@ package org.cloudfoundry.identity.uaa.zone; -import org.cloudfoundry.identity.uaa.client.UaaClientDetails; import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; import org.springframework.security.oauth2.provider.ClientAlreadyExistsException; import org.springframework.security.oauth2.provider.ClientDetails; @@ -59,11 +58,6 @@ public void updateClientDetails(ClientDetails clientDetails, String zoneId) thro addClientDetails(clientDetails, zoneId); } - @Override - public void addUaaClientDetails(UaaClientDetails uaaClientDetails, String zoneId) throws ClientAlreadyExistsException { - getInMemoryService(zoneId).put(uaaClientDetails.getClientId(), (BaseClientDetails) uaaClientDetails); - } - @Override public void updateClientSecret(String clientId, String secret, String zoneId) throws NoSuchClientException { ofNullable((BaseClientDetails) loadClientByClientId(clientId, zoneId)).ifPresent(client -> diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java index 67bd965e06d..5e02c99d69a 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java @@ -543,7 +543,7 @@ void updateClientJwtConfig() { UaaClientDetails clientDetails = new UaaClientDetails(); clientDetails.setClientId("newClientIdWithClientJwtConfig"); clientDetails.setClientJwtConfig("small"); - service.addUaaClientDetails(clientDetails, mockIdentityZoneManager.getCurrentIdentityZoneId()); + service.addClientDetails(clientDetails, mockIdentityZoneManager.getCurrentIdentityZoneId()); Map map = jdbcTemplate.queryForMap(SELECT_SQL, "newClientIdWithClientJwtConfig"); From aecb64536b822c1037df6b1f00d3f28b490cb365 Mon Sep 17 00:00:00 2001 From: strehle Date: Fri, 8 Sep 2023 13:51:37 +0200 Subject: [PATCH 14/22] review --- .../identity/uaa/client/UaaClientDetails.java | 10 ++--- .../uaa/client/UaaClientDetailsTest.java | 5 ++- ...titenantJdbcClientDetailsServiceTests.java | 40 +++++++++---------- 3 files changed, 26 insertions(+), 29 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java index 708ac46949d..0803b2b7f8d 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetails.java @@ -56,14 +56,12 @@ public boolean equals(Object o) { if (this == o) { return true; } - if (o == null || getClass() != o.getClass()) { + if (o == null || getClass() != o.getClass() || !super.equals(o)) { return false; } - if (o instanceof UaaClientDetails) { - UaaClientDetails uaaClientDetails = (UaaClientDetails) o; - return Objects.equals(clientJwtConfig, uaaClientDetails.clientJwtConfig); - } - return false; + + UaaClientDetails uaaClientDetails = (UaaClientDetails) o; + return Objects.equals(clientJwtConfig, uaaClientDetails.clientJwtConfig); } @Override diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsTest.java index bcd561d9ee1..53aa8104d35 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsTest.java @@ -83,6 +83,8 @@ void testEquals() { assertNotEquals(copy, new UaaClientDetails()); copy.setClientJwtConfig(null); assertEquals(copy, copy2); + assertEquals(copy, copy); + assertNotEquals(copy, new BaseClientDetails()); } @Test @@ -90,10 +92,9 @@ void testHashCode() { UaaClientDetails copy = new UaaClientDetails(testClient); UaaClientDetails copy2 = new UaaClientDetails(testClient.getClientId(), "", "test.none", "", "test.admin", null); + assertEquals(copy.hashCode(), copy2.hashCode()); copy.setClientJwtConfig("test"); assertNotEquals(copy.hashCode(), copy2.hashCode()); - copy.setClientJwtConfig(null); - assertEquals(copy.hashCode(), copy2.hashCode()); } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java index 5e02c99d69a..5bd106db6ba 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java @@ -308,27 +308,25 @@ void loadingClientIdWithSingleDetails() { ClientDetails clientDetails = service .loadClientByClientId("clientIdWithSingleDetails"); - assertEquals("clientIdWithSingleDetails", clientDetails.getClientId()); - assertTrue(clientDetails.isSecretRequired()); - assertEquals("mySecret", clientDetails.getClientSecret()); - assertTrue(clientDetails.isScoped()); - assertEquals(1, clientDetails.getScope().size()); - assertEquals("myScope", clientDetails.getScope().iterator().next()); - assertEquals(1, clientDetails.getResourceIds().size()); - assertEquals("myResource", clientDetails.getResourceIds().iterator() - .next()); - assertEquals(1, clientDetails.getAuthorizedGrantTypes().size()); - assertEquals("myAuthorizedGrantType", clientDetails - .getAuthorizedGrantTypes().iterator().next()); - assertEquals("myRedirectUri", clientDetails.getRegisteredRedirectUri() - .iterator().next()); - assertEquals(1, clientDetails.getAuthorities().size()); - assertEquals("myAuthority", clientDetails.getAuthorities().iterator() - .next().getAuthority()); - assertEquals(new Integer(100), - clientDetails.getAccessTokenValiditySeconds()); - assertEquals(new Integer(200), - clientDetails.getRefreshTokenValiditySeconds()); + assertNotNull(clientDetails); + assertTrue(clientDetails instanceof UaaClientDetails); + + UaaClientDetails uaaClientDetails = (UaaClientDetails) clientDetails; + assertEquals("clientIdWithSingleDetails", uaaClientDetails.getClientId()); + assertTrue(uaaClientDetails.isSecretRequired()); + assertEquals("mySecret", uaaClientDetails.getClientSecret()); + assertTrue(uaaClientDetails.isScoped()); + assertEquals(1, uaaClientDetails.getScope().size()); + assertEquals("myScope", uaaClientDetails.getScope().iterator().next()); + assertEquals(1, uaaClientDetails.getResourceIds().size()); + assertEquals("myResource", uaaClientDetails.getResourceIds().iterator().next()); + assertEquals(1, uaaClientDetails.getAuthorizedGrantTypes().size()); + assertEquals("myAuthorizedGrantType", uaaClientDetails .getAuthorizedGrantTypes().iterator().next()); + assertEquals("myRedirectUri", uaaClientDetails.getRegisteredRedirectUri() .iterator().next()); + assertEquals(1, uaaClientDetails.getAuthorities().size()); + assertEquals("myAuthority", uaaClientDetails.getAuthorities().iterator() .next().getAuthority()); + assertEquals(new Integer(100), uaaClientDetails.getAccessTokenValiditySeconds()); + assertEquals(new Integer(200), uaaClientDetails.getRefreshTokenValiditySeconds()); } @Test From e7c0bc2be9b778a81ecd3c6bbc80bff68d14c8a8 Mon Sep 17 00:00:00 2001 From: strehle Date: Mon, 18 Sep 2023 14:21:14 +0200 Subject: [PATCH 15/22] own events for jwt client configuration --- .../oauth/client/ClientJwtChangeRequest.java | 10 ++++--- .../identity/uaa/audit/AuditEventType.java | 4 ++- .../uaa/client/ClientAdminEndpoints.java | 26 ++++++++++++------- .../client/ClientAdminEndpointsValidator.java | 2 ++ .../client/event/ClientJwtChangeEvent.java | 18 +++++++++++++ .../client/event/ClientJwtFailureEvent.java | 26 +++++++++++++++++++ .../WEB-INF/spring/client-admin-endpoints.xml | 5 ++++ 7 files changed, 76 insertions(+), 15 deletions(-) create mode 100644 server/src/main/java/org/cloudfoundry/identity/uaa/client/event/ClientJwtChangeEvent.java create mode 100644 server/src/main/java/org/cloudfoundry/identity/uaa/client/event/ClientJwtFailureEvent.java diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java index 2e19627b2c6..38170e2c35d 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java @@ -1,14 +1,12 @@ package org.cloudfoundry.identity.uaa.oauth.client; - import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import static org.cloudfoundry.identity.uaa.oauth.client.ClientJwtChangeRequest.ChangeMode.ADD; +import static org.cloudfoundry.identity.uaa.oauth.client.ClientJwtChangeRequest.ChangeMode.DELETE; -/** - */ @JsonInclude(JsonInclude.Include.NON_NULL) @JsonIgnoreProperties(ignoreUnknown = true) public class ClientJwtChangeRequest { @@ -75,7 +73,11 @@ public void setKeyId(String keyId) { this.keyId = keyId; } - public String getKey() { + public String getChangeValue() { + // Depending on change mode, allow different values + if (changeMode == DELETE && keyId != null) { + return keyId; + } return jsonWebKeyUri != null ? jsonWebKeyUri : jsonWebKeySet; } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/audit/AuditEventType.java b/server/src/main/java/org/cloudfoundry/identity/uaa/audit/AuditEventType.java index da1dacb7771..e032d2b9f9f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/audit/AuditEventType.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/audit/AuditEventType.java @@ -61,7 +61,9 @@ public enum AuditEventType { IdentityProviderAuthenticationSuccess(37), IdentityProviderAuthenticationFailure(38), MfaAuthenticationSuccess(39), - MfaAuthenticationFailure(40); + MfaAuthenticationFailure(40), + ClientJwtChangeSuccess(41), + ClientJwtChangeFailure(42); private final int code; diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java index 6d52740e88a..377d2e3b653 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java @@ -115,6 +115,7 @@ public class ClientAdminEndpoints implements ApplicationEventPublisherAware { private final AtomicInteger clientUpdates; private final AtomicInteger clientDeletes; private final AtomicInteger clientSecretChanges; + private final AtomicInteger clientJwtChanges; private ApplicationEventPublisher publisher; @@ -155,6 +156,7 @@ public ClientAdminEndpoints(final SecurityContextAccessor securityContextAccesso this.clientUpdates = new AtomicInteger(); this.clientDeletes = new AtomicInteger(); this.clientSecretChanges = new AtomicInteger(); + this.clientJwtChanges = new AtomicInteger(); } @ManagedMetric(metricType = MetricType.COUNTER, displayName = "Client Registration Count") @@ -177,6 +179,11 @@ public int getClientSecretChanges() { return clientSecretChanges.get(); } + @ManagedMetric(metricType = MetricType.COUNTER, displayName = "Client Jwt Config Change Count (Since Startup)") + public int getClientJwtChanges() { + return clientJwtChanges.get(); + } + @ManagedMetric(displayName = "Errors Since Startup") public Map getErrorCounts() { return errorCounts; @@ -553,13 +560,11 @@ public ActionResult changeClientJwt(@PathVariable String client_id, @RequestBody throw new InvalidClientDetailsException(e.getMessage()); } - ClientJwtConfiguration clientKeyConfig = ClientJwtConfiguration.readValue(uaaClientDetails); - ActionResult result; switch (change.getChangeMode()){ case ADD : - if (change.getKey() != null) { - clientRegistrationService.addClientJwtConfig(client_id, change.getKey(), IdentityZoneHolder.get().getId(), false); + if (change.getChangeValue() != null) { + clientRegistrationService.addClientJwtConfig(client_id, change.getChangeValue(), IdentityZoneHolder.get().getId(), false); result = new ActionResult("ok", "Client jwt configuration is added"); } else { result = new ActionResult("ok", "No key added"); @@ -567,18 +572,19 @@ public ActionResult changeClientJwt(@PathVariable String client_id, @RequestBody break; case DELETE : - String deleteString = change.getKeyId() == null ? change.getKey() : change.getKeyId(); - if (clientKeyConfig != null && deleteString != null) { - clientRegistrationService.deleteClientJwtConfig(client_id, deleteString, IdentityZoneHolder.get().getId()); + if (ClientJwtConfiguration.readValue(uaaClientDetails) != null && change.getChangeValue() != null) { + clientRegistrationService.deleteClientJwtConfig(client_id, change.getChangeValue(), IdentityZoneHolder.get().getId()); + result = new ActionResult("ok", "Client jwt configuration is deleted"); + } else { + result = new ActionResult("ok", "No key deleted"); } - result = new ActionResult("ok", "Client jwt configuration is deleted"); break; default: - clientRegistrationService.addClientJwtConfig(client_id, change.getKey(), IdentityZoneHolder.get().getId(), true); + clientRegistrationService.addClientJwtConfig(client_id, change.getChangeValue(), IdentityZoneHolder.get().getId(), true); result = new ActionResult("ok", "Client jwt configuration updated"); } - clientSecretChanges.incrementAndGet(); + clientJwtChanges.incrementAndGet(); return result; } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java index 6c1879656b6..f334dadbbb9 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java @@ -256,6 +256,8 @@ public ClientDetails validate(ClientDetails prototype, boolean create, boolean c clientJwtConfiguration.writeValue(client); } else { logger.warn("Client with client jwt configuration not valid"); + throw new InvalidClientDetailsException( + "Client with client jwt configuration not valid"); } } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/event/ClientJwtChangeEvent.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/event/ClientJwtChangeEvent.java new file mode 100644 index 00000000000..bd65441bce0 --- /dev/null +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/event/ClientJwtChangeEvent.java @@ -0,0 +1,18 @@ +package org.cloudfoundry.identity.uaa.client.event; + +import org.cloudfoundry.identity.uaa.audit.AuditEventType; +import org.springframework.security.core.Authentication; +import org.springframework.security.oauth2.provider.ClientDetails; + +public class ClientJwtChangeEvent extends AbstractClientAdminEvent { + + public ClientJwtChangeEvent(ClientDetails client, Authentication principal, String zoneId) { + super(client, principal, zoneId); + } + + @Override + public AuditEventType getAuditEventType() { + return AuditEventType.ClientJwtChangeSuccess; + } + +} diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/event/ClientJwtFailureEvent.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/event/ClientJwtFailureEvent.java new file mode 100644 index 00000000000..659bddeee8c --- /dev/null +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/event/ClientJwtFailureEvent.java @@ -0,0 +1,26 @@ +package org.cloudfoundry.identity.uaa.client.event; + +import org.cloudfoundry.identity.uaa.audit.AuditEventType; +import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; +import org.springframework.security.core.Authentication; +import org.springframework.security.oauth2.provider.ClientDetails; + +public class ClientJwtFailureEvent extends AbstractClientAdminEvent { + + private String message; + + public ClientJwtFailureEvent(String message, Authentication principal) { + this(message, null, principal, IdentityZoneHolder.getCurrentZoneId()); + } + + public ClientJwtFailureEvent(String message, ClientDetails client, Authentication principal, String zoneId) { + super(client, principal, zoneId); + this.message = message; + } + + @Override + public AuditEventType getAuditEventType() { + return AuditEventType.ClientJwtChangeFailure; + } + +} diff --git a/uaa/src/main/webapp/WEB-INF/spring/client-admin-endpoints.xml b/uaa/src/main/webapp/WEB-INF/spring/client-admin-endpoints.xml index 285f71ac09d..705be2d8425 100644 --- a/uaa/src/main/webapp/WEB-INF/spring/client-admin-endpoints.xml +++ b/uaa/src/main/webapp/WEB-INF/spring/client-admin-endpoints.xml @@ -87,6 +87,11 @@ + + From fe955c9bdef4faf2504b56614f42d58f751aa4c0 Mon Sep 17 00:00:00 2001 From: strehle Date: Mon, 18 Sep 2023 14:22:54 +0200 Subject: [PATCH 16/22] review * throw exceptions * --- .../uaa/client/ClientAdminBootstrap.java | 12 +- .../uaa/client/ClientJwtConfiguration.java | 8 +- .../event/ClientAdminEventPublisher.java | 8 ++ .../MultitenantJdbcClientDetailsService.java | 5 + .../uaa/client/ClientAdminEndpointsTests.java | 9 +- .../ClientAdminEndpointsIntegrationTests.java | 36 ++++++ .../ClientAdminEndpointsMockMvcTests.java | 104 ++++++++++++++++++ 7 files changed, 167 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java index dcd5783c363..789b28ec424 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java @@ -19,7 +19,6 @@ import org.cloudfoundry.identity.uaa.authentication.SystemAuthentication; import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.user.UaaAuthority; -import org.cloudfoundry.identity.uaa.util.UaaUrlUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import org.cloudfoundry.identity.uaa.zone.MultitenantClientServices; @@ -211,17 +210,14 @@ private void addNewClients() { client.setAdditionalInformation(info); - if (map.get("jwks_uri") instanceof String) { + if (map.get("jwks_uri") instanceof String || map.get("jwks") instanceof String) { String jwksUri = (String) map.get("jwks_uri"); - ClientJwtConfiguration keyConfig = ClientJwtConfiguration.parse(UaaUrlUtils.normalizeUri(jwksUri), null); - if (keyConfig != null && keyConfig.getCleanString() != null) { - keyConfig.writeValue(client); - } - } else if (map.get("jwks") instanceof String) { String jwks = (String) map.get("jwks"); - ClientJwtConfiguration keyConfig = ClientJwtConfiguration.parse(null, jwks); + ClientJwtConfiguration keyConfig = ClientJwtConfiguration.parse(jwksUri, jwks); if (keyConfig != null && keyConfig.getCleanString() != null) { keyConfig.writeValue(client); + } else { + throw new InvalidClientDetailsException("Client jwt configuration invalid syntax. ClientID: " + client.getClientId()); } } 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 78e8c318737..1891af777b4 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 @@ -123,7 +123,13 @@ public static ClientJwtConfiguration parse(String privateKeyConfig) { public static ClientJwtConfiguration parse(String privateKeyUrl, String privateKeyJwt) { ClientJwtConfiguration clientJwtConfiguration = null; if (privateKeyUrl != null) { - clientJwtConfiguration = new ClientJwtConfiguration(privateKeyUrl, null); + String normalizedUri = null; + try { + normalizedUri = UaaUrlUtils.normalizeUri(privateKeyUrl); + } catch (IllegalArgumentException e) { + throw new InvalidClientDetailsException("Client jwt configuration with invalid URI", e); + } + clientJwtConfiguration = new ClientJwtConfiguration(normalizedUri, null); clientJwtConfiguration.validateJwksUri(); } else if (privateKeyJwt != null && privateKeyJwt.contains("{") && privateKeyJwt.contains("}")) { HashMap jsonMap = JsonUtils.readValue(privateKeyJwt, HashMap.class); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/event/ClientAdminEventPublisher.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/event/ClientAdminEventPublisher.java index 10e7c186561..3ce02a7153f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/event/ClientAdminEventPublisher.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/event/ClientAdminEventPublisher.java @@ -112,6 +112,14 @@ public void secretChange(String clientId) { publish(new SecretChangeEvent(getClient(clientId), getPrincipal(), identityZoneManager.getCurrentIdentityZoneId())); } + public void clientJwtFailure(String clientId, Exception e) { + publish(new ClientJwtFailureEvent(e.getMessage(), getClient(clientId), getPrincipal(), identityZoneManager.getCurrentIdentityZoneId())); + } + + public void clientJwtChange(String clientId) { + publish(new ClientJwtChangeEvent(getClient(clientId), getPrincipal(), identityZoneManager.getCurrentIdentityZoneId())); + } + private ClientDetails getClient(String clientId) { try { return clientDetailsService.loadClientByClientId(clientId, identityZoneManager.getCurrentIdentityZoneId()); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java index a7eeb636ebd..a043d8f4167 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java @@ -2,6 +2,7 @@ import org.cloudfoundry.identity.uaa.audit.event.SystemDeletable; import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal; +import org.cloudfoundry.identity.uaa.client.InvalidClientDetailsException; import org.cloudfoundry.identity.uaa.client.UaaClientDetails; import org.cloudfoundry.identity.uaa.client.ClientJwtConfiguration; import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; @@ -302,6 +303,8 @@ public void addClientJwtConfig(String clientId, String keyConfig, String zoneId, result.writeValue(uaaClientDetails); } updateClientDetails(uaaClientDetails, zoneId); + } else { + throw new InvalidClientDetailsException("Invalid jwt configuration configuration"); } } @@ -322,6 +325,8 @@ public void deleteClientJwtConfig(String clientId, String keyConfig, String zone ClientJwtConfiguration.resetConfiguration(uaaClientDetails); } updateClientDetails(uaaClientDetails, zoneId); + } else { + throw new InvalidClientDetailsException("Invalid jwt configuration configuration"); } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java index e9966583362..758389bf213 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java @@ -216,6 +216,7 @@ void testValidateClientsTransferAutoApproveScopeSet() { void testStatistics() { assertEquals(0, endpoints.getClientDeletes()); assertEquals(0, endpoints.getClientSecretChanges()); + assertEquals(0, endpoints.getClientJwtChanges()); assertEquals(0, endpoints.getClientUpdates()); assertEquals(0, endpoints.getErrorCounts().size()); assertEquals(0, endpoints.getTotalClients()); @@ -1092,12 +1093,8 @@ void testCreateClientWithJsonWebKeyUriInvalid() { detail.setAuthorizedGrantTypes(input.getAuthorizedGrantTypes()); ClientDetailsCreation createRequest = createClientDetailsCreation(input); createRequest.setJsonWebKeySet(jwksUri); - ClientDetails result = endpoints.createClientDetails(createRequest); - assertNull(result.getClientSecret()); - ArgumentCaptor clientCaptor = ArgumentCaptor.forClass(UaaClientDetails.class); - verify(clientDetailsService).create(clientCaptor.capture(), anyString()); - UaaClientDetails created = clientCaptor.getValue(); - assertNull(ClientJwtConfiguration.readValue(created)); + assertThrows(InvalidClientDetailsException.class, + () -> endpoints.createClientDetails(createClientDetailsCreation(detail))); } @Test diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ClientAdminEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ClientAdminEndpointsIntegrationTests.java index 866e1cae89a..a831ae07a44 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ClientAdminEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ClientAdminEndpointsIntegrationTests.java @@ -20,6 +20,7 @@ import org.cloudfoundry.identity.uaa.integration.util.IntegrationTestUtils; import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsCreation; import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsModification; +import org.cloudfoundry.identity.uaa.oauth.client.ClientJwtChangeRequest; import org.cloudfoundry.identity.uaa.oauth.client.SecretChangeRequest; import org.cloudfoundry.identity.uaa.resources.SearchResults; import org.cloudfoundry.identity.uaa.test.TestAccountSetup; @@ -600,6 +601,41 @@ public void testChangeSecret() throws Exception { assertEquals(HttpStatus.OK, result.getStatusCode()); } + @Test + public void testChangeJwtConfig() throws Exception { + headers = getAuthenticatedHeaders(getClientCredentialsAccessToken("clients.read,clients.write,clients.secret,uaa.admin")); + BaseClientDetails client = createClient("client_credentials"); + + client.setResourceIds(Collections.singleton("foo")); + + ClientJwtChangeRequest def = new ClientJwtChangeRequest(null, null, null); + def.setJsonWebKeyUri("http://localhost:8080/uaa/token_key"); + def.setClientId("admin"); + + ResponseEntity result = serverRunning.getRestTemplate().exchange( + serverRunning.getUrl("/oauth/clients/{client}/clientjwt"), + HttpMethod.PUT, new HttpEntity(def, headers), Void.class, + client.getClientId()); + assertEquals(HttpStatus.OK, result.getStatusCode()); + } + + @Test + public void testChangeJwtConfigInvalidTokenKey() throws Exception { + headers = getAuthenticatedHeaders(getClientCredentialsAccessToken("clients.read,clients.write,clients.secret,uaa.admin")); + BaseClientDetails client = createClient("client_credentials"); + + client.setResourceIds(Collections.singleton("foo")); + + ClientJwtChangeRequest def = new ClientJwtChangeRequest(null, null, null); + def.setJsonWebKeyUri("no uri"); + def.setClientId("admin"); + + ResponseEntity result = serverRunning.getRestTemplate().exchange( + serverRunning.getUrl("/oauth/clients/{client}/clientjwt"), + HttpMethod.PUT, new HttpEntity(def, headers), Void.class, + client.getClientId()); + assertEquals(HttpStatus.BAD_REQUEST, result.getStatusCode()); + } @Test public void testCreateClientsWithStrictSecretPolicy() throws Exception { diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcTests.java index 3a78d0591f2..3f265263763 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcTests.java @@ -16,6 +16,8 @@ import org.cloudfoundry.identity.uaa.client.event.ClientApprovalsDeletedEvent; import org.cloudfoundry.identity.uaa.client.event.ClientCreateEvent; import org.cloudfoundry.identity.uaa.client.event.ClientDeleteEvent; +import org.cloudfoundry.identity.uaa.client.event.ClientJwtChangeEvent; +import org.cloudfoundry.identity.uaa.client.event.ClientJwtFailureEvent; import org.cloudfoundry.identity.uaa.client.event.ClientUpdateEvent; import org.cloudfoundry.identity.uaa.client.event.SecretChangeEvent; import org.cloudfoundry.identity.uaa.client.event.SecretFailureEvent; @@ -23,6 +25,7 @@ import org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils; import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsCreation; import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsModification; +import org.cloudfoundry.identity.uaa.oauth.client.ClientJwtChangeRequest; import org.cloudfoundry.identity.uaa.oauth.client.SecretChangeRequest; import org.cloudfoundry.identity.uaa.resources.ActionResult; import org.cloudfoundry.identity.uaa.resources.SearchResults; @@ -1943,6 +1946,107 @@ void testPutClientModifyName() throws Exception { assertThat(client.getAdditionalInformation(), hasEntry(is("name"), PredicateMatcher.is(value -> value.equals("New Client Name")))); } + @Test + void testAddNewClientJwtKeyUri() throws Exception { + String token = testClient.getClientCredentialsOAuthAccessToken( + testAccounts.getAdminClientId(), + testAccounts.getAdminClientSecret(), + "uaa.admin,clients.secret"); + String id = generator.generate(); + ClientDetails client = createClient(token, id, SECRET, Collections.singleton("client_credentials")); + ClientJwtChangeRequest request = new ClientJwtChangeRequest(null, null, null); + request.setJsonWebKeyUri("http://localhost:8080/uaa/token_key"); + request.setClientId("admin"); + request.setChangeMode(ClientJwtChangeRequest.ChangeMode.ADD); + MockHttpServletResponse response = mockMvc.perform(put("/oauth/clients/{client_id}/clientjwt", client.getClientId()) + .header("Authorization", "Bearer " + token) + .accept(APPLICATION_JSON) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(request))) + .andExpect(status().isOk()) + .andReturn().getResponse(); + + ActionResult actionResult = JsonUtils.readValue(response.getContentAsString(), ActionResult.class); + assertEquals("ok", actionResult.getStatus()); + assertEquals("Client jwt configuration is added", actionResult.getMessage()); + + verify(mockApplicationEventPublisher, times(2)).publishEvent(abstractUaaEventCaptor.capture()); + assertEquals(ClientJwtChangeEvent.class, abstractUaaEventCaptor.getValue().getClass()); + ClientJwtChangeEvent event = (ClientJwtChangeEvent) abstractUaaEventCaptor.getValue(); + assertEquals(id, event.getAuditEvent().getPrincipalId()); + } + + @Test + void testAddNewClientJwtKeyUriButInvalidChange() throws Exception { + String token = testClient.getClientCredentialsOAuthAccessToken( + testAccounts.getAdminClientId(), + testAccounts.getAdminClientSecret(), + "uaa.admin,clients.secret"); + String id = generator.generate(); + ClientDetails client = createClient(token, id, SECRET, Collections.singleton("client_credentials")); + ClientJwtChangeRequest request = new ClientJwtChangeRequest(null, null, null); + request.setJsonWebKeyUri("http://localhost:8080/uaa/token_key"); + request.setClientId("admin"); + request.setChangeMode(ClientJwtChangeRequest.ChangeMode.ADD); + MockHttpServletResponse response = mockMvc.perform(put("/oauth/clients/{client_id}/clientjwt", client.getClientId()) + .header("Authorization", "Bearer " + token) + .accept(APPLICATION_JSON) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(request))) + .andExpect(status().isOk()) + .andReturn().getResponse(); + + ActionResult actionResult = JsonUtils.readValue(response.getContentAsString(), ActionResult.class); + assertEquals("ok", actionResult.getStatus()); + assertEquals("Client jwt configuration is added", actionResult.getMessage()); + + verify(mockApplicationEventPublisher, times(2)).publishEvent(abstractUaaEventCaptor.capture()); + assertEquals(ClientJwtChangeEvent.class, abstractUaaEventCaptor.getValue().getClass()); + ClientJwtChangeEvent event = (ClientJwtChangeEvent) abstractUaaEventCaptor.getValue(); + assertEquals(id, event.getAuditEvent().getPrincipalId()); + + request = new ClientJwtChangeRequest("admin", null, "{\"keys\":[{\"kty\":\"RSA\",\"e\":\"AQAB\",\"use\":\"sig\",\"alg\":\"RS256\",\"n\":\"n\"}]}"); + request.setChangeMode(ClientJwtChangeRequest.ChangeMode.UPDATE); + response = mockMvc.perform(put("/oauth/clients/{client_id}/clientjwt", client.getClientId()) + .header("Authorization", "Bearer " + token) + .accept(APPLICATION_JSON) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(request))) + .andExpect(status().isBadRequest()) + .andReturn().getResponse(); + + verify(mockApplicationEventPublisher, times(3)).publishEvent(abstractUaaEventCaptor.capture()); + assertEquals(ClientJwtFailureEvent.class, abstractUaaEventCaptor.getValue().getClass()); + ClientJwtFailureEvent eventUpdate = (ClientJwtFailureEvent) abstractUaaEventCaptor.getValue(); + assertEquals(client.getClientId(), eventUpdate.getAuditEvent().getPrincipalId()); + } + + @Test + void testInvalidClientJwtKeyUri() throws Exception { + String token = testClient.getClientCredentialsOAuthAccessToken( + testAccounts.getAdminClientId(), + testAccounts.getAdminClientSecret(), + "uaa.admin,clients.secret"); + String id = generator.generate(); + ClientDetails client = createClient(token, id, SECRET, Collections.singleton("client_credentials")); + ClientJwtChangeRequest request = new ClientJwtChangeRequest(null, null, null); + request.setJsonWebKeyUri("no uri"); + request.setClientId("admin"); + request.setChangeMode(ClientJwtChangeRequest.ChangeMode.ADD); + MockHttpServletResponse response = mockMvc.perform(put("/oauth/clients/{client_id}/clientjwt", client.getClientId()) + .header("Authorization", "Bearer " + token) + .accept(APPLICATION_JSON) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(request))) + .andExpect(status().isBadRequest()) + .andReturn().getResponse(); + + verify(mockApplicationEventPublisher, times(2)).publishEvent(abstractUaaEventCaptor.capture()); + assertEquals(ClientJwtFailureEvent.class, abstractUaaEventCaptor.getValue().getClass()); + ClientJwtFailureEvent event = (ClientJwtFailureEvent) abstractUaaEventCaptor.getValue(); + assertEquals(client.getClientId(), event.getAuditEvent().getPrincipalId()); + } + private BaseClientDetails createClient(List authorities) throws Exception { String clientId = generator.generate().toLowerCase(); List scopes = Arrays.asList("foo", "bar", "oauth.approvals"); From 4c1bf3995b4475308e312195dce946779c1a4432 Mon Sep 17 00:00:00 2001 From: strehle Date: Mon, 18 Sep 2023 16:01:38 +0200 Subject: [PATCH 17/22] doc: Add documentation --- .../source/index.html.md.erb | 23 ++++++++++++ .../mock/clients/ClientAdminEndpointDocs.java | 35 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/uaa/slateCustomizations/source/index.html.md.erb b/uaa/slateCustomizations/source/index.html.md.erb index 26a06bc4f23..3476e859b30 100644 --- a/uaa/slateCustomizations/source/index.html.md.erb +++ b/uaa/slateCustomizations/source/index.html.md.erb @@ -2472,6 +2472,29 @@ _Request Fields_ <%= render('ClientAdminEndpointDocs/changeClientSecret/request-fields.md') %> +## Change Client JWT +This configuration can be done if client authentication is performed with method private_key_jwt +instead of secret based client authentication. See details for client authentiction with [OAuth2](https://oauth.net/2/client-authentication/) and/or +[OpenID Connect](https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication). + +The client can send a client_assertion for authentication. The signature of the assertion is validated either with the jwks_uri or the internal jwks public keys. + +<%= render('ClientAdminEndpointDocs/changeClientJwt/curl-request.md') %> +<%= render('ClientAdminEndpointDocs/changeClientJwt/http-request.md') %> +<%= render('ClientAdminEndpointDocs/changeClientJwt/http-response.md') %> + +_Path Parameters_ + +<%= render('ClientAdminEndpointDocs/changeClientJwt/path-parameters.md') %> + +_Request Headers_ + +<%= render('ClientAdminEndpointDocs/changeClientJwt/request-headers.md') %> + +_Request Fields_ + +<%= render('ClientAdminEndpointDocs/changeClientJwt/request-fields.md') %> + ## List <%= render('ClientAdminEndpointDocs/listClients/curl-request.md') %> diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointDocs.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointDocs.java index 7a0e31be0e5..8377c9f82eb 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointDocs.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointDocs.java @@ -77,6 +77,13 @@ class ClientAdminEndpointDocs extends AdminClientCreator { fieldWithPath("changeMode").optional(UPDATE).type(STRING).description("If change mode is set to `"+ADD+"`, the new `secret` will be added to the existing one and if the change mode is set to `"+DELETE+"`, the old secret will be deleted to support secret rotation. Currently only two client secrets are supported at any given time.") }; + private static final FieldDescriptor[] clientJwtChangeFields = new FieldDescriptor[]{ + fieldWithPath("client_id").required().description(clientIdDescription), + fieldWithPath("kid").optional(UPDATE).type(STRING).description("If change mode is set to `"+DELETE+"`, the `id of the key` that will be deleted. The kid parameter is only possible if jwks configuration is used."), + fieldWithPath("jwks").constrained("Optional if jwks_uri is used. Required otherwise.").type(STRING).description("A valid JSON string according JSON Web Key Set standard, see [RFC 7517](https://www.rfc-editor.org/rfc/rfc7517), e.g. content of /token_keys endpoint from UAA"), + fieldWithPath("jwks_uri").constrained("Optional if jwks is used. Required otherwise.").type(STRING).description("A valid URI to token keys endpoint. Must be compliant to jwks_uri from [OpenID Discovery](https://openid.net/specs/openid-connect-discovery-1_0.html).") + }; + @BeforeEach void setup() throws Exception { clientAdminToken = testClient.getClientCredentialsOAuthAccessToken( @@ -255,6 +262,34 @@ void changeClientSecret() throws Exception { ); } + @Test + void changeClientJwt() throws Exception { + ClientDetails createdClientDetails = JsonUtils.readValue(createClientHelper().andReturn().getResponse().getContentAsString(), BaseClientDetails.class); + + ResultActions resultActions = mockMvc.perform(put("/oauth/clients/{client_id}/clientjwt", createdClientDetails.getClientId()) + .header("Authorization", "Bearer " + clientAdminToken) + .contentType(APPLICATION_JSON) + .accept(APPLICATION_JSON) + .content(writeValueAsString(map( + entry("client_id", createdClientDetails.getClientId()), + entry("jwks_uri", "http://localhost:8080/uaa/token_keys") + )))) + .andExpect(status().isOk()); + + resultActions.andDo(document("{ClassName}/{methodName}", preprocessRequest(prettyPrint()), preprocessResponse(prettyPrint()), + pathParameters( + parameterWithName("client_id").required().description(clientIdDescription) + ), + requestHeaders( + authorizationHeader, + IDENTITY_ZONE_ID_HEADER, + IDENTITY_ZONE_SUBDOMAIN_HEADER + ), + requestFields(clientJwtChangeFields) + ) + ); + } + @Test void deleteClient() throws Exception { ClientDetails createdClientDetails = JsonUtils.readValue(createClientHelper().andReturn().getResponse().getContentAsString(), BaseClientDetails.class); From 6650eb37880c0e6f8b04aac7af8b93d78110b554 Mon Sep 17 00:00:00 2001 From: strehle Date: Mon, 18 Sep 2023 16:36:30 +0200 Subject: [PATCH 18/22] Add new scope clients.trust This can be used for JWT client trust configuration calls Similar to clients.secret --- docs/UAA-APIs.rst | 21 ++++++++++++++++++- .../WEB-INF/spring/client-admin-endpoints.xml | 12 +++++++++++ .../webapp/WEB-INF/spring/oauth-clients.xml | 2 +- .../ClientAdminEndpointsIntegrationTests.java | 21 ++++++++++++++++++- .../mock/clients/ClientAdminEndpointDocs.java | 2 +- 5 files changed, 54 insertions(+), 4 deletions(-) diff --git a/docs/UAA-APIs.rst b/docs/UAA-APIs.rst index 6bd1204d750..56079d778fe 100644 --- a/docs/UAA-APIs.rst +++ b/docs/UAA-APIs.rst @@ -10,7 +10,7 @@ Overview The User Account and Authentication Service (UAA): * is a separate application from Cloud Foundry the Cloud Controller -* owns the user accounts and authentication sources (SAML, LDAP, Keystone) +* owns the user accounts and authentication sources (SAML, OpenID Connect, LDAP, Keystone) * is invoked via JSON APIs * supports standard protocols to provide single sign-on and delegated authorization to web applications in addition to JSON APIs to support the Cloud Controller and team features of Cloud Foundry * supports APIs and a basic login/approval UI for web client apps @@ -35,6 +35,7 @@ Here is a summary of the different scopes that are known to the UAA. * **clients.write** - scope required to create and modify clients. The scopes are limited to be prefixed with the scope holder's client id. For example, id:testclient authorities:client.write may create a client that has scopes that have the 'testclient.' prefix. Authorities are limited to uaa.resource * **clients.read** - scope to read information about clients * **clients.secret** - ``/oauth/clients/*/secret`` endpoint. Scope required to change the password of a client. Considered an admin scope. +* **clients.trust** - ``/oauth/clients/*/clientjwt`` endpoint. Scope required to change the JWT configuration of a client. Considered an admin scope. * **scim.write** - Admin write access to all SCIM endpoints, ``/Users``, ``/Groups/``. * **scim.read** - Admin read access to all SCIM endpoints, ``/Users``, ``/Groups/``. * **scim.create** - Reduced scope to be able to create a user using ``POST /Users`` (get verification links ``GET /Users/{id}/verify-link`` or verify their account using ``GET /Users/{id}/verify``) but not be able to modify, read or delete users. @@ -3103,6 +3104,24 @@ Example:: } +Change Client JWT Configuration: ``PUT /oauth/clients/{client_id}/clientjwt`` +--------------------------------------------------------------- + +============== =============================================== +Request ``PUT /oauth/clients/{client_id}/clientjwt`` +Request body *jwt trust configuration change request* +Reponse code ``200 OK`` if successful +Response body a status message (hash) +============== =============================================== + +Example:: + + PUT /oauth/clients/foo/clientjwt + { + "jwks_uri": "http://localhost:8080/uaa/token_keys" + } + + Register Multiple Clients: ``POST /oauth/clients/tx`` ----------------------------------------------------- diff --git a/uaa/src/main/webapp/WEB-INF/spring/client-admin-endpoints.xml b/uaa/src/main/webapp/WEB-INF/spring/client-admin-endpoints.xml index 705be2d8425..7b1e86f385d 100644 --- a/uaa/src/main/webapp/WEB-INF/spring/client-admin-endpoints.xml +++ b/uaa/src/main/webapp/WEB-INF/spring/client-admin-endpoints.xml @@ -19,6 +19,18 @@ + + + + + + + + + value="uaa.admin,clients.read,clients.write,clients.secret,clients.trust,scim.read,scim.write,clients.admin"/> diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ClientAdminEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ClientAdminEndpointsIntegrationTests.java index a831ae07a44..744025d25e4 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ClientAdminEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ClientAdminEndpointsIntegrationTests.java @@ -603,7 +603,7 @@ public void testChangeSecret() throws Exception { @Test public void testChangeJwtConfig() throws Exception { - headers = getAuthenticatedHeaders(getClientCredentialsAccessToken("clients.read,clients.write,clients.secret,uaa.admin")); + headers = getAuthenticatedHeaders(getClientCredentialsAccessToken("clients.read,clients.write,clients.trust,uaa.admin")); BaseClientDetails client = createClient("client_credentials"); client.setResourceIds(Collections.singleton("foo")); @@ -619,6 +619,25 @@ public void testChangeJwtConfig() throws Exception { assertEquals(HttpStatus.OK, result.getStatusCode()); } + @Test + public void testChangeJwtConfigNoAuthorization() throws Exception { + headers = getAuthenticatedHeaders(getClientCredentialsAccessToken("clients.read,clients.write,clients.trust,uaa.admin")); + BaseClientDetails client = createClient("client_credentials"); + headers = getAuthenticatedHeaders(getClientCredentialsAccessToken("clients.read,clients.write")); + + client.setResourceIds(Collections.singleton("foo")); + + ClientJwtChangeRequest def = new ClientJwtChangeRequest(null, null, null); + def.setJsonWebKeyUri("http://localhost:8080/uaa/token_key"); + def.setClientId("admin"); + + ResponseEntity result = serverRunning.getRestTemplate().exchange( + serverRunning.getUrl("/oauth/clients/{client}/clientjwt"), + HttpMethod.PUT, new HttpEntity(def, headers), Void.class, + client.getClientId()); + assertEquals(HttpStatus.FORBIDDEN, result.getStatusCode()); + } + @Test public void testChangeJwtConfigInvalidTokenKey() throws Exception { headers = getAuthenticatedHeaders(getClientCredentialsAccessToken("clients.read,clients.write,clients.secret,uaa.admin")); diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointDocs.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointDocs.java index 8377c9f82eb..28bc081a592 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointDocs.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointDocs.java @@ -281,7 +281,7 @@ void changeClientJwt() throws Exception { parameterWithName("client_id").required().description(clientIdDescription) ), requestHeaders( - authorizationHeader, + headerWithName("Authorization").description("Bearer token containing `clients.trust`, `clients.admin` or `zones.{zone.id}.admin`"), IDENTITY_ZONE_ID_HEADER, IDENTITY_ZONE_SUBDOMAIN_HEADER ), From b3c3ea5cca50d0c7b94fc13b322261403e551169 Mon Sep 17 00:00:00 2001 From: strehle Date: Mon, 18 Sep 2023 23:23:50 +0200 Subject: [PATCH 19/22] review --- .../client/ClientAdminEndpointsValidator.java | 1 - .../uaa/client/ClientJwtConfiguration.java | 58 +++++++++++-------- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java index f334dadbbb9..0f3849577f1 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java @@ -255,7 +255,6 @@ public ClientDetails validate(ClientDetails prototype, boolean create, boolean c if (clientJwtConfiguration != null) { clientJwtConfiguration.writeValue(client); } else { - logger.warn("Client with client jwt configuration not valid"); throw new InvalidClientDetailsException( "Client with client jwt configuration not valid"); } 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 1891af777b4..8e1cea0dcd3 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 @@ -112,40 +112,48 @@ public String getCleanString() { @JsonIgnore public static ClientJwtConfiguration parse(String privateKeyConfig) { - if (UaaUrlUtils.isUrl(privateKeyConfig)) { - return parse(privateKeyConfig, null); - } else { - return parse(null, privateKeyConfig); - } + return UaaUrlUtils.isUrl(privateKeyConfig) ? parseJwksUri(privateKeyConfig) : parseJwkSet(privateKeyConfig); } @JsonIgnore public static ClientJwtConfiguration parse(String privateKeyUrl, String privateKeyJwt) { ClientJwtConfiguration clientJwtConfiguration = null; if (privateKeyUrl != null) { - String normalizedUri = null; - try { - normalizedUri = UaaUrlUtils.normalizeUri(privateKeyUrl); - } catch (IllegalArgumentException e) { - throw new InvalidClientDetailsException("Client jwt configuration with invalid URI", e); - } - clientJwtConfiguration = new ClientJwtConfiguration(normalizedUri, null); - clientJwtConfiguration.validateJwksUri(); + clientJwtConfiguration = parseJwksUri(privateKeyUrl); } else if (privateKeyJwt != null && privateKeyJwt.contains("{") && privateKeyJwt.contains("}")) { - HashMap jsonMap = JsonUtils.readValue(privateKeyJwt, HashMap.class); - String cleanJwtString; - try { - if (jsonMap.containsKey("keys")) { - cleanJwtString = JWKSet.parse(jsonMap).toString(true); - } else { - cleanJwtString = JWK.parse(jsonMap).toPublicJWK().toString(); - } - clientJwtConfiguration = new ClientJwtConfiguration(null, JsonWebKeyHelper.deserialize(cleanJwtString)); - clientJwtConfiguration.validateJwkSet(); - } catch (ParseException e) { - throw new InvalidClientDetailsException("Client jwt configuration cannot be parsed", e); + clientJwtConfiguration = parseJwkSet(privateKeyJwt); + } + return clientJwtConfiguration; + } + + private static ClientJwtConfiguration parseJwkSet(String privateKeyJwt) { + ClientJwtConfiguration clientJwtConfiguration; + HashMap jsonMap = JsonUtils.readValue(privateKeyJwt, HashMap.class); + String cleanJwtString; + try { + if (jsonMap.containsKey("keys")) { + cleanJwtString = JWKSet.parse(jsonMap).toString(true); + } else { + cleanJwtString = JWK.parse(jsonMap).toPublicJWK().toString(); } + clientJwtConfiguration = new ClientJwtConfiguration(null, JsonWebKeyHelper.deserialize(cleanJwtString)); + clientJwtConfiguration.validateJwkSet(); + } catch (ParseException e) { + throw new InvalidClientDetailsException("Client jwt configuration cannot be parsed", e); + } + return clientJwtConfiguration; + } + + private static ClientJwtConfiguration parseJwksUri(String privateKeyUrl) { + ClientJwtConfiguration clientJwtConfiguration; + String normalizedUri = null; + try { + normalizedUri = UaaUrlUtils.normalizeUri(privateKeyUrl); + } catch (IllegalArgumentException e) { + throw new InvalidClientDetailsException("Client jwt configuration with invalid URI", e); } + clientJwtConfiguration = new ClientJwtConfiguration(normalizedUri, null); + clientJwtConfiguration.validateJwksUri(); return clientJwtConfiguration; } From 2782dfa714db8fa9a98337489f23c12b332c9665 Mon Sep 17 00:00:00 2001 From: strehle Date: Mon, 18 Sep 2023 23:24:56 +0200 Subject: [PATCH 20/22] more tests --- .../client/ClientDetailsCreationTest.java | 20 +++++++++++ .../uaa/client/ClientJwtConfiguration.java | 4 +-- .../MultitenantJdbcClientDetailsService.java | 10 ++---- .../uaa/client/ClientAdminBootstrapTests.java | 13 ++++++++ .../uaa/client/ClientAdminEndpointsTests.java | 6 ++-- .../event/ClientAdminEventPublisherTests.java | 21 ++++++++++++ ...titenantJdbcClientDetailsServiceTests.java | 33 +++++++++++++++++++ 7 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/ClientDetailsCreationTest.java diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/ClientDetailsCreationTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/ClientDetailsCreationTest.java new file mode 100644 index 00000000000..96a62aef809 --- /dev/null +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/ClientDetailsCreationTest.java @@ -0,0 +1,20 @@ +package org.cloudfoundry.identity.uaa.oauth.client; + +import org.cloudfoundry.identity.uaa.util.JsonUtils; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class ClientDetailsCreationTest { + + ClientDetailsCreation clientDetailsCreation = new ClientDetailsCreation(); + + @Test + void testRequestSerialization() { + clientDetailsCreation.setJsonWebKeyUri("https://uri.domain.net"); + clientDetailsCreation.setJsonWebKeySet("{}"); + String jsonRequest = JsonUtils.writeValueAsString(clientDetailsCreation); + ClientDetailsCreation request = JsonUtils.readValue(jsonRequest, ClientDetailsCreation.class); + assertEquals(clientDetailsCreation, request); + } +} \ No newline at end of file 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 8e1cea0dcd3..65d03269008 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 @@ -128,9 +128,9 @@ public static ClientJwtConfiguration parse(String privateKeyUrl, String privateK private static ClientJwtConfiguration parseJwkSet(String privateKeyJwt) { ClientJwtConfiguration clientJwtConfiguration; - HashMap jsonMap = JsonUtils.readValue(privateKeyJwt, HashMap.class); String cleanJwtString; try { + HashMap jsonMap = JsonUtils.readValue(privateKeyJwt, HashMap.class); if (jsonMap.containsKey("keys")) { cleanJwtString = JWKSet.parse(jsonMap).toString(true); } else { @@ -138,7 +138,7 @@ private static ClientJwtConfiguration parseJwkSet(String privateKeyJwt) { } clientJwtConfiguration = new ClientJwtConfiguration(null, JsonWebKeyHelper.deserialize(cleanJwtString)); clientJwtConfiguration.validateJwkSet(); - } catch (ParseException e) { + } catch (ParseException | JsonUtils.JsonUtilException e) { throw new InvalidClientDetailsException("Client jwt configuration cannot be parsed", e); } return clientJwtConfiguration; diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java index a043d8f4167..b15b8e0d01f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java @@ -300,9 +300,8 @@ public void addClientJwtConfig(String clientId, String keyConfig, String zoneId, ClientJwtConfiguration existingConfig = ClientJwtConfiguration.readValue(uaaClientDetails); ClientJwtConfiguration result = ClientJwtConfiguration.merge(existingConfig, clientJwtConfiguration, overwrite); if (result != null) { - result.writeValue(uaaClientDetails); + updateClientJwtConfig(clientId, JsonUtils.writeValueAsString(result), zoneId); } - updateClientDetails(uaaClientDetails, zoneId); } else { throw new InvalidClientDetailsException("Invalid jwt configuration configuration"); } @@ -319,12 +318,7 @@ public void deleteClientJwtConfig(String clientId, String keyConfig, String zone if (clientJwtConfiguration != null) { UaaClientDetails uaaClientDetails = (UaaClientDetails) loadClientByClientId(clientId, zoneId); ClientJwtConfiguration result = ClientJwtConfiguration.delete(ClientJwtConfiguration.readValue(uaaClientDetails), clientJwtConfiguration); - if (result != null) { - result.writeValue(uaaClientDetails); - } else { - ClientJwtConfiguration.resetConfiguration(uaaClientDetails); - } - updateClientDetails(uaaClientDetails, zoneId); + updateClientJwtConfig(clientId, result != null ? JsonUtils.writeValueAsString(result) : null, zoneId); } else { throw new InvalidClientDetailsException("Invalid jwt configuration configuration"); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java index fb2c063dca4..1264d0b3ccd 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java @@ -280,6 +280,19 @@ void simpleAddClientWithJwkSet() throws Exception { assertNotNull(clientDetails.getClientJwtConfig()); } + @Test + void simpleInvalidClientWithJwkSet() throws Exception { + Map map = new HashMap<>(); + map.put("id", "foo-jwks"); + map.put("secret", "bar"); + map.put("scope", "openid"); + map.put("authorized-grant-types", GRANT_TYPE_AUTHORIZATION_CODE); + map.put("authorities", "uaa.none"); + map.put("redirect-uri", "http://localhost/callback"); + map.put("jwks", "invalid"); + assertThrows(InvalidClientDetailsException.class, () -> doSimpleTest(map, clientAdminBootstrap, multitenantJdbcClientDetailsService, clients)); + } + @Test void clientMetadata_getsBootstrapped() { Map map = new HashMap<>(); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java index 758389bf213..ecbaad859f7 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java @@ -1083,8 +1083,6 @@ void testCreateClientWithJsonWebKeyUri() { @Test void testCreateClientWithJsonWebKeyUriInvalid() { - // invalid jwks_uri - String jwksUri = "http://myhost/openid/jwks-uri"; when(clientDetailsService.retrieve(anyString(), anyString())).thenReturn(input); when(mockSecurityContextAccessor.getClientId()).thenReturn(detail.getClientId()); when(mockSecurityContextAccessor.isClient()).thenReturn(true); @@ -1092,9 +1090,9 @@ void testCreateClientWithJsonWebKeyUriInvalid() { input.setClientSecret("secret"); detail.setAuthorizedGrantTypes(input.getAuthorizedGrantTypes()); ClientDetailsCreation createRequest = createClientDetailsCreation(input); - createRequest.setJsonWebKeySet(jwksUri); + createRequest.setJsonWebKeySet("invalid"); assertThrows(InvalidClientDetailsException.class, - () -> endpoints.createClientDetails(createClientDetailsCreation(detail))); + () -> endpoints.createClientDetails(createRequest)); } @Test diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/event/ClientAdminEventPublisherTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/event/ClientAdminEventPublisherTests.java index 36f1b6577fb..2a3ee4d4b13 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/event/ClientAdminEventPublisherTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/event/ClientAdminEventPublisherTests.java @@ -1,8 +1,10 @@ package org.cloudfoundry.identity.uaa.oauth.event; import org.aspectj.lang.ProceedingJoinPoint; +import org.cloudfoundry.identity.uaa.audit.AuditEventType; import org.cloudfoundry.identity.uaa.authentication.UaaAuthentication; import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationTestFactory; +import org.cloudfoundry.identity.uaa.client.UaaClientDetails; import org.cloudfoundry.identity.uaa.client.event.*; import org.cloudfoundry.identity.uaa.zone.MultitenantClientServices; import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; @@ -19,6 +21,7 @@ import java.util.Collections; +import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.*; class ClientAdminEventPublisherTests { @@ -91,4 +94,22 @@ void secretFailureMissingClient() { subject.secretFailure("foo", new RuntimeException("planned")); verify(mockApplicationEventPublisher).publishEvent(isA(SecretFailureEvent.class)); } + + @Test + void clientJwtChange() { + UaaClientDetails uaaClientDetails = new UaaClientDetails("foo", null, null, "client_credentials", "none", null); + when(mockMultitenantClientServices.loadClientByClientId("foo")).thenReturn(uaaClientDetails); + subject.clientJwtChange("foo"); + verify(mockApplicationEventPublisher).publishEvent(isA(ClientJwtChangeEvent.class)); + assertEquals(AuditEventType.ClientJwtChangeSuccess, new ClientJwtChangeEvent(uaaClientDetails, SecurityContextHolder.getContext().getAuthentication(), "uaa").getAuditEvent().getType()); + } + + @Test + void clientJwtFailure() { + UaaClientDetails uaaClientDetails = new UaaClientDetails("foo", null, null, "client_credentials", "none", null); + when(mockMultitenantClientServices.loadClientByClientId("foo")).thenReturn(uaaClientDetails); + subject.clientJwtFailure("foo", new RuntimeException("planned")); + verify(mockApplicationEventPublisher).publishEvent(isA(ClientJwtFailureEvent.class)); + assertEquals(AuditEventType.ClientJwtChangeFailure, new ClientJwtFailureEvent("", uaaClientDetails, SecurityContextHolder.getContext().getAuthentication(), "uaa").getAuditEvent().getType()); + } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java index 5bd106db6ba..ecf45d04d14 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsServiceTests.java @@ -529,6 +529,39 @@ void deleteClientSecret() { assertEquals(clientSecretBeforeDelete.split(" ")[1], clientSecret); } + @Test + void updateClientJwt() { + BaseClientDetails clientDetails = new BaseClientDetails(); + clientDetails.setClientId("newClientIdWithNoDetails"); + service.addClientDetails(clientDetails); + service.addClientJwtConfig(clientDetails.getClientId(), "http://localhost:8080/uaa/token_keys", currentZoneId, true); + + Map map = jdbcTemplate.queryForMap(SELECT_SQL, + "newClientIdWithNoDetails"); + + assertEquals("newClientIdWithNoDetails", map.get("client_id")); + assertTrue(map.containsKey("client_jwt_config")); + assertEquals("{\"jwks_uri\":\"http://localhost:8080/uaa/token_keys\"}", (String) map.get("client_jwt_config")); + } + + @Test + void deleteClientJwt() { + String clientId = "client_id_test_delete"; + BaseClientDetails clientDetails = new BaseClientDetails(); + clientDetails.setClientId(clientId); + service.addClientDetails(clientDetails); + service.addClientJwtConfig(clientDetails.getClientId(), "http://localhost:8080/uaa/token_keys", currentZoneId, true); + + Map map = jdbcTemplate.queryForMap(SELECT_SQL, clientId); + assertTrue(map.containsKey("client_jwt_config")); + assertEquals("{\"jwks_uri\":\"http://localhost:8080/uaa/token_keys\"}", (String) map.get("client_jwt_config")); + service.deleteClientJwtConfig(clientId, "http://localhost:8080/uaa/token_keys", currentZoneId); + + map = jdbcTemplate.queryForMap(SELECT_SQL, clientId); + assertNull(map.get("client_jwt_config")); + assertFalse(map.containsValue("client_jwt_config")); + } + @Test void deleteClientSecretForInvalidClient() { assertThrowsWithMessageThat(NoSuchClientException.class, From 379fe73eba192cc5637bd250d3c0a8dc14dd0cc4 Mon Sep 17 00:00:00 2001 From: strehle Date: Mon, 18 Sep 2023 23:44:36 +0200 Subject: [PATCH 21/22] sonar findings --- .../uaa/oauth/client/ClientJwtChangeRequest.java | 7 +++++-- .../identity/uaa/client/ClientAdminBootstrap.java | 10 ++++++---- .../identity/uaa/client/ClientJwtConfiguration.java | 8 ++++++-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java index 38170e2c35d..337a2282889 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java @@ -11,6 +11,9 @@ @JsonIgnoreProperties(ignoreUnknown = true) public class ClientJwtChangeRequest { + public static final String JWKS_URI = "jwks_uri"; + public static final String JWKS = "jwks"; + public enum ChangeMode { UPDATE, ADD, @@ -18,9 +21,9 @@ public enum ChangeMode { } @JsonProperty("kid") private String keyId; - @JsonProperty("jwks_uri") + @JsonProperty(JWKS_URI) private String jsonWebKeyUri; - @JsonProperty("jwks") + @JsonProperty(JWKS) private String jsonWebKeySet; @JsonProperty("client_id") private String clientId; diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java index 789b28ec424..feb597eccda 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java @@ -1,6 +1,8 @@ package org.cloudfoundry.identity.uaa.client; import static java.util.Optional.ofNullable; +import static org.cloudfoundry.identity.uaa.client.ClientJwtConfiguration.JWKS; +import static org.cloudfoundry.identity.uaa.client.ClientJwtConfiguration.JWKS_URI; import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_AUTHORIZATION_CODE; import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_IMPLICIT; import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_REFRESH_TOKEN; @@ -204,15 +206,15 @@ private void addNewClients() { } for (String key : Arrays.asList("resource-ids", "scope", "authorized-grant-types", "authorities", "redirect-uri", "secret", "id", "override", "access-token-validity", - "refresh-token-validity", "show-on-homepage", "app-launch-url", "app-icon", "jwks", "jwks_uri")) { + "refresh-token-validity", "show-on-homepage", "app-launch-url", "app-icon", JWKS, JWKS_URI)) { info.remove(key); } client.setAdditionalInformation(info); - if (map.get("jwks_uri") instanceof String || map.get("jwks") instanceof String) { - String jwksUri = (String) map.get("jwks_uri"); - String jwks = (String) map.get("jwks"); + if (map.get(JWKS_URI) instanceof String || map.get(JWKS) instanceof String) { + String jwksUri = (String) map.get(JWKS_URI); + String jwks = (String) map.get(JWKS); ClientJwtConfiguration keyConfig = ClientJwtConfiguration.parse(jwksUri, jwks); if (keyConfig != null && keyConfig.getCleanString() != null) { keyConfig.writeValue(client); 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 65d03269008..b7ed03e12c8 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 @@ -6,6 +6,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.nimbusds.jose.jwk.JWK; import com.nimbusds.jose.jwk.JWKSet; +import org.cloudfoundry.identity.uaa.oauth.client.ClientJwtChangeRequest; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKeyHelper; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKeySet; @@ -29,13 +30,16 @@ @JsonIgnoreProperties(ignoreUnknown = true) public class ClientJwtConfiguration implements Cloneable{ + public static final String JWKS_URI = ClientJwtChangeRequest.JWKS_URI; + public static final String JWKS = ClientJwtChangeRequest.JWKS; + @JsonIgnore private static final int MAX_KEY_SIZE = 10; - @JsonProperty("jwks_uri") + @JsonProperty(JWKS_URI) private String jwksUri; - @JsonProperty("jwks") + @JsonProperty(JWKS) private JsonWebKeySet jwkSet; public ClientJwtConfiguration() { From 5a85e5459a1407d1c855a8091079d062d7426147 Mon Sep 17 00:00:00 2001 From: strehle Date: Tue, 19 Sep 2023 10:05:53 +0200 Subject: [PATCH 22/22] fix sonar issues --- .../uaa/client/ClientAdminEndpoints.java | 3 ++- .../uaa/client/ClientJwtConfiguration.java | 21 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java index 377d2e3b653..84cf4aa7d29 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java @@ -66,6 +66,7 @@ import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; @@ -543,7 +544,7 @@ public ActionResult changeSecret(@PathVariable String client_id, @RequestBody Se return result; } - @RequestMapping(value = "/oauth/clients/{client_id}/clientjwt", method = RequestMethod.PUT) + @PutMapping(value = "/oauth/clients/{client_id}/clientjwt") @ResponseBody public ActionResult changeClientJwt(@PathVariable String client_id, @RequestBody ClientJwtChangeRequest change) { 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 b7ed03e12c8..792499e100b 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 @@ -149,14 +149,13 @@ private static ClientJwtConfiguration parseJwkSet(String privateKeyJwt) { } private static ClientJwtConfiguration parseJwksUri(String privateKeyUrl) { - ClientJwtConfiguration clientJwtConfiguration; - String normalizedUri = null; + String normalizedUri; try { normalizedUri = UaaUrlUtils.normalizeUri(privateKeyUrl); } catch (IllegalArgumentException e) { throw new InvalidClientDetailsException("Client jwt configuration with invalid URI", e); } - clientJwtConfiguration = new ClientJwtConfiguration(normalizedUri, null); + ClientJwtConfiguration clientJwtConfiguration = new ClientJwtConfiguration(normalizedUri, null); clientJwtConfiguration.validateJwksUri(); return clientJwtConfiguration; } @@ -167,7 +166,7 @@ private boolean validateJwkSet() { throw new InvalidClientDetailsException("Invalid private_key_jwt: jwk set is empty of exceeds to maximum of keys. max: + " + MAX_KEY_SIZE); } Set keyId = new HashSet<>(); - keyList.forEach(key -> { + keyList.forEach((JsonWebKey key) -> { if (!StringUtils.hasText(key.getKid())) { throw new InvalidClientDetailsException("Invalid private_key_jwt: kid is required attribute"); } @@ -180,19 +179,19 @@ private boolean validateJwkSet() { } private boolean validateJwksUri() { - URI jwksUri; + URI validateJwksUri; try { - jwksUri = URI.create(this.jwksUri); + validateJwksUri = URI.create(this.jwksUri); } catch (IllegalArgumentException e) { throw new InvalidClientDetailsException("Invalid private_key_jwt: jwks_uri must be URI complaint", e); } - if (!jwksUri.isAbsolute()) { + if (!validateJwksUri.isAbsolute()) { throw new InvalidClientDetailsException("Invalid private_key_jwt: jwks_uri must be an absolute URL"); } - if (!"https".equals(jwksUri.getScheme()) && !"http".equals(jwksUri.getScheme())) { + if (!"https".equals(validateJwksUri.getScheme()) && !"http".equals(validateJwksUri.getScheme())) { throw new InvalidClientDetailsException("Invalid private_key_jwt: jwks_uri must be either using https or http"); } - if ("http".equals(jwksUri.getScheme()) && !jwksUri.getHost().endsWith("localhost")) { + if ("http".equals(validateJwksUri.getScheme()) && !validateJwksUri.getHost().endsWith("localhost")) { throw new InvalidClientDetailsException("Invalid private_key_jwt: jwks_uri with http is not on localhost"); } return true; @@ -212,7 +211,7 @@ public static ClientJwtConfiguration readValue(UaaClientDetails clientDetails) { !(clientDetails.getClientJwtConfig() instanceof String)) { return null; } - return JsonUtils.readValue((String) clientDetails.getClientJwtConfig(), ClientJwtConfiguration.class); + return JsonUtils.readValue(clientDetails.getClientJwtConfig(), ClientJwtConfiguration.class); } /** @@ -272,7 +271,7 @@ public static ClientJwtConfiguration merge(ClientJwtConfiguration existingConfig JsonWebKeySet existingKeySet = existingConfig.jwkSet; List existingKeys = new ArrayList<>(existingKeySet.getKeys()); List newKeys = new ArrayList<>(); - newConfig.getJwkSet().getKeys().forEach(key -> { + newConfig.getJwkSet().getKeys().forEach((JsonWebKey key) -> { if (existingKeys.contains(key)) { if (overwrite) { existingKeys.remove(key);