diff --git a/account-gui/pom.xml b/account-gui/pom.xml index 9f6695eb..e739e3f6 100644 --- a/account-gui/pom.xml +++ b/account-gui/pom.xml @@ -4,7 +4,7 @@ org.openconext myconext - 7.4.2 + 7.4.3 ../pom.xml account-gui diff --git a/myconext-gui/pom.xml b/myconext-gui/pom.xml index a43c9bd3..a3326d41 100644 --- a/myconext-gui/pom.xml +++ b/myconext-gui/pom.xml @@ -4,7 +4,7 @@ org.openconext myconext - 7.4.2 + 7.4.3 ../pom.xml myconext-gui diff --git a/myconext-server/pom.xml b/myconext-server/pom.xml index 7d0df917..183e198b 100644 --- a/myconext-server/pom.xml +++ b/myconext-server/pom.xml @@ -4,7 +4,7 @@ org.openconext myconext - 7.4.2 + 7.4.3 ../pom.xml myconext-server diff --git a/myconext-server/src/main/java/myconext/cron/InactivityMail.java b/myconext-server/src/main/java/myconext/cron/InactivityMail.java index 030402e5..449bad09 100644 --- a/myconext-server/src/main/java/myconext/cron/InactivityMail.java +++ b/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 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 userInactivitiesWithNullElement(UserInactivity userInactivity) { diff --git a/myconext-server/src/main/java/myconext/cron/InstitutionMailUsage.java b/myconext-server/src/main/java/myconext/cron/InstitutionMailUsage.java index 735f4d85..13696c65 100644 --- a/myconext-server/src/main/java/myconext/cron/InstitutionMailUsage.java +++ b/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 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); } diff --git a/myconext-server/src/main/java/myconext/cron/NudgeAppMail.java b/myconext-server/src/main/java/myconext/cron/NudgeAppMail.java index 6d60723e..5b9a8998 100644 --- a/myconext-server/src/main/java/myconext/cron/NudgeAppMail.java +++ b/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 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); } diff --git a/myconext-server/src/main/resources/application.yml b/myconext-server/src/main/resources/application.yml index 71a71417..d32f8229 100644 --- a/myconext-server/src/main/resources/application.yml +++ b/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 diff --git a/myconext-server/src/test/java/myconext/AbstractMailBoxTest.java b/myconext-server/src/test/java/myconext/AbstractMailBoxTest.java index 2f97bef7..4256cf95 100644 --- a/myconext-server/src/test/java/myconext/AbstractMailBoxTest.java +++ b/myconext-server/src/test/java/myconext/AbstractMailBoxTest.java @@ -2,6 +2,9 @@ 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; @@ -9,7 +12,10 @@ 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 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); + + } + + } \ No newline at end of file diff --git a/myconext-server/src/test/java/myconext/cron/InactivityMailDryRunTest.java b/myconext-server/src/test/java/myconext/cron/InactivityMailDryRunTest.java new file mode 100644 index 00000000..3d3bdded --- /dev/null +++ b/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); + } + +} \ No newline at end of file diff --git a/myconext-server/src/test/java/myconext/cron/InactivityMailTest.java b/myconext-server/src/test/java/myconext/cron/InactivityMailTest.java index 8b299860..2c132e6b 100644 --- a/myconext-server/src/test/java/myconext/cron/InactivityMailTest.java +++ b/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); - - } - } \ No newline at end of file diff --git a/myconext-server/src/test/java/myconext/cron/InstitutionMailUsageDryRunTest.java b/myconext-server/src/test/java/myconext/cron/InstitutionMailUsageDryRunTest.java new file mode 100644 index 00000000..f4bf5a2a --- /dev/null +++ b/myconext-server/src/test/java/myconext/cron/InstitutionMailUsageDryRunTest.java @@ -0,0 +1,43 @@ +package myconext.cron; + +import myconext.AbstractMailBoxTest; +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 static org.junit.jupiter.api.Assertions.assertEquals; +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 InstitutionMailUsageDryRunTest extends AbstractMailBoxTest { + + @Autowired + protected InstitutionMailUsage institutionMailUsage; + + @Test + public void mailUsersWithInstitutionMail() { + institutionMailUsage.mailUsersWithInstitutionMail(); + + assertThrows(ConditionTimeoutException.class, this::mailMessages); + } +} \ No newline at end of file diff --git a/myconext-server/src/test/java/myconext/cron/NudgeAppMailDryRunTest.java b/myconext-server/src/test/java/myconext/cron/NudgeAppMailDryRunTest.java new file mode 100644 index 00000000..618436e6 --- /dev/null +++ b/myconext-server/src/test/java/myconext/cron/NudgeAppMailDryRunTest.java @@ -0,0 +1,40 @@ +package myconext.cron; + +import myconext.AbstractMailBoxTest; +import org.awaitility.core.ConditionTimeoutException; +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; + +import static org.junit.Assert.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 NudgeAppMailDryRunTest extends AbstractMailBoxTest { + + @Autowired + protected NudgeAppMail nudgeAppMail; + + @Test + public void mailUsersWithoutApp() { + nudgeAppMail.mailUsersWithoutApp(); + + assertThrows(ConditionTimeoutException.class, this::mailMessages); + } + +} \ No newline at end of file diff --git a/pom.xml b/pom.xml index 09364b00..c818636c 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ 4.0.0 org.openconext myconext - 7.4.2 + 7.4.3 pom myconext My OpenConext diff --git a/public-gui/pom.xml b/public-gui/pom.xml index 4f37fe01..242a7504 100644 --- a/public-gui/pom.xml +++ b/public-gui/pom.xml @@ -4,7 +4,7 @@ org.openconext myconext - 7.4.2 + 7.4.3 ../pom.xml public-gui diff --git a/tiqr-mock/pom.xml b/tiqr-mock/pom.xml index afe2b97f..d5aead18 100644 --- a/tiqr-mock/pom.xml +++ b/tiqr-mock/pom.xml @@ -4,7 +4,7 @@ org.openconext myconext - 7.4.2 + 7.4.3 ../pom.xml tiqr-mock