Skip to content

Commit

Permalink
Fix client's tokens lifetime and authentication method management (#677)
Browse files Browse the repository at this point in the history
  • Loading branch information
@enricovianello
enricovianello authored Dec 11, 2023
1 parent 8a10e8e commit 07b5dd4
Show file tree
Hide file tree
Showing 22 changed files with 513 additions and 151 deletions.
24 changes: 15 additions & 9 deletions ...ain/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

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 static org.mitre.oauth2.model.ClientDetailsEntity.AuthMethod.NONE;

import java.text.ParseException;
import java.time.Clock;
Expand All @@ -29,7 +31,6 @@
import org.mitre.oauth2.model.ClientDetailsEntity;
import org.mitre.oauth2.model.OAuth2AccessTokenEntity;
import org.mitre.openid.connect.service.OIDCTokenService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
Expand Down Expand Up @@ -71,8 +72,6 @@ public class DefaultClientManagementService implements ClientManagementService {
private final IamTokenService tokenService;
private final ApplicationEventPublisher eventPublisher;


@Autowired
public DefaultClientManagementService(Clock clock, ClientService clientService,
ClientConverter converter, ClientDefaultsService defaultsService, UserConverter userConverter,
IamAccountRepository accountRepo, OIDCTokenService oidcTokenService,
Expand Down Expand Up @@ -106,13 +105,13 @@ public ListResponseDTO<RegisteredClientDTO> retrieveAllClients(Pageable pageable

@Override
public Optional<RegisteredClientDTO> 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);
Expand Down Expand Up @@ -150,6 +149,12 @@ public RegisteredClientDTO updateClient(String clientId, RegisteredClientDTO cli
newClient.setAuthorities(oldClient.getAuthorities());
newClient.setDynamicallyRegistered(oldClient.isDynamicallyRegistered());

if (NONE.equals(newClient.getTokenEndpointAuthMethod())) {
newClient.setClientSecret(null);
} else if (isNull(client.getClientSecret())) {
client.setClientSecret(defaultsService.generateClientSecret());
}

newClient = clientService.updateClient(newClient);
eventPublisher.publishEvent(new ClientUpdatedEvent(this, newClient));
return converter.registeredClientDtoFromEntity(newClient);
Expand Down Expand Up @@ -227,15 +232,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));
Expand Down
8 changes: 2 additions & 6 deletions ...java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.mitre.oauth2.model.OAuth2AccessTokenEntity;
import org.mitre.oauth2.service.SystemScopeService;
import org.mitre.openid.connect.service.OIDCTokenService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.security.access.AccessDeniedException;
Expand All @@ -61,7 +60,6 @@
import it.infn.mw.iam.audit.events.client.ClientRegistrationAccessTokenRotatedEvent;
import it.infn.mw.iam.audit.events.client.ClientRemovedEvent;
import it.infn.mw.iam.audit.events.client.ClientUpdatedEvent;
import it.infn.mw.iam.config.IamProperties;
import it.infn.mw.iam.config.client_registration.ClientRegistrationProperties;
import it.infn.mw.iam.config.client_registration.ClientRegistrationProperties.ClientRegistrationAuthorizationPolicy;
import it.infn.mw.iam.core.IamTokenService;
Expand Down Expand Up @@ -99,13 +97,11 @@ public class DefaultClientRegistrationService implements ClientRegistrationServi
private final ApplicationEventPublisher eventPublisher;


@Autowired
public DefaultClientRegistrationService(Clock clock, ClientService clientService,
AccountUtils accountUtils, ClientConverter converter, ClientDefaultsService defaultsService,
OIDCTokenService clientTokenService, IamTokenService tokenService,
SystemScopeService scopeService, ClientRegistrationProperties registrationProperties,
IamProperties iamProperties, ScopeMatcherRegistry scopeMatcherRegistry,
ApplicationEventPublisher aep) {
ScopeMatcherRegistry scopeMatcherRegistry, ApplicationEventPublisher aep) {

this.clock = clock;
this.clientService = clientService;
Expand Down Expand Up @@ -439,7 +435,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));
Expand Down
61 changes: 27 additions & 34 deletions iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.mitre.oauth2.model.ClientDetailsEntity;
import org.mitre.oauth2.model.ClientDetailsEntity.AuthMethod;
import org.mitre.oauth2.model.PKCEAlgorithm;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

import com.google.common.base.Strings;
Expand All @@ -46,15 +45,14 @@ public class ClientConverter {
private final IamProperties iamProperties;

private final String clientRegistrationBaseUrl;
private final ClientRegistrationProperties clientRegistrationProperties;

private final ClientRegistrationProperties clientProperties;

@Autowired
public ClientConverter(IamProperties properties, ClientRegistrationProperties clientProperties) {
public ClientConverter(IamProperties properties,
ClientRegistrationProperties clientRegistrationProperties) {
this.iamProperties = properties;
this.clientRegistrationProperties = clientRegistrationProperties;
clientRegistrationBaseUrl =
String.format("%s%s", iamProperties.getBaseUrl(), ClientRegistrationApiController.ENDPOINT);
this.clientProperties = clientProperties;
}

private <T> Set<T> cloneSet(Set<T> stringSet) {
Expand All @@ -70,28 +68,16 @@ public ClientDetailsEntity entityFromClientManagementRequest(RegisteredClientDTO
throws ParseException {
ClientDetailsEntity client = entityFromRegistrationRequest(dto);

if (dto.getAccessTokenValiditySeconds() != null) {
if (dto.getAccessTokenValiditySeconds() != null && dto.getAccessTokenValiditySeconds() > 0) {
client.setAccessTokenValiditySeconds(dto.getAccessTokenValiditySeconds());
} else {
client.setAccessTokenValiditySeconds(
clientProperties.getClientDefaults().getDefaultAccessTokenValiditySeconds());
}

if (dto.getRefreshTokenValiditySeconds() != null) {
// Refresh Token validity seconds zero value is valid and means infinite duration
if (dto.getRefreshTokenValiditySeconds() != null && dto.getRefreshTokenValiditySeconds() >= 0) {
client.setRefreshTokenValiditySeconds(dto.getRefreshTokenValiditySeconds());
} else {
client.setRefreshTokenValiditySeconds(
clientProperties.getClientDefaults().getDefaultRefreshTokenValiditySeconds());
}

if (dto.getIdTokenValiditySeconds() != null) {
if (dto.getIdTokenValiditySeconds() <= 0) {
client.setIdTokenValiditySeconds(null);
} else {
client.setIdTokenValiditySeconds(dto.getIdTokenValiditySeconds());
}
if (dto.getIdTokenValiditySeconds() != null && dto.getIdTokenValiditySeconds() > 0) {
client.setIdTokenValiditySeconds(dto.getIdTokenValiditySeconds());
}

if (dto.getDeviceCodeValiditySeconds() != null && dto.getDeviceCodeValiditySeconds() > 0) {
client.setDeviceCodeValiditySeconds(dto.getDeviceCodeValiditySeconds());
}
Expand All @@ -105,6 +91,11 @@ public ClientDetailsEntity entityFromClientManagementRequest(RegisteredClientDTO
client.setCodeChallengeMethod(pkceAlgo);
}

if (dto.getTokenEndpointAuthMethod() != null) {
client
.setTokenEndpointAuthMethod(AuthMethod.getByValue(dto.getTokenEndpointAuthMethod().name()));
}

client.setRequireAuthTime(Boolean.valueOf(dto.isRequireAuthTime()));

return client;
Expand Down Expand Up @@ -196,16 +187,19 @@ public ClientDetailsEntity entityFromRegistrationRequest(RegisteredClientDTO dto

client.setLogoUri(dto.getLogoUri());
client.setPolicyUri(dto.getPolicyUri());

client.setRedirectUris(cloneSet(dto.getRedirectUris()));

client.setScope(cloneSet(dto.getScope()));

client.setGrantTypes(new HashSet<>());
client.setGrantTypes(new HashSet<>());

if (!isNull(dto.getGrantTypes())) {
client.setGrantTypes(
dto.getGrantTypes().stream().map(AuthorizationGrantType::getGrantType).collect(toSet()));
dto.getGrantTypes()
.stream()
.map(AuthorizationGrantType::getGrantType)
.collect(toSet()));
}

if (dto.getScope().contains("offline_access")) {
Expand All @@ -219,9 +213,7 @@ public ClientDetailsEntity entityFromRegistrationRequest(RegisteredClientDTO dto

client.setContacts(cloneSet(dto.getContacts()));

if (isNull(dto.getTokenEndpointAuthMethod())) {
client.setTokenEndpointAuthMethod(AuthMethod.SECRET_BASIC);
} else {
if (!isNull(dto.getTokenEndpointAuthMethod())) {
client
.setTokenEndpointAuthMethod(AuthMethod.getByValue(dto.getTokenEndpointAuthMethod().name()));
}
Expand All @@ -231,10 +223,11 @@ public ClientDetailsEntity entityFromRegistrationRequest(RegisteredClientDTO dto
client.setCodeChallengeMethod(pkceAlgo);
}

client.setAccessTokenValiditySeconds(
clientProperties.getClientDefaults().getDefaultAccessTokenValiditySeconds());
client.setRefreshTokenValiditySeconds(
clientProperties.getClientDefaults().getDefaultRefreshTokenValiditySeconds());
// bypasses MitreID default setting to zero inside client's entity
client.setAccessTokenValiditySeconds(clientRegistrationProperties.getClientDefaults().getDefaultAccessTokenValiditySeconds());
client.setRefreshTokenValiditySeconds(clientRegistrationProperties.getClientDefaults().getDefaultRefreshTokenValiditySeconds());
client.setIdTokenValiditySeconds(clientRegistrationProperties.getClientDefaults().getDefaultIdTokenValiditySeconds());
client.setDeviceCodeValiditySeconds(clientRegistrationProperties.getClientDefaults().getDefaultDeviceCodeValiditySeconds());

return client;
}
Expand Down
34 changes: 24 additions & 10 deletions ...service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientDefaultsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,12 @@
import java.math.BigInteger;
import java.security.SecureRandom;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.Set;
import java.util.UUID;

import org.apache.commons.codec.binary.Base64;
import org.mitre.oauth2.model.ClientDetailsEntity;
import org.mitre.oauth2.model.ClientDetailsEntity.AuthMethod;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;

import com.google.common.collect.Sets;
Expand All @@ -46,7 +44,6 @@ public class DefaultClientDefaultsService implements ClientDefaultsService {

private final ClientRegistrationProperties properties;

@Autowired
public DefaultClientDefaultsService(ClientRegistrationProperties properties) {
this.properties = properties;
}
Expand All @@ -58,19 +55,36 @@ public ClientDetailsEntity setupClientDefaults(ClientDetailsEntity client) {
client.setClientId(UUID.randomUUID().toString());
}

client
.setIdTokenValiditySeconds(properties.getClientDefaults().getDefaultIdTokenValiditySeconds());
if (client.getAccessTokenValiditySeconds() == null
|| client.getAccessTokenValiditySeconds() == 0) {
client.setAccessTokenValiditySeconds(
properties.getClientDefaults().getDefaultAccessTokenValiditySeconds());
}

if (client.getRefreshTokenValiditySeconds() == null) {
client.setRefreshTokenValiditySeconds(
properties.getClientDefaults().getDefaultRefreshTokenValiditySeconds());
}

client.setDeviceCodeValiditySeconds(
properties.getClientDefaults().getDefaultDeviceCodeValiditySeconds());
if (client.getIdTokenValiditySeconds() == null || client.getIdTokenValiditySeconds() == 0) {
client.setIdTokenValiditySeconds(
properties.getClientDefaults().getDefaultIdTokenValiditySeconds());
}

if (client.getDeviceCodeValiditySeconds() == null
|| client.getDeviceCodeValiditySeconds() == 0) {
client.setDeviceCodeValiditySeconds(
properties.getClientDefaults().getDefaultDeviceCodeValiditySeconds());
}

client.setAllowIntrospection(true);

if (isNull(client.getContacts())) {
client.setContacts(new HashSet<>());
if (isNull(client.getTokenEndpointAuthMethod())) {
client.setTokenEndpointAuthMethod(AuthMethod.SECRET_BASIC);
}

if (AUTH_METHODS_REQUIRING_SECRET.contains(client.getTokenEndpointAuthMethod())) {
if (isNull(client.getClientSecret())
&& AUTH_METHODS_REQUIRING_SECRET.contains(client.getTokenEndpointAuthMethod())) {
client.setClientSecret(generateClientSecret());
}

Expand Down
14 changes: 6 additions & 8 deletions ...src/main/java/it/infn/mw/iam/config/client_registration/ClientRegistrationProperties.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

import static it.infn.mw.iam.config.client_registration.ClientRegistrationProperties.ClientRegistrationAuthorizationPolicy.ANYONE;

import java.util.concurrent.TimeUnit;

import javax.validation.constraints.NotNull;

import org.springframework.boot.context.properties.ConfigurationProperties;
Expand All @@ -30,14 +28,14 @@ public class ClientRegistrationProperties {

public static class ClientDefaultsProperties {

@NotNull(message = "Provide a default access token lifetime")
@NotNull(message = "Provide a default access token lifetime")
private int defaultAccessTokenValiditySeconds;

@NotNull(message = "Provide a default refresh token lifetime")
@NotNull(message = "Provide a default id token lifetime")
private int defaultIdTokenValiditySeconds;
@NotNull(message = "Provide a default device code lifetime")
private int defaultDeviceCodeValiditySeconds;
@NotNull(message = "Provide a default refresh token lifetime")
private int defaultRefreshTokenValiditySeconds;

private int defaultIdTokenValiditySeconds = (int) TimeUnit.MINUTES.toSeconds(10);
private int defaultDeviceCodeValiditySeconds = (int) TimeUnit.MINUTES.toSeconds(10);

private int defaultRegistrationAccessTokenValiditySeconds = -1;

Expand Down
6 changes: 3 additions & 3 deletions iam-login-service/src/main/java/it/infn/mw/iam/core/web/util/IamViewInfoInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ public class IamViewInfoInterceptor implements HandlerInterceptor {

@Autowired
IamProperties iamProperties;

@Autowired
ClientRegistrationProperties clientRegistrationProperties;

@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler)
throws Exception {
Expand All @@ -81,7 +81,7 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
request.setAttribute(IAM_SAML_PROPERTIES_KEY, samlProperties);

request.setAttribute(RCAUTH_ENABLED_KEY, rcAuthProperties.isEnabled());

request.setAttribute(CLIENT_DEFAULTS_PROPERTIES_KEY, clientRegistrationProperties.getClientDefaults());

if (iamProperties.getVersionedStaticResources().isEnableVersioning()) {
Expand Down
5 changes: 3 additions & 2 deletions iam-login-service/src/main/resources/application.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,9 @@ client-registration:
enable: ${IAM_CLIENT_REGISTRATION_ENABLE:true}
client-defaults:
default-access-token-validity-seconds: ${DEFAULT_ACCESS_TOKEN_VALIDITY_SECONDS:3600}
default-refresh-token-validity-seconds: ${DEFAULT_REFRESH_TOKEN_VALIDITY_SECONDS:108000}

default-device-code-validity-seconds: ${DEFAULT_DEVICE_CODE_VALIDITY_SECONDS:600}
default-id-token-validity-seconds: ${DEFAULT_ID_TOKEN_VALIDITY_SECONDS:600}
default-refresh-token-validity-seconds: ${DEFAULT_REFRESH_TOKEN_VALIDITY_SECONDS:2592000}

management:
health:
Expand Down
Loading

0 comments on commit 07b5dd4

Please sign in to comment.