From d18af899b0bb9813c3e1bc3111171787e85ccfc4 Mon Sep 17 00:00:00 2001 From: Remigiusz Janeczek Date: Fri, 28 Feb 2020 12:27:12 +0100 Subject: 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 Change-Id: I3d6a7f534d49ff64177917989727a7330e3f6869 --- certServiceClient/pom.xml | 6 +- .../conversion/PKCS12FilesCreator.java | 10 +- .../client/configuration/EnvValidationUtils.java | 2 +- .../configuration/EnvValidationUtilsTest.java | 41 +++++++ .../model/CsrConfigurationFactoryTest.java | 129 ++++++++++++++++----- pom.xml | 8 +- 6 files changed, 152 insertions(+), 44 deletions(-) create mode 100644 certServiceClient/src/test/java/org/onap/aaf/certservice/client/configuration/EnvValidationUtilsTest.java 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 @@ org.junit.jupiter - junit-jupiter-engine - - - org.junit.jupiter - junit-jupiter-api + junit-jupiter org.mockito 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 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 @@ org.junit.jupiter - junit-jupiter-engine - ${junit.version} - test - - - org.junit.jupiter - junit-jupiter-api + junit-jupiter ${junit.version} test -- cgit 1.2.3-korg