Remove option Nerver Expires for tokens in Advanced OIDC client configuration

Closes https://github.com/keycloak/keycloak/issues/21927

docs/documentation/upgrading/topics/keycloak/changes-22_0_2.adoc

@@ -0,0 +1,3 @@
+= Never expires option removed from client advanced settings combos
+
+The option `Never expires` is now removed from all the combos of the Advanced Settings client tab. This option was misleading because the different lifespans or idle timeouts were never infinite, but limited by the general user session or realm values. Therefore, this option is removed in favor of the other two remaining options: `Inherits from the realm settings` (the client uses general realm timeouts) and `Expires in` (the value is overriden for the client). Internally the `Never expires` was represented by `-1`. Now that value is shown with a warning in the Admin Console and cannot be set directly by the administrator.

docs/documentation/upgrading/topics/keycloak/changes.adoc

@@ -4,6 +4,10 @@
 
 include::changes-23_0_0.adoc[leveloffset=3]
 
+=== Migrating to 22.0.2
+
+include::changes-22_0_2.adoc[leveloffset=3]
+
 === Migrating to 22.0.0
 
 include::changes-22_0_0.adoc[leveloffset=3]

js/apps/admin-ui/src/clients/advanced/AdvancedSettings.tsx

@@ -132,7 +132,11 @@ export const AdvancedSettings = ({
             name={convertAttributeNameToForm(
               "attributes.client.offline.session.max.lifespan",
             )}
-            defaultValue={realm?.offlineSessionMaxLifespan}
+            defaultValue={
+              realm?.offlineSessionMaxLifespanEnabled
+                ? realm?.offlineSessionMaxLifespan
+                : undefined
+            }
             units={["minute", "day", "hour"]}
           />
 

js/apps/admin-ui/src/clients/advanced/TokenLifespan.tsx

@@ -24,7 +24,6 @@ type TokenLifespanProps = {
 };
 
 const inherited = "tokenLifespan.inherited";
-const never = "tokenLifespan.never";
 const expires = "tokenLifespan.expires";
 
 export const TokenLifespan = ({
@@ -42,8 +41,8 @@ export const TokenLifespan = ({
 
   const { control } = useFormContext();
   const isExpireSet = (value: string | number) =>
-    (typeof value === "number" && value !== -1) ||
+    typeof value === "number" ||
-    (typeof value === "string" && value !== "" && value !== "-1") ||
+    (typeof value === "string" && value !== "") ||
     focused;
 
   return (
@@ -73,30 +72,28 @@ export const TokenLifespan = ({
                   setOpen(false);
                 }}
                 selections={[
-                  isExpireSet(field.value)
+                  isExpireSet(field.value) ? t(expires) : t(inherited),
-                    ? t(expires)
-                    : field.value === ""
-                    ? t(inherited)
-                    : t(never),
                 ]}
               >
                 <SelectOption value="">{t(inherited)}</SelectOption>
-                <SelectOption value={-1}>{t(never)}</SelectOption>
                 <SelectOption value={60}>{t(expires)}</SelectOption>
               </Select>
             </SplitItem>
             <SplitItem>
-              {field.value !== "-1" && field.value !== -1 && (
+              <TimeSelector
-                <TimeSelector
+                validated={
-                  units={units}
+                  isExpireSet(field.value) && field.value! < 1
-                  value={field.value === "" ? defaultValue : field.value}
+                    ? "warning"
-                  onChange={field.onChange}
+                    : "default"
-                  onFocus={onFocus}
+                }
-                  onBlur={onBlur}
+                units={units}
-                  min={1}
+                value={field.value === "" ? defaultValue : field.value}
-                  isDisabled={field.value === ""}
+                onChange={field.onChange}
-                />
+                onFocus={onFocus}
-              )}
+                onBlur={onBlur}
+                min={1}
+                isDisabled={!isExpireSet(field.value)}
+              />
             </SplitItem>
           </Split>
         )}

server-spi-private/src/main/java/org/keycloak/models/utils/SessionExpirationUtils.java

@@ -21,6 +21,7 @@ import org.keycloak.models.ClientModel;
 import org.keycloak.models.Constants;
 import org.keycloak.models.RealmModel;
 import org.keycloak.protocol.oidc.OIDCConfigAttributes;
+import org.keycloak.utils.StringUtil;
 
 /**
  * <p>Shared methods to calculate the session expiration and idle.</p>
@@ -92,30 +93,29 @@ public class SessionExpirationUtils {
             long clientSessionCreated, long userSessionCreated, RealmModel realm, ClientModel client) {
         long timestamp = -1;
         if (offline) {
-            if (realm.isOfflineSessionMaxLifespanEnabled()) {
+            long clientOfflineSessionMaxLifespan = getClientAttributeTimeout(client, OIDCConfigAttributes.CLIENT_OFFLINE_SESSION_MAX_LIFESPAN);
-                long clientOfflineSessionMaxLifespan = TimeUnit.SECONDS.toMillis(getOfflineSessionMaxLifespan(realm));
+            if (realm.isOfflineSessionMaxLifespanEnabled() || clientOfflineSessionMaxLifespan > 0) {
-
+                if (clientOfflineSessionMaxLifespan > 0) {
-                String clientOfflineSessionMaxLifespanPerClient = client == null? null : client.getAttribute(OIDCConfigAttributes.CLIENT_OFFLINE_SESSION_MAX_LIFESPAN);
+                    clientOfflineSessionMaxLifespan = TimeUnit.SECONDS.toMillis(clientOfflineSessionMaxLifespan);
-                if (clientOfflineSessionMaxLifespanPerClient != null && !clientOfflineSessionMaxLifespanPerClient.trim().isEmpty()) {
-                    clientOfflineSessionMaxLifespan = TimeUnit.SECONDS.toMillis(Long.parseLong(clientOfflineSessionMaxLifespanPerClient));
                 } else if (realm.getClientOfflineSessionMaxLifespan() > 0) {
                     clientOfflineSessionMaxLifespan = TimeUnit.SECONDS.toMillis(realm.getClientOfflineSessionMaxLifespan());
+                } else {
+                    clientOfflineSessionMaxLifespan = TimeUnit.SECONDS.toMillis(getOfflineSessionMaxLifespan(realm));
                 }
-
                 timestamp = clientSessionCreated + clientOfflineSessionMaxLifespan;
 
                 long userSessionExpires = calculateUserSessionMaxLifespanTimestamp(offline, isRememberMe, userSessionCreated, realm);
 
-                timestamp = Math.min(timestamp, userSessionExpires);
+                timestamp = userSessionExpires > 0? Math.min(timestamp, userSessionExpires) : timestamp;
             }
         } else {
             long clientSessionMaxLifespan = TimeUnit.SECONDS.toMillis(getSsoSessionMaxLifespan(realm));
             if (isRememberMe) {
                 clientSessionMaxLifespan = Math.max(clientSessionMaxLifespan, TimeUnit.SECONDS.toMillis(realm.getSsoSessionMaxLifespanRememberMe()));
             }
-            String clientSessionMaxLifespanPerClient = client == null? null : client.getAttribute(OIDCConfigAttributes.CLIENT_SESSION_MAX_LIFESPAN);
+            long clientSessionMaxLifespanPerClient = getClientAttributeTimeout(client, OIDCConfigAttributes.CLIENT_SESSION_MAX_LIFESPAN);
-            if (clientSessionMaxLifespanPerClient != null && !clientSessionMaxLifespanPerClient.trim().isEmpty()) {
+            if (clientSessionMaxLifespanPerClient > 0) {
-                clientSessionMaxLifespan = TimeUnit.SECONDS.toMillis(Long.parseLong(clientSessionMaxLifespanPerClient));
+                clientSessionMaxLifespan = TimeUnit.SECONDS.toMillis(clientSessionMaxLifespanPerClient);
             } else if (realm.getClientSessionMaxLifespan() > 0) {
                 clientSessionMaxLifespan = TimeUnit.SECONDS.toMillis(realm.getClientSessionMaxLifespan());
             }
@@ -144,9 +144,9 @@ public class SessionExpirationUtils {
         long timestamp;
         if (offline) {
             long clientOfflineSessionIdleTimeout = TimeUnit.SECONDS.toMillis(getOfflineSessionIdleTimeout(realm));
-            String clientOfflineSessionIdleTimeoutPerClient = client == null? null : client.getAttribute(OIDCConfigAttributes.CLIENT_OFFLINE_SESSION_IDLE_TIMEOUT);
+            long clientOfflineSessionIdleTimeoutPerClient = getClientAttributeTimeout(client, OIDCConfigAttributes.CLIENT_OFFLINE_SESSION_IDLE_TIMEOUT);
-            if (clientOfflineSessionIdleTimeoutPerClient != null && !clientOfflineSessionIdleTimeoutPerClient.trim().isEmpty()) {
+            if (clientOfflineSessionIdleTimeoutPerClient > 0) {
-                clientOfflineSessionIdleTimeout = TimeUnit.SECONDS.toMillis(Long.parseLong(clientOfflineSessionIdleTimeoutPerClient));
+                clientOfflineSessionIdleTimeout = TimeUnit.SECONDS.toMillis(clientOfflineSessionIdleTimeoutPerClient);
             } else if (realm.getClientOfflineSessionIdleTimeout() > 0) {
                 clientOfflineSessionIdleTimeout = TimeUnit.SECONDS.toMillis(realm.getClientOfflineSessionIdleTimeout());
             }
@@ -157,9 +157,9 @@ public class SessionExpirationUtils {
             if (isRememberMe) {
                 clientSessionIdleTimeout = Math.max(clientSessionIdleTimeout, TimeUnit.SECONDS.toMillis(realm.getSsoSessionIdleTimeoutRememberMe()));
             }
-            String clientSessionIdleTimeoutPerClient = client == null? null : client.getAttribute(OIDCConfigAttributes.CLIENT_SESSION_IDLE_TIMEOUT);
+            long clientSessionIdleTimeoutPerClient = getClientAttributeTimeout(client, OIDCConfigAttributes.CLIENT_SESSION_IDLE_TIMEOUT);
-            if (clientSessionIdleTimeoutPerClient != null && !clientSessionIdleTimeoutPerClient.trim().isEmpty()) {
+            if (clientSessionIdleTimeoutPerClient > 0) {
-                clientSessionIdleTimeout = TimeUnit.SECONDS.toMillis(Long.parseLong(clientSessionIdleTimeoutPerClient));
+                clientSessionIdleTimeout = TimeUnit.SECONDS.toMillis(clientSessionIdleTimeoutPerClient);
             } else if (realm.getClientSessionIdleTimeout() > 0){
                 clientSessionIdleTimeout = TimeUnit.SECONDS.toMillis(realm.getClientSessionIdleTimeout());
             }
@@ -200,4 +200,18 @@ public class SessionExpirationUtils {
         }
         return idle;
     }
+
+    private static long getClientAttributeTimeout(ClientModel client, String attr) {
+        if (client != null) {
+            final String value = client.getAttribute(attr);
+            if (StringUtil.isNotBlank(value)) {
+                try {
+                    return Long.parseLong(value);
+                } catch (NumberFormatException e) {
+                    // no-op
+                }
+            }
+        }
+        return -1;
+    }
 }

server-spi-private/src/test/java/org/keycloak/models/utils/SessionExpirationUtilsTest.java

@@ -180,6 +180,9 @@ public class SessionExpirationUtilsTest {
         realmMap.put("getSsoSessionMaxLifespan", 1000);
         realmMap.put("getSsoSessionMaxLifespanRememberMe", 2000);
         Assert.assertEquals(2000 * 1000L, SessionExpirationUtils.calculateClientSessionMaxLifespanTimestamp(false, true, t, t, realm, client) - t);
+        // set -1 in the client and should be not taken into account
+        clientMap.put(OIDCConfigAttributes.CLIENT_SESSION_MAX_LIFESPAN, "-1");
+        Assert.assertEquals(2000 * 1000L, SessionExpirationUtils.calculateClientSessionMaxLifespanTimestamp(false, true, t, t, realm, client) - t);
     }
 
     @Test
@@ -207,6 +210,13 @@ public class SessionExpirationUtilsTest {
         long t2 = t - 100;
         realmMap.put("getOfflineSessionMaxLifespan", 2000);
         Assert.assertEquals(2000 * 1000L, SessionExpirationUtils.calculateClientSessionMaxLifespanTimestamp(true, false, t, t2, realm, client) - t2);
+        // set -1 in the client and should be not taken into account
+        clientMap.put(OIDCConfigAttributes.CLIENT_OFFLINE_SESSION_MAX_LIFESPAN, "-1");
+        Assert.assertEquals(2000 * 1000L, SessionExpirationUtils.calculateClientSessionMaxLifespanTimestamp(true, false, t, t, realm, client) - t);
+        // set no expiration at realm but set expiration at client level
+        realmMap.put("isOfflineSessionMaxLifespanEnabled", false);
+        clientMap.put(OIDCConfigAttributes.CLIENT_OFFLINE_SESSION_MAX_LIFESPAN, "2000");
+        Assert.assertEquals(2000 * 1000L, SessionExpirationUtils.calculateClientSessionMaxLifespanTimestamp(true, false, t, t, realm, client) - t);
     }
 
     @Test
@@ -230,6 +240,9 @@ public class SessionExpirationUtilsTest {
         // override value in client
         clientMap.put(OIDCConfigAttributes.CLIENT_SESSION_IDLE_TIMEOUT, "3000");
         Assert.assertEquals(3000 * 1000L, SessionExpirationUtils.calculateClientSessionIdleTimestamp(false, false, t, realm, client) - t);
+        // set -1 in the client and should be not taken into account
+        clientMap.put(OIDCConfigAttributes.CLIENT_SESSION_IDLE_TIMEOUT, "-1");
+        Assert.assertEquals(4000 * 1000L, SessionExpirationUtils.calculateClientSessionIdleTimestamp(false, false, t, realm, client) - t);
     }
 
     @Test
@@ -253,5 +266,8 @@ public class SessionExpirationUtilsTest {
         // override value in client
         clientMap.put(OIDCConfigAttributes.CLIENT_OFFLINE_SESSION_IDLE_TIMEOUT, "3000");
         Assert.assertEquals(3000 * 1000L, SessionExpirationUtils.calculateClientSessionIdleTimestamp(true, false, t, realm, client) - t);
+        // set -1 in the client and should be not taken into account
+        clientMap.put(OIDCConfigAttributes.CLIENT_OFFLINE_SESSION_IDLE_TIMEOUT, "-1");
+        Assert.assertEquals(4000 * 1000L, SessionExpirationUtils.calculateClientSessionIdleTimestamp(true, false, t, realm, client) - t);
     }
 }