From b1d702c9ff4c7dfa1ddb4072781c37154a172be7 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 24 Mar 2022 18:22:59 +0100 Subject: [PATCH 1/9] refactor(validation): move email, url and username validation into package #8531 --- .../harvard/iq/dataverse/DatasetFieldValueValidator.java | 2 ++ src/main/java/edu/harvard/iq/dataverse/DatasetPage.java | 1 + .../java/edu/harvard/iq/dataverse/DataverseContact.java | 3 ++- src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java | 2 ++ src/main/java/edu/harvard/iq/dataverse/Shib.java | 1 + src/main/java/edu/harvard/iq/dataverse/api/Admin.java | 4 ++-- .../authorization/providers/builtin/BuiltinUser.java | 5 ++--- .../authorization/providers/builtin/DataverseUserPage.java | 4 ++-- .../providers/oauth2/OAuth2FirstLoginPage.java | 6 +++--- .../iq/dataverse/authorization/providers/shib/ShibUtil.java | 2 +- .../iq/dataverse/authorization/users/AuthenticatedUser.java | 3 +-- .../iq/dataverse/passwordreset/PasswordResetPage.java | 2 +- .../iq/dataverse/{ => validation}/EMailValidator.java | 2 +- .../harvard/iq/dataverse/{ => validation}/URLValidator.java | 2 +- .../iq/dataverse/{ => validation}/UserNameValidator.java | 2 +- .../iq/dataverse/{ => validation}/ValidateEmail.java | 2 +- .../harvard/iq/dataverse/{ => validation}/ValidateURL.java | 2 +- .../iq/dataverse/{ => validation}/ValidateUserName.java | 2 +- .../iq/dataverse/{ => validation}/EMailValidatorTest.java | 2 +- .../iq/dataverse/{ => validation}/URLValidatorTest.java | 3 ++- .../dataverse/{ => validation}/UserNameValidatorTest.java | 3 ++- 21 files changed, 31 insertions(+), 24 deletions(-) rename src/main/java/edu/harvard/iq/dataverse/{ => validation}/EMailValidator.java (97%) rename src/main/java/edu/harvard/iq/dataverse/{ => validation}/URLValidator.java (96%) rename src/main/java/edu/harvard/iq/dataverse/{ => validation}/UserNameValidator.java (97%) rename src/main/java/edu/harvard/iq/dataverse/{ => validation}/ValidateEmail.java (94%) rename src/main/java/edu/harvard/iq/dataverse/{ => validation}/ValidateURL.java (92%) rename src/main/java/edu/harvard/iq/dataverse/{ => validation}/ValidateUserName.java (94%) rename src/test/java/edu/harvard/iq/dataverse/{ => validation}/EMailValidatorTest.java (97%) rename src/test/java/edu/harvard/iq/dataverse/{ => validation}/URLValidatorTest.java (94%) rename src/test/java/edu/harvard/iq/dataverse/{ => validation}/UserNameValidatorTest.java (95%) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java b/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java index 08f30343ad8..de215a7318d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java @@ -15,6 +15,8 @@ import java.util.regex.Pattern; import javax.validation.ConstraintValidator; import javax.validation.ConstraintValidatorContext; + +import edu.harvard.iq.dataverse.validation.EMailValidator; import org.apache.commons.lang3.StringUtils; import org.apache.commons.validator.routines.UrlValidator; diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java index 29f08d885cd..c0235c512fe 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java @@ -56,6 +56,7 @@ import edu.harvard.iq.dataverse.util.StringUtil; import edu.harvard.iq.dataverse.util.SystemConfig; +import edu.harvard.iq.dataverse.validation.URLValidator; import edu.harvard.iq.dataverse.workflows.WorkflowComment; import java.io.File; diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseContact.java b/src/main/java/edu/harvard/iq/dataverse/DataverseContact.java index d19f20d5ebd..46021ddbc9b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataverseContact.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataverseContact.java @@ -16,7 +16,8 @@ import javax.persistence.JoinColumn; import javax.persistence.ManyToOne; import javax.persistence.Table; -import org.hibernate.validator.constraints.Email; + +import edu.harvard.iq.dataverse.validation.ValidateEmail; import org.hibernate.validator.constraints.NotBlank; /** diff --git a/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java index 04956ccc756..89972e188ce 100644 --- a/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java @@ -38,6 +38,8 @@ import javax.mail.internet.AddressException; import javax.mail.internet.InternetAddress; import javax.mail.internet.MimeMessage; + +import edu.harvard.iq.dataverse.validation.EMailValidator; import org.apache.commons.lang3.StringUtils; /** diff --git a/src/main/java/edu/harvard/iq/dataverse/Shib.java b/src/main/java/edu/harvard/iq/dataverse/Shib.java index b71fe3cd566..dbd14f9f679 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Shib.java +++ b/src/main/java/edu/harvard/iq/dataverse/Shib.java @@ -15,6 +15,7 @@ import edu.harvard.iq.dataverse.util.BundleUtil; import edu.harvard.iq.dataverse.util.JsfHelper; import edu.harvard.iq.dataverse.util.SystemConfig; +import edu.harvard.iq.dataverse.validation.EMailValidator; import org.apache.commons.lang3.StringUtils; import java.io.IOException; diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java index 4085b504578..fcc05d78b14 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java @@ -13,7 +13,7 @@ import edu.harvard.iq.dataverse.DataverseRequestServiceBean; import edu.harvard.iq.dataverse.DataverseSession; import edu.harvard.iq.dataverse.DvObject; -import edu.harvard.iq.dataverse.EMailValidator; +import edu.harvard.iq.dataverse.validation.EMailValidator; import edu.harvard.iq.dataverse.EjbDataverseEngine; import edu.harvard.iq.dataverse.GlobalId; import edu.harvard.iq.dataverse.UserServiceBean; @@ -97,7 +97,7 @@ import java.io.IOException; import java.io.OutputStream; -import edu.harvard.iq.dataverse.util.json.JsonPrinter; + import static edu.harvard.iq.dataverse.util.json.JsonPrinter.json; import static edu.harvard.iq.dataverse.util.json.JsonPrinter.rolesToJson; import static edu.harvard.iq.dataverse.util.json.JsonPrinter.toJsonArray; diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java index 9ae4e4b0e87..99badc27974 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java @@ -1,10 +1,9 @@ package edu.harvard.iq.dataverse.authorization.providers.builtin; -import edu.harvard.iq.dataverse.ValidateEmail; -import edu.harvard.iq.dataverse.ValidateUserName; +import edu.harvard.iq.dataverse.validation.ValidateUserName; import edu.harvard.iq.dataverse.authorization.AuthenticatedUserDisplayInfo; import edu.harvard.iq.dataverse.passwordreset.PasswordResetData; -import static edu.harvard.iq.dataverse.util.StringUtil.nonEmpty; + import java.io.Serializable; import javax.persistence.CascadeType; import javax.persistence.Column; diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java index 177c59c5873..746cf9a11f1 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java @@ -10,12 +10,12 @@ import edu.harvard.iq.dataverse.DataverseServiceBean; import edu.harvard.iq.dataverse.DataverseSession; import edu.harvard.iq.dataverse.DvObject; -import edu.harvard.iq.dataverse.EMailValidator; +import edu.harvard.iq.dataverse.validation.EMailValidator; import edu.harvard.iq.dataverse.PermissionServiceBean; import edu.harvard.iq.dataverse.PermissionsWrapper; import edu.harvard.iq.dataverse.RoleAssignment; import edu.harvard.iq.dataverse.SettingsWrapper; -import edu.harvard.iq.dataverse.UserNameValidator; +import edu.harvard.iq.dataverse.validation.UserNameValidator; import edu.harvard.iq.dataverse.UserNotification; import edu.harvard.iq.dataverse.UserNotificationServiceBean; import edu.harvard.iq.dataverse.UserServiceBean; diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java index 4c0170a906b..7efe9263d26 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java @@ -1,11 +1,11 @@ package edu.harvard.iq.dataverse.authorization.providers.oauth2; import edu.harvard.iq.dataverse.DataverseSession; -import edu.harvard.iq.dataverse.EMailValidator; +import edu.harvard.iq.dataverse.validation.EMailValidator; import edu.harvard.iq.dataverse.UserNotification; import edu.harvard.iq.dataverse.UserNotificationServiceBean; -import edu.harvard.iq.dataverse.ValidateEmail; -import edu.harvard.iq.dataverse.UserNameValidator; +import edu.harvard.iq.dataverse.validation.ValidateEmail; +import edu.harvard.iq.dataverse.validation.UserNameValidator; import edu.harvard.iq.dataverse.authorization.AuthTestDataServiceBean; import edu.harvard.iq.dataverse.authorization.AuthUtil; import edu.harvard.iq.dataverse.authorization.AuthenticatedUserDisplayInfo; diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/shib/ShibUtil.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/shib/ShibUtil.java index 8d34bed4f2f..0d59316fe31 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/shib/ShibUtil.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/shib/ShibUtil.java @@ -4,7 +4,7 @@ import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonParser; -import edu.harvard.iq.dataverse.EMailValidator; +import edu.harvard.iq.dataverse.validation.EMailValidator; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java index 9d76ce0e47c..65e62debf76 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java @@ -3,8 +3,7 @@ import edu.harvard.iq.dataverse.Cart; import edu.harvard.iq.dataverse.DatasetLock; import edu.harvard.iq.dataverse.UserNotification; -import edu.harvard.iq.dataverse.ValidateEmail; -import edu.harvard.iq.dataverse.authorization.AccessRequest; +import edu.harvard.iq.dataverse.validation.ValidateEmail; import edu.harvard.iq.dataverse.authorization.AuthenticatedUserDisplayInfo; import edu.harvard.iq.dataverse.authorization.AuthenticatedUserLookup; import edu.harvard.iq.dataverse.authorization.providers.oauth2.OAuth2TokenData; diff --git a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java index 4ef4fd51737..b9eabf45159 100644 --- a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java @@ -3,7 +3,7 @@ import edu.harvard.iq.dataverse.DataverseServiceBean; import edu.harvard.iq.dataverse.DataverseSession; import edu.harvard.iq.dataverse.SettingsWrapper; -import edu.harvard.iq.dataverse.ValidateEmail; +import edu.harvard.iq.dataverse.validation.ValidateEmail; import edu.harvard.iq.dataverse.actionlogging.ActionLogRecord; import edu.harvard.iq.dataverse.actionlogging.ActionLogServiceBean; import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean; diff --git a/src/main/java/edu/harvard/iq/dataverse/EMailValidator.java b/src/main/java/edu/harvard/iq/dataverse/validation/EMailValidator.java similarity index 97% rename from src/main/java/edu/harvard/iq/dataverse/EMailValidator.java rename to src/main/java/edu/harvard/iq/dataverse/validation/EMailValidator.java index 4091c9dfee9..47c773a329b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/EMailValidator.java +++ b/src/main/java/edu/harvard/iq/dataverse/validation/EMailValidator.java @@ -3,7 +3,7 @@ * To change this template file, choose Tools | Templates * and open the template in the editor. */ -package edu.harvard.iq.dataverse; +package edu.harvard.iq.dataverse.validation; import javax.validation.ConstraintValidator; import javax.validation.ConstraintValidatorContext; diff --git a/src/main/java/edu/harvard/iq/dataverse/URLValidator.java b/src/main/java/edu/harvard/iq/dataverse/validation/URLValidator.java similarity index 96% rename from src/main/java/edu/harvard/iq/dataverse/URLValidator.java rename to src/main/java/edu/harvard/iq/dataverse/validation/URLValidator.java index ac8620b0123..08b99aa2eaf 100644 --- a/src/main/java/edu/harvard/iq/dataverse/URLValidator.java +++ b/src/main/java/edu/harvard/iq/dataverse/validation/URLValidator.java @@ -1,4 +1,4 @@ -package edu.harvard.iq.dataverse; +package edu.harvard.iq.dataverse.validation; import edu.harvard.iq.dataverse.util.BundleUtil; import javax.validation.ConstraintValidator; import javax.validation.ConstraintValidatorContext; diff --git a/src/main/java/edu/harvard/iq/dataverse/UserNameValidator.java b/src/main/java/edu/harvard/iq/dataverse/validation/UserNameValidator.java similarity index 97% rename from src/main/java/edu/harvard/iq/dataverse/UserNameValidator.java rename to src/main/java/edu/harvard/iq/dataverse/validation/UserNameValidator.java index a86f95f6a3f..d15e41f56e4 100644 --- a/src/main/java/edu/harvard/iq/dataverse/UserNameValidator.java +++ b/src/main/java/edu/harvard/iq/dataverse/validation/UserNameValidator.java @@ -3,7 +3,7 @@ * To change this template file, choose Tools | Templates * and open the template in the editor. */ -package edu.harvard.iq.dataverse; +package edu.harvard.iq.dataverse.validation; import java.util.regex.Matcher; import java.util.regex.Pattern; diff --git a/src/main/java/edu/harvard/iq/dataverse/ValidateEmail.java b/src/main/java/edu/harvard/iq/dataverse/validation/ValidateEmail.java similarity index 94% rename from src/main/java/edu/harvard/iq/dataverse/ValidateEmail.java rename to src/main/java/edu/harvard/iq/dataverse/validation/ValidateEmail.java index cadbbbf1407..1abb893be83 100644 --- a/src/main/java/edu/harvard/iq/dataverse/ValidateEmail.java +++ b/src/main/java/edu/harvard/iq/dataverse/validation/ValidateEmail.java @@ -3,7 +3,7 @@ * To change this template file, choose Tools | Templates * and open the template in the editor. */ -package edu.harvard.iq.dataverse; +package edu.harvard.iq.dataverse.validation; import static java.lang.annotation.ElementType.*; import static java.lang.annotation.RetentionPolicy.*; diff --git a/src/main/java/edu/harvard/iq/dataverse/ValidateURL.java b/src/main/java/edu/harvard/iq/dataverse/validation/ValidateURL.java similarity index 92% rename from src/main/java/edu/harvard/iq/dataverse/ValidateURL.java rename to src/main/java/edu/harvard/iq/dataverse/validation/ValidateURL.java index 106876e423e..492110b34b1 100644 --- a/src/main/java/edu/harvard/iq/dataverse/ValidateURL.java +++ b/src/main/java/edu/harvard/iq/dataverse/validation/ValidateURL.java @@ -1,5 +1,5 @@ -package edu.harvard.iq.dataverse; +package edu.harvard.iq.dataverse.validation; import java.lang.annotation.Documented; import static java.lang.annotation.ElementType.FIELD; diff --git a/src/main/java/edu/harvard/iq/dataverse/ValidateUserName.java b/src/main/java/edu/harvard/iq/dataverse/validation/ValidateUserName.java similarity index 94% rename from src/main/java/edu/harvard/iq/dataverse/ValidateUserName.java rename to src/main/java/edu/harvard/iq/dataverse/validation/ValidateUserName.java index 0cd8fcb4443..b3c46c63fdc 100644 --- a/src/main/java/edu/harvard/iq/dataverse/ValidateUserName.java +++ b/src/main/java/edu/harvard/iq/dataverse/validation/ValidateUserName.java @@ -3,7 +3,7 @@ * To change this template file, choose Tools | Templates * and open the template in the editor. */ -package edu.harvard.iq.dataverse; +package edu.harvard.iq.dataverse.validation; import java.lang.annotation.Documented; import static java.lang.annotation.ElementType.FIELD; diff --git a/src/test/java/edu/harvard/iq/dataverse/EMailValidatorTest.java b/src/test/java/edu/harvard/iq/dataverse/validation/EMailValidatorTest.java similarity index 97% rename from src/test/java/edu/harvard/iq/dataverse/EMailValidatorTest.java rename to src/test/java/edu/harvard/iq/dataverse/validation/EMailValidatorTest.java index 2518f6486e3..9f4b9d3942e 100644 --- a/src/test/java/edu/harvard/iq/dataverse/EMailValidatorTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/validation/EMailValidatorTest.java @@ -1,4 +1,4 @@ -package edu.harvard.iq.dataverse; +package edu.harvard.iq.dataverse.validation; import org.apache.commons.validator.routines.EmailValidator; import org.junit.jupiter.params.ParameterizedTest; diff --git a/src/test/java/edu/harvard/iq/dataverse/URLValidatorTest.java b/src/test/java/edu/harvard/iq/dataverse/validation/URLValidatorTest.java similarity index 94% rename from src/test/java/edu/harvard/iq/dataverse/URLValidatorTest.java rename to src/test/java/edu/harvard/iq/dataverse/validation/URLValidatorTest.java index f994809a0c0..a464e6f549f 100644 --- a/src/test/java/edu/harvard/iq/dataverse/URLValidatorTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/validation/URLValidatorTest.java @@ -1,10 +1,11 @@ -package edu.harvard.iq.dataverse; +package edu.harvard.iq.dataverse.validation; import static org.junit.Assert.assertEquals; import javax.validation.ConstraintValidatorContext; +import edu.harvard.iq.dataverse.validation.URLValidator; import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorContextImpl; import org.hibernate.validator.internal.engine.path.PathImpl; import javax.validation.Validation; diff --git a/src/test/java/edu/harvard/iq/dataverse/UserNameValidatorTest.java b/src/test/java/edu/harvard/iq/dataverse/validation/UserNameValidatorTest.java similarity index 95% rename from src/test/java/edu/harvard/iq/dataverse/UserNameValidatorTest.java rename to src/test/java/edu/harvard/iq/dataverse/validation/UserNameValidatorTest.java index 000651d829a..2602556a9fb 100644 --- a/src/test/java/edu/harvard/iq/dataverse/UserNameValidatorTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/validation/UserNameValidatorTest.java @@ -3,13 +3,14 @@ * To change this template file, choose Tools | Templates * and open the template in the editor. */ -package edu.harvard.iq.dataverse; +package edu.harvard.iq.dataverse.validation; import static org.junit.Assert.assertEquals; import java.util.Arrays; import java.util.Collection; +import edu.harvard.iq.dataverse.validation.UserNameValidator; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; From b77b7a3b3030f2cd86c0aff5e1e5bffdacf5c1de Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 24 Mar 2022 20:56:05 +0100 Subject: [PATCH 2/9] build(deps): add necessary deps for bean validation #8531 --- pom.xml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 7663bb3e6e6..c0b66a47115 100644 --- a/pom.xml +++ b/pom.xml @@ -200,11 +200,18 @@ omnifaces 3.8 + + + jakarta.validation + jakarta.validation-api + provided + org.hibernate.validator hibernate-validator + provided - + org.glassfish jakarta.el provided From c1a03642d782826f5e164674b1ae8bbf2577f375 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 24 Mar 2022 20:57:46 +0100 Subject: [PATCH 3/9] refactor(validation): reshape EMailValidator and add tests #8531 --- src/main/java/ValidationMessages.properties | 2 + .../dataverse/validation/EMailValidator.java | 38 ++----- .../dataverse/validation/ValidateEmail.java | 4 +- src/main/java/propertyFiles/Bundle.properties | 3 - .../validation/EMailValidatorTest.java | 102 ++++++++++++------ 5 files changed, 83 insertions(+), 66 deletions(-) diff --git a/src/main/java/ValidationMessages.properties b/src/main/java/ValidationMessages.properties index 877f76b0f9b..08859ccccb0 100644 --- a/src/main/java/ValidationMessages.properties +++ b/src/main/java/ValidationMessages.properties @@ -42,3 +42,5 @@ password.validate=Password reset page default email message. guestbook.name=Enter a name for the guestbook guestbook.response.nameLength=Please limit response to 255 characters + +email.invalid=is not a valid email address. diff --git a/src/main/java/edu/harvard/iq/dataverse/validation/EMailValidator.java b/src/main/java/edu/harvard/iq/dataverse/validation/EMailValidator.java index 47c773a329b..5050aad5bf7 100644 --- a/src/main/java/edu/harvard/iq/dataverse/validation/EMailValidator.java +++ b/src/main/java/edu/harvard/iq/dataverse/validation/EMailValidator.java @@ -1,14 +1,8 @@ -/* - * To change this license header, choose License Headers in Project Properties. - * To change this template file, choose Tools | Templates - * and open the template in the editor. - */ package edu.harvard.iq.dataverse.validation; import javax.validation.ConstraintValidator; import javax.validation.ConstraintValidatorContext; -import edu.harvard.iq.dataverse.util.BundleUtil; import org.apache.commons.validator.routines.EmailValidator; /** @@ -17,32 +11,18 @@ */ public class EMailValidator implements ConstraintValidator { - @Override - public void initialize(ValidateEmail constraintAnnotation) { - - } - @Override public boolean isValid(String value, ConstraintValidatorContext context) { - - return isEmailValid(value, context); - - + return isEmailValid(value); } - public static boolean isEmailValid(String value, ConstraintValidatorContext context) { - //this null check is not needed any more as the null check is done in datasetfieldvaluevalidator -// if (value == null) { -// //A null email id is not valid, as email is a required field. -// return false; -// } - boolean isValid = EmailValidator.getInstance().isValid(value); - if (!isValid) { - if (context != null) { - context.buildConstraintViolationWithTemplate(value + " " + BundleUtil.getStringFromBundle("email.invalid")).addConstraintViolation(); - } - return false; - } - return true; + /** + * Validate an email address in a null safe way (null = valid). + * (Null is valid to allow for optional values - use @NotNull to enforce!) + * @param value The email address to validate + * @return true when valid, false when invalid (null = valid!) + */ + public static boolean isEmailValid(String value) { + return value == null || EmailValidator.getInstance().isValid(value); } } diff --git a/src/main/java/edu/harvard/iq/dataverse/validation/ValidateEmail.java b/src/main/java/edu/harvard/iq/dataverse/validation/ValidateEmail.java index 1abb893be83..310dc950858 100644 --- a/src/main/java/edu/harvard/iq/dataverse/validation/ValidateEmail.java +++ b/src/main/java/edu/harvard/iq/dataverse/validation/ValidateEmail.java @@ -21,8 +21,8 @@ @Retention(RUNTIME) @Constraint(validatedBy = {EMailValidator.class}) @Documented -public @interface ValidateEmail { - String message() default "Failed Validation Email Address"; +public @interface ValidateEmail { + String message() default "'${validatedValue}' {email.invalid}"; Class[] groups() default {}; diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 4c0938ee33f..5b331e404b5 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -2292,9 +2292,6 @@ dataset.file.uploadWarning=upload warning dataset.file.uploadWorked=upload worked dataset.file.upload.popup.explanation.tip=For more information, please refer to the Duplicate Files section of the User Guide. -#EmailValidator.java -email.invalid=is not a valid email address. - #URLValidator.java url.invalid=is not a valid URL. diff --git a/src/test/java/edu/harvard/iq/dataverse/validation/EMailValidatorTest.java b/src/test/java/edu/harvard/iq/dataverse/validation/EMailValidatorTest.java index 9f4b9d3942e..80d848248c0 100644 --- a/src/test/java/edu/harvard/iq/dataverse/validation/EMailValidatorTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/validation/EMailValidatorTest.java @@ -1,47 +1,85 @@ package edu.harvard.iq.dataverse.validation; -import org.apache.commons.validator.routines.EmailValidator; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import javax.validation.ConstraintViolation; +import javax.validation.Validation; +import javax.validation.Validator; + +import java.util.Set; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; public class EMailValidatorTest { - static EmailValidator validator = EmailValidator.getInstance(); + private final Validator validator = Validation.buildDefaultValidatorFactory().getValidator(); + + public static Stream emailExamples() { + return Stream.of( + Arguments.of("true", "pete@mailinator.com"), + Arguments.of("false", " leadingWhitespace@mailinator.com"), + Arguments.of("false", "trailingWhitespace@mailinator.com "), + Arguments.of("false", "elisah.da mota@example.com"), + Arguments.of("false", "pete1@mailinator.com;pete2@mailinator.com"), + /* + * These examples are all from https://randomuser.me and seem to be + * valid according to + * http://sphinx.mythic-beasts.com/~pdw/cgi-bin/emailvalidate (except رونیکا.محمدخان@example.com) + */ + Arguments.of("true", "michélle.pereboom@example.com"), + Arguments.of("true", "begüm.vriezen@example.com"), + Arguments.of("true", "lótus.gonçalves@example.com"), + Arguments.of("true", "lótus.gonçalves@éxample.com"), + Arguments.of("true", "begüm.vriezen@example.cologne"), + Arguments.of("true", "رونیکا.محمدخان@example.com"), + Arguments.of("false", "lótus.gonçalves@example.cóm"), + Arguments.of("false", "dora@.com"), + Arguments.of("false", ""), + // Null means "no value given" - that's valid, you'll need to use @NotNull or similar to enforce a value! + Arguments.of("true", null), + + // add tests for 4601 + Arguments.of("true", "blah@wiso.uni-unc.de"), + Arguments.of("true", "foo@essex.co.uk"), + Arguments.of("true", "jack@bu.cloud") + ); + } + @ParameterizedTest - @CsvSource(value = { - "true, 'pete@mailinator.com'", - "false, ' leadingWhitespace@mailinator.com'", - "false, 'trailingWhitespace@mailinator.com '", + @MethodSource("emailExamples") + public void testIsEmailValid(boolean expected, String mail) { + assertEquals(expected, EMailValidator.isEmailValid(mail)); + } - "false, 'elisah.da mota@example.com'", - "false, 'pete1@mailinator.com;pete2@mailinator.com'", + /** + * This is a simple test class, as we defined the annotation for fields only. + */ + private final class SubjectClass { + @ValidateEmail + String mail; - /** - * These examples are all from https://randomuser.me and seem to be - * valid according to - * http://sphinx.mythic-beasts.com/~pdw/cgi-bin/emailvalidate (except - * رونیکا.محمدخان@example.com). - */ - "true, 'michélle.pereboom@example.com'", - "true, 'begüm.vriezen@example.com'", - "true, 'lótus.gonçalves@example.com'", - "true, 'lótus.gonçalves@éxample.com'", - "true, 'begüm.vriezen@example.cologne'", - "true, 'رونیکا.محمدخان@example.com'", - "false, 'lótus.gonçalves@example.cóm'", - "false, 'dora@.com'", - "false, ''", - "false, NULL", + SubjectClass(String mail) { + this.mail = mail; + } + } - // add tests for 4601 - "true, 'blah@wiso.uni-unc.de'", - "true, 'foo@essex.co.uk'", - "true, 'jack@bu.cloud'" - }, nullValues = "NULL") - public void testIsEmailValid(boolean expected, String mail) { - assertEquals(expected, validator.isValid(mail)); + @ParameterizedTest + @MethodSource("emailExamples") + public void testConstraint(boolean expected, String mail) { + // given + SubjectClass sut = new SubjectClass(mail); + + //when + Set> violations = validator.validate(sut); + + // then + assertEquals(expected ? 0 : 1, violations.size()); + violations.stream().findFirst().ifPresent( c -> { + assertTrue(c.getMessage().contains(mail)); }); } } From 2d82f8443b55b503057ec09b5c32674c5404b542 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 24 Mar 2022 21:22:37 +0100 Subject: [PATCH 4/9] refactor(validation): change EMailValidator users sending null context #8531 --- src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java | 2 +- src/main/java/edu/harvard/iq/dataverse/Shib.java | 4 ++-- src/main/java/edu/harvard/iq/dataverse/api/Admin.java | 4 ++-- .../authorization/providers/builtin/DataverseUserPage.java | 2 +- .../authorization/providers/oauth2/OAuth2FirstLoginPage.java | 2 +- .../iq/dataverse/authorization/providers/shib/ShibUtil.java | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java index 89972e188ce..bd5f27b9e83 100644 --- a/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java @@ -190,7 +190,7 @@ public void sendMail(String reply, String to, String subject, String messageText logger.severe(ex.getMessage()); } msg.setFrom(fromAddress); - if (EMailValidator.isEmailValid(reply, null)) { + if (EMailValidator.isEmailValid(reply)) { // But set the reply-to address to direct replies to the requested 'from' party if it is a valid email address msg.setReplyTo(new Address[] {new InternetAddress(reply)}); } else { diff --git a/src/main/java/edu/harvard/iq/dataverse/Shib.java b/src/main/java/edu/harvard/iq/dataverse/Shib.java index dbd14f9f679..324f6e185a6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Shib.java +++ b/src/main/java/edu/harvard/iq/dataverse/Shib.java @@ -191,12 +191,12 @@ public void init() { } } - if (!EMailValidator.isEmailValid(emailAddressInAssertion, null)) { + if (!EMailValidator.isEmailValid(emailAddressInAssertion)) { String msg = "The SAML assertion contained an invalid email address: \"" + emailAddressInAssertion + "\"."; logger.info(msg); msg=BundleUtil.getStringFromBundle("shib.invalidEmailAddress", Arrays.asList(emailAddressInAssertion)); String singleEmailAddress = ShibUtil.findSingleValue(emailAddressInAssertion); - if (EMailValidator.isEmailValid(singleEmailAddress, null)) { + if (EMailValidator.isEmailValid(singleEmailAddress)) { msg = "Multiple email addresses were asserted by the Identity Provider (" + emailAddressInAssertion + " ). These were sorted and the first was chosen: " + singleEmailAddress; logger.info(msg); emailAddress = singleEmailAddress; diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java index fcc05d78b14..101cd570ccd 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java @@ -659,7 +659,7 @@ public Response builtin2shib(String content) { String overwriteEmail = randomUser.get("email"); overwriteEmail = newEmailAddressToUse; logger.info("overwriteEmail: " + overwriteEmail); - boolean validEmail = EMailValidator.isEmailValid(overwriteEmail, null); + boolean validEmail = EMailValidator.isEmailValid(overwriteEmail); if (!validEmail) { // See https://github.com/IQSS/dataverse/issues/2998 return error(Response.Status.BAD_REQUEST, "invalid email: " + overwriteEmail); @@ -816,7 +816,7 @@ public Response builtin2oauth(String content) { String overwriteEmail = randomUser.get("email"); overwriteEmail = newEmailAddressToUse; logger.info("overwriteEmail: " + overwriteEmail); - boolean validEmail = EMailValidator.isEmailValid(overwriteEmail, null); + boolean validEmail = EMailValidator.isEmailValid(overwriteEmail); if (!validEmail) { // See https://github.com/IQSS/dataverse/issues/2998 return error(Response.Status.BAD_REQUEST, "invalid email: " + overwriteEmail); diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java index 746cf9a11f1..da0500c734d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java @@ -225,7 +225,7 @@ public void validateUserName(FacesContext context, UIComponent toValidate, Objec public void validateUserEmail(FacesContext context, UIComponent toValidate, Object value) { String userEmail = (String) value; - boolean emailValid = EMailValidator.isEmailValid(userEmail, null); + boolean emailValid = EMailValidator.isEmailValid(userEmail); if (!emailValid) { ((UIInput) toValidate).setValid(false); FacesMessage message = new FacesMessage(FacesMessage.SEVERITY_ERROR, BundleUtil.getStringFromBundle("oauth2.newAccount.emailInvalid"), null); diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java index 7efe9263d26..54ba3ec6a05 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java @@ -259,7 +259,7 @@ public void validateUserName(FacesContext context, UIComponent toValidate, Objec */ public void validateUserEmail(FacesContext context, UIComponent toValidate, Object value) { String userEmail = (String) value; - boolean emailValid = EMailValidator.isEmailValid(userEmail, null); + boolean emailValid = EMailValidator.isEmailValid(userEmail); if (!emailValid) { ((UIInput) toValidate).setValid(false); FacesMessage message = new FacesMessage(FacesMessage.SEVERITY_ERROR, BundleUtil.getStringFromBundle("oauth2.newAccount.emailInvalid"), null); diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/shib/ShibUtil.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/shib/ShibUtil.java index 0d59316fe31..8d523ceae2f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/shib/ShibUtil.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/shib/ShibUtil.java @@ -151,7 +151,7 @@ public static String generateFriendlyLookingUserIdentifer(String usernameAsserti logger.fine(ex + " parsing " + email); } } else { - boolean passedValidation = EMailValidator.isEmailValid(email, null); + boolean passedValidation = EMailValidator.isEmailValid(email); logger.fine("Odd email address. No @ sign ('" + email + "'). Passed email validation: " + passedValidation); } } else { From 237d5baefcfeb2264d66eece81f76e2904bbc992 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 24 Mar 2022 21:24:26 +0100 Subject: [PATCH 5/9] fix(validation): integrate refactored EMailValidator with DatasetFieldValueValidator #8503 The context parameter has been removed, need to add the violation message ourselfs. Also adding a true test for email validation. --- .../dataverse/DatasetFieldValueValidator.java | 7 +-- .../DatasetFieldValueValidatorTest.java | 63 +++++++++++-------- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java b/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java index de215a7318d..7cd823211bf 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java @@ -184,12 +184,11 @@ public boolean isValid(DatasetFieldValue value, ConstraintValidatorContext conte } if (fieldType.equals(FieldType.EMAIL) && !lengthOnly) { - if(value.getDatasetField().isRequired() && value.getValue()==null){ + boolean isValidMail = EMailValidator.isEmailValid(value.getValue()); + if (!isValidMail) { + context.buildConstraintViolationWithTemplate(dsfType.getDisplayName() + " " + value.getValue() + " {email.invalid}").addConstraintViolation(); return false; } - - return EMailValidator.isEmailValid(value.getValue(), context); - } return true; diff --git a/src/test/java/edu/harvard/iq/dataverse/DatasetFieldValueValidatorTest.java b/src/test/java/edu/harvard/iq/dataverse/DatasetFieldValueValidatorTest.java index 7e3f326923a..a637848851e 100644 --- a/src/test/java/edu/harvard/iq/dataverse/DatasetFieldValueValidatorTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/DatasetFieldValueValidatorTest.java @@ -5,42 +5,25 @@ */ package edu.harvard.iq.dataverse; +import java.util.Set; import java.util.regex.Pattern; import javax.validation.ConstraintValidatorContext; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Test; -import static org.junit.Assert.*; +import javax.validation.ConstraintViolation; +import javax.validation.Validation; +import javax.validation.Validator; + +import org.junit.jupiter.api.Test; import org.mockito.Mockito; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; + /** * * @author skraffmi */ public class DatasetFieldValueValidatorTest { - - - public DatasetFieldValueValidatorTest() { - } - - @BeforeClass - public static void setUpClass() { - } - - @AfterClass - public static void tearDownClass() { - } - - @Before - public void setUp() { - } - - @After - public void tearDown() { - } - /** * Test of isValid method, of class DatasetFieldValueValidator. @@ -199,4 +182,30 @@ public void testIsValidAuthorIdentifierGnd() { assertFalse(validator.isValidAuthorIdentifier("junk", pattern)); } + final Validator validator = Validation.buildDefaultValidatorFactory().getValidator(); + + @Test + public void testInvalidEmail() { + // given + String fieldName = "testField"; + String invalidMail = "myinvalidmail"; + + DatasetField field = new DatasetField(); + field.setDatasetFieldType(new DatasetFieldType(fieldName, DatasetFieldType.FieldType.EMAIL, false)); + DatasetFieldValue sut = new DatasetFieldValue(field); + sut.setValue(invalidMail); + + // when + Set> violations = validator.validate(sut); + + // then + assertTrue(violations.size() == 1); + violations.stream().findFirst().ifPresent(c -> { + assertTrue(c.getMessage().startsWith(fieldName + " " + invalidMail + " ")); + assertTrue(c.getMessage().contains("not")); + assertTrue(c.getMessage().contains("email")); + }); + + } + } From 223d5a265b068d2d5a37dda8343d850c7d8a077a Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 24 Mar 2022 22:31:53 +0100 Subject: [PATCH 6/9] refactor(validation): simplify ValidateURL #8531 - Simplify the code for URLValidator - Make it nullsafe - Make allowed schemes configurable from annotation - Rewrite tests to JUnit5, more examples and test with real subject classes - Move message string to validation bundle --- src/main/java/ValidationMessages.properties | 1 + .../iq/dataverse/validation/URLValidator.java | 50 +++---- .../iq/dataverse/validation/ValidateURL.java | 4 +- src/main/java/propertyFiles/Bundle.properties | 3 - .../validation/URLValidatorTest.java | 131 ++++++++++++------ 5 files changed, 119 insertions(+), 70 deletions(-) diff --git a/src/main/java/ValidationMessages.properties b/src/main/java/ValidationMessages.properties index 08859ccccb0..651d86b7290 100644 --- a/src/main/java/ValidationMessages.properties +++ b/src/main/java/ValidationMessages.properties @@ -44,3 +44,4 @@ guestbook.name=Enter a name for the guestbook guestbook.response.nameLength=Please limit response to 255 characters email.invalid=is not a valid email address. +url.invalid=is not a valid URL. diff --git a/src/main/java/edu/harvard/iq/dataverse/validation/URLValidator.java b/src/main/java/edu/harvard/iq/dataverse/validation/URLValidator.java index 08b99aa2eaf..846ae48783a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/validation/URLValidator.java +++ b/src/main/java/edu/harvard/iq/dataverse/validation/URLValidator.java @@ -10,40 +10,40 @@ */ public class URLValidator implements ConstraintValidator { + private String[] allowedSchemes; @Override public void initialize(ValidateURL constraintAnnotation) { - + this.allowedSchemes = constraintAnnotation.schemes(); } @Override public boolean isValid(String value, ConstraintValidatorContext context) { - - boolean valid = isURLValid(value); - if (context != null && !valid) { - context.buildConstraintViolationWithTemplate(value + " " + BundleUtil.getStringFromBundle("url.invalid")).addConstraintViolation(); - } - return valid; + return isURLValid(value, this.allowedSchemes); } - + + /** + * Check if a URL is valid in a nullsafe way. (null = valid to allow optional values). + * Empty values are no valid URLs. This variant allows default schemes HTTP, HTTPS and FTP. + * + * @param value The URL to validate + * @return true when valid (null is also valid) or false + */ public static boolean isURLValid(String value) { - if (value == null || value.isEmpty()) { - return true; - } - - String[] schemes = {"http","https", "ftp"}; + // default schemes == ValidateURL schemes() default + return isURLValid(value, new String[]{"http", "https", "ftp"}); + } + + /** + * Check if a URL is valid in a nullsafe way. (null = valid to allow optional values). + * Empty values are no valid URLs. This variant allows any schemes you hand over. + * + * @param value The URL to validate + * @param schemes The list of allowed schemes + * @return true when valid (null is also valid) or false + */ + public static boolean isURLValid(String value, String[] schemes) { UrlValidator urlValidator = new UrlValidator(schemes); - - try { - if (urlValidator.isValid(value)) { - } else { - return false; - } - } catch (NullPointerException npe) { - return false; - } - - return true; - + return value == null || urlValidator.isValid(value); } diff --git a/src/main/java/edu/harvard/iq/dataverse/validation/ValidateURL.java b/src/main/java/edu/harvard/iq/dataverse/validation/ValidateURL.java index 492110b34b1..5aaab0c2e8e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/validation/ValidateURL.java +++ b/src/main/java/edu/harvard/iq/dataverse/validation/ValidateURL.java @@ -14,8 +14,8 @@ @Constraint(validatedBy = {URLValidator.class}) @Documented public @interface ValidateURL { - - String message() default "Failed Validation for Validate URL"; + String message() default "'${validatedValue}' {url.invalid}"; + String[] schemes() default {"http", "https", "ftp"}; Class[] groups() default {}; diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 5b331e404b5..e9081565b82 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -2292,9 +2292,6 @@ dataset.file.uploadWarning=upload warning dataset.file.uploadWorked=upload worked dataset.file.upload.popup.explanation.tip=For more information, please refer to the Duplicate Files section of the User Guide. -#URLValidator.java -url.invalid=is not a valid URL. - #HarvestingClientsPage.java harvest.start.error=Sorry, harvest could not be started for the selected harvesting client configuration (unknown server error). harvest.delete.error=Selected harvesting client cannot be deleted; unknown exception: diff --git a/src/test/java/edu/harvard/iq/dataverse/validation/URLValidatorTest.java b/src/test/java/edu/harvard/iq/dataverse/validation/URLValidatorTest.java index a464e6f549f..3fe8501bbbf 100644 --- a/src/test/java/edu/harvard/iq/dataverse/validation/URLValidatorTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/validation/URLValidatorTest.java @@ -1,56 +1,107 @@ - package edu.harvard.iq.dataverse.validation; -import static org.junit.Assert.assertEquals; +import javax.validation.ConstraintViolation; +import javax.validation.Validation; +import javax.validation.Validator; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; -import javax.validation.ConstraintValidatorContext; +import java.util.Set; +import java.util.stream.Stream; -import edu.harvard.iq.dataverse.validation.URLValidator; -import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorContextImpl; -import org.hibernate.validator.internal.engine.path.PathImpl; -import javax.validation.Validation; -import javax.validation.ValidatorFactory; -import org.junit.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * * @author skraffmi */ public class URLValidatorTest { - ValidatorFactory validatorFactory = Validation.buildDefaultValidatorFactory(); - - @Test - public void testIsURLValid() { - assertEquals(true, URLValidator.isURLValid(null)); - assertEquals(true, URLValidator.isURLValid("")); - assertEquals(true, URLValidator.isURLValid("https://twitter.com/")); - - assertEquals(false, URLValidator.isURLValid("cnn.com")); - + final Validator validator = Validation.buildDefaultValidatorFactory().getValidator(); + + public static Stream stdUrlExamples() { + return Stream.of( + Arguments.of(true, null), + Arguments.of(false, ""), + Arguments.of(true, "https://twitter.com"), + Arguments.of(true, "http://foobar.com:9101"), + Arguments.of(true, "ftp://user@foobar.com"), + Arguments.of(false, "cnn.com"), + Arguments.of(false, "smb://user@foobar.com") + ); } - - @Test - public void testIsValidWithUnspecifiedContext() { - String value = "https://twitter.com/"; - ConstraintValidatorContext context = null; - assertEquals(true, new URLValidator().isValid(value, context)); + + @ParameterizedTest + @MethodSource("stdUrlExamples") + public void testIsURLValid(boolean expected, String url) { + assertEquals(expected, URLValidator.isURLValid(url)); } - - @Test - public void testIsValidWithContextAndValidURL() { - String value = "https://twitter.com/"; - ConstraintValidatorContext context = new ConstraintValidatorContextImpl(validatorFactory.getClockProvider(), PathImpl.createPathFromString(""),null, null); - - assertEquals(true, new URLValidator().isValid(value, context)); + + /** + * This is a simple test class, as we defined the annotation for fields only. + */ + private final class SubjectClass { + @ValidateURL + String url; + + SubjectClass(String url) { + this.url = url; + } } - - @Test - public void testIsValidWithContextButInvalidURL() { - String value = "cnn.com"; - ConstraintValidatorContext context = new ConstraintValidatorContextImpl(validatorFactory.getClockProvider(), PathImpl.createPathFromString(""),null, null); - - assertEquals(false, new URLValidator().isValid(value, context)); + + @ParameterizedTest + @MethodSource("stdUrlExamples") + public void testConstraint(boolean expected, String url) { + // given + URLValidatorTest.SubjectClass sut = new URLValidatorTest.SubjectClass(url); + + //when + Set> violations = validator.validate(sut); + + // then + assertEquals(expected ? 0 : 1, violations.size()); + violations.stream().findFirst().ifPresent( c -> { + assertTrue(c.getMessage().contains(url)); }); + } + + public static Stream fancyUrlExamples() { + return Stream.of( + Arguments.of(true, null), + Arguments.of(false, ""), + Arguments.of(false, "https://twitter.com"), + Arguments.of(true, "http://foobar.com:9101"), + Arguments.of(false, "ftp://user@foobar.com"), + Arguments.of(false, "cnn.com"), + Arguments.of(true, "smb://user@foobar.com") + ); + } + + /** + * This is a simple test class like above, but with a scheme given + */ + private final class SubjectSchemeClass { + @ValidateURL(schemes = {"http", "smb"}) + String url; + + SubjectSchemeClass(String url) { + this.url = url; + } + } + + @ParameterizedTest + @MethodSource("fancyUrlExamples") + public void testConstraintWithSchemes(boolean expected, String url) { + // given + URLValidatorTest.SubjectSchemeClass sut = new URLValidatorTest.SubjectSchemeClass(url); + + //when + Set> violations = validator.validate(sut); + + // then + assertEquals(expected ? 0 : 1, violations.size()); + violations.stream().findFirst().ifPresent( c -> { + assertTrue(c.getMessage().contains(url)); }); } - } From 85dcbbf999fb6633c035e7d47a6c7e432f38cafd Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 24 Mar 2022 23:43:16 +0100 Subject: [PATCH 7/9] refactor(validation): cleanup UserNameValidator #8531 - Reuse existing constraints instead of creating our own validator - Provide programmatic method to use the same from code like OAuth2 etc - Add much more tests, also for actual bean validation --- src/main/java/ValidationMessages.properties | 2 +- .../providers/builtin/BuiltinUser.java | 8 +- .../validation/UserNameValidator.java | 75 +++++------ .../validation/ValidateUserName.java | 22 +++- .../validation/UserNameValidatorTest.java | 124 +++++++++--------- 5 files changed, 114 insertions(+), 117 deletions(-) diff --git a/src/main/java/ValidationMessages.properties b/src/main/java/ValidationMessages.properties index 651d86b7290..7245c0841c7 100644 --- a/src/main/java/ValidationMessages.properties +++ b/src/main/java/ValidationMessages.properties @@ -2,7 +2,7 @@ user.firstName=Please enter your first name. user.lastName=Please enter your last name. user.invalidEmail=Please enter a valid email address. user.enterUsername=Please enter a username. -user.usernameLength=Username must be between 2 and 60 characters. +user.usernameLength=Username must be between {min} and {max} characters. user.illegalCharacters=Found an illegal character(s). Valid characters are a-Z, 0-9, '_', '-', and '.'. user.enterNickname=Please enter a nickname. diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java index 99badc27974..c2510b8b043 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java @@ -17,8 +17,8 @@ import javax.persistence.OneToOne; import javax.persistence.Table; import javax.persistence.Transient; +import javax.validation.constraints.NotBlank; import javax.validation.constraints.Size; -import org.hibernate.validator.constraints.NotBlank; /** * @@ -41,10 +41,8 @@ public class BuiltinUser implements Serializable { @Id @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; - - @NotBlank(message = "{user.enterUsername}") - @Size(min=2, max=60, message = "{user.usernameLength}") - @ValidateUserName(message = "{user.illegalCharacters}") + + @ValidateUserName @Column(nullable = false, unique=true) private String userName; diff --git a/src/main/java/edu/harvard/iq/dataverse/validation/UserNameValidator.java b/src/main/java/edu/harvard/iq/dataverse/validation/UserNameValidator.java index d15e41f56e4..873512299ea 100644 --- a/src/main/java/edu/harvard/iq/dataverse/validation/UserNameValidator.java +++ b/src/main/java/edu/harvard/iq/dataverse/validation/UserNameValidator.java @@ -1,61 +1,46 @@ -/* - * To change this license header, choose License Headers in Project Properties. - * To change this template file, choose Tools | Templates - * and open the template in the editor. - */ package edu.harvard.iq.dataverse.validation; import java.util.regex.Matcher; import java.util.regex.Pattern; -import javax.validation.ConstraintValidator; -import javax.validation.ConstraintValidatorContext; /** - * - * @author sarahferry - * Modeled after PasswordValidator and EMailValidator + * This class is not implementing the ConstraintValidatorMatcher interface, as this is not necessary any more. + * It serves as the storage for the annotation configuration and convenient interface to a programmatic + * validation of usernames. */ - -public class UserNameValidator implements ConstraintValidator { - @Override - public void initialize(ValidateUserName constraintAnnotation) { - - } +public class UserNameValidator { - // note: while the ConstraintValidatorContext is not used in this method, - // it is required in order to impelement the ConstraintValidator interface - @Override - public boolean isValid(String value, ConstraintValidatorContext context) { - return isUserNameValid(value); - } + public static final int MIN_CHARS = 2; + public static final int MAX_CHARS = 60; + + // NOTE: the size is checked by either the @Size annotation of @ValidateUserName or programmatically below! + public static final String USERNAME_PATTERN = "[a-zA-Z0-9\\_\\-\\.]*"; + + /* + * If you would like to support accents or chinese characters in usernames, choose one of the below. + * + * With support for accents: + private static final String USERNAME_PATTERN = "[a-zA-Z0-9\\_\\-\\.À-ÿ\\u00C0-\\u017F]"; + * + * With support for chinese characters: + private static final String USERNAME_PATTERN = "[a-zA-Z0-9\\_\\-\\.\\x{4e00}-\\x{9fa5}]"; + */ + + private static final Matcher usernameMatcher = Pattern.compile(USERNAME_PATTERN).matcher(""); /** - * Here we will validate the username + * Validate a username against the pattern in {@link #USERNAME_PATTERN} + * and check for length: min {@link #MIN_CHARS} chars, max {@link #MAX_CHARS} chars. + * Nullsafe - null is considered invalid (as is empty). * - * @param username - * @return boolean + * @param username The username to validate + * @return true if matching, false otherwise */ public static boolean isUserNameValid(final String username) { - if (username == null) { - return false; - } - //TODO: What other characters do we need to support? - String validCharacters = "[a-zA-Z0-9\\_\\-\\."; - /* - * if you would like to support accents or chinese characters, uncomment this - * - //support accents - validCharacters += "À-ÿ\\u00C0-\\u017F"; - //support chinese characters - validCharacters += "\\x{4e00}-\\x{9fa5}"; - * - */ - //end - validCharacters += "]"; - validCharacters += "{2,60}"; //must be between 2 and 60 characters for user name - Pattern p = Pattern.compile(validCharacters); - Matcher m = p.matcher(username); - return m.matches(); + return username != null && + username.length() >= MIN_CHARS && + username.length() <= MAX_CHARS && + usernameMatcher.reset(username).matches(); } } diff --git a/src/main/java/edu/harvard/iq/dataverse/validation/ValidateUserName.java b/src/main/java/edu/harvard/iq/dataverse/validation/ValidateUserName.java index b3c46c63fdc..0583b70df49 100644 --- a/src/main/java/edu/harvard/iq/dataverse/validation/ValidateUserName.java +++ b/src/main/java/edu/harvard/iq/dataverse/validation/ValidateUserName.java @@ -12,6 +12,9 @@ import java.lang.annotation.Target; import javax.validation.Constraint; import javax.validation.Payload; +import javax.validation.constraints.NotBlank; +import javax.validation.constraints.Size; +import javax.validation.constraints.Pattern; /** * @@ -20,14 +23,19 @@ @Target({FIELD}) @Retention(RUNTIME) -@Constraint(validatedBy = {UserNameValidator.class}) -@Documented -public @interface ValidateUserName { - String message() default "Failed Validation Username"; +// This is empty by intention - we are just recombining existing bean validation here. +// The class UserNameValidator is just for convenience and historic reasons. +@Constraint(validatedBy = {}) - Class[] groups() default {}; +@NotBlank(message = "{user.enterUsername}") +@Size(min = UserNameValidator.MIN_CHARS, max = UserNameValidator.MAX_CHARS, message = "{user.usernameLength}") +@Pattern(regexp = UserNameValidator.USERNAME_PATTERN, message = "{user.illegalCharacters}") - Class[] payload() default {}; +@Documented +public @interface ValidateUserName { + String message() default ""; + Class[] groups() default {}; + + Class[] payload() default {}; } - diff --git a/src/test/java/edu/harvard/iq/dataverse/validation/UserNameValidatorTest.java b/src/test/java/edu/harvard/iq/dataverse/validation/UserNameValidatorTest.java index 2602556a9fb..a9816f81dca 100644 --- a/src/test/java/edu/harvard/iq/dataverse/validation/UserNameValidatorTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/validation/UserNameValidatorTest.java @@ -1,76 +1,82 @@ -/* - * To change this license header, choose License Headers in Project Properties. - * To change this template file, choose Tools | Templates - * and open the template in the editor. - */ package edu.harvard.iq.dataverse.validation; -import static org.junit.Assert.assertEquals; +import java.util.Set; +import java.util.stream.Stream; -import java.util.Arrays; -import java.util.Collection; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; -import edu.harvard.iq.dataverse.validation.UserNameValidator; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameters; +import javax.validation.ConstraintViolation; +import javax.validation.Validation; +import javax.validation.Validator; -/** - * - * @author sarahferry - * @author alexscheitlin - */ -@RunWith(Parameterized.class) -public class UserNameValidatorTest { - - public boolean isValid; - public String userName; - - public UserNameValidatorTest(boolean isValid, String userName) { - this.isValid = isValid; - this.userName = userName; - } +import static org.junit.jupiter.api.Assertions.assertEquals; - @Parameters - public static Collection parameters() { - return Arrays.asList(new Object[][] { +public class UserNameValidatorTest { + + public static Stream usernameExamples() { + return Stream.of( // good usernames - { true, "sarah" }, - { true, ".-_5Arah_-." }, - + Arguments.of(true, "sarah"), + Arguments.of(true, ".-_5Arah_-."), // dont allow accents - { false, "àèìòùÀÈÌÒÙáéíóúýÁÉÍÓÚÝâêîôûÂÊÎÔÛãñõÃÑÕäëïöüÿÄËÏÖÜŸçÇßØøÅåÆæœ" }, - + Arguments.of(false, "àèìòùÀÈÌÒÙáéíóúýÁÉÍÓÚÝâêîôûÂÊÎÔÛãñõÃÑÕäëïöüÿÄËÏÖÜŸçÇßØøÅåÆæœ"), // dont allow chinese characters - { false, "谁日吧爸好" }, - + Arguments.of(false, "谁日吧爸好"), // dont allow middle white space - { false, "sarah f" }, - + Arguments.of(false, "sarah f"), // dont allow leading white space - { false, " sarah" }, - + Arguments.of(false, " sarah"), // dont allow trailing white space - { false, "sarah " }, - + Arguments.of(false, "sarah "), // dont allow symbols - { false, "sarah!" }, - { false, "sarah?" }, - { false, "sarah:(" }, - { false, "💲🅰️®️🅰️🚧" }, - + Arguments.of(false, "sarah!"), + Arguments.of(false, "sarah?"), + Arguments.of(false, "sarah:("), + Arguments.of(false, "💲🅰️®️🅰️🚧"), // only allow between 2 and 60 characters - { false, "q" }, - { true, "q2" }, - { false, "q2jsalfhjopiwurtiosfhkdhasjkdhfgkfhkfrhnefcn4cqonroclmooi4oiqwhrfq4jrlqhaskdalwehrlwhflhklasdjfglq0kkajfelirhilwhakjgv" }, - { false, "" }, - { false, null } - }); + Arguments.of(false, "q"), + Arguments.of(true, "q2"), + Arguments.of(false, "q2jsalfhjopiwurtiosfhkdhasjkdhfgkfhkfrhnefcn4cqonroclmooi4oiqwhrfq4jrlqhaskdalwehrlwhflhklasdjfglq0kkajfelirhilwhakjgv"), + Arguments.of(false, "q2jsalfhjopiwurtiosfhkdhasj!1! !#fhkfrhnefcn4cqonroclmooi4oiqwhrfq4jrlqhaskdalwehrlwhflhklasdjfglq0kkajfelirhilwhakjgv"), + Arguments.of(false, ""), + Arguments.of(false, null) + ); } - - @Test - public void testIsUserNameValid() { - assertEquals(isValid, UserNameValidator.isUserNameValid(userName)); + + @ParameterizedTest + @MethodSource("usernameExamples") + public void testUsernames(boolean expected, String username) { + assertEquals(expected, UserNameValidator.isUserNameValid(username)); + } + + /** + * This is a simple test class, as we defined the annotation for fields only. + */ + private final class SubjectClass { + @ValidateUserName + String username; + + SubjectClass(String username) { + this.username = username; + } + } + + private final Validator validator = Validation.buildDefaultValidatorFactory().getValidator(); + + @ParameterizedTest + @MethodSource("usernameExamples") + public void testConstraint(boolean expected, String username) { + // given + UserNameValidatorTest.SubjectClass sut = new UserNameValidatorTest.SubjectClass(username); + + //when + Set> violations = validator.validate(sut); + + // then + assertEquals(expected, violations.size() < 1); + // TODO: one might add tests for the two violations (size and illegal chars) here... } + } From 9589d3f42adf385f75755f252768a02c9866a72c Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 24 Mar 2022 23:56:56 +0100 Subject: [PATCH 8/9] refactor(validation): cleanup DatasetFieldValueValidator URL validation #8531 --- .../dataverse/DatasetFieldValueValidator.java | 16 ++--- .../DatasetFieldValueValidatorTest.java | 63 ++++++++++--------- 2 files changed, 38 insertions(+), 41 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java b/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java index 7cd823211bf..8b807f78bca 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java @@ -17,6 +17,7 @@ import javax.validation.ConstraintValidatorContext; import edu.harvard.iq.dataverse.validation.EMailValidator; +import edu.harvard.iq.dataverse.validation.URLValidator; import org.apache.commons.lang3.StringUtils; import org.apache.commons.validator.routines.UrlValidator; @@ -167,20 +168,11 @@ public boolean isValid(DatasetFieldValue value, ConstraintValidatorContext conte // Note, length validation for FieldType.TEXT was removed to accommodate migrated data that is greater than 255 chars. if (fieldType.equals(FieldType.URL) && !lengthOnly) { - - String[] schemes = {"http","https", "ftp"}; - UrlValidator urlValidator = new UrlValidator(schemes); - - try { - if (urlValidator.isValid(value.getValue())) { - } else { - context.buildConstraintViolationWithTemplate(dsfType.getDisplayName() + " " + value.getValue() + " is not a valid URL.").addConstraintViolation(); - return false; - } - } catch (NullPointerException npe) { + boolean isValidUrl = URLValidator.isURLValid(value.getValue()); + if (!isValidUrl) { + context.buildConstraintViolationWithTemplate(dsfType.getDisplayName() + " " + value.getValue() + " {url.invalid}").addConstraintViolation(); return false; } - } if (fieldType.equals(FieldType.EMAIL) && !lengthOnly) { diff --git a/src/test/java/edu/harvard/iq/dataverse/DatasetFieldValueValidatorTest.java b/src/test/java/edu/harvard/iq/dataverse/DatasetFieldValueValidatorTest.java index a637848851e..ceaa69ade4e 100644 --- a/src/test/java/edu/harvard/iq/dataverse/DatasetFieldValueValidatorTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/DatasetFieldValueValidatorTest.java @@ -13,6 +13,8 @@ import javax.validation.Validator; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.mockito.Mockito; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -107,33 +109,6 @@ public void testIsValid() { value.setValue("12.14"); result = instance.isValid(value, ctx); assertEquals(false, result); - - //URL - dft.setFieldType(DatasetFieldType.FieldType.URL); - value.setValue("https://www.google.com"); - result = instance.isValid(value, ctx); - assertEquals(true, result); - - value.setValue("http://google.com"); - result = instance.isValid(value, ctx); - assertEquals(true, result); - - value.setValue("https://do-not-exist-123-123.com/"); // does not exist - result = instance.isValid(value, ctx); - assertEquals(true, result); - - value.setValue("ftp://somesite.com"); - result = instance.isValid(value, ctx); - assertEquals(true, result); - - value.setValue("google.com"); - result = instance.isValid(value, ctx); - assertEquals(false, result); - - value.setValue("git@github.com:IQSS/dataverse.git"); - result = instance.isValid(value, ctx); - assertEquals(false, result); - } @Test @@ -184,6 +159,38 @@ public void testIsValidAuthorIdentifierGnd() { final Validator validator = Validation.buildDefaultValidatorFactory().getValidator(); + @ParameterizedTest + @CsvSource( + { + "true, https://www.google.com", + "true, http://google.com", + "true, https://do-not-exist-123-123.com/", + "true, ftp://somesite.com", + "false, google.com", + "false, git@github.com:IQSS/dataverse.git" + } + ) + public void testInvalidURL(boolean expected, String url) { + // given + String fieldName = "testField"; + + DatasetField field = new DatasetField(); + field.setDatasetFieldType(new DatasetFieldType(fieldName, DatasetFieldType.FieldType.URL, false)); + DatasetFieldValue sut = new DatasetFieldValue(field); + sut.setValue(url); + + // when + Set> violations = validator.validate(sut); + + // then + assertEquals(expected, violations.size() < 1); + violations.stream().findFirst().ifPresent(c -> { + assertTrue(c.getMessage().startsWith(fieldName + " " + url + " ")); + assertTrue(c.getMessage().contains("not")); + assertTrue(c.getMessage().contains("URL")); + }); + } + @Test public void testInvalidEmail() { // given @@ -205,7 +212,5 @@ public void testInvalidEmail() { assertTrue(c.getMessage().contains("not")); assertTrue(c.getMessage().contains("email")); }); - } - } From 43126e17ddc366ce4db8cb0c26f40949ae1ab6b3 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 25 Mar 2022 13:47:51 +0100 Subject: [PATCH 9/9] fix(groups): replace invalid Hibernate CollectionHelper usage with JDK set generator #8531 --- .../groups/impl/builtin/BuiltInGroupsProvider.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/groups/impl/builtin/BuiltInGroupsProvider.java b/src/main/java/edu/harvard/iq/dataverse/authorization/groups/impl/builtin/BuiltInGroupsProvider.java index af9ab080443..d67d3caa24c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/groups/impl/builtin/BuiltInGroupsProvider.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/groups/impl/builtin/BuiltInGroupsProvider.java @@ -9,7 +9,6 @@ import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import java.util.Collections; import java.util.Set; -import org.hibernate.validator.internal.util.CollectionHelper; /** * Provider for the built-in, hard coded groups. This class is a singleton (no @@ -56,7 +55,7 @@ public Set groupsFor(DataverseRequest req) { @Override public Set groupsFor(RoleAssignee ra) { if (ra instanceof AuthenticatedUser){ - return CollectionHelper.asSet(AllUsers.get(), AuthenticatedUsers.get()); + return Set.of(AllUsers.get(), AuthenticatedUsers.get()); } else if ( ra instanceof User) { return Collections.singleton(AllUsers.get()); } else { @@ -72,6 +71,6 @@ public Group get(String groupAlias) { @Override public Set findGlobalGroups() { - return CollectionHelper.asSet(AllUsers.get(), AuthenticatedUsers.get()); + return Set.of(AllUsers.get(), AuthenticatedUsers.get()); } }