Skip to content

Commit

Permalink
Refactoring because of usage of client_jwt_config now from oauth_clie…
Browse files Browse the repository at this point in the history 
…nt_details

additional_information is not used anymore
  • Loading branch information
@strehle
strehle committed Aug 24, 2023
1 parent e4ea431 commit 415983b
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 82 deletions.
1 change: 0 additions & 1 deletion model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
31 changes: 16 additions & 15 deletions server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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) {
Expand All @@ -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());
}
Expand Down
8 changes: 4 additions & 4 deletions server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()){
Expand Down
34 changes: 13 additions & 21 deletions server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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{
Expand Down Expand Up @@ -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<String, Object> 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<String, Object> 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);
}
}

Expand Down
18 changes: 9 additions & 9 deletions ...src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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);
}
}

Expand Down
26 changes: 13 additions & 13 deletions server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,29 +255,29 @@ void simpleAddClientWithSignupSuccessRedirectUrl() throws Exception {
@Test
void simpleAddClientWithJwksUri() throws Exception {
Map<String, Object> 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<String, Object> 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
Expand Down Expand Up @@ -355,42 +355,42 @@ void setUp() {
@Test
void simpleAddClientWithAutoApprove() {
Map<String, Object> 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");
}

@Test
void simpleAddClientWithAllowPublic() {
Map<String, Object> 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");
}

@Test
void simpleAddClientWithAllowPublicNoClient() {
Map<String, Object> 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());
Expand Down
12 changes: 6 additions & 6 deletions server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1074,9 +1074,9 @@ void testCreateClientWithJsonWebKeyUri() {
createRequest.setJsonWebKeyUri(jwksUri);
ClientDetails result = endpoints.createClientDetails(createRequest);
assertNull(result.getClientSecret());
ArgumentCaptor<BaseClientDetails> clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class);
ArgumentCaptor<UaaClientDetails> 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));
}

Expand All @@ -1094,9 +1094,9 @@ void testCreateClientWithJsonWebKeyUriInvalid() {
createRequest.setJsonWebKeySet(jwksUri);
ClientDetails result = endpoints.createClientDetails(createRequest);
assertNull(result.getClientSecret());
ArgumentCaptor<BaseClientDetails> clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class);
ArgumentCaptor<UaaClientDetails> clientCaptor = ArgumentCaptor.forClass(UaaClientDetails.class);
verify(clientDetailsService).create(clientCaptor.capture(), anyString());
BaseClientDetails created = clientCaptor.getValue();
UaaClientDetails created = clientCaptor.getValue();
assertNull(ClientJwtConfiguration.readValue(created));
}

Expand Down Expand Up @@ -1173,9 +1173,9 @@ void testCreateClientWithJsonKeyWebSet() {
createRequest.setJsonWebKeySet(jsonJwk);
ClientDetails result = endpoints.createClientDetails(createRequest);
assertNull(result.getClientSecret());
ArgumentCaptor<BaseClientDetails> clientCaptor = ArgumentCaptor.forClass(BaseClientDetails.class);
ArgumentCaptor<UaaClientDetails> 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));
Expand Down
Loading

0 comments on commit 415983b

Please sign in to comment.