From 46e67f9bd05747acc46525822dca7fe2d6260e31 Mon Sep 17 00:00:00 2001 From: Dominik Mizyn Date: Thu, 30 May 2019 11:52:03 +0200 Subject: [PATCH] XSS Vulnerability fix in MicroserviceController @SafeHtml annotation is used to fix this problem. This commit also fix: * redundant local variable issue * sonar issue: Replace the type specification in this constructor call with the diamond operator ("<>"). * performance issue - String concatenation argument as argument to 'StringBuilder.append()' call * redundant cast * redundant 'throws Exception'. 'Exception' is never thrown * access static member via instance reference * unused declarations Issue-ID: PORTAL-602 Change-Id: Id92fe2d9cfe239474403f611f3d5d0170acf63cc Signed-off-by: Dominik Mizyn --- .../portal/controller/MicroserviceController.java | 59 +++++++++---- .../portalapp/portal/domain/MicroserviceData.java | 18 ++-- .../portal/domain/MicroserviceParameter.java | 5 +- .../controller/MicroserviceControllerTest.java | 97 +++++++++++++--------- 4 files changed, 112 insertions(+), 67 deletions(-) diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/MicroserviceController.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/MicroserviceController.java index 50eaa600..2f956cc3 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/MicroserviceController.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/MicroserviceController.java @@ -39,9 +39,15 @@ package org.onap.portalapp.portal.controller; import java.util.List; +import java.util.Set; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.validation.ConstraintViolation; +import javax.validation.Valid; +import javax.validation.Validation; +import javax.validation.Validator; +import javax.validation.ValidatorFactory; import org.onap.portalapp.controller.EPRestrictedBaseController; import org.onap.portalapp.portal.domain.MicroserviceData; import org.onap.portalapp.portal.domain.WidgetCatalog; @@ -72,6 +78,7 @@ import org.springframework.web.client.RestTemplate; @EnableAspectJAutoProxy @EPAuditLog public class MicroserviceController extends EPRestrictedBaseController { + public static final ValidatorFactory VALIDATOR_FACTORY = Validation.buildDefaultValidatorFactory(); String whatService = "widgets-service"; RestTemplate template = new RestTemplate(); @@ -84,53 +91,68 @@ public class MicroserviceController extends EPRestrictedBaseController { @RequestMapping(value = { "/portalApi/microservices" }, method = RequestMethod.POST) public PortalRestResponse createMicroservice(HttpServletRequest request, HttpServletResponse response, - @RequestBody MicroserviceData newServiceData) throws Exception { + @Valid @RequestBody MicroserviceData newServiceData) throws Exception { if (newServiceData == null) { - return new PortalRestResponse(PortalRestStatusEnum.ERROR, "FAILURE", - "MicroserviceData cannot be null or empty"); + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "FAILURE", + "MicroserviceData cannot be null or empty"); + }else { + Validator validator = VALIDATOR_FACTORY.getValidator(); + + Set> constraintViolations = validator.validate(newServiceData); + if(!constraintViolations.isEmpty()){ + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, + "ERROR", "MicroserviceData is not valid"); + } } long serviceId = microserviceService.saveMicroservice(newServiceData); try { microserviceService.saveServiceParameters(serviceId, newServiceData.getParameterList()); } catch (Exception e) { - return new PortalRestResponse(PortalRestStatusEnum.ERROR, "FAILURE", e.getMessage()); + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "FAILURE", e.getMessage()); } - return new PortalRestResponse(PortalRestStatusEnum.OK, "SUCCESS", ""); + return new PortalRestResponse<>(PortalRestStatusEnum.OK, "SUCCESS", ""); } @RequestMapping(value = { "/portalApi/microservices" }, method = RequestMethod.GET) public List getMicroservice(HttpServletRequest request, HttpServletResponse response) throws Exception { - List list = microserviceService.getMicroserviceData(); - return list; + return microserviceService.getMicroserviceData(); } @RequestMapping(value = { "/portalApi/microservices/{serviceId}" }, method = RequestMethod.PUT) public PortalRestResponse updateMicroservice(HttpServletRequest request, HttpServletResponse response, - @PathVariable("serviceId") long serviceId, @RequestBody MicroserviceData newServiceData) throws Exception { + @PathVariable("serviceId") long serviceId, @Valid @RequestBody MicroserviceData newServiceData) { if (newServiceData == null) { - return new PortalRestResponse(PortalRestStatusEnum.ERROR, "FAILURE", - "MicroserviceData cannot be null or empty"); + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "FAILURE", + "MicroserviceData cannot be null or empty"); + }else { + Validator validator = VALIDATOR_FACTORY.getValidator(); + + Set> constraintViolations = validator.validate(newServiceData); + if(!constraintViolations.isEmpty()){ + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, + "ERROR", "MicroserviceData is not valid"); + } } try { microserviceService.updateMicroservice(serviceId, newServiceData); } catch (Exception e) { - return new PortalRestResponse(PortalRestStatusEnum.ERROR, "FAILURE", e.getMessage()); + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "FAILURE", e.getMessage()); } - return new PortalRestResponse(PortalRestStatusEnum.OK, "SUCCESS", ""); + return new PortalRestResponse<>(PortalRestStatusEnum.OK, "SUCCESS", ""); } @RequestMapping(value = { "/portalApi/microservices/{serviceId}" }, method = RequestMethod.DELETE) public PortalRestResponse deleteMicroservice(HttpServletRequest request, HttpServletResponse response, - @PathVariable("serviceId") long serviceId) throws Exception { + @PathVariable("serviceId") long serviceId) { try { ParameterizedTypeReference> typeRef = new ParameterizedTypeReference>() { }; // If this service is assoicated with widgets, cannnot be deleted - ResponseEntity> ans = (ResponseEntity>) template.exchange( + ResponseEntity> ans = template.exchange( EcompPortalUtils.widgetMsProtocol() + "://" + consulHealthService.getServiceLocation(whatService, SystemProperties.getProperty("microservices.widget.local.port")) + "/widget/microservices/widgetCatalog/service/" + serviceId, HttpMethod.GET, new HttpEntity(WidgetServiceHeaders.getInstance()), typeRef); @@ -140,17 +162,18 @@ public class MicroserviceController extends EPRestrictedBaseController { else{ StringBuilder sb = new StringBuilder(); for(int i = 0; i < widgets.size(); i++){ - sb.append("'" + widgets.get(i).getName() + "' "); + sb.append("'").append(widgets.get(i).getName()).append("' "); if(i < (widgets.size()-1)){ sb.append(","); } } - return new PortalRestResponse(PortalRestStatusEnum.WARN, "SOME WIDGETS ASSOICATE WITH THIS SERVICE", sb.toString()); + return new PortalRestResponse<>(PortalRestStatusEnum.WARN, "SOME WIDGETS ASSOICATE WITH THIS SERVICE", + sb.toString()); } } catch (Exception e) { - return new PortalRestResponse(PortalRestStatusEnum.ERROR, "FAILURE", e.getMessage()); + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "FAILURE", e.getMessage()); } - return new PortalRestResponse(PortalRestStatusEnum.OK, "SUCCESS", ""); + return new PortalRestResponse<>(PortalRestStatusEnum.OK, "SUCCESS", ""); } } diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/MicroserviceData.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/MicroserviceData.java index f62b8928..b8f79d06 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/MicroserviceData.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/MicroserviceData.java @@ -44,6 +44,8 @@ import javax.persistence.GeneratedValue; import javax.persistence.GenerationType; import javax.persistence.Id; +import javax.validation.Valid; +import org.hibernate.validator.constraints.SafeHtml; import org.onap.portalsdk.core.domain.support.DomainVo; public class MicroserviceData extends DomainVo { @@ -55,23 +57,23 @@ public class MicroserviceData extends DomainVo { } private Long id; - + @SafeHtml private String name; - + @SafeHtml private String active; - + @SafeHtml private String desc; private long appId; - + @SafeHtml private String url; - + @SafeHtml private String securityType; - + @SafeHtml private String username; - + @SafeHtml private String password; - + @Valid private List parameterList; public Long getId() { diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/MicroserviceParameter.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/MicroserviceParameter.java index 0c645716..848c6a2a 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/MicroserviceParameter.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/MicroserviceParameter.java @@ -37,6 +37,7 @@ */ package org.onap.portalapp.portal.domain; +import org.hibernate.validator.constraints.SafeHtml; import org.onap.portalsdk.core.domain.support.DomainVo; public class MicroserviceParameter extends DomainVo { @@ -50,9 +51,9 @@ public class MicroserviceParameter extends DomainVo { private Long id; private long serviceId; - + @SafeHtml private String para_key; - + @SafeHtml private String para_value; public Long getId() { diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/MicroserviceControllerTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/MicroserviceControllerTest.java index 21d0cf70..81e1f8b2 100644 --- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/MicroserviceControllerTest.java +++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/MicroserviceControllerTest.java @@ -96,7 +96,7 @@ public class MicroserviceControllerTest extends MockitoTestSuite{ @SuppressWarnings("rawtypes") @Mock - ResponseEntity> ans = new ResponseEntity>(HttpStatus.OK); + ResponseEntity> ans = new ResponseEntity<>(HttpStatus.OK); @Before public void setup() { @@ -114,11 +114,10 @@ public class MicroserviceControllerTest extends MockitoTestSuite{ @Test public void createMicroserviceIfServiceDataNullTest() throws Exception { - PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); + PortalRestResponse expectedportalRestResponse = new PortalRestResponse<>(); expectedportalRestResponse.setMessage("FAILURE"); expectedportalRestResponse.setResponse("MicroserviceData cannot be null or empty"); - PortalRestStatusEnum portalRestStatusEnum = null; - expectedportalRestResponse.setStatus(portalRestStatusEnum.ERROR); + expectedportalRestResponse.setStatus(PortalRestStatusEnum.ERROR); MicroserviceData microserviceData = null; PortalRestResponse actualportalRestResponse = microserviceController.createMicroservice(mockedRequest, mockedResponse, microserviceData); @@ -127,23 +126,35 @@ public class MicroserviceControllerTest extends MockitoTestSuite{ @Test public void createMicroserviceTest() throws Exception { - PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); + PortalRestResponse expectedportalRestResponse = new PortalRestResponse<>(); expectedportalRestResponse.setMessage("SUCCESS"); expectedportalRestResponse.setResponse(""); - PortalRestStatusEnum portalRestStatusEnum = null; - expectedportalRestResponse.setStatus(portalRestStatusEnum.OK); + expectedportalRestResponse.setStatus(PortalRestStatusEnum.OK); PortalRestResponse actualportalRestResponse = microserviceController.createMicroservice(mockedRequest, mockedResponse, microserviceData); assertEquals(actualportalRestResponse, expectedportalRestResponse); } + @Test + public void createMicroserviceXSSTest() throws Exception { + PortalRestResponse expectedportalRestResponse = new PortalRestResponse<>(); + expectedportalRestResponse.setMessage("ERROR"); + expectedportalRestResponse.setResponse("MicroserviceData is not valid"); + expectedportalRestResponse.setStatus(PortalRestStatusEnum.ERROR); + MicroserviceData XSSMicroserviceData = new MicroserviceData(); + XSSMicroserviceData.setActive(""); + XSSMicroserviceData.setName(""); + PortalRestResponse actualportalRestResponse = microserviceController.createMicroservice(mockedRequest, + mockedResponse, XSSMicroserviceData); + assertEquals(expectedportalRestResponse, actualportalRestResponse); + } + @Test public void createMicroserviceExceptionTest() throws Exception { - PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); + PortalRestResponse expectedportalRestResponse = new PortalRestResponse<>(); expectedportalRestResponse.setMessage("FAILURE"); expectedportalRestResponse.setResponse(null); - PortalRestStatusEnum portalRestStatusEnum = null; - expectedportalRestResponse.setStatus(portalRestStatusEnum.ERROR); + expectedportalRestResponse.setStatus(PortalRestStatusEnum.ERROR); Mockito.when(microserviceService.saveMicroservice(microserviceData)).thenReturn((long) 1); Mockito.when(microserviceData.getParameterList()).thenThrow(nullPointerException); PortalRestResponse actualportalRestResponse = microserviceController.createMicroservice(mockedRequest, @@ -159,12 +170,11 @@ public class MicroserviceControllerTest extends MockitoTestSuite{ } @Test - public void updateMicroserviceIfServiceISNullTest() throws Exception { - PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); + public void updateMicroserviceIfServiceISNullTest() { + PortalRestResponse expectedportalRestResponse = new PortalRestResponse<>(); expectedportalRestResponse.setMessage("FAILURE"); expectedportalRestResponse.setResponse("MicroserviceData cannot be null or empty"); - PortalRestStatusEnum portalRestStatusEnum = null; - expectedportalRestResponse.setStatus(portalRestStatusEnum.ERROR); + expectedportalRestResponse.setStatus(PortalRestStatusEnum.ERROR); MicroserviceData microserviceData = null; PortalRestResponse actualportalRestResponse = microserviceController.updateMicroservice(mockedRequest, mockedResponse, 1, microserviceData); @@ -172,24 +182,36 @@ public class MicroserviceControllerTest extends MockitoTestSuite{ } @Test - public void updateMicroserviceTest() throws Exception { - PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); + public void updateMicroserviceTest() { + PortalRestResponse expectedportalRestResponse = new PortalRestResponse<>(); expectedportalRestResponse.setMessage("SUCCESS"); expectedportalRestResponse.setResponse(""); - PortalRestStatusEnum portalRestStatusEnum = null; - expectedportalRestResponse.setStatus(portalRestStatusEnum.OK); + expectedportalRestResponse.setStatus(PortalRestStatusEnum.OK); PortalRestResponse actualportalRestResponse = microserviceController.updateMicroservice(mockedRequest, - mockedResponse, 1, microserviceData); + mockedResponse, 1, microserviceData); assertEquals(actualportalRestResponse, expectedportalRestResponse); } @Test - public void updateMicroserviceExceptionTest() throws Exception { - PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); + public void updateMicroserviceXSSTest() { + PortalRestResponse expectedportalRestResponse = new PortalRestResponse<>(); + expectedportalRestResponse.setMessage("ERROR"); + expectedportalRestResponse.setResponse("MicroserviceData is not valid"); + expectedportalRestResponse.setStatus(PortalRestStatusEnum.ERROR); + MicroserviceData XSSMicroserviceData = new MicroserviceData(); + XSSMicroserviceData.setActive(""); + XSSMicroserviceData.setName(""); + PortalRestResponse actualportalRestResponse = microserviceController.updateMicroservice(mockedRequest, + mockedResponse, 1, XSSMicroserviceData); + assertEquals(expectedportalRestResponse, actualportalRestResponse); + } + + @Test + public void updateMicroserviceExceptionTest() { + PortalRestResponse expectedportalRestResponse = new PortalRestResponse<>(); expectedportalRestResponse.setMessage("FAILURE"); expectedportalRestResponse.setResponse(null); - PortalRestStatusEnum portalRestStatusEnum = null; - expectedportalRestResponse.setStatus(portalRestStatusEnum.ERROR); + expectedportalRestResponse.setStatus(PortalRestStatusEnum.ERROR); Mockito.when(microserviceController.updateMicroservice(mockedRequest, mockedResponse, 1, microserviceData)) .thenThrow(nullPointerException); PortalRestResponse actualportalRestResponse = microserviceController.updateMicroservice(mockedRequest, @@ -198,14 +220,14 @@ public class MicroserviceControllerTest extends MockitoTestSuite{ } @Test - public void deleteMicroserviceExceptionTest() throws Exception { - PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); + public void deleteMicroserviceExceptionTest() { + PortalRestResponse expectedportalRestResponse = new PortalRestResponse<>(); expectedportalRestResponse.setMessage("FAILURE"); PowerMockito.mockStatic(EcompPortalUtils.class); expectedportalRestResponse.setResponse( - "I/O error on GET request for \"" + EcompPortalUtils.widgetMsProtocol() + "://null/widget/microservices/widgetCatalog/service/1\":null; nested exception is java.net.UnknownHostException: null"); - PortalRestStatusEnum portalRestStatusEnum = null; - expectedportalRestResponse.setStatus(portalRestStatusEnum.ERROR); + "I/O error on GET request for \"" + org.onap.portalapp.portal.utils.EcompPortalUtils.widgetMsProtocol() + + "://null/widget/microservices/widgetCatalog/service/1\":null; nested exception is java.net.UnknownHostException: null"); + expectedportalRestResponse.setStatus(PortalRestStatusEnum.ERROR); PowerMockito.mockStatic(WidgetServiceHeaders.class); PortalRestResponse actuaPportalRestResponse = microserviceController.deleteMicroservice(mockedRequest, mockedResponse, 1); @@ -215,13 +237,11 @@ public class MicroserviceControllerTest extends MockitoTestSuite{ @SuppressWarnings("unchecked") @Test public void deleteMicroserviceTest() throws Exception { - String HTTPS = "https://"; - PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); + PortalRestResponse expectedportalRestResponse = new PortalRestResponse<>(); expectedportalRestResponse.setMessage("SOME WIDGETS ASSOICATE WITH THIS SERVICE"); expectedportalRestResponse.setResponse("'null' ,'null' "); - PortalRestStatusEnum portalRestStatusEnum = null; - expectedportalRestResponse.setStatus(portalRestStatusEnum.WARN); - List List = new ArrayList(); + expectedportalRestResponse.setStatus(PortalRestStatusEnum.WARN); + List List = new ArrayList<>(); WidgetCatalog widgetCatalog = new WidgetCatalog(); widgetCatalog.setId(1); WidgetCatalog widgetCatalog1 = new WidgetCatalog(); @@ -236,7 +256,7 @@ public class MicroserviceControllerTest extends MockitoTestSuite{ ParameterizedTypeReference> typeRef = new ParameterizedTypeReference>() { }; Mockito.when(template.exchange( - EcompPortalUtils.widgetMsProtocol() + "://" + consulHealthService.getServiceLocation(whatService, SystemProperties.getProperty("microservices.widget.local.port")) + org.onap.portalapp.portal.utils.EcompPortalUtils.widgetMsProtocol() + "://" + consulHealthService.getServiceLocation(whatService, SystemProperties.getProperty("microservices.widget.local.port")) + "/widget/microservices/widgetCatalog/service/" + 1, HttpMethod.GET, new HttpEntity(WidgetServiceHeaders.getInstance()), typeRef)).thenReturn(ans); @@ -248,12 +268,11 @@ public class MicroserviceControllerTest extends MockitoTestSuite{ @SuppressWarnings("unchecked") @Test public void deleteMicroserviceWhenNoWidgetsAssociatedTest() throws Exception { - PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); + PortalRestResponse expectedportalRestResponse = new PortalRestResponse<>(); expectedportalRestResponse.setMessage("SUCCESS"); expectedportalRestResponse.setResponse(""); - PortalRestStatusEnum portalRestStatusEnum = null; - expectedportalRestResponse.setStatus(portalRestStatusEnum.OK); - List List = new ArrayList(); + expectedportalRestResponse.setStatus(PortalRestStatusEnum.OK); + List List = new ArrayList<>(); PowerMockito.mockStatic(WidgetServiceHeaders.class); PowerMockito.mockStatic(EcompPortalUtils.class); String whatService = "widgets-service"; @@ -262,7 +281,7 @@ public class MicroserviceControllerTest extends MockitoTestSuite{ ParameterizedTypeReference> typeRef = new ParameterizedTypeReference>() { }; Mockito.when(template.exchange( - EcompPortalUtils.widgetMsProtocol() + "://" + consulHealthService.getServiceLocation(whatService, SystemProperties.getProperty("microservices.widget.local.port")) + org.onap.portalapp.portal.utils.EcompPortalUtils.widgetMsProtocol() + "://" + consulHealthService.getServiceLocation(whatService, SystemProperties.getProperty("microservices.widget.local.port")) + "/widget/microservices/widgetCatalog/service/" + 1, HttpMethod.GET, new HttpEntity(WidgetServiceHeaders.getInstance()), typeRef)).thenReturn(ans); PortalRestResponse actuaPportalRestResponse = microserviceController.deleteMicroservice(mockedRequest, -- 2.16.6