Dry-run feature for cron emails

account-gui/pom.xml

@@ -4,7 +4,7 @@
     <parent>
         <groupId>org.openconext</groupId>
         <artifactId>myconext</artifactId>
-        <version>7.4.2</version>
+        <version>7.4.3</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
     <artifactId>account-gui</artifactId>

myconext-gui/pom.xml

@@ -4,7 +4,7 @@
     <parent>
         <groupId>org.openconext</groupId>
         <artifactId>myconext</artifactId>
-        <version>7.4.2</version>
+        <version>7.4.3</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
     <artifactId>myconext-gui</artifactId>

myconext-server/pom.xml

@@ -4,7 +4,7 @@
     <parent>
         <groupId>org.openconext</groupId>
         <artifactId>myconext</artifactId>
-        <version>7.4.2</version>
+        <version>7.4.3</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
     <artifactId>myconext-server</artifactId>

myconext-server/src/main/java/myconext/cron/InactivityMail.java

@@ -31,16 +31,19 @@ public class InactivityMail {
     private final boolean cronJobResponsible;
     private final DateFormat dateFormatUS;
     private final DateFormat dateFormatNL;
+    private final boolean dryRunEmail;
 
     @Autowired
     public InactivityMail(MailBox mailBox,
                           UserRepository userRepository,
                           @Value("${cron.node-cron-job-responsible}") boolean cronJobResponsible,
-                          @Value("${feature.mail_inactivity_mails}") boolean mailInactivityMails) {
+                          @Value("${feature.mail_inactivity_mails}") boolean mailInactivityMails,
+                          @Value("${cron.dry-run-email}") boolean dryRunEmail) {
         this.mailBox = mailBox;
         this.userRepository = userRepository;
         this.cronJobResponsible = cronJobResponsible;
         this.mailInactivityMails = mailInactivityMails;
+        this.dryRunEmail = dryRunEmail;
         this.dateFormatUS = DateFormat.getDateInstance(DateFormat.LONG, Locale.of("us"));
         this.dateFormatNL = DateFormat.getDateInstance(DateFormat.LONG, Locale.of("nl"));
     }
@@ -79,14 +82,21 @@ private void doMailInactiveUsers(UserInactivity userInactivity) {
         localeVariables.put("deletion_period_nl", userInactivity.getDeletionPeriodNl());
         localeVariables.put("account_delete_date_en", dateFormatUS.format(date));
         localeVariables.put("account_delete_date_nl", dateFormatNL.format(date));
-        users.forEach(user -> {
-            mailBox.sendUserInactivityMail(user, localeVariables,
-                    userInactivity.equals(YEAR_1_INTERVAL) || userInactivity.equals(YEAR_3_INTERVAL));
-            user.setUserInactivity(userInactivity);
-            userRepository.save(user);
-        });
-        LOG.info(String.format("Mailed %s users who has been inactive for %s period in for %s ms",
-                users.size(), userInactivity, System.currentTimeMillis() - nowInMillis));
+
+        if (!dryRunEmail) {
+            users.forEach(user -> {
+                mailBox.sendUserInactivityMail(user, localeVariables,
+                        userInactivity.equals(YEAR_1_INTERVAL) || userInactivity.equals(YEAR_3_INTERVAL));
+                user.setUserInactivity(userInactivity);
+                //Ensure users who receive their last warning are not deleted the next run, but after one week
+                if (userInactivity.equals(WEEK_1_BEFORE_5_YEARS)) {
+                    user.setLastLogin(nowInMillis - (WEEK_1_BEFORE_5_YEARS.getInactivityDays() * ONE_DAY_IN_MILLIS));
+                }
+                userRepository.save(user);
+            });
+        }
+        LOG.info(String.format("Mailed %s users who has been inactive for %s period in for %s ms, dry run: ",
+                users.size(), userInactivity, System.currentTimeMillis() - nowInMillis, dryRunEmail));
     }
 
     private void doDeleteInactiveUsers() {
@@ -95,10 +105,13 @@ private void doDeleteInactiveUsers() {
 
         long lastLoginBefore = nowInMillis - (oneDayInMillis * 5L * 365);
         List<User> users = userRepository.findByLastLoginBeforeAndUserInactivityIn(lastLoginBefore, List.of(WEEK_1_BEFORE_5_YEARS));
-        userRepository.deleteAll(users);
-        LOG.info(String.format("Deleted %s users (%s) who has been inactive for 5 years in for %s ms",
+        if (!dryRunEmail) {
+            userRepository.deleteAll(users);
+        }
+        LOG.info(String.format("Deleted %s users (%s) who has been inactive for 5 years in for %s ms, dry-run: %s",
                 users.size(), users.stream().map(User::getEmail).collect(Collectors.joining(", ")),
-                System.currentTimeMillis() - nowInMillis));
+                System.currentTimeMillis() - nowInMillis,
+                dryRunEmail));
     }
 
     private List<UserInactivity> userInactivitiesWithNullElement(UserInactivity userInactivity) {

myconext-server/src/main/java/myconext/cron/InstitutionMailUsage.java

@@ -23,18 +23,21 @@ public class InstitutionMailUsage {
     private final UserRepository userRepository;
     private final boolean mailInstitutionMailUsage;
     private final boolean cronJobResponsible;
+    private final boolean dryRunEmail;
 
     @Autowired
     public InstitutionMailUsage(Manage manage,
                                 MailBox mailBox,
                                 UserRepository userRepository,
                                 @Value("${cron.node-cron-job-responsible}") boolean cronJobResponsible,
-                                @Value("${feature.mail_institution_mail_usage}") boolean mailInstitutionMailUsage) {
+                                @Value("${feature.mail_institution_mail_usage}") boolean mailInstitutionMailUsage,
+                                @Value("${cron.dry-run-email}") boolean dryRunEmail) {
         this.manage = manage;
         this.mailBox = mailBox;
         this.userRepository = userRepository;
         this.cronJobResponsible = cronJobResponsible;
         this.mailInstitutionMailUsage = mailInstitutionMailUsage;
+        this.dryRunEmail = dryRunEmail;
     }
 
     @Scheduled(cron = "${cron.mail-institution-mail-usage-expression}")
@@ -52,10 +55,11 @@ public void mailUsersWithInstitutionMail() {
             String regex = "@" + String.join("|", queryList) + "$";
             List<User> users = userRepository.findByEmailDomain(regex);
 
-            users.forEach(mailBox::sendInstitutionMailWarning);
-
-            LOG.info(String.format("Mailed %s users who use their institution domain in %s ms",
-                    users.size(), System.currentTimeMillis() - start));
+            if (!dryRunEmail) {
+                users.forEach(mailBox::sendInstitutionMailWarning);
+            }
+            LOG.info(String.format("Mailed %s users who use their institution domain in %s ms, dry-run: %s",
+                    users.size(), System.currentTimeMillis() - start, dryRunEmail));
         } catch (Exception e) {
             LOG.error("Error in mailUsersWithInstitutionMail", e);
         }

myconext-server/src/main/java/myconext/cron/NudgeAppMail.java

@@ -1,7 +1,6 @@
 package myconext.cron;
 
 import myconext.mail.MailBox;
-import myconext.manage.Manage;
 import myconext.model.User;
 import myconext.repository.UserRepository;
 import org.apache.commons.logging.Log;
@@ -24,18 +23,21 @@ public class NudgeAppMail {
     private final boolean nudgeAppMailFeature;
     private final boolean cronJobResponsible;
     private final long nudgeAppMailDaysAfterCreation;
+    private final boolean dryRunEmail;
 
     @Autowired
     public NudgeAppMail(MailBox mailBox,
                         UserRepository userRepository,
                         @Value("${cron.node-cron-job-responsible}") boolean cronJobResponsible,
                         @Value("${cron.nudge-app-mail-days-after-creation}") long nudgeAppMailDaysAfterCreation,
-                        @Value("${feature.nudge_app_mail}") boolean nudgeAppMailFeature) {
+                        @Value("${feature.nudge_app_mail}") boolean nudgeAppMailFeature,
+                        @Value("${cron.dry-run-email}") boolean dryRunEmail) {
         this.mailBox = mailBox;
         this.userRepository = userRepository;
         this.cronJobResponsible = cronJobResponsible;
         this.nudgeAppMailDaysAfterCreation = nudgeAppMailDaysAfterCreation;
         this.nudgeAppMailFeature = nudgeAppMailFeature;
+        this.dryRunEmail = dryRunEmail;
     }
 
     @Scheduled(cron = "${cron.nudge-app-mail-expression}")
@@ -48,15 +50,15 @@ public void mailUsersWithoutApp() {
         try {
             long createdBefore = (new Date().getTime() - (nudgeAppMailDaysAfterCreation * 24 * 60 * 60 * 1000)) / 1000;
             List<User> users = userRepository.findByNoEduIDApp(createdBefore);
-
-            users.forEach(user -> {
-                mailBox.sendNudgeAppMail(user);
-                user.setNudgeAppMailSend(true);
-                userRepository.save(user);
-            });
-
-            LOG.info(String.format("Mailed %s users to nudge the app in %s ms",
-                    users.size(), System.currentTimeMillis() - start));
+            if (!dryRunEmail) {
+                users.forEach(user -> {
+                    mailBox.sendNudgeAppMail(user);
+                    user.setNudgeAppMailSend(true);
+                    userRepository.save(user);
+                });
+            }
+            LOG.info(String.format("Mailed %s users to nudge the app in %s ms, dry-run: %s",
+                    users.size(), System.currentTimeMillis() - start, dryRunEmail));
         } catch (Exception e) {
             LOG.error("Error in mailUsersWithInstitutionMail", e);
         }

myconext-server/src/main/resources/application.yml

@@ -56,6 +56,8 @@ cron:
   nudge-app-mail-days-after-creation: 14
   # Every day at 7:30AM
   inactivity-users-expression: "0 30 7 * * ?"
+  # Set to true to disable sending emails
+  dry-run-email: false
 
 manage:
   username: myconext

myconext-server/src/test/java/myconext/AbstractMailBoxTest.java

@@ -2,14 +2,20 @@
 
 import com.icegreen.greenmail.junit.GreenMailRule;
 import com.icegreen.greenmail.util.ServerSetup;
+import myconext.model.User;
+import myconext.model.UserInactivity;
+import myconext.repository.UserRepository;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 
 import javax.mail.internet.MimeMessage;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
 
+import static myconext.cron.InactivityMail.ONE_DAY_IN_MILLIS;
+import static myconext.cron.InactivityMailTest.DELETED_EMAIL;
 import static org.awaitility.Awaitility.await;
 
 @SuppressWarnings("deprecation")
@@ -41,4 +47,24 @@ protected List<MimeMessage> mailMessages() {
         return List.of(greenMail.getReceivedMessages());
     }
 
+    protected void inactivityUserSeed(String language) {
+        long yesterday = System.currentTimeMillis() - ONE_DAY_IN_MILLIS;
+        Stream.of(UserInactivity.values()).forEach(userInactivity -> {
+            User user = new User();
+            user.setLastLogin(yesterday - (userInactivity.getInactivityDays() * ONE_DAY_IN_MILLIS));
+            user.setEmail(userInactivity.name());
+            user.setPreferredLanguage(language);
+            user.setUserInactivity(userInactivity.getPreviousUserInactivity());
+            userRepository.save(user);
+        });
+        //And one extra User who is to be deleted
+        User user = new User();
+        user.setLastLogin(yesterday - (((5L * 365) + 5) * ONE_DAY_IN_MILLIS));
+        user.setEmail(DELETED_EMAIL);
+        user.setUserInactivity(UserInactivity.WEEK_1_BEFORE_5_YEARS);
+        userRepository.save(user);
+
+    }
+
+
 }

myconext-server/src/test/java/myconext/cron/InactivityMailDryRunTest.java

@@ -0,0 +1,70 @@
+package myconext.cron;
+
+import lombok.SneakyThrows;
+import myconext.AbstractMailBoxTest;
+import myconext.model.User;
+import myconext.model.UserInactivity;
+import myconext.repository.UserRepository;
+import org.awaitility.core.ConditionTimeoutException;
+import org.junit.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+
+import javax.mail.internet.MimeMessage;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static myconext.cron.InactivityMail.ONE_DAY_IN_MILLIS;
+import static myconext.cron.InactivityMailTest.DELETED_EMAIL;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
+        properties = {
+                "mongodb_db=surf_id_test",
+                "cron.node-cron-job-responsible=true",
+                "email_guessing_sleep_millis=1",
+                "sp_entity_id=https://engine.test.surfconext.nl/authentication/sp/metadata",
+                "sp_entity_metadata_url=https://engine.test.surfconext.nl/authentication/sp/metadata",
+                "spring.main.lazy-initialization=true",
+                "eduid_api.oidcng_introspection_uri=http://localhost:8098/introspect",
+                "cron.service-name-resolver-initial-delay-milliseconds=60000",
+                "oidc.base-url=http://localhost:8098/",
+                "sso_mfa_duration_seconds=-1000",
+                "feature.requires_signed_authn_request=false",
+                "feature.deny_disposable_email_providers=false",
+                "verify.base_uri=http://localhost:8098",
+                "cron.dry-run-email=true"
+        })
+public class InactivityMailDryRunTest extends AbstractMailBoxTest {
+
+    @Autowired
+    protected InactivityMail inactivityMail;
+
+    @SneakyThrows
+    @Test
+    public void mailInactivityMail() {
+        inactivityUserSeed("en");
+
+        inactivityMail.mailInactiveUsers();
+
+        assertThrows(ConditionTimeoutException.class, this::mailMessages);
+    }
+
+    @SneakyThrows
+    @Test
+    public void mailInactivityFirstRun() {
+        //See https://github.com/OpenConext/OpenConext-myconext/issues/656
+        User user = new User();
+        long yesterday = System.currentTimeMillis() - ONE_DAY_IN_MILLIS;
+        user.setLastLogin(yesterday - (UserInactivity.WEEK_1_BEFORE_5_YEARS.getInactivityDays() * ONE_DAY_IN_MILLIS));
+        user.setEmail(UserInactivity.WEEK_1_BEFORE_5_YEARS.name());
+        //Explicit set userInactivity to null for self-explanation of the test code
+        user.setUserInactivity(null);
+        userRepository.save(user);
+
+        inactivityMail.mailInactiveUsers();
+
+        assertThrows(ConditionTimeoutException.class, this::mailMessages);
+    }
+
+}

myconext-server/src/test/java/myconext/cron/InactivityMailTest.java

@@ -38,7 +38,7 @@
         })
 public class InactivityMailTest extends AbstractMailBoxTest {
 
-    private final String DELETED_EMAIL = "DELETED";
+    public static final String DELETED_EMAIL = "DELETED";
 
     @Autowired
     protected InactivityMail inactivityMail;
@@ -70,7 +70,8 @@ public void mailInactivityMail() {
     public void mailInactivityFirstRun() {
         //See https://github.com/OpenConext/OpenConext-myconext/issues/656
         User user = new User();
-        long yesterday = System.currentTimeMillis() - ONE_DAY_IN_MILLIS;
+        long now = System.currentTimeMillis();
+        long yesterday = now - ONE_DAY_IN_MILLIS;
         user.setLastLogin(yesterday - (UserInactivity.WEEK_1_BEFORE_5_YEARS.getInactivityDays() * ONE_DAY_IN_MILLIS));
         user.setEmail(UserInactivity.WEEK_1_BEFORE_5_YEARS.name());
         //Explicit set userInactivity to null for self-explanation of the test code
@@ -83,6 +84,9 @@ public void mailInactivityFirstRun() {
         assertEquals(1, mimeMessages.size());
         User userFromDB = userRepository.findOneUserByEmail(UserInactivity.WEEK_1_BEFORE_5_YEARS.name());
         assertEquals(UserInactivity.WEEK_1_BEFORE_5_YEARS, userFromDB.getUserInactivity());
+        //Ensure users which have received the last warning have a new lastLogin which is one week before the deletion threshold
+        long newLastLoginDelta = (UserInactivity.WEEK_1_BEFORE_5_YEARS.getInactivityDays() + 6) * ONE_DAY_IN_MILLIS;
+        assertTrue(userFromDB.getLastLogin() >= (now - newLastLoginDelta) );
         //Idempotency check
         greenMail.purgeEmailFromAllMailboxes();
         inactivityMail.mailInactiveUsers();
@@ -112,23 +116,4 @@ private String messageContent(MimeMessage mimeMessage) {
         return IOUtils.toString(mimeMessage.getInputStream(), Charset.defaultCharset());
     }
 
-    private void inactivityUserSeed(String preferredLanguage) {
-        long yesterday = System.currentTimeMillis() - ONE_DAY_IN_MILLIS;
-        Stream.of(UserInactivity.values()).forEach(userInactivity -> {
-            User user = new User();
-            user.setLastLogin(yesterday - (userInactivity.getInactivityDays() * ONE_DAY_IN_MILLIS));
-            user.setEmail(userInactivity.name());
-            user.setPreferredLanguage(preferredLanguage);
-            user.setUserInactivity(userInactivity.getPreviousUserInactivity());
-            userRepository.save(user);
-        });
-        //And one extra User who is to be deleted
-        User user = new User();
-        user.setLastLogin(yesterday - (((5L * 365) + 5) * ONE_DAY_IN_MILLIS));
-        user.setEmail(DELETED_EMAIL);
-        user.setUserInactivity(UserInactivity.WEEK_1_BEFORE_5_YEARS);
-        userRepository.save(user);
-
-    }
-
 }