Adjust scheduled action time so that it is always based on the previous action

Closes #42385

Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

model/jpa/src/main/java/org/keycloak/models/policy/JpaResourcePolicyStateProvider.java

@@ -54,7 +54,7 @@ public class JpaResourcePolicyStateProvider implements ResourcePolicyStateProvid
     }
 
     @Override
-    public void scheduleAction(ResourcePolicy policy, ResourceAction action, long scheduledTimeOffset, String resourceId) {
+    public void scheduleAction(ResourcePolicy policy, ResourceAction action, String resourceId) {
         ResourcePolicyStateEntity.PrimaryKey pk = new ResourcePolicyStateEntity.PrimaryKey(resourceId, policy.getId());
         ResourcePolicyStateEntity entity = em.find(ResourcePolicyStateEntity.class, pk);
         if (entity == null) {
@@ -63,12 +63,12 @@ public class JpaResourcePolicyStateProvider implements ResourcePolicyStateProvid
             entity.setPolicyId(policy.getId());
             entity.setPolicyProviderId(policy.getProviderId());
             entity.setScheduledActionId(action.getId());
-            entity.setScheduledActionTimestamp(Time.currentTimeMillis() + scheduledTimeOffset);
+            entity.setScheduledActionTimestamp(Time.currentTimeMillis() + action.getAfter());
             em.persist(entity);
         }
         else {
             entity.setScheduledActionId(action.getId());
-            entity.setScheduledActionTimestamp(Time.currentTimeMillis() + scheduledTimeOffset);
+            entity.setScheduledActionTimestamp(Time.currentTimeMillis() + action.getAfter());
         }
     }
 

model/storage-private/src/main/java/org/keycloak/models/policy/ResourcePolicyStateProvider.java

@@ -51,11 +51,7 @@ public interface ResourcePolicyStateProvider extends Provider {
      */
     void removeAll();
 
-    default void scheduleAction(ResourcePolicy policy, ResourceAction action, String resourceId) {
-        this.scheduleAction(policy, action, action.getAfter(), resourceId);
-    }
-
-    void scheduleAction(ResourcePolicy policy, ResourceAction action, long scheduledTimeOffset, String resourceId);
+    void scheduleAction(ResourcePolicy policy, ResourceAction action, String resourceId);
 
     ScheduledAction getScheduledAction(String policyId, String resourceId);
 

services/src/main/java/org/keycloak/models/policy/NotifyUserActionProvider.java

@@ -87,8 +87,7 @@ public class NotifyUserActionProvider implements ResourceActionProvider {
         }
         
         // Return default subject key based on next action type
-        String defaultSubjectKey = getDefaultSubjectKey(nextActionType);
-        return defaultSubjectKey;
+        return getDefaultSubjectKey(nextActionType);
     }
 
     private String getBodyTemplate() {
@@ -124,35 +123,22 @@ public class NotifyUserActionProvider implements ResourceActionProvider {
     }
 
     private String getNextActionType() {
-        ComponentModel nextAction = getNextNonNotificationAction();
-        return nextAction != null ? nextAction.getProviderId() : "unknown-action";
+        Map<ComponentModel, Long> nextActionMap = getNextNonNotificationAction();
+        return nextActionMap.isEmpty() ? "unknown-action" : nextActionMap.keySet().iterator().next().getProviderId();
     }
 
     private int calculateDaysUntilNextAction() {
-        ComponentModel nextAction = getNextNonNotificationAction();
-        if (nextAction == null) {
-            return 0;
-        }
-        
-        String currentAfter = actionModel.get(AFTER_KEY);
-        String nextAfter = nextAction.get(AFTER_KEY);
-        
-        if (currentAfter == null || nextAfter == null) {
-            return 0;
-        }
-        
-        try {
-            long currentMillis = Long.parseLong(currentAfter);
-            long nextMillis = Long.parseLong(nextAfter);
-            Duration difference = Duration.ofMillis(nextMillis - currentMillis);
-            return Math.toIntExact(difference.toDays());
-        } catch (NumberFormatException e) {
-            log.warnv("Invalid days format: current={0}, next={1}", currentAfter, nextAfter);
+        Map<ComponentModel, Long> nextActionMap = getNextNonNotificationAction();
+        if (nextActionMap.isEmpty()) {
             return 0;
         }
+        Long timeToNextAction = nextActionMap.values().iterator().next();
+        return Math.toIntExact(Duration.ofMillis(timeToNextAction).toDays());
     }
 
-    private ComponentModel getNextNonNotificationAction() {
+    private Map<ComponentModel, Long> getNextNonNotificationAction() {
+        long timeToNextNonNotificationAction = 0L;
+
         RealmModel realm = session.getContext().getRealm();
         ComponentModel policyModel = realm.getComponent(actionModel.getParentId());
         
@@ -167,15 +153,19 @@ public class NotifyUserActionProvider implements ResourceActionProvider {
         // Find current action and return next non-notification action
         boolean foundCurrent = false;
         for (ComponentModel action : actions) {
-            if (foundCurrent && !action.getProviderId().equals("notify-user-action-provider")) {
-                return action;
+            if (foundCurrent) {
+                timeToNextNonNotificationAction += action.get(AFTER_KEY, 0L);
+                if (!action.getProviderId().equals("notify-user-action-provider")) {
+                    // we found the next non-notification action, accumulate its time and break
+                    return Map.of(action, timeToNextNonNotificationAction);
+                }
             }
             if (action.getId().equals(actionModel.getId())) {
                 foundCurrent = true;
             }
         }
         
-        return null;
+        return Map.of();
     }
     
     private String getDefaultSubjectKey(String actionType) {

services/src/main/java/org/keycloak/models/policy/ResourcePolicyManager.java

@@ -259,7 +259,7 @@ public class ResourcePolicyManager {
                         if (actions.size() > i + 1) {
                             // schedule the next action using the time offset difference between the actions.
                             ResourceAction nextAction = actions.get(i + 1);
-                            policyStateProvider.scheduleAction(policy, nextAction, nextAction.getAfter() - currentAction.getAfter(), scheduled.resourceId());
+                            policyStateProvider.scheduleAction(policy, nextAction, scheduled.resourceId());
                         } else {
                             // this was the last action, check if the policy is recurring - i.e. if we need to schedule the first action again
                             if (policy.isRecurring()) {

tests/base/src/test/java/org/keycloak/tests/admin/model/policy/ActionRunnerScheduledTaskTest.java

@@ -83,7 +83,7 @@ public class ActionRunnerScheduledTaskTest {
                                 .withConfig("message", "message")
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(10))
+                                .after(Duration.ofDays(5))
                                 .build()
                 ).build()).close();
 

tests/base/src/test/java/org/keycloak/tests/admin/model/policy/ResourcePolicyManagementTest.java

@@ -102,7 +102,7 @@ public class ResourcePolicyManagementTest {
                                 .after(Duration.ofDays(5))
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(10))
+                                .after(Duration.ofDays(5))
                                 .build()
                 ).build();
 
@@ -130,7 +130,7 @@ public class ResourcePolicyManagementTest {
                                 .after(Duration.ofDays(5))
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(10))
+                                .after(Duration.ofDays(5))
                                 .build()
                 ).build();
 
@@ -198,7 +198,7 @@ public class ResourcePolicyManagementTest {
                                 .after(Duration.ofDays(5))
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(10))
+                                .after(Duration.ofDays(5))
                                 .build()
                 ).build();
 
@@ -230,7 +230,7 @@ public class ResourcePolicyManagementTest {
                                 .after(Duration.ofDays(5))
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(10))
+                                .after(Duration.ofDays(5))
                                 .build()
                 ).build()).close();
 
@@ -309,7 +309,7 @@ public class ResourcePolicyManagementTest {
                                 .after(Duration.ofDays(5))
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(10))
+                                .after(Duration.ofDays(5))
                                 .build()
                 ).build()).close();
 
@@ -405,7 +405,7 @@ public class ResourcePolicyManagementTest {
                                 .after(Duration.ofDays(5))
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(10))
+                                .after(Duration.ofDays(5))
                                 .build()
                 ).build()).close();
 
@@ -567,7 +567,7 @@ public class ResourcePolicyManagementTest {
                                 .withConfig("message", "message")
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(2))
+                                .after(Duration.ofDays(1))
                                 .build()
                 ).build()).close();
 
@@ -594,7 +594,7 @@ public class ResourcePolicyManagementTest {
                                 .withConfig("reason", "inactivity")
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(10))
+                                .after(Duration.ofDays(3))
                                 .build()
                 ).build()).close();
 
@@ -632,7 +632,7 @@ public class ResourcePolicyManagementTest {
                                 .withConfig("reason", "inactivity")
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DeleteUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(30))
+                                .after(Duration.ofDays(15))
                                 .build()
                 ).build()).close();
 
@@ -672,7 +672,7 @@ public class ResourcePolicyManagementTest {
                                 .withConfig("custom_subject_key", "customComplianceSubject")
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(7))
+                                .after(Duration.ofDays(2))
                                 .build()
                 ).build()).close();
 
@@ -707,7 +707,7 @@ public class ResourcePolicyManagementTest {
                                 .after(Duration.ofDays(5))
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(10))
+                                .after(Duration.ofDays(5))
                                 .build()
                 ).build()).close();
 
@@ -747,7 +747,7 @@ public class ResourcePolicyManagementTest {
                                 .withConfig("reason", "inactivity")
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(30))
+                                .after(Duration.ofDays(15))
                                 .build()
                 ).build()).close();
 

tests/base/src/test/java/org/keycloak/tests/admin/model/policy/UserCreationTimePolicyTest.java

@@ -69,7 +69,7 @@ public class UserCreationTimePolicyTest {
                                 .after(Duration.ofDays(5))
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(10))
+                                .after(Duration.ofDays(5))
                                 .build()
                 ).build()).close();
 

tests/base/src/test/java/org/keycloak/tests/admin/model/policy/UserSessionRefreshTimePolicyTest.java

@@ -108,7 +108,7 @@ public class UserSessionRefreshTimePolicyTest {
                                 .after(Duration.ofDays(5))
                                 .build(),
                         ResourcePolicyActionRepresentation.create().of(DisableUserActionProviderFactory.ID)
-                                .after(Duration.ofDays(10))
+                                .after(Duration.ofDays(5))
                                 .build()
                 ).build()).close();