From 87226ef722c24e261da01bcc3872485427c8211c Mon Sep 17 00:00:00 2001 From: Federica Agostini Date: Wed, 6 Dec 2023 17:22:26 +0100 Subject: [PATCH 1/2] Reset client secret when authN is set to none or else, generate a new Random secret. --- .../DefaultClientRegistrationService.java | 1 - .../api/client/service/ClientConverter.java | 25 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java index a88b25808..e009d674f 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java @@ -383,7 +383,6 @@ public RegisteredClientDTO updateClient(String clientId, RegisteredClientDTO req ClientDetailsEntity newClient = converter.entityFromRegistrationRequest(request); newClient.setId(oldClient.getId()); - newClient.setClientSecret(oldClient.getClientSecret()); newClient.setAccessTokenValiditySeconds(oldClient.getAccessTokenValiditySeconds()); newClient.setIdTokenValiditySeconds(oldClient.getIdTokenValiditySeconds()); newClient.setRefreshTokenValiditySeconds(oldClient.getRefreshTokenValiditySeconds()); diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java index 54b4692ab..78bcbf2cc 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java @@ -15,6 +15,9 @@ */ package it.infn.mw.iam.api.client.service; +import java.math.BigInteger; +import java.security.SecureRandom; + import static java.util.Objects.isNull; import static java.util.stream.Collectors.toSet; @@ -40,6 +43,9 @@ import it.infn.mw.iam.config.IamProperties; import it.infn.mw.iam.config.client_registration.ClientRegistrationProperties; +import org.apache.commons.codec.binary.Base64; + + @Component public class ClientConverter { @@ -48,6 +54,9 @@ public class ClientConverter { private final String clientRegistrationBaseUrl; private final ClientRegistrationProperties clientProperties; + + private static final int SECRET_SIZE = 512; + private static final SecureRandom RNG = new SecureRandom(); @Autowired public ClientConverter(IamProperties properties, ClientRegistrationProperties clientProperties) { @@ -173,6 +182,10 @@ public RegisteredClientDTO registeredClientDtoFromEntity(ClientDetailsEntity ent clientDTO.setRequireAuthTime(false); } + if (entity.getTokenEndpointAuthMethod() == AuthMethod.NONE) { + clientDTO.setClientSecret(null); + } + return clientDTO; } @@ -221,6 +234,12 @@ public ClientDetailsEntity entityFromRegistrationRequest(RegisteredClientDTO dto if (isNull(dto.getTokenEndpointAuthMethod())) { client.setTokenEndpointAuthMethod(AuthMethod.SECRET_BASIC); + } else if (dto.getTokenEndpointAuthMethod().equals(TokenEndpointAuthenticationMethod.none)) { + client.setTokenEndpointAuthMethod(AuthMethod.NONE); + client.setClientSecret(null); + } else if (!dto.getTokenEndpointAuthMethod().equals(TokenEndpointAuthenticationMethod.none) + && isNull(client.getClientSecret())) { + client.setClientSecret(generateClientSecret()); } else { client .setTokenEndpointAuthMethod(AuthMethod.getByValue(dto.getTokenEndpointAuthMethod().name())); @@ -247,4 +266,10 @@ public RegisteredClientDTO registrationResponseFromClient(ClientDetailsEntity en return response; } + public String generateClientSecret() { + return Base64.encodeBase64URLSafeString(new BigInteger(SECRET_SIZE, RNG).toByteArray()) + .replace("=", ""); + + } + } From 2a7ec8ddaaa6f129b5389d8bac11833fb7534d96 Mon Sep 17 00:00:00 2001 From: Federica Agostini Date: Wed, 6 Dec 2023 19:18:52 +0100 Subject: [PATCH 2/2] Move logic at service level --- .../DefaultClientManagementService.java | 23 ++++++++++++++----- .../DefaultClientRegistrationService.java | 11 ++++++++- .../api/client/service/ClientConverter.java | 20 ---------------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java index eb7faf6ec..239e385b5 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java @@ -17,6 +17,7 @@ import static it.infn.mw.iam.api.client.util.ClientSuppliers.accountNotFound; import static it.infn.mw.iam.api.client.util.ClientSuppliers.clientNotFound; +import static java.util.Objects.isNull; import java.text.ParseException; import java.time.Clock; @@ -27,6 +28,7 @@ import javax.validation.constraints.NotBlank; import org.mitre.oauth2.model.ClientDetailsEntity; +import org.mitre.oauth2.model.ClientDetailsEntity.AuthMethod; import org.mitre.oauth2.model.OAuth2AccessTokenEntity; import org.mitre.openid.connect.service.OIDCTokenService; import org.springframework.beans.factory.annotation.Autowired; @@ -106,13 +108,13 @@ public ListResponseDTO retrieveAllClients(Pageable pageable @Override public Optional retrieveClientByClientId(String clientId) { - return clientService.findClientByClientId(clientId).map(converter::registeredClientDtoFromEntity); + return clientService.findClientByClientId(clientId) + .map(converter::registeredClientDtoFromEntity); } @Validated(OnClientCreation.class) @Override - public RegisteredClientDTO saveNewClient(RegisteredClientDTO client) - throws ParseException { + public RegisteredClientDTO saveNewClient(RegisteredClientDTO client) throws ParseException { ClientDetailsEntity entity = converter.entityFromClientManagementRequest(client); entity.setDynamicallyRegistered(false); @@ -150,6 +152,14 @@ public RegisteredClientDTO updateClient(String clientId, RegisteredClientDTO cli newClient.setAuthorities(oldClient.getAuthorities()); newClient.setDynamicallyRegistered(oldClient.isDynamicallyRegistered()); + if (newClient.getTokenEndpointAuthMethod().equals(AuthMethod.NONE)) { + newClient.setTokenEndpointAuthMethod(AuthMethod.NONE); + newClient.setClientSecret(null); + } else if (!newClient.getTokenEndpointAuthMethod().equals(AuthMethod.NONE) + && isNull(newClient.getClientSecret())) { + newClient.setClientSecret(defaultsService.generateClientSecret()); + } + newClient = clientService.updateClient(newClient); eventPublisher.publishEvent(new ClientUpdatedEvent(this, newClient)); return converter.registeredClientDtoFromEntity(newClient); @@ -227,15 +237,16 @@ private OAuth2AccessTokenEntity createRegistrationAccessTokenForClient( return tokenService.saveAccessToken(token); } + @Override public RegisteredClientDTO rotateRegistrationAccessToken(@NotBlank String clientId) { ClientDetailsEntity client = clientService.findClientByClientId(clientId).orElseThrow(clientNotFound(clientId)); OAuth2AccessTokenEntity rat = - Optional.ofNullable(oidcTokenService.rotateRegistrationAccessTokenForClient(client)) - .orElse(createRegistrationAccessTokenForClient(client)); - + Optional.ofNullable(oidcTokenService.rotateRegistrationAccessTokenForClient(client)) + .orElse(createRegistrationAccessTokenForClient(client)); + tokenService.saveAccessToken(rat); eventPublisher.publishEvent(new ClientRegistrationAccessTokenRotatedEvent(this, client)); diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java index e009d674f..108f3e880 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java @@ -33,6 +33,7 @@ import javax.validation.constraints.NotBlank; import org.mitre.oauth2.model.ClientDetailsEntity; +import org.mitre.oauth2.model.ClientDetailsEntity.AuthMethod; import org.mitre.oauth2.model.OAuth2AccessTokenEntity; import org.mitre.oauth2.service.SystemScopeService; import org.mitre.openid.connect.service.OIDCTokenService; @@ -393,6 +394,14 @@ public RegisteredClientDTO updateClient(String clientId, RegisteredClientDTO req newClient.setCreatedAt(oldClient.getCreatedAt()); newClient.setReuseRefreshToken(oldClient.isReuseRefreshToken()); + if (newClient.getTokenEndpointAuthMethod().equals(AuthMethod.NONE)) { + newClient.setTokenEndpointAuthMethod(AuthMethod.NONE); + newClient.setClientSecret(null); + } else if (!newClient.getTokenEndpointAuthMethod().equals(AuthMethod.NONE) + && isNull(newClient.getClientSecret())) { + newClient.setClientSecret(defaultsService.generateClientSecret()); + } + ClientDetailsEntity savedClient = clientService.updateClient(newClient); eventPublisher.publishEvent(new ClientUpdatedEvent(this, savedClient)); @@ -438,7 +447,7 @@ public RegisteredClientDTO redeemClient(@NotBlank String clientId, final IamAccount account = accountUtils.getAuthenticatedUserAccount(authentication).orElseThrow(noAuthUserError()); - + client = clientService.linkClientToAccount(client, account); eventPublisher.publishEvent(new AccountClientOwnerAssigned(this, account, client)); diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java index 78bcbf2cc..16dfcfda1 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java @@ -15,9 +15,6 @@ */ package it.infn.mw.iam.api.client.service; -import java.math.BigInteger; -import java.security.SecureRandom; - import static java.util.Objects.isNull; import static java.util.stream.Collectors.toSet; @@ -43,8 +40,6 @@ import it.infn.mw.iam.config.IamProperties; import it.infn.mw.iam.config.client_registration.ClientRegistrationProperties; -import org.apache.commons.codec.binary.Base64; - @Component public class ClientConverter { @@ -54,9 +49,6 @@ public class ClientConverter { private final String clientRegistrationBaseUrl; private final ClientRegistrationProperties clientProperties; - - private static final int SECRET_SIZE = 512; - private static final SecureRandom RNG = new SecureRandom(); @Autowired public ClientConverter(IamProperties properties, ClientRegistrationProperties clientProperties) { @@ -234,12 +226,6 @@ public ClientDetailsEntity entityFromRegistrationRequest(RegisteredClientDTO dto if (isNull(dto.getTokenEndpointAuthMethod())) { client.setTokenEndpointAuthMethod(AuthMethod.SECRET_BASIC); - } else if (dto.getTokenEndpointAuthMethod().equals(TokenEndpointAuthenticationMethod.none)) { - client.setTokenEndpointAuthMethod(AuthMethod.NONE); - client.setClientSecret(null); - } else if (!dto.getTokenEndpointAuthMethod().equals(TokenEndpointAuthenticationMethod.none) - && isNull(client.getClientSecret())) { - client.setClientSecret(generateClientSecret()); } else { client .setTokenEndpointAuthMethod(AuthMethod.getByValue(dto.getTokenEndpointAuthMethod().name())); @@ -266,10 +252,4 @@ public RegisteredClientDTO registrationResponseFromClient(ClientDetailsEntity en return response; } - public String generateClientSecret() { - return Base64.encodeBase64URLSafeString(new BigInteger(SECRET_SIZE, RNG).toByteArray()) - .replace("=", ""); - - } - }