Add more checks for AupDTO

in particular, enable class annotation that checks both aup reminders and signature validity. For aup reminders, also check that they are smaller than the aup signature validity
indigo-iam · Jul 2, 2024 · cf5ce67 · cf5ce67
1 parent dc2892e
commit cf5ce67
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 21 deletions.
diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/aup/model/AupDTO.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/aup/model/AupDTO.java
@@ -17,9 +17,7 @@
 
 import java.util.Date;
 
-import javax.validation.constraints.Min;
 import javax.validation.constraints.NotBlank;
-import javax.validation.constraints.NotNull;
 import javax.validation.constraints.Size;
 
 import org.hibernate.validator.constraints.URL;
@@ -30,6 +28,7 @@
 import it.infn.mw.iam.api.scim.controller.utils.JsonDateSerializer;
 import it.infn.mw.iam.api.validators.NoQueryParamsUrl;
 
+@AupRemindersAndSignature
 public class AupDTO {
 
   @NotBlank(message = "Invalid AUP: the AUP URL cannot be blank")
@@ -43,11 +42,8 @@ public class AupDTO {
       message = "Invalid AUP: the description string must be at most 128 characters long")
   String description;
 
-  @NotNull(message = "Invalid AUP: signatureValidityInDays is required")
-  @Min(value = 0L, message = "Invalid AUP: signatureValidityInDays must be >= 0")
   Long signatureValidityInDays;
 
-  @AupReminders
   String aupRemindersInDays;
 
   @JsonSerialize(using = JsonDateSerializer.class)

diff --git a/...fn/mw/iam/api/aup/model/AupReminders.java → ...i/aup/model/AupRemindersAndSignature.java b/...fn/mw/iam/api/aup/model/AupReminders.java → ...i/aup/model/AupRemindersAndSignature.java
@@ -15,7 +15,7 @@
  */
 package it.infn.mw.iam.api.aup.model;
 
-import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.TYPE;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 
 import java.lang.annotation.Retention;
@@ -25,11 +25,11 @@
 import javax.validation.Payload;
 
 @Retention(RUNTIME)
-@Target({FIELD})
-@Constraint(validatedBy = AupRemindersValidator.class)
-public @interface AupReminders {
+@Target({TYPE})
+@Constraint(validatedBy = AupRemindersAndSignatureValidator.class)
+public @interface AupRemindersAndSignature {
 
-  String message() default "Invalid AUP: aupRemindersInDays must be a comma-separated list of positive integers with no duplicates";
+  String message() default "Invalid AUP";
 
   Class<?>[] groups() default {};
 

diff --git a/.../api/aup/model/AupRemindersValidator.java → ...el/AupRemindersAndSignatureValidator.java b/.../api/aup/model/AupRemindersValidator.java → ...el/AupRemindersAndSignatureValidator.java
@@ -24,27 +24,66 @@
 import javax.validation.ConstraintValidator;
 import javax.validation.ConstraintValidatorContext;
 
-public class AupRemindersValidator implements ConstraintValidator<AupReminders, String> {
+public class AupRemindersAndSignatureValidator implements ConstraintValidator<AupRemindersAndSignature, Object> {
 
   @Override
-  public boolean isValid(String value, ConstraintValidatorContext context) {
+  public boolean isValid(Object value, ConstraintValidatorContext context) {
 
-    if (value == null || value.isEmpty()) {
+    if (!(value instanceof AupDTO)) {
+      throw new IllegalArgumentException("@AupReminders only applies to AupDTO");
+    }
+
+    AupDTO aup = (AupDTO) value;
+    Long signatureValidityInDays = aup.getSignatureValidityInDays();
+    String aupRemindersInDays = aup.getAupRemindersInDays();
+
+    if (signatureValidityInDays == null) {
+      context.disableDefaultConstraintViolation();
+      context
+        .buildConstraintViolationWithTemplate("Invalid AUP: signatureValidityInDays is required")
+        .addPropertyNode("signatureValidityInDays")
+        .addConstraintViolation();
+      return false;
+    }
+
+    if (signatureValidityInDays < 0) {
+      context.disableDefaultConstraintViolation();
+      context
+        .buildConstraintViolationWithTemplate("Invalid AUP: signatureValidityInDays must be >= 0")
+        .addPropertyNode("signatureValidityInDays")
+        .addConstraintViolation();
+      return false;
+    }
+
+    if (aupRemindersInDays == null || aupRemindersInDays.isEmpty()) {
       context.disableDefaultConstraintViolation();
-      context.buildConstraintViolationWithTemplate("Invalid AUP: aupRemindersInDays cannot be empty or null")
+      context
+        .buildConstraintViolationWithTemplate(
+            "Invalid AUP: aupRemindersInDays cannot be empty or null")
         .addConstraintViolation();
       return false;
     }
 
     try {
-      List<Integer> numbers = Arrays.stream(value.split(","))
+      List<Integer> numbers = Arrays.stream(aupRemindersInDays.split(","))
         .map(String::trim)
         .map(Integer::parseInt)
         .collect(Collectors.toList());
 
       if (numbers.stream().anyMatch(i -> i <= 0)) {
         context.disableDefaultConstraintViolation();
-        context.buildConstraintViolationWithTemplate("Invalid AUP: zero or negative values are not allowed")
+        context
+          .buildConstraintViolationWithTemplate(
+              "Invalid AUP: zero or negative values are not allowed")
+          .addConstraintViolation();
+        return false;
+      }
+
+      if (numbers.stream().anyMatch(i -> i >= signatureValidityInDays)) {
+        context.disableDefaultConstraintViolation();
+        context
+          .buildConstraintViolationWithTemplate(
+              "Invalid AUP: aupRemindersInDays must be smaller than signatureValidityInDays")
           .addConstraintViolation();
         return false;
       }

diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/aup/AupReminderTask.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/aup/AupReminderTask.java
@@ -64,6 +64,7 @@ public void sendAupReminders() {
       List<IamAupSignature> expiredSignatures = aupSignatureRepo.findByAupAndSignatureTime(aup,
           expirationDateAsDate, expirationDatePlusOneDayAsDate);
 
+      // check if an email of type AUP_EXPIRATION does not already exist, because it is never deleted
       expiredSignatures.forEach(s -> {
         if (emailNotificationRepo
           .countAupExpirationMessPerAccount(s.getAccount().getUserInfo().getEmail()) == 0) {
@@ -87,6 +88,8 @@ private void processRemindersForInterval(IamAup aup, LocalDate currentDate, Inte
 
     List<IamAupSignature> signatures = aupSignatureRepo.findByAupAndSignatureTime(aup,
         reminderDateAsDate, reminderDatePlusOneAsDate);
+
+    // check if an email of type AUP_REMINDER does not already exist, because it is never deleted
     signatures.forEach(s -> {
       if (emailNotificationRepo.countAupRemindersPerAccount(s.getAccount().getUserInfo().getEmail(),
           tomorrowAsDate) == 0) {

diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/aup/AupIntegrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/aup/AupIntegrationTests.java
@@ -228,7 +228,6 @@ public void aupDescriptionNoLongerThan128Chars() throws JsonProcessingException,
 
   }
 
-
   @Test
   @WithMockUser(username = "admin", roles = {"ADMIN", "USER"})
   public void aupCreationRequiresSignatureValidityDays() throws JsonProcessingException, Exception {
@@ -337,7 +336,7 @@ public void aupCreationRequiresNoLettersInAupRemindersInDays() throws Exception
   @Test
   @WithMockUser(username = "admin", roles = {"ADMIN", "USER"})
   public void aupCreationRequiresNoDuplicationInAupRemindersInDays() throws Exception {
-    AupDTO aup = new AupDTO(DEFAULT_AUP_URL, DEFAULT_AUP_TEXT, null, 3L, null, null, "30,15,15");
+    AupDTO aup = new AupDTO(DEFAULT_AUP_URL, DEFAULT_AUP_TEXT, null, 31L, null, null, "30,15,15");
     Date now = new Date();
     mockTimeProvider.setTime(now.getTime());
 
@@ -349,6 +348,21 @@ public void aupCreationRequiresNoDuplicationInAupRemindersInDays() throws Except
           "Invalid AUP: duplicate values are not allowed"));
   }
 
+  @Test
+  @WithMockUser(username = "admin", roles = {"ADMIN", "USER"})
+  public void aupCreationRequiresAupRemindersInDaysSmallerThanAupExpirationDays() throws Exception {
+    AupDTO aup = new AupDTO(DEFAULT_AUP_URL, DEFAULT_AUP_TEXT, null, 3L, null, null, "4");
+    Date now = new Date();
+    mockTimeProvider.setTime(now.getTime());
+
+    mvc
+      .perform(
+          post("/iam/aup").contentType(APPLICATION_JSON).content(mapper.writeValueAsString(aup)))
+      .andExpect(status().isBadRequest())
+      .andExpect(jsonPath("$.error").value(
+          "Invalid AUP: aupRemindersInDays must be smaller than signatureValidityInDays"));
+  }
+
   @Test
   @WithMockUser(username = "admin", roles = {"ADMIN", "USER"})
   public void aupCreationWorks() throws JsonProcessingException, Exception {
@@ -382,7 +396,7 @@ public void aupCreationWorks() throws JsonProcessingException, Exception {
   @Test
   @WithMockUser(username = "admin", roles = {"ADMIN", "USER"})
   public void whiteSpacesAllowedAmongAupRemindersDays() throws Exception {
-    AupDTO aup = new AupDTO(DEFAULT_AUP_URL, DEFAULT_AUP_TEXT, null, 3L, null, null, " 30, 15, 7 ");
+    AupDTO aup = new AupDTO(DEFAULT_AUP_URL, DEFAULT_AUP_TEXT, null, 31L, null, null, " 30, 15, 7 ");
 
     Date now = new Date();
     mockTimeProvider.setTime(now.getTime());
@@ -520,7 +534,7 @@ public void aupUpdateWorks() throws JsonProcessingException, Exception {
 
     aup.setUrl(UPDATED_AUP_URL);
     aup.setDescription(UPDATED_AUP_DESC);
-    aup.setSignatureValidityInDays(18L);
+    aup.setSignatureValidityInDays(31L);
 
     // Time travel 1 minute in the future
     Date then = new Date(now.getTime() + TimeUnit.MINUTES.toMillis(1));
@@ -540,7 +554,7 @@ public void aupUpdateWorks() throws JsonProcessingException, Exception {
     assertThat(updatedAup.getDescription(), equalTo(UPDATED_AUP_DESC));
     assertThat(updatedAup.getCreationTime(), new DateEqualModulo1Second(now));
     assertThat(updatedAup.getLastUpdateTime(), new DateEqualModulo1Second(now));
-    assertThat(updatedAup.getSignatureValidityInDays(), equalTo(18L));
+    assertThat(updatedAup.getSignatureValidityInDays(), equalTo(31L));
   }
 
 }