Added name allowed character validation #1314

- changed model validation, inspect now for illegal characters accepted only now: `a-z`,`0-9`,`-` or `_` - refactored existing parts from model validation: moved parts from abstract simple string validation into simple string utils and reused implementation - added some missing SPDX headers to sources - added test scases
mercedes-benz · May 6, 2022 · 33c5e55 · 33c5e55
1 parent f453048
commit 33c5e55
Show file tree

Hide file tree

Showing 10 changed files with 142 additions and 30 deletions.
diff --git a/...mmons-core/src/main/java/com/mercedesbenz/sechub/commons/core/util/SimpleStringUtils.java b/...mmons-core/src/main/java/com/mercedesbenz/sechub/commons/core/util/SimpleStringUtils.java
@@ -101,4 +101,43 @@ public static String truncateWhenTooLong(String string, int maxLength) {
         }
         return string.substring(0, maxLength - 3) + "...";
     }
+
+    /**
+     * Will test if given string does contain only alphabetic characters (a-z,A-Z),
+     * digits (0-9) or additionally allowed characters.
+     *
+     * @param string            the content to inspect.
+     * @param additionalAllowed
+     * @return <code>true</code> when only allowed characters are contained, or
+     *         given string is <code>null</code> or empty. <code>false</code>
+     *         otherwise.
+     */
+    public static boolean hasOnlyAlphabeticDigitOrAdditionalAllowedCharacters(String string, char... additionalAllowed) {
+        if (string == null) {
+            return true;
+        }
+        if (string.isEmpty()) {
+            return true;
+        }
+        for (char c : string.toCharArray()) {
+            if (Character.isDigit(c)) {
+                continue;
+            }
+            if (Character.isAlphabetic(c)) {
+                continue;
+            }
+            boolean ok = false;
+            for (char allowed : additionalAllowed) {
+                if (c == allowed) {
+                    ok = true;
+                    continue;
+                }
+            }
+            if (ok) {
+                continue;
+            }
+            return false;
+        }
+        return true;
+    }
 }
diff --git a/...s-core/src/test/java/com/mercedesbenz/sechub/commons/core/util/SimpleStringUtilsTest.java b/...s-core/src/test/java/com/mercedesbenz/sechub/commons/core/util/SimpleStringUtilsTest.java
@@ -5,9 +5,35 @@
 import static org.hamcrest.MatcherAssert.*;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 
 public class SimpleStringUtilsTest {
 
+    @ParameterizedTest
+    @CsvSource({ "name", "age", "test" })
+    void validNames_hasOnlyAlphabeticDigitOrAdditionalAllowedCharacters_no_additional_is_true(String string) {
+        assertThat(SimpleStringUtils.hasOnlyAlphabeticDigitOrAdditionalAllowedCharacters(string), is(true));
+    }
+
+    @ParameterizedTest
+    @CsvSource({ "name-", "age_", "test-1" })
+    void invalidNames_hasOnlyAlphabeticDigitOrAdditionalAllowedCharacters_no_additional_is_false(String string) {
+        assertThat(SimpleStringUtils.hasOnlyAlphabeticDigitOrAdditionalAllowedCharacters(string), is(false));
+    }
+
+    @ParameterizedTest
+    @CsvSource({ "name-", "age_", "test-1" })
+    void validNames_hasOnlyAlphabeticDigitOrAdditionalAllowedCharacters_with_additional_is_true(String string) {
+        assertThat(SimpleStringUtils.hasOnlyAlphabeticDigitOrAdditionalAllowedCharacters(string, '-', '_'), is(true));
+    }
+
+    @ParameterizedTest
+    @CsvSource({ "n$me-", "a@e_", "t§st-1" })
+    void invalidNames_hasOnlyAlphabeticDigitOrAdditionalAllowedCharacters_with_additional_is_false(String string) {
+        assertThat(SimpleStringUtils.hasOnlyAlphabeticDigitOrAdditionalAllowedCharacters(string, '-', '_'), is(false));
+    }
+
     @Test
     void isTrimmedEquals_null_null_is_true() {
         assertThat(SimpleStringUtils.isTrimmedEqual(null, null), is(true));

diff --git a/...n/java/com/mercedesbenz/sechub/commons/model/SecHubConfigurationModelValidationError.java b/...n/java/com/mercedesbenz/sechub/commons/model/SecHubConfigurationModelValidationError.java
@@ -27,6 +27,8 @@ public enum SecHubConfigurationModelValidationError {
 
     REFERENCED_DATA_CONFIG_OBJECT_NAME_NOT_EXISTING("The referenced config object name was not found in object!"),
 
+    DATA_CONFIG_OBJECT_NAME_CONTAINS_ILLEGAL_CHARACTERS("Data configuration object contains illegal characters!"),
+
     ;
 
     private String defaultMessage;

diff --git a/...rc/main/java/com/mercedesbenz/sechub/commons/model/SecHubConfigurationModelValidator.java b/...rc/main/java/com/mercedesbenz/sechub/commons/model/SecHubConfigurationModelValidator.java
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: MIT
 package com.mercedesbenz.sechub.commons.model;
 
+import static com.mercedesbenz.sechub.commons.core.util.SimpleStringUtils.*;
 import static com.mercedesbenz.sechub.commons.model.SecHubConfigurationModelValidationError.*;
 
 import java.net.URI;
@@ -161,6 +162,12 @@ private void validateNameUniqueAndNotNull(InternalValidationContext context, Col
                 result.addError(DATA_CONFIG_OBJECT_NAME_IS_NULL);
                 continue;
             }
+            if (!hasOnlyAlphabeticDigitOrAdditionalAllowedCharacters(uniqueName, '-', '_')) {
+                result.addError(DATA_CONFIG_OBJECT_NAME_CONTAINS_ILLEGAL_CHARACTERS,
+                        "Name '" + uniqueName + "' may only contain 'a-z','0-9', '-' or '_' characters");
+                continue;
+            }
+
             if (uniqueName.length() < MIN_NAME_LENGTH) {
                 result.addError(DATA_CONFIG_OBJECT_NAME_LENGTH_TOO_SHORT, "Name '" + uniqueName + "' lengh < " + MIN_NAME_LENGTH + " characters");
             }

diff --git a/...el/src/main/java/com/mercedesbenz/sechub/commons/model/SecHubWebScanApiConfiguration.java b/...el/src/main/java/com/mercedesbenz/sechub/commons/model/SecHubWebScanApiConfiguration.java
@@ -6,20 +6,19 @@
 
 public class SecHubWebScanApiConfiguration implements SecHubDataConfigurationUsageByName {
 
-
     public static final String PROPERTY_TYPE = "type";
     public static final String PROPERTY_LOGIN = "use";
-    
+
     private SecHubWebScanApiType type;
-    
+
     public SecHubWebScanApiType getType() {
         return type;
     }
-    
+
     public void setType(SecHubWebScanApiType type) {
         this.type = type;
     }
-    
+
     private Set<String> namesOfUsedDataConfigurationObjects = new LinkedHashSet<>();
 
     @Override

diff --git a/...mmons-model/src/main/java/com/mercedesbenz/sechub/commons/model/SecHubWebScanApiType.java b/...mmons-model/src/main/java/com/mercedesbenz/sechub/commons/model/SecHubWebScanApiType.java
@@ -1,10 +1,11 @@
+// SPDX-License-Identifier: MIT
 package com.mercedesbenz.sechub.commons.model;
 
 import com.fasterxml.jackson.annotation.JsonAlias;
 
 public enum SecHubWebScanApiType {
 
-    @JsonAlias({ "openApi"})
+    @JsonAlias({ "openApi" })
     OPEN_API;
 
 }
diff --git a/...model/src/main/java/com/mercedesbenz/sechub/commons/model/SecHubWebScanConfiguration.java b/...model/src/main/java/com/mercedesbenz/sechub/commons/model/SecHubWebScanConfiguration.java
@@ -19,7 +19,7 @@ public class SecHubWebScanConfiguration {
     Optional<WebScanDurationConfiguration> maxScanDuration = Optional.empty();
 
     Optional<SecHubWebScanApiConfiguration> api = Optional.empty();
-    
+
     URI uri;
 
     Optional<List<String>> includes = Optional.empty();
@@ -28,8 +28,8 @@ public class SecHubWebScanConfiguration {
     public URI getUri() {
         return uri;
     }
-    
-    public Optional<SecHubWebScanApiConfiguration> getApi(){
+
+    public Optional<SecHubWebScanApiConfiguration> getApi() {
         return api;
     }
 

diff --git a/...est/java/com/mercedesbenz/sechub/commons/model/SecHubConfigurationModelValidatorTest.java b/...est/java/com/mercedesbenz/sechub/commons/model/SecHubConfigurationModelValidatorTest.java
@@ -10,12 +10,16 @@
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 
 import com.mercedesbenz.sechub.commons.model.SecHubConfigurationModelValidationResult.SecHubConfigurationModelValidationErrorData;
 import com.mercedesbenz.sechub.commons.model.SecHubConfigurationModelValidator.SecHubConfigurationModelValidationException;
 
 class SecHubConfigurationModelValidatorTest {
 
+    private static final String VALID_NAME_WITH_MAX_LENGTH = "---------1---------2---------3---------4---------5---------6---------7---------8";
+    private static final String VALID_NAME_BUT_ONE_CHAR_TOO_LONG = VALID_NAME_WITH_MAX_LENGTH + "-";
     private SecHubConfigurationModelValidator validatorToTest;
 
     @BeforeEach
@@ -184,6 +188,45 @@ void api_version_unsupported_results_in_one_error() {
 
     }
 
+    @ParameterizedTest
+    @CsvSource({ "n", "_", "-", "1", "name1", "referenced-name_", "the_name_with_slashes", VALID_NAME_WITH_MAX_LENGTH })
+    void model_having_a_data_configuration_with_valid_name_has_no_error(String name) {
+        /* prepare */
+        /* prepare */
+        SecHubConfigurationModel model = createDefaultValidModel();
+        SecHubDataConfiguration data = new SecHubDataConfiguration();
+
+        SecHubSourceDataConfiguration config1 = new SecHubSourceDataConfiguration();
+        config1.setUniqueName(name);
+        data.getSources().add(config1);
+        model.setData(data);
+
+        /* execute + test */
+        SecHubConfigurationModelValidationResult result = validatorToTest.validate(model);
+
+        /* test */
+        assertFalse(result.hasErrors());
+    }
+
+    @ParameterizedTest
+    @CsvSource({ "$", "n$me-", "a@e_", "t§st-1", "config-object-name$1", VALID_NAME_BUT_ONE_CHAR_TOO_LONG })
+    void model_having_a_data_configuration_with_invalid_name_has_error(String name) {
+        /* prepare */
+        SecHubConfigurationModel model = createDefaultValidModel();
+        SecHubDataConfiguration data = new SecHubDataConfiguration();
+
+        SecHubSourceDataConfiguration config1 = new SecHubSourceDataConfiguration();
+        config1.setUniqueName(name);
+        data.getSources().add(config1);
+        model.setData(data);
+
+        /* execute + test */
+        SecHubConfigurationModelValidationResult result = validatorToTest.validate(model);
+
+        /* test */
+        assertTrue(result.hasErrors());
+    }
+
     @Test
     void model_having_a_code_scan_configuration_which_references_a_wellknown_data_object_results_in_no_error() {
         /* prepare */

diff --git a/...mercedesbenz/sechub/integrationtest/api/AbstractHttpStatusCodeExceptionTestValidator.java b/...mercedesbenz/sechub/integrationtest/api/AbstractHttpStatusCodeExceptionTestValidator.java
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: MIT
 package com.mercedesbenz.sechub.integrationtest.api;
 
 import static org.junit.Assert.*;

diff --git a/.../java/com/mercedesbenz/sechub/sharedkernel/validation/AbstractSimpleStringValidation.java b/.../java/com/mercedesbenz/sechub/sharedkernel/validation/AbstractSimpleStringValidation.java
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: MIT
 package com.mercedesbenz.sechub.sharedkernel.validation;
 
+import static com.mercedesbenz.sechub.commons.core.util.SimpleStringUtils.*;
+
 public abstract class AbstractSimpleStringValidation extends AbstractValidation<String> {
 
     protected void validateNoUpperCaseCharacters(ValidationContext<String> context) {
@@ -37,32 +39,24 @@ final protected void validateWithoutWhitespaces(ValidationContext<String> contex
 
     protected void validateOnlyAlphabeticDigitOrAllowedParts(ValidationContext<String> context, char... alsoAllowed) {
         String string = context.objectToValidate;
-        if (string == null) {
-            return;
-        }
-        if (string.isEmpty()) {
+        if (hasOnlyAlphabeticDigitOrAdditionalAllowedCharacters(string, alsoAllowed)) {
             return;
         }
-        for (char c : string.toCharArray()) {
-            if (Character.isDigit(c)) {
-                continue;
-            }
-            if (Character.isAlphabetic(c)) {
-                continue;
-            }
-            boolean ok = false;
-            for (char allowed : alsoAllowed) {
-                if (c == allowed) {
-                    ok = true;
-                    continue;
+        StringBuilder sb = new StringBuilder();
+        sb.append("The string '");
+        sb.append(string);
+        sb.append("' contains at least one character being neither a digit, alphebtical");
+        if (alsoAllowed.length > 0) {
+            sb.append("or ");
+            for (int i = 0; i < alsoAllowed.length; i++) {
+                if (i != 0) {
+                    sb.append(',');
                 }
+                sb.append(alsoAllowed[i]);
             }
-            if (ok) {
-                continue;
-            }
-            addErrorMessage(context, "Character must be one of alloweds, but found '" + c + "'.");
-            return;
+            sb.append("]");
         }
+        addErrorMessage(context, sb.toString());
     }
 
     protected void validateNotContainingCharackters(ValidationContext<String> context, char... chars) {