Merge "XSS Vulnerability fix in SharedContextRestController"
diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/SharedContextRestController.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/SharedContextRestController.java
index ba77c56..9e3428e 100644
--- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/SharedContextRestController.java
+++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/SharedContextRestController.java
@@ -48,10 +48,13 @@
import org.onap.portalapp.controller.EPRestrictedRESTfulBaseController;
import org.onap.portalapp.portal.domain.SharedContext;
+import org.onap.portalapp.portal.exceptions.NotValidDataException;
import org.onap.portalapp.portal.logging.aop.EPAuditLog;
import org.onap.portalapp.portal.service.SharedContextService;
import org.onap.portalapp.portal.utils.EPCommonSystemProperties;
import org.onap.portalapp.portal.utils.PortalConstants;
+import org.onap.portalapp.validation.DataValidator;
+import org.onap.portalapp.validation.SecureString;
import org.onap.portalsdk.core.logging.logic.EELFLoggerDelegate;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Configuration;
@@ -85,33 +88,20 @@
@EnableAspectJAutoProxy
@EPAuditLog
public class SharedContextRestController extends EPRestrictedRESTfulBaseController {
+ private static final DataValidator dataValidator = new DataValidator();
+ private static final EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(SharedContextRestController.class);
+ private static final ObjectMapper mapper = new ObjectMapper();
- /**
- * Model for a one-element JSON object returned by many methods.
- */
- class SharedContextJsonResponse {
- String response;
+ private SharedContextService contextService;
+
+ @Autowired
+ public SharedContextRestController(SharedContextService contextService) {
+ this.contextService = contextService;
}
/**
- * Access to the database
- */
- @Autowired
- private SharedContextService contextService;
-
- /**
- * Logger for debug etc.
- */
- private EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(SharedContextRestController.class);
-
- /**
- * Reusable JSON (de)serializer
- */
- private final ObjectMapper mapper = new ObjectMapper();
-
- /**
* Gets a value for the specified context and key (RESTful service method).
- *
+ *
* @param request
* HTTP servlet request
* @param context_id
@@ -127,13 +117,18 @@
@RequestMapping(value = { "/get" }, method = RequestMethod.GET, produces = "application/json")
public String getContext(HttpServletRequest request, @RequestParam String context_id, @RequestParam String ckey)
throws Exception {
-
logger.debug(EELFLoggerDelegate.debugLogger, "getContext for ID " + context_id + ", key " + ckey);
if (context_id == null || ckey == null)
throw new Exception("Received null for context_id and/or ckey");
+ SecureString secureContextId = new SecureString(context_id);
+ SecureString secureCKey = new SecureString(ckey);
+
+ if(!dataValidator.isValid(secureContextId) || !dataValidator.isValid(secureCKey)){
+ throw new NotValidDataException("Received not valid for context_id and/or ckey");
+ }
SharedContext context = contextService.getSharedContext(context_id, ckey);
- String jsonResponse = "";
+ String jsonResponse;
if (context == null)
jsonResponse = convertResponseToJSON(context);
else
@@ -144,7 +139,7 @@
/**
* Gets user information for the specified context (RESTful service method).
- *
+ *
* @param request
* HTTP servlet request
* @param context_id
@@ -162,8 +157,11 @@
logger.debug(EELFLoggerDelegate.debugLogger, "getUserContext for ID " + context_id);
if (context_id == null)
throw new Exception("Received null for context_id");
+ SecureString secureContextId = new SecureString(context_id);
+ if (!dataValidator.isValid(secureContextId))
+ throw new NotValidDataException("context_id is not valid");
- List<SharedContext> listSharedContext = new ArrayList<SharedContext>();
+ List<SharedContext> listSharedContext = new ArrayList<>();
SharedContext firstNameContext = contextService.getSharedContext(context_id,
EPCommonSystemProperties.USER_FIRST_NAME);
SharedContext lastNameContext = contextService.getSharedContext(context_id,
@@ -179,14 +177,13 @@
listSharedContext.add(emailContext);
if (orgUserIdContext != null)
listSharedContext.add(orgUserIdContext);
- String jsonResponse = convertResponseToJSON(listSharedContext);
- return jsonResponse;
+ return convertResponseToJSON(listSharedContext);
}
/**
* Tests for presence of the specified key in the specified context (RESTful
* service method).
- *
+ *
* @param request
* HTTP servlet request
* @param context_id
@@ -208,19 +205,24 @@
if (context_id == null || ckey == null)
throw new Exception("Received null for contextId and/or key");
+ SecureString secureContextId = new SecureString(context_id);
+ SecureString secureCKey = new SecureString(ckey);
+
+ if (!dataValidator.isValid(secureContextId) || !dataValidator.isValid(secureCKey))
+ throw new NotValidDataException("Not valid data for contextId and/or key");
+
String response = null;
SharedContext context = contextService.getSharedContext(context_id, ckey);
if (context != null)
response = "exists";
- String jsonResponse = convertResponseToJSON(response);
- return jsonResponse;
+ return convertResponseToJSON(response);
}
/**
* Removes the specified key in the specified context (RESTful service
* method).
- *
+ *
* @param request
* HTTP servlet request
* @param context_id
@@ -242,6 +244,12 @@
if (context_id == null || ckey == null)
throw new Exception("Received null for contextId and/or key");
+ SecureString secureContextId = new SecureString(context_id);
+ SecureString secureCKey = new SecureString(ckey);
+
+ if (!dataValidator.isValid(secureContextId) || !dataValidator.isValid(secureCKey))
+ throw new NotValidDataException("Not valid data for contextId and/or key");
+
SharedContext context = contextService.getSharedContext(context_id, ckey);
String response = null;
if (context != null) {
@@ -249,14 +257,13 @@
response = "removed";
}
- String jsonResponse = convertResponseToJSON(response);
- return jsonResponse;
+ return convertResponseToJSON(response);
}
/**
* Clears all key-value pairs in the specified context (RESTful service
* method).
- *
+ *
* @param request
* HTTP servlet request
* @param context_id
@@ -275,16 +282,20 @@
if (context_id == null)
throw new Exception("clearContext: Received null for contextId");
+ SecureString secureContextId = new SecureString(context_id);
+
+ if (!dataValidator.isValid(secureContextId))
+ throw new NotValidDataException("Not valid data for contextId");
+
int count = contextService.deleteSharedContexts(context_id);
- String jsonResponse = convertResponseToJSON(Integer.toString(count));
- return jsonResponse;
+ return convertResponseToJSON(Integer.toString(count));
}
/**
* Sets a context value for the specified context and key (RESTful service
* method). Creates the context if no context with the specified ID-key pair
* exists, overwrites the value if it exists already.
- *
+ *
* @param request
* HTTP servlet request
* @param userJson
@@ -302,6 +313,11 @@
@ApiOperation(value = "Sets a context value for the specified context and key. Creates the context if no context with the specified ID-key pair exists, overwrites the value if it exists already.", response = SharedContextJsonResponse.class)
@RequestMapping(value = { "/set" }, method = RequestMethod.POST, produces = "application/json")
public String setContext(HttpServletRequest request, @RequestBody String userJson) throws Exception {
+ if (userJson !=null){
+ SecureString secureUserJson = new SecureString(userJson);
+ if (!dataValidator.isValid(secureUserJson))
+ throw new NotValidDataException("Not valid data for userJson");
+ }
@SuppressWarnings("unchecked")
Map<String, Object> userData = mapper.readValue(userJson, Map.class);
@@ -313,7 +329,7 @@
throw new Exception("setContext: received null for contextId and/or key");
logger.debug(EELFLoggerDelegate.debugLogger, "setContext: ID " + contextId + ", key " + key + "->" + value);
- String response = null;
+ String response;
SharedContext existing = contextService.getSharedContext(contextId, key);
if (existing == null) {
contextService.addSharedContext(contextId, key, value);
@@ -322,53 +338,49 @@
contextService.saveSharedContext(existing);
}
response = existing == null ? "added" : "replaced";
- String jsonResponse = convertResponseToJSON(response);
- return jsonResponse;
+ return convertResponseToJSON(response);
}
/**
* Creates a two-element JSON object tagged "response".
- *
+ *
* @param responseBody
* @return JSON object as String
* @throws JsonProcessingException
*/
private String convertResponseToJSON(String responseBody) throws JsonProcessingException {
- Map<String, String> responseMap = new HashMap<String, String>();
+ Map<String, String> responseMap = new HashMap<>();
responseMap.put("response", responseBody);
- String response = mapper.writeValueAsString(responseMap);
- return response;
+ return mapper.writeValueAsString(responseMap);
}
/**
* Converts a list of SharedContext objects to a JSON array.
- *
+ *
* @param contextList
* @return JSON array as String
* @throws JsonProcessingException
*/
private String convertResponseToJSON(List<SharedContext> contextList) throws JsonProcessingException {
- String jsonArray = mapper.writeValueAsString(contextList);
- return jsonArray;
+ return mapper.writeValueAsString(contextList);
}
/**
* Creates a JSON object with the content of the shared context; null is ok.
- *
+ *
* @param context
* @return tag "response" with collection of context object's fields
* @throws JsonProcessingException
*/
private String convertResponseToJSON(SharedContext context) throws JsonProcessingException {
- Map<String, Object> responseMap = new HashMap<String, Object>();
+ Map<String, Object> responseMap = new HashMap<>();
responseMap.put("response", context);
- String responseBody = mapper.writeValueAsString(responseMap);
- return responseBody;
+ return mapper.writeValueAsString(responseMap);
}
/**
* Handles any exception thrown by a method in this controller.
- *
+ *
* @param e
* Exception
* @param response
@@ -382,3 +394,7 @@
}
}
+class SharedContextJsonResponse {
+ String response;
+}
+
diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/exceptions/NotValidDataException.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/exceptions/NotValidDataException.java
new file mode 100644
index 0000000..2a26ab3
--- /dev/null
+++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/exceptions/NotValidDataException.java
@@ -0,0 +1,51 @@
+/*-
+ * ============LICENSE_START==========================================
+ * ONAP Portal
+ * ===================================================================
+ * Copyright (C) 2017 AT&T Intellectual Property. All rights reserved.
+ * ===================================================================
+ *
+ * Unless otherwise specified, all software contained herein is licensed
+ * under the Apache License, Version 2.0 (the "License");
+ * you may not use this software 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.
+ *
+ * Unless otherwise specified, all documentation contained herein is licensed
+ * under the Creative Commons License, Attribution 4.0 Intl. (the "License");
+ * you may not use this documentation except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * https://creativecommons.org/licenses/by/4.0/
+ *
+ * Unless required by applicable law or agreed to in writing, documentation
+ * 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.portalapp.portal.exceptions;
+
+public class NotValidDataException extends Exception {
+
+ public NotValidDataException(String msg) {
+ super(msg);
+ }
+
+ @Override
+ public String toString() {
+ return "NotValidDataException{}: " + this.getMessage();
+ }
+}
diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/SharedContextRestControllerTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/SharedContextRestControllerTest.java
index 1607f42..49cccae 100644
--- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/SharedContextRestControllerTest.java
+++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/SharedContextRestControllerTest.java
@@ -38,24 +38,19 @@
*/
-import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
-import java.io.IOException;
+import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;
-
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
-import org.drools.core.command.assertion.AssertEquals;
import org.json.JSONObject;
-import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -64,24 +59,15 @@
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
-import org.onap.portalapp.portal.controller.SharedContextRestClient;
-import org.onap.portalapp.portal.controller.SharedContextTestProperties;
import org.onap.portalapp.portal.core.MockEPUser;
-import org.onap.portalapp.portal.domain.CentralV2RoleFunction;
import org.onap.portalapp.portal.domain.SharedContext;
+import org.onap.portalapp.portal.exceptions.NotValidDataException;
import org.onap.portalapp.portal.framework.MockitoTestSuite;
-import org.onap.portalapp.portal.scheduler.SchedulerProperties;
import org.onap.portalapp.portal.service.SharedContextService;
import org.onap.portalapp.portal.utils.EPCommonSystemProperties;
-import org.onap.portalsdk.core.util.SystemProperties;
-import org.onap.portalsdk.core.web.support.UserUtils;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
-import org.springframework.beans.factory.annotation.Autowired;
-
-import com.fasterxml.jackson.databind.DeserializationFeature;
-import com.fasterxml.jackson.databind.ObjectMapper;
/**
* Tests the endpoints exposed by the Shared Context controller in Portal.
@@ -95,7 +81,7 @@
SharedContextService contextService;
@InjectMocks
- SharedContextRestController sharedContextRestController=new SharedContextRestController();
+ SharedContextRestController sharedContextRestController=new SharedContextRestController(contextService);
@Before
public void setup() {
@@ -220,11 +206,31 @@
public void getContextTestWithException() throws Exception{
sharedContextRestController.getContext(mockedRequest, null,null);
}
+
+ @Test(expected=NotValidDataException.class)
+ public void getContextTestNotValidDataException() throws Exception{
+ sharedContextRestController.getContext(mockedRequest, "<script>alert(\"hellox worldss\");</script>","test");
+ }
+
+ @Test(expected=NotValidDataException.class)
+ public void getContextTest2NotValidDataException() throws Exception{
+ sharedContextRestController.getContext(mockedRequest, "test","“><script>alert(“XSS”)</script>");
+ }
+
+ @Test(expected=NotValidDataException.class)
+ public void getContextTest3NotValidDataException() throws Exception{
+ sharedContextRestController.getContext(mockedRequest, "<ScRipT>alert(\"XSS\");</ScRipT>","“><script>alert(“XSS”)</script>");
+ }
- @Test(expected=Exception.class)
+ @Test(expected= Exception.class)
public void getUserContextTest() throws Exception{
sharedContextRestController.getUserContext(mockedRequest, null);
}
+
+ @Test(expected= NotValidDataException.class)
+ public void getUserContextXSSTest() throws Exception{
+ sharedContextRestController.getUserContext(mockedRequest, "<svg><script x:href='https://dl.dropbox.com/u/13018058/js.js' {Opera}");
+ }
@Test
public void getUserContextTestWithContext() throws Exception{
@@ -257,6 +263,16 @@
Mockito.when(contextService.getSharedContext(Matchers.any(),Matchers.any())).thenReturn(sharedContext);
sharedContextRestController.checkContext(mockedRequest, null,null);
}
+
+ @Test(expected=NotValidDataException.class)
+ public void checkContextTestWithContextXSSl() throws Exception{
+ SharedContext sharedContext=new SharedContext();
+ sharedContext.setContext_id("test_contextid");
+ sharedContext.setCkey("test_ckey");
+ Mockito.when(contextService.getSharedContext(Matchers.any(),Matchers.any())).thenReturn(sharedContext);
+ sharedContextRestController.checkContext(mockedRequest,
+ "<ScRipT 5-0*3+9/3=>prompt(1)</ScRipT giveanswerhere=?","<script>alert(123);</script>");
+ }
@Test
public void removeContextTest() throws Exception{
@@ -283,6 +299,20 @@
assertNotNull(actual);
}
+
+ @Test(expected=NotValidDataException.class)
+ public void removeContextTestWithContextXSS() throws Exception{
+ SharedContext sharedContext=new SharedContext();
+ sharedContext.setContext_id("test_contextid");
+ sharedContext.setCkey("test_ckey");
+ Mockito.when(contextService.getSharedContext(Matchers.any(),Matchers.any())).thenReturn(sharedContext);
+
+ //Mockito.when(contextService.deleteSharedContext(sharedContext));
+ String actual=sharedContextRestController.removeContext(mockedRequest,
+ "<script>alert(“XSS”)</script> ","<script>alert(/XSS/)</script>");
+ assertNotNull(actual);
+
+ }
@Test(expected=Exception.class)
public void clearContextTestwithContextIdNull() throws Exception{
@@ -293,6 +323,16 @@
assertNotNull(actual);
}
+
+ @Test(expected=NotValidDataException.class)
+ public void clearContextTestwithContextXSS() throws Exception{
+
+ Mockito.when(contextService.deleteSharedContexts(Matchers.any())).thenReturn(12);
+
+ String actual=sharedContextRestController.clearContext(mockedRequest,"<script>alert(123)</script>");
+ assertNotNull(actual);
+
+ }
@Test
public void clearContextTest() throws Exception{
@@ -350,4 +390,27 @@
String actual=sharedContextRestController.setContext(mockedRequest,testUserJson.toString());
}
+
+ @Test(expected=NotValidDataException.class)
+ public void setContextTestWithContextXSS() throws Exception{
+ ObjectMapper mapper = new ObjectMapper();
+ Map<String, Object> userData = new HashMap<String, Object>();
+ userData.put("context_id", "test_contextId");
+ userData.put("ckey", "<script>alert(‘XSS’)</script>");
+ userData.put("cvalue", "test_cvalue");
+ //String testUserJson=Matchers.anyString();
+ JSONObject testUserJson = new JSONObject();
+ testUserJson.put("context_id", "test1ContextId");
+ testUserJson.put("ckey", "testCkey");
+ testUserJson.put("cvalue", "<script>alert(‘XSS’)</script>");
+ Map<String, Object> userData1 = mapper.readValue(testUserJson.toString(), Map.class);
+ SharedContext sharedContext=new SharedContext();
+ sharedContext.setContext_id("test_contextid");
+ sharedContext.setCkey("test_ckey");
+ Mockito.when(contextService.getSharedContext(Matchers.any(),Matchers.any())).thenReturn(sharedContext);
+ // Mockito.when(mapper.readValue("true", Map.class)).thenReturn(userData);
+ String actual=sharedContextRestController.setContext(mockedRequest,testUserJson.toString());
+
+ }
+
}