Improve paths validation and creation in certServiceClient, improve coverage

Add tests for path validation
Add tests for CsrConfigurationFactory
Ensure path from env can end with "/"
Ensure path built in file creator is not dependent on representation of root path

Issue-ID: AAF-996
Signed-off-by: Remigiusz Janeczek <remigiusz.janeczek@nokia.com>
Change-Id: I3d6a7f534d49ff64177917989727a7330e3f6869

6 files changed, 152 insertions, 44 deletions

diff --git a/certServiceClient/pom.xml b/certServiceClient/pom.xml
index 5e11f583..4b7c0cf9 100644
--- a/certServiceClient/pom.xml
+++ b/certServiceClient/pom.xml
@@ -155,11 +155,7 @@
         </dependency>
         <dependency>
             <groupId>org.junit.jupiter</groupId>
-            <artifactId>junit-jupiter-engine</artifactId>
-        </dependency>
-        <dependency>
-            <groupId>org.junit.jupiter</groupId>
-            <artifactId>junit-jupiter-api</artifactId>
+            <artifactId>junit-jupiter</artifactId>
         </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
diff --git a/certServiceClient/src/main/java/org/onap/aaf/certservice/client/certification/conversion/PKCS12FilesCreator.java b/certServiceClient/src/main/java/org/onap/aaf/certservice/client/certification/conversion/PKCS12FilesCreator.java
index 60121b03..d8c41bfd 100644
--- a/certServiceClient/src/main/java/org/onap/aaf/certservice/client/certification/conversion/PKCS12FilesCreator.java
+++ b/certServiceClient/src/main/java/org/onap/aaf/certservice/client/certification/conversion/PKCS12FilesCreator.java
@@ -21,6 +21,8 @@ package org.onap.aaf.certservice.client.certification.conversion;
 
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.nio.file.Path;
+
 import org.onap.aaf.certservice.client.certification.exception.PemToPKCS12ConverterException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -39,10 +41,10 @@ class PKCS12FilesCreator {
 
 
     PKCS12FilesCreator(String path) {
-        keystoreJksPath = path + KEYSTORE_JKS;
-        keystorePassPath = path + KEYSTORE_PASS;
-        truststoreJksPath = path + TRUSTSTORE_JKS;
-        truststorePassPath = path + TRUSTSTORE_PASS;
+        keystoreJksPath = Path.of(path, KEYSTORE_JKS).toString();
+        keystorePassPath = Path.of(path, KEYSTORE_PASS).toString();
+        truststoreJksPath = Path.of(path, TRUSTSTORE_JKS).toString();
+        truststorePassPath = Path.of(path, TRUSTSTORE_PASS).toString();
     }
 
     void saveKeystoreData(byte[] keystoreData, String keystorePassword) throws PemToPKCS12ConverterException {
diff --git a/certServiceClient/src/main/java/org/onap/aaf/certservice/client/configuration/EnvValidationUtils.java b/certServiceClient/src/main/java/org/onap/aaf/certservice/client/configuration/EnvValidationUtils.java
index e65d688d..b0405274 100644
--- a/certServiceClient/src/main/java/org/onap/aaf/certservice/client/configuration/EnvValidationUtils.java
+++ b/certServiceClient/src/main/java/org/onap/aaf/certservice/client/configuration/EnvValidationUtils.java
@@ -27,7 +27,7 @@ public final class EnvValidationUtils {
     private EnvValidationUtils() {}
 
     public static Boolean isPathValid(String path) {
-        return path.matches("^/|(/[a-zA-Z0-9_-]+)+$");
+        return path.matches("^/|(/[a-zA-Z0-9_-]+)+/?$");
     }
 
     public static Boolean isAlphaNumeric(String caName) {
diff --git a/certServiceClient/src/test/java/org/onap/aaf/certservice/client/configuration/EnvValidationUtilsTest.java b/certServiceClient/src/test/java/org/onap/aaf/certservice/client/configuration/EnvValidationUtilsTest.java
new file mode 100644
index 00000000..8f4fe6a3
--- /dev/null
+++ b/certServiceClient/src/test/java/org/onap/aaf/certservice/client/configuration/EnvValidationUtilsTest.java
@@ -0,0 +1,41 @@
+/*============LICENSE_START=======================================================
+ * aaf-certservice-client
+ * ================================================================================
+ * Copyright (C) 2020 Nokia. All rights reserved.
+ * ================================================================================
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ============LICENSE_END=========================================================
+ */
+
+package org.onap.aaf.certservice.client.configuration;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class EnvValidationUtilsTest {
+
+    @ParameterizedTest
+    @ValueSource(strings = {"/var/log", "/", "/var/log/", "/second_var", "/second-var"})
+    public void shouldAcceptValidPath(String path){
+        assertTrue(EnvValidationUtils.isPathValid(path));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"/var/log?", "", "var_", "var", "//", "/var//log"})
+    public void shouldRejectInvalidPath(String path){
+        assertFalse(EnvValidationUtils.isPathValid(path));
+    }
+}
\ No newline at end of file
diff --git a/certServiceClient/src/test/java/org/onap/aaf/certservice/client/configuration/model/CsrConfigurationFactoryTest.java b/certServiceClient/src/test/java/org/onap/aaf/certservice/client/configuration/model/CsrConfigurationFactoryTest.java
index 707094c0..bb566e81 100644
--- a/certServiceClient/src/test/java/org/onap/aaf/certservice/client/configuration/model/CsrConfigurationFactoryTest.java
+++ b/certServiceClient/src/test/java/org/onap/aaf/certservice/client/configuration/model/CsrConfigurationFactoryTest.java
@@ -20,13 +20,17 @@
 
 package org.onap.aaf.certservice.client.configuration.model;
 
+import org.assertj.core.api.Condition;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.onap.aaf.certservice.client.api.ExitCode;
 import org.onap.aaf.certservice.client.configuration.CsrConfigurationEnvs;
 import org.onap.aaf.certservice.client.configuration.EnvsForCsr;
 import org.onap.aaf.certservice.client.configuration.exception.CsrConfigurationException;
 import org.onap.aaf.certservice.client.configuration.factory.CsrConfigurationFactory;
 
 import java.util.Optional;
+import java.util.function.Predicate;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
@@ -43,23 +47,30 @@ public class CsrConfigurationFactoryTest {
     private final String ORGANIZATION_UNIT_VALID = "ONAP";
     private final String STATE_VALID = "California";
     private final String COMMON_NAME_INVALID = "onap.org*&";
+    private final String COUNTRY_INVALID = "PLA";
+    private final String ORGANIZATION_INVALID = "Linux?Foundation";
 
     private EnvsForCsr envsForCsr = mock(EnvsForCsr.class);
-
+    private CsrConfigurationFactory testedFactory;
+    private Condition<CsrConfigurationException> expectedExitCodeCondition = new Condition<>("Correct exit code"){
+        @Override
+        public boolean matches(CsrConfigurationException e) {
+            return e.applicationExitCode() == ExitCode.CSR_CONFIGURATION_EXCEPTION.getValue();
+        }
+    };
+
+    @BeforeEach
+    void setUp() {
+        testedFactory = new CsrConfigurationFactory(envsForCsr);
+    }
 
     @Test
-    void create_shouldReturnSuccessWhenAllVariablesAreSetAndValid() throws CsrConfigurationException {
+    void shouldReturnCorrectConfiguration_WhenAllVariablesAreSetAndValid() throws CsrConfigurationException {
         // given
-        when(envsForCsr.getCommonName()).thenReturn(Optional.of(COMMON_NAME_VALID));
-        when(envsForCsr.getSubjectAlternativesName()).thenReturn(Optional.of(SANS_VALID));
-        when(envsForCsr.getCountry()).thenReturn(Optional.of(COUNTRY_VALID));
-        when(envsForCsr.getLocation()).thenReturn(Optional.of(LOCATION_VALID));
-        when(envsForCsr.getOrganization()).thenReturn(Optional.of(ORGANIZATION_VALID));
-        when(envsForCsr.getOrganizationUnit()).thenReturn(Optional.of(ORGANIZATION_UNIT_VALID));
-        when(envsForCsr.getState()).thenReturn(Optional.of(STATE_VALID));
+        mockEnvsWithAllValidParameters();
 
         // when
-        CsrConfiguration configuration = new CsrConfigurationFactory(envsForCsr).create();
+        CsrConfiguration configuration = testedFactory.create();
 
         // then
         assertThat(configuration.getCommonName()).isEqualTo(COMMON_NAME_VALID);
@@ -72,15 +83,12 @@ public class CsrConfigurationFactoryTest {
     }
 
     @Test
-    void create_shouldReturnSuccessWhenNotRequiredVariablesAreNotSet() throws CsrConfigurationException {
+    void shouldReturnCorrectConfiguration_WhenNotRequiredVariablesAreNotSet() throws CsrConfigurationException {
         // given
-        when(envsForCsr.getCommonName()).thenReturn(Optional.of(COMMON_NAME_VALID));
-        when(envsForCsr.getState()).thenReturn(Optional.of(STATE_VALID));
-        when(envsForCsr.getCountry()).thenReturn(Optional.of(COUNTRY_VALID));
-        when(envsForCsr.getOrganization()).thenReturn(Optional.of(ORGANIZATION_VALID));
+        mockEnvsWithValidRequiredParameters();
 
         // when
-        CsrConfiguration configuration = new CsrConfigurationFactory(envsForCsr).create();
+        CsrConfiguration configuration = testedFactory.create();
 
         // then
         assertThat(configuration.getCommonName()).isEqualTo(COMMON_NAME_VALID);
@@ -91,22 +99,89 @@ public class CsrConfigurationFactoryTest {
 
 
     @Test
-    void create_shouldReturnCsrConfigurationExceptionWhenCommonNameContainsSpecialCharacters() {
+    void shouldThrowCsrConfigurationException_WhenCommonNameInvalid() {
         // given
-        when(envsForCsr.getCommonName()).thenReturn(Optional.of(COMMON_NAME_INVALID));
+        mockEnvsWithInvalidCommonName();
+
+        // when/then
+        assertThatExceptionOfType(CsrConfigurationException.class)
+                .isThrownBy(testedFactory::create)
+                .withMessageContaining(CsrConfigurationEnvs.COMMON_NAME + " is invalid.")
+                .has(expectedExitCodeCondition);
+    }
+
+    @Test
+    void shouldThrowCsrConfigurationException_WhenOrganizationInvalid() {
+        // given
+        mockEnvsWithInvalidOrganization();
+
+        // when/then
+        assertThatExceptionOfType(CsrConfigurationException.class)
+                .isThrownBy(testedFactory::create)
+                .withMessageContaining(CsrConfigurationEnvs.ORGANIZATION + " is invalid.")
+                .has(expectedExitCodeCondition);
+
+    }
+
+    @Test
+    void shouldThrowCsrConfigurationException_WhenCountryInvalid() {
+        // given
+        mockEnvsWithInvalidCountry();
+
+        // when/then
+        assertThatExceptionOfType(CsrConfigurationException.class)
+                .isThrownBy(testedFactory::create)
+                .withMessageContaining(CsrConfigurationEnvs.COUNTRY + " is invalid.")
+                .has(expectedExitCodeCondition);
+
+    }
+
+    @Test
+    void shouldThrowCsrConfigurationExceptionWhenStateInvalid() {
+        // given
+        mockEnvsWithInvalidState();
+        // when/then
+        assertThatExceptionOfType(CsrConfigurationException.class)
+                .isThrownBy(testedFactory::create)
+                .withMessageContaining(CsrConfigurationEnvs.STATE + " is invalid.")
+                .has(expectedExitCodeCondition);
+    }
+
+    private void mockEnvsWithAllValidParameters() {
+        mockEnvsWithValidRequiredParameters();
+        mockEnvsWithValidOptionalParameters();
+    }
+
+    private void mockEnvsWithValidOptionalParameters() {
+        when(envsForCsr.getOrganizationUnit()).thenReturn(Optional.of(ORGANIZATION_UNIT_VALID));
+        when(envsForCsr.getLocation()).thenReturn(Optional.of(LOCATION_VALID));
         when(envsForCsr.getSubjectAlternativesName()).thenReturn(Optional.of(SANS_VALID));
+    }
+
+    private void mockEnvsWithValidRequiredParameters() {
+        when(envsForCsr.getCommonName()).thenReturn(Optional.of(COMMON_NAME_VALID));
         when(envsForCsr.getCountry()).thenReturn(Optional.of(COUNTRY_VALID));
-        when(envsForCsr.getLocation()).thenReturn(Optional.of(LOCATION_VALID));
         when(envsForCsr.getOrganization()).thenReturn(Optional.of(ORGANIZATION_VALID));
-        when(envsForCsr.getOrganizationUnit()).thenReturn(Optional.of(ORGANIZATION_UNIT_VALID));
-        when(envsForCsr.getState()).thenReturn(Optional.of(SANS_VALID));
+        when(envsForCsr.getState()).thenReturn(Optional.of(STATE_VALID));
+    }
 
-        // when
-        CsrConfigurationFactory configurationFactory = new CsrConfigurationFactory(envsForCsr);
+    private void mockEnvsWithInvalidCommonName() {
+        mockEnvsWithAllValidParameters();
+        when(envsForCsr.getCommonName()).thenReturn(Optional.of(COMMON_NAME_INVALID));
+    }
 
-        // when/then
-        assertThatExceptionOfType(CsrConfigurationException.class)
-                .isThrownBy(configurationFactory::create)
-                .withMessageContaining(CsrConfigurationEnvs.COMMON_NAME + " is invalid.");
+    private void mockEnvsWithInvalidCountry() {
+        mockEnvsWithAllValidParameters();
+        when(envsForCsr.getCountry()).thenReturn(Optional.of(COUNTRY_INVALID));
+    }
+
+    private void mockEnvsWithInvalidOrganization() {
+        mockEnvsWithAllValidParameters();
+        when(envsForCsr.getOrganization()).thenReturn(Optional.of(ORGANIZATION_INVALID));
+    }
+
+    private void mockEnvsWithInvalidState() {
+        mockEnvsWithAllValidParameters();
+        when(envsForCsr.getState()).thenReturn(Optional.empty());
     }
 }
diff --git a/pom.xml b/pom.xml
index c9e829c8..3eef61b1 100644
--- a/pom.xml
+++ b/pom.xml
@@ -263,13 +263,7 @@
             </dependency>
             <dependency>
                 <groupId>org.junit.jupiter</groupId>
-                <artifactId>junit-jupiter-engine</artifactId>
-                <version>${junit.version}</version>
-                <scope>test</scope>
-            </dependency>
-            <dependency>
-                <groupId>org.junit.jupiter</groupId>
-                <artifactId>junit-jupiter-api</artifactId>
+                <artifactId>junit-jupiter</artifactId>
                 <version>${junit.version}</version>
                 <scope>test</scope>
             </dependency>