Fix property name regex

property name should contain only letters, numbers and / or underscores

Change-Id: Ic6b8d627a30f5f157bed1617a8af819b66352136
Issue-ID: SDC-2243
Signed-off-by: talio <tali.orenbach@amdocs.com>

5 files changed, 193 insertions, 59 deletions

diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/PropertyBusinessLogic.java b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/PropertyBusinessLogic.java
index 3c49e21471..275d1726ed 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/PropertyBusinessLogic.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/PropertyBusinessLogic.java
@@ -22,6 +22,13 @@ package org.openecomp.sdc.be.components.impl;
 
 import com.google.gson.JsonElement;
 import fj.data.Either;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Supplier;
+import javax.servlet.ServletContext;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang3.tuple.ImmutablePair;
@@ -54,10 +61,6 @@ import org.openecomp.sdc.common.log.wrappers.Logger;
 import org.openecomp.sdc.exception.ResponseFormat;
 import org.springframework.web.context.WebApplicationContext;
 
-import javax.servlet.ServletContext;
-import java.util.*;
-import java.util.function.Supplier;
-
 @org.springframework.stereotype.Component("propertyBusinessLogic")
 public class PropertyBusinessLogic extends BaseBusinessLogic {
 
@@ -467,31 +470,6 @@ public class PropertyBusinessLogic extends BaseBusinessLogic {
         return propertyCandidate.isPresent();
     }
 
-    private boolean isPropertyExist(List<PropertyDefinition> properties, String resourceUid, String propertyName, String propertyType) {
-        boolean result = false;
-        if (!CollectionUtils.isEmpty(properties)) {
-            for (PropertyDefinition propertyDefinition : properties) {
-
-                if ( propertyDefinition.getName().equals(propertyName) &&
-                    (propertyDefinition.getParentUniqueId().equals(resourceUid) || !propertyDefinition.getType().equals(propertyType)) ) {
-                    result = true;
-                    break;
-                }
-            }
-        }
-        return result;
-    }
-
-    private Either<PropertyDefinition, StorageOperationStatus> handleProperty(PropertyDefinition newPropertyDefinition, Map<String, DataTypeDefinition> dataTypes) {
-
-        StorageOperationStatus validateAndUpdateProperty = validateAndUpdateProperty(newPropertyDefinition, dataTypes);
-        if (validateAndUpdateProperty != StorageOperationStatus.OK) {
-            return Either.right(validateAndUpdateProperty);
-        }
-
-        return Either.left(newPropertyDefinition);
-    }
-
     private StorageOperationStatus validateAndUpdateProperty(IComplexDefaultValue propertyDefinition, Map<String, DataTypeDefinition> dataTypes) {
 
         log.trace("Going to validate property type and value. {}", propertyDefinition);
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/BeGenericServlet.java b/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/BeGenericServlet.java
index b397439a99..8e7d406bea 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/BeGenericServlet.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/BeGenericServlet.java
@@ -33,6 +33,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Supplier;
 import javax.servlet.ServletContext;
@@ -96,6 +97,8 @@ public class BeGenericServlet extends BasicServlet {
 
     private static final Logger log = Logger.getLogger(BeGenericServlet.class);
 
+    private static final String PROPERTY_NAME_REGEX = "[\\w,\\d,_]+";
+
     /******************** New error response mechanism
      * @param requestErrorWrapper **************/
 
@@ -315,12 +318,11 @@ public class BeGenericServlet extends BasicServlet {
         }
     }
 
-    protected Either<Map<String, PropertyDefinition>, ActionStatus> getPropertyModel(String componentId,
-                                                                                     String data) {
+    protected Either<Map<String, PropertyDefinition>, ActionStatus> getPropertyModel(String componentId, String data) {
         JSONParser parser = new JSONParser();
         JSONObject root;
         try {
-            Map<String, PropertyDefinition> properties = new HashMap<String, PropertyDefinition>();
+            Map<String, PropertyDefinition> properties = new HashMap<>();
             root = (JSONObject) parser.parse(data);
 
             Set entrySet = root.entrySet();
@@ -328,16 +330,20 @@ public class BeGenericServlet extends BasicServlet {
             while (iterator.hasNext()) {
                 Entry next = (Entry) iterator.next();
                 String propertyName = (String) next.getKey();
+
+                if(!isPropertyNameValid(propertyName)) {
+                    return Either.right(ActionStatus.INVALID_PROPERTY_NAME);
+                }
+
                 JSONObject value = (JSONObject) next.getValue();
-                String jsonString = value.toJSONString();
-                Either<PropertyDefinition, ActionStatus> convertJsonToObject = convertJsonToObject(jsonString, PropertyDefinition.class);
-                if (convertJsonToObject.isRight()) {
-                    return Either.right(convertJsonToObject.right().value());
+                Either<PropertyDefinition, ActionStatus> propertyDefinitionEither =
+                        getPropertyDefinitionFromJson(componentId, propertyName, value);
+
+                if(propertyDefinitionEither.isRight()) {
+                    return Either.right(propertyDefinitionEither.right().value());
                 }
-                PropertyDefinition propertyDefinition = convertJsonToObject.left().value();
-                String uniqueId = UniqueIdBuilder.buildPropertyUniqueId(componentId, (String) propertyName);
-                propertyDefinition.setUniqueId(uniqueId);
-                properties.put(propertyName, propertyDefinition);
+
+                properties.put(propertyName, propertyDefinitionEither.left().value());
             }
 
             return Either.left(properties);
@@ -347,6 +353,25 @@ public class BeGenericServlet extends BasicServlet {
         }
     }
 
+    protected boolean isPropertyNameValid(String propertyName) {
+        return Objects.nonNull(propertyName)
+                       && propertyName.matches(PROPERTY_NAME_REGEX);
+
+    }
+
+    private Either<PropertyDefinition, ActionStatus> getPropertyDefinitionFromJson(String componentId, String propertyName, JSONObject value) {
+        String jsonString = value.toJSONString();
+        Either<PropertyDefinition, ActionStatus> convertJsonToObject = convertJsonToObject(jsonString, PropertyDefinition.class);
+        if (convertJsonToObject.isRight()) {
+            return Either.right(convertJsonToObject.right().value());
+        }
+        PropertyDefinition propertyDefinition = convertJsonToObject.left().value();
+        String uniqueId = UniqueIdBuilder.buildPropertyUniqueId(componentId, propertyName);
+        propertyDefinition.setUniqueId(uniqueId);
+
+        return Either.left(propertyDefinition);
+    }
+
     protected Either<Map<String, PropertyDefinition>, ActionStatus> getPropertiesListForUpdate(String data) {
 
         Map<String, PropertyDefinition> properties = new HashMap<>();
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/ComponentPropertyServlet.java b/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/ComponentPropertyServlet.java
index 38c04a131c..037bd95190 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/ComponentPropertyServlet.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/ComponentPropertyServlet.java
@@ -17,30 +17,14 @@
 package org.openecomp.sdc.be.servlets;
 
 import com.jcabi.aspects.Loggable;
-
-import java.util.List;
-import java.util.Map;
-
 import fj.data.Either;
 import io.swagger.annotations.Api;
 import io.swagger.annotations.ApiOperation;
 import io.swagger.annotations.ApiParam;
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
-import org.openecomp.sdc.be.components.impl.PropertyBusinessLogic;
-import org.openecomp.sdc.be.config.BeEcompErrorManager;
-import org.openecomp.sdc.be.dao.api.ActionStatus;
-import org.openecomp.sdc.be.datamodel.utils.PropertyValueConstraintValidationUtil;
-import org.openecomp.sdc.be.model.PropertyDefinition;
-import org.openecomp.sdc.be.model.User;
-import org.openecomp.sdc.be.model.cache.ApplicationDataTypeCache;
-import org.openecomp.sdc.be.resources.data.EntryData;
-import org.openecomp.sdc.common.api.Constants;
-import org.openecomp.sdc.exception.ResponseFormat;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.springframework.beans.factory.annotation.Autowired;
-
+import java.util.List;
+import java.util.Map;
 import javax.inject.Singleton;
 import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletRequest;
@@ -56,6 +40,19 @@ import javax.ws.rs.Produces;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
+import org.openecomp.sdc.be.components.impl.PropertyBusinessLogic;
+import org.openecomp.sdc.be.config.BeEcompErrorManager;
+import org.openecomp.sdc.be.dao.api.ActionStatus;
+import org.openecomp.sdc.be.datamodel.utils.PropertyValueConstraintValidationUtil;
+import org.openecomp.sdc.be.model.PropertyDefinition;
+import org.openecomp.sdc.be.model.User;
+import org.openecomp.sdc.be.model.cache.ApplicationDataTypeCache;
+import org.openecomp.sdc.be.resources.data.EntryData;
+import org.openecomp.sdc.common.api.Constants;
+import org.openecomp.sdc.exception.ResponseFormat;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
 
 @Loggable(prepend = true, value = Loggable.DEBUG, trim = false)
 @Path("/v1/catalog")
@@ -216,6 +213,7 @@ public class ComponentPropertyServlet extends BeGenericServlet {
     log.debug("Start handle request of {} modifier id is {} data is {}", url, userId, data);
 
     try{
+      PropertyBusinessLogic propertyBL = getPropertyBL(context);
       Either<Map<String, PropertyDefinition>, ActionStatus> propertyDefinition =
               getPropertyModel(componentId, data);
       if (propertyDefinition.isRight()) {
@@ -235,7 +233,6 @@ public class ComponentPropertyServlet extends BeGenericServlet {
       newPropertyDefinition.setParentUniqueId(componentId);
       String propertyName = newPropertyDefinition.getName();
 
-      PropertyBusinessLogic propertyBL = getPropertyBL(context);
       Either<EntryData<String, PropertyDefinition>, ResponseFormat> addPropertyEither =
               propertyBL.addPropertyToComponent(componentId, propertyName, newPropertyDefinition, userId);
 
diff --git a/catalog-be/src/main/resources/config/error-configuration.yaml b/catalog-be/src/main/resources/config/error-configuration.yaml
index 312d90c307..f9e5407e30 100644
--- a/catalog-be/src/main/resources/config/error-configuration.yaml
+++ b/catalog-be/src/main/resources/config/error-configuration.yaml
@@ -2268,4 +2268,10 @@ errors:
         code: 400,
         message: "Error: Invalid property values provided:\n %1",
         messageId: "SVC4735"
+    }
+#---------SVC4727------------------------------
+    INVALID_PROPERTY_NAME: {
+        code: 400,
+        message: "Error: Property name contains invalid characters. It should have only letters, numbers and underscores.",
+        messageId: "SVC4727"
     }
\ No newline at end of file
diff --git a/catalog-be/src/test/java/org/openecomp/sdc/be/servlets/ComponentPropertyServletTest.java b/catalog-be/src/test/java/org/openecomp/sdc/be/servlets/ComponentPropertyServletTest.java
new file mode 100644
index 0000000000..b816702317
--- /dev/null
+++ b/catalog-be/src/test/java/org/openecomp/sdc/be/servlets/ComponentPropertyServletTest.java
@@ -0,0 +1,128 @@
+package org.openecomp.sdc.be.servlets;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.when;
+
+import com.google.gson.Gson;
+import fj.data.Either;
+import javax.servlet.ServletContext;
+import javax.servlet.http.HttpSession;
+import javax.ws.rs.core.Response;
+import org.glassfish.grizzly.http.util.HttpStatus;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.openecomp.sdc.be.components.impl.PropertyBusinessLogic;
+import org.openecomp.sdc.be.dao.api.ActionStatus;
+import org.openecomp.sdc.be.impl.ComponentsUtils;
+import org.openecomp.sdc.be.impl.WebAppContextWrapper;
+import org.openecomp.sdc.be.model.PropertyDefinition;
+import org.openecomp.sdc.be.resources.data.EntryData;
+import org.openecomp.sdc.common.api.Constants;
+import org.openecomp.sdc.exception.ResponseFormat;
+import org.springframework.web.context.WebApplicationContext;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ComponentPropertyServletTest extends JerseySpringBaseTest {
+    @Mock
+    private static HttpSession session;
+    @Mock
+    private static ServletContext context;
+    @Mock
+    private static WebAppContextWrapper wrapper;
+    @Mock
+    private static WebApplicationContext webAppContext;
+    @Mock
+    private static PropertyBusinessLogic propertyBl;
+    @Mock
+    private static ComponentsUtils componentsUtils;
+    @InjectMocks
+    @Spy
+    private ComponentPropertyServlet componentPropertyServlet;
+
+    private static final String SERVICE_ID = "service1";
+    private final static String USER_ID = "jh0003";
+    private static final String VALID_PROPERTY_NAME = "valid_name_123";
+    private static final String INVALID_PROPERTY_NAME = "invalid_name_$.&";
+    private static final String STRING_TYPE = "string";
+
+    @Before
+    public void initClass() {
+        initMockitoStubbings();
+    }
+
+    @Test
+    public void testCreatePropertyOnService_success() {
+        PropertyDefinition property = new PropertyDefinition();
+        property.setName(VALID_PROPERTY_NAME);
+        property.setType(STRING_TYPE);
+
+        EntryData<String, PropertyDefinition> propertyEntry = new EntryData<>(VALID_PROPERTY_NAME, property);
+        when(propertyBl.addPropertyToComponent(eq(SERVICE_ID), any(), any(), any())).thenReturn(Either.left(propertyEntry));
+
+        Response propertyInService =
+                componentPropertyServlet.createPropertyInService(SERVICE_ID, getValidProperty(), request, USER_ID);
+
+        Assert.assertEquals(HttpStatus.OK_200.getStatusCode(), propertyInService.getStatus());
+    }
+
+    @Test
+    public void testCreatePropertyInvalidName_failure() {
+        PropertyDefinition property = new PropertyDefinition();
+        property.setName(INVALID_PROPERTY_NAME);
+        property.setType(STRING_TYPE);
+
+        ResponseFormat responseFormat = new ResponseFormat();
+        responseFormat.setStatus(HttpStatus.BAD_REQUEST_400.getStatusCode());
+
+        when(componentsUtils.getResponseFormat(eq(ActionStatus.INVALID_PROPERTY_NAME))).thenReturn(responseFormat);
+
+
+        Response propertyInService =
+                componentPropertyServlet.createPropertyInService(SERVICE_ID, getInvalidProperty(), request, USER_ID);
+
+        Assert.assertEquals(HttpStatus.BAD_REQUEST_400.getStatusCode(), propertyInService.getStatus());
+    }
+
+    private static void initMockitoStubbings() {
+        when(request.getSession()).thenReturn(session);
+        when(session.getServletContext()).thenReturn(context);
+        when(context.getAttribute(eq(Constants.WEB_APPLICATION_CONTEXT_WRAPPER_ATTR))).thenReturn(wrapper);
+        when(wrapper.getWebAppContext(any())).thenReturn(webAppContext);
+        when(webAppContext.getBean(eq(PropertyBusinessLogic.class))).thenReturn(propertyBl);
+        when(webAppContext.getBean(eq(ComponentsUtils.class))).thenReturn(componentsUtils);
+    }
+
+    private String getValidProperty() {
+        return "{\n"
+                       + "  \"valid_name_123\": {\n"
+                       + "    \"schema\": {\n"
+                       + "      \"property\": {\n"
+                       + "        \"type\": \"\"\n"
+                       + "      }\n" + "    },\n"
+                       + "    \"type\": \"string\",\n"
+                       + "    \"name\": \"valid_name_123\"\n"
+                       + "  }\n"
+                       + "}";
+    }
+
+    private String getInvalidProperty() {
+        return "{\n"
+                       + "  \"invalid_name_$.&\": {\n"
+                       + "    \"schema\": {\n"
+                       + "      \"property\": {\n"
+                       + "        \"type\": \"\"\n"
+                       + "      }\n" + "    },\n"
+                       + "    \"type\": \"string\",\n"
+                       + "    \"name\": \"invalid_name_$.&\"\n"
+                       + "  }\n"
+                       + "}";
+    }
+
+}