feat(impl): [#528] relax validation of policyId to safe path paramete…

…r characters New pattern "[a-zA-Z0-9\-_~.:]+" (safe path variable characters). Reason: Avoid problems with too strict validation.
eclipse-tractusx · May 13, 2024 · d13dfb5 · d13dfb5
1 parent cc5ca9b
commit d13dfb5
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 27 deletions.
diff --git a/...c/main/java/org/eclipse/tractusx/irs/policystore/validators/ListOfPolicyIdsValidator.java b/...c/main/java/org/eclipse/tractusx/irs/policystore/validators/ListOfPolicyIdsValidator.java
@@ -34,7 +34,8 @@ public class ListOfPolicyIdsValidator implements ConstraintValidator<ListOfPolic
     @Override
     public boolean isValid(final List<String> value, final ConstraintValidatorContext context) {
 
-        // allow null and empty here (in order to allow flexible combination with @NotNull and @NotEmpty)
+        // allow null and empty here
+        // (in order to allow flexible combination with @NotNull and @NotEmpty)
         if (value == null || value.isEmpty()) {
             return true;
         }
@@ -49,17 +50,17 @@ public boolean isValid(final List<String> value, final ConstraintValidatorContex
         for (int index = 0; index < value.size(); index++) {
             if (!PolicyIdValidator.isValid(value.get(index))) {
                 context.disableDefaultConstraintViolation();
-                final String msg = "The policyId at index %d is invalid (must be a valid UUID)";
-                context.buildConstraintViolationWithTemplate(msg.formatted(index)).addConstraintViolation();
+                final String msg = "The policyId at index %d is invalid (%s)";
+                context.buildConstraintViolationWithTemplate(msg.formatted(index, ValidPolicyId.DEFAULT_MESSAGE))
+                       .addConstraintViolation();
                 return false;
             }
         }
 
         return true;
     }
 
-    /* package */
-    static boolean containsDuplicates(final List<String> strings) {
+    protected static boolean containsDuplicates(final List<String> strings) {
         final Set<String> set = new HashSet<>(strings);
         return set.size() < strings.size();
     }

diff --git a/...tore/src/main/java/org/eclipse/tractusx/irs/policystore/validators/PolicyIdValidator.java b/...tore/src/main/java/org/eclipse/tractusx/irs/policystore/validators/PolicyIdValidator.java
@@ -19,17 +19,18 @@
  ********************************************************************************/
 package org.eclipse.tractusx.irs.policystore.validators;
 
-import java.util.UUID;
+import java.util.regex.Pattern;
 
 import jakarta.validation.ConstraintValidator;
 import jakarta.validation.ConstraintValidatorContext;
-import org.apache.commons.lang3.StringUtils;
 
 /**
  * Validator for list of business partner numbers (BPN).
  */
 public class PolicyIdValidator implements ConstraintValidator<ValidPolicyId, String> {
 
+    private static final Pattern PATTERN_SAFE_PATH_VARIABLE_CHARACTERS = Pattern.compile("[a-zA-Z0-9\\-_~.:]+");
+
     @Override
     public boolean isValid(final String value, final ConstraintValidatorContext context) {
 
@@ -40,15 +41,7 @@ public boolean isValid(final String value, final ConstraintValidatorContext cont
     }
 
     public static boolean isValid(final String policyId) {
-        return validateUUID(policyId);
+        return PATTERN_SAFE_PATH_VARIABLE_CHARACTERS.matcher(policyId).matches();
     }
 
-    private static boolean validateUUID(final String uuidStr) {
-        try {
-            UUID.fromString(StringUtils.removeStart(uuidStr, "urn:uuid:"));
-            return true;
-        } catch (IllegalArgumentException e) {
-            return false;
-        }
-    }
 }
diff --git a/...cy-store/src/main/java/org/eclipse/tractusx/irs/policystore/validators/ValidPolicyId.java b/...cy-store/src/main/java/org/eclipse/tractusx/irs/policystore/validators/ValidPolicyId.java
@@ -39,7 +39,9 @@
 @Retention(RetentionPolicy.RUNTIME)
 public @interface ValidPolicyId {
 
-    String message() default "must be a valid UUID";
+    String DEFAULT_MESSAGE = "must only contain safe URL path variable characters";
+
+    String message() default DEFAULT_MESSAGE;
 
     Class<?>[] groups() default { };
 

diff --git a/.../test/java/org/eclipse/tractusx/irs/policystore/validators/PolicyIdListValidatorTest.java b/.../test/java/org/eclipse/tractusx/irs/policystore/validators/PolicyIdListValidatorTest.java
@@ -68,7 +68,7 @@ void withValidListOfStrings() {
 
     @Test
     void withListContainingInvalidPolicyId() {
-        List<String> invalidList = Arrays.asList(UUID.randomUUID().toString(), "_INVALID_POLICY_ID_");
+        List<String> invalidList = Arrays.asList(UUID.randomUUID().toString(), "###INVALID_POLICY_ID###");
         assertThat(validator.isValid(invalidList, contextMock)).isFalse();
         verify(contextMock).buildConstraintViolationWithTemplate(messageCaptor.capture());
         assertThat(messageCaptor.getValue()).contains("policyId").contains(" index 1 ").contains("invalid");

diff --git a/.../src/test/java/org/eclipse/tractusx/irs/policystore/validators/PolicyIdValidatorTest.java b/.../src/test/java/org/eclipse/tractusx/irs/policystore/validators/PolicyIdValidatorTest.java
@@ -79,8 +79,7 @@ private static Stream<String> uuidStream() {
     }
 
     @ParameterizedTest
-    @ValueSource(strings = { "a_",
-                             "*",
+    @ValueSource(strings = { "*",
                              "(",
                              ")",
                              "<",
@@ -89,11 +88,10 @@ private static Stream<String> uuidStream() {
                              "]",
                              "{",
                              "}",
-                             "p3",
-                             "123",
-                             "abc",
-                             "abc123",
-                             "abc-def-4444"
+                             "a/policyId",
+                             "a\\policyId",
+                             "my?policyId",
+                             "my#policyId"
     })
     void withInvalidPolicyId(final String invalidPolicyId) {
         assertThat(validator.isValid(invalidPolicyId, contextMock)).isFalse();

diff --git a/...re/src/test/java/org/eclipse/tractusx/irs/policystore/validators/PolicyValidatorTest.java b/...re/src/test/java/org/eclipse/tractusx/irs/policystore/validators/PolicyValidatorTest.java
@@ -36,10 +36,11 @@ class PolicyValidatorTest {
 
     @Test
     void invalidPolicyId() {
-        final Policy policy = Policy.builder().policyId("_invalid_policy_id_").permissions(createPermissions()).build();
+        final Policy policy = Policy.builder().policyId("?invalid_policy_id#").permissions(createPermissions()).build();
         assertThatThrownBy(() -> PolicyValidator.validate(policy)).isInstanceOf(ConstraintViolationException.class)
                                                                   .hasMessageContaining("policyId")
-                                                                  .hasMessageContaining("must be a valid UUID");
+                                                                  .hasMessageContaining(
+                                                                          "must only contain safe URL path variable characters");
     }
 
     private List<Permission> createPermissions() {