Revert "Fix management of tokens lifetime following RFC9068 (#620)"

This reverts commit 3749a72.
indigo-iam · Dec 10, 2023 · ad11c01 · ad11c01
1 parent 2c11a7c
commit ad11c01
Show file tree

Hide file tree

Showing 12 changed files with 76 additions and 67 deletions.
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
@@ -38,7 +38,6 @@
 import it.infn.mw.iam.api.common.client.RegisteredClientDTO;
 import it.infn.mw.iam.api.common.client.TokenEndpointAuthenticationMethod;
 import it.infn.mw.iam.config.IamProperties;
-import it.infn.mw.iam.config.client_registration.ClientRegistrationProperties;
 
 @Component
 public class ClientConverter {
@@ -47,14 +46,11 @@ public class ClientConverter {
 
   private final String clientRegistrationBaseUrl;
 
-  private final ClientRegistrationProperties clientProperties;
-
   @Autowired
-  public ClientConverter(IamProperties properties, ClientRegistrationProperties clientProperties) {
+  public ClientConverter(IamProperties properties) {
     this.iamProperties = properties;
     clientRegistrationBaseUrl =
         String.format("%s%s", iamProperties.getBaseUrl(), ClientRegistrationApiController.ENDPOINT);
-    this.clientProperties = clientProperties;
   }
 
   private <T> Set<T> cloneSet(Set<T> stringSet) {
@@ -71,17 +67,18 @@ public ClientDetailsEntity entityFromClientManagementRequest(RegisteredClientDTO
     ClientDetailsEntity client = entityFromRegistrationRequest(dto);
 
     if (dto.getAccessTokenValiditySeconds() != null) {
-      client.setAccessTokenValiditySeconds(dto.getAccessTokenValiditySeconds());
-    } else {
-      client.setAccessTokenValiditySeconds(
-          clientProperties.getClientDefaults().getDefaultAccessTokenValiditySeconds());
+      if (dto.getAccessTokenValiditySeconds() <= 0) {
+        client.setAccessTokenValiditySeconds(null);
+      } else {
+        client.setAccessTokenValiditySeconds(dto.getAccessTokenValiditySeconds());
+      }
     }
-
     if (dto.getRefreshTokenValiditySeconds() != null) {
-      client.setRefreshTokenValiditySeconds(dto.getRefreshTokenValiditySeconds());
-    } else {
-      client.setRefreshTokenValiditySeconds(
-          clientProperties.getClientDefaults().getDefaultRefreshTokenValiditySeconds());
+      if (dto.getRefreshTokenValiditySeconds() <= 0) {
+        client.setRefreshTokenValiditySeconds(null);
+      } else {
+        client.setRefreshTokenValiditySeconds(dto.getRefreshTokenValiditySeconds());
+      }
     }
 
     if (dto.getIdTokenValiditySeconds() != null) {
@@ -196,16 +193,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")) {
@@ -231,11 +231,6 @@ public ClientDetailsEntity entityFromRegistrationRequest(RegisteredClientDTO dto
       client.setCodeChallengeMethod(pkceAlgo);
     }
 
-    client.setAccessTokenValiditySeconds(
-        clientProperties.getClientDefaults().getDefaultAccessTokenValiditySeconds());
-    client.setRefreshTokenValiditySeconds(
-        clientProperties.getClientDefaults().getDefaultRefreshTokenValiditySeconds());
-
     return client;
   }
 

diff --git a/...service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientDefaultsService.java b/...service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientDefaultsService.java
@@ -58,18 +58,33 @@ public ClientDetailsEntity setupClientDefaults(ClientDetailsEntity client) {
       client.setClientId(UUID.randomUUID().toString());
     }
 
+    client.setAccessTokenValiditySeconds(
+        properties.getClientDefaults().getDefaultAccessTokenValiditySeconds());
+
     client
       .setIdTokenValiditySeconds(properties.getClientDefaults().getDefaultIdTokenValiditySeconds());
 
     client.setDeviceCodeValiditySeconds(
         properties.getClientDefaults().getDefaultDeviceCodeValiditySeconds());
 
+    final int rtSecs = properties.getClientDefaults().getDefaultRefreshTokenValiditySeconds();
+
+    if (rtSecs < 0) {
+      client.setRefreshTokenValiditySeconds(null);
+    } else {
+      client.setRefreshTokenValiditySeconds(rtSecs);
+    }
+
     client.setAllowIntrospection(true);
 
     if (isNull(client.getContacts())) {
       client.setContacts(new HashSet<>());
     }
 
+    if (isNull(client.getClientId())) {
+      client.setClientId(UUID.randomUUID().toString());
+    }
+
     if (AUTH_METHODS_REQUIRING_SECRET.contains(client.getTokenEndpointAuthMethod())) {
       client.setClientSecret(generateClientSecret());
     }

diff --git a/...src/main/java/it/infn/mw/iam/config/client_registration/ClientRegistrationProperties.java b/...src/main/java/it/infn/mw/iam/config/client_registration/ClientRegistrationProperties.java
@@ -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;
@@ -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;
 

diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/util/IamViewInfoInterceptor.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/util/IamViewInfoInterceptor.java
@@ -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 {
@@ -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()) {

diff --git a/iam-login-service/src/main/resources/application.yml b/iam-login-service/src/main/resources/application.yml
@@ -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:

diff --git a/iam-login-service/src/main/webapp/WEB-INF/tags/iamHeader.tag b/iam-login-service/src/main/webapp/WEB-INF/tags/iamHeader.tag
@@ -81,21 +81,13 @@ function getRegistrationEnabled() {
 }
 
 function getAccountLinkingEnabled() {
-    return ${loginPageConfiguration.accountLinkingEnabled};
+  return ${loginPageConfiguration.accountLinkingEnabled};
 }
 
 function getOrganisationName() {
  return '${iamOrganisationName}'; 
 }
 
-function getAccessTokenValiditySeconds() {
-  return ${clientDefaultsProperties.defaultAccessTokenValiditySeconds};
-}
-
-function getRefreshTokenValiditySeconds() {
-  return ${clientDefaultsProperties.defaultRefreshTokenValiditySeconds};
-}
-
 function getOidcEnabled() {
   return ${loginPageConfiguration.oidcEnabled};
 }
@@ -109,6 +101,14 @@ function getRcauthEnabled() {
 }
 
 function getExternalAuthenticationEnabled() {
-    return ${loginPageConfiguration.externalAuthenticationEnabled};
+  return ${loginPageConfiguration.externalAuthenticationEnabled};
+}
+
+function getAccessTokenValiditySeconds() {
+  return ${clientDefaultsProperties.defaultAccessTokenValiditySeconds};
+}
+
+function getRefreshTokenValiditySeconds() {
+  return ${clientDefaultsProperties.defaultRefreshTokenValiditySeconds};
 }
 </script>
diff --git a/...m/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.html b/...m/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.html
@@ -20,20 +20,19 @@
         <div class="form-group">
             <label for="at_timeout">Access token timeout (seconds)</label>
             <input id="at_timeout" class="form-control" type="text"
-                ng-model="$ctrl.client.access_token_validity_seconds">
+                ng-model="$ctrl.client.access_token_validity_seconds"/>
         </div>
-
     </div>
     <div class="form-group">
         <label for="id_timeout">ID token timeout (seconds)</label>
 
         <input id="id_timeout" class="form-control" type="text" ng-model="$ctrl.client.id_token_validity_seconds"
-            placeholder="600" ng-required ng-min="30">
+            placeholder="600" ng-required ng-min="30"/>
     </div>
     <div class="form-group">
         <div class="checkbox">
             <label>
-                <input type="checkbox" ng-model="$ctrl.client.require_auth_time">
+                <input type="checkbox" ng-model="$ctrl.client.require_auth_time"/>
                 Always require authentication time in ID tokens
             </label>
         </div>
@@ -57,11 +56,11 @@
         <label for="rt_timeout">Refresh token timeout (seconds)</label>
 
         <input id="rt_timeout" class="form-control" type="text" ng-model="$ctrl.client.refresh_token_validity_seconds"
-            ng-required ng-min="30">
-            <p class="help-block">
-                This will setup after how many seconds the refresh token
-                expires. Type 0 to set an infinite lifetime.
-            </p>
+            ng-required ng-min="30"/>
+        <p class="help-block">
+           This will setup after how many seconds the refresh token
+           expires. Type 0 to set an infinite lifetime.
+        </p>
 
     </div>
     <div class="form-group">

diff --git a/...iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.js b/...iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.js
@@ -26,27 +26,28 @@
         self.toggleOfflineAccess = toggleOfflineAccess;
         self.canIssueRefreshTokens = false;
         self.hasDeviceCodeGrantType = false;
-        self.accessTokenValiditySeconds = getAccessTokenValiditySeconds();
-        self.refreshTokenValiditySeconds = getRefreshTokenValiditySeconds();
+
+        self.accessTokenDefaultValiditySeconds = getAccessTokenValiditySeconds();
+        self.refreshTokenDefaultValiditySeconds = getRefreshTokenValiditySeconds();
+
 
         self.$onInit = function () {
             console.debug('TokenSettingsController.self', self);
             if (self.client.access_token_validity_seconds == null) {
-                self.client.access_token_validity_seconds = self.accessTokenValiditySeconds;
+                self.client.access_token_validity_seconds = self.accessTokenDefaultValiditySeconds;
             }
-
             if (self.client.refresh_token_validity_seconds == null) {
-                self.client.refresh_token_validity_seconds = self.refreshTokenValiditySeconds;
+                self.client.refresh_token_validity_seconds = self.refreshTokenDefaultValiditySeconds;
             }
 
             $scope.$watch('$ctrl.client.access_token_validity_seconds', function handleChange(newVal, oldVal) {
-                if (newVal <= 0) {
+                if (newVal < 0) {
                     self.client.access_token_validity_seconds = 0;
                 }
             });
 
             $scope.$watch('$ctrl.client.refresh_token_validity_seconds', function handleChange(newVal, oldVal) {
-                if (newVal <= 0) {
+                if (newVal < 0) {
                     self.client.refresh_token_validity_seconds = 0;
                 }
             });

diff --git a/...ice/src/test/java/it/infn/mw/iam/test/api/client/ClientManagementAPIIntegrationTests.java b/...ice/src/test/java/it/infn/mw/iam/test/api/client/ClientManagementAPIIntegrationTests.java
@@ -20,7 +20,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasSize;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.springframework.http.MediaType.APPLICATION_JSON;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
@@ -261,4 +261,5 @@ public void negativeTokenLifetimesNotAllowed() throws Exception {
       .andExpect(BAD_REQUEST)
       .andExpect(jsonPath("$.error", containsString("must be greater than or equal to 0")));
   }
+
 }
diff --git a/...e/src/test/java/it/infn/mw/iam/test/api/client/ClientRegistrationAPIIntegrationTests.java b/...e/src/test/java/it/infn/mw/iam/test/api/client/ClientRegistrationAPIIntegrationTests.java
@@ -137,4 +137,4 @@ public void tokenLifetimesAreNotEditable() throws Exception {
 
   }
 
-}
+}
diff --git a/...est/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTestSupport.java b/...est/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTestSupport.java
@@ -105,8 +105,7 @@ public String build() {
       json.add(CLAIMS_REDIRECT_URIS, getAsArray(newHashSet(), true));
       json.add(REQUEST_URIS, getAsArray(newHashSet(), true));
       json.add(CONTACTS, getAsArray(newHashSet("test@iam.test")));
-      json.addProperty("access_token_validity_seconds", accessTokenValiditySeconds);
-      json.addProperty("refresh_token_validity_seconds", refreshTokenValiditySeconds);
+
       return json.toString();
     }
 

diff --git a/pom.xml b/pom.xml
@@ -50,7 +50,7 @@
 
     <testcontainers.version>1.16.2</testcontainers.version>
 
-    <mitreid.version>1.3.6.cnaf-20231113</mitreid.version>
+    <mitreid.version>1.3.6.cnaf-20231129</mitreid.version>
     <spring-security-oauth2.version>2.5.2.RELEASE</spring-security-oauth2.version>
 
     <voms.version>3.3.2</voms.version>