XSS Vulnerability fix in MicroserviceController 53/88853/3
authorDominik Mizyn <d.mizyn@samsung.com>
Thu, 30 May 2019 09:52:03 +0000 (11:52 +0200)
committerDominik Mizyn <d.mizyn@samsung.com>
Thu, 30 May 2019 11:42:05 +0000 (13:42 +0200)
@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 <d.mizyn@samsung.com>
ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/MicroserviceController.java
ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/MicroserviceData.java
ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/MicroserviceParameter.java
ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/MicroserviceControllerTest.java

index 50eaa60..2f956cc 100644 (file)
@@ -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<String> createMicroservice(HttpServletRequest request, HttpServletResponse response,
-                       @RequestBody MicroserviceData newServiceData) throws Exception {
+                       @Valid @RequestBody MicroserviceData newServiceData) throws Exception {
                if (newServiceData == null) {
-                       return new PortalRestResponse<String>(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<ConstraintViolation<MicroserviceData>> 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<String>(PortalRestStatusEnum.ERROR, "FAILURE", e.getMessage());
+                       return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "FAILURE", e.getMessage());
                }
 
-               return new PortalRestResponse<String>(PortalRestStatusEnum.OK, "SUCCESS", "");
+               return new PortalRestResponse<>(PortalRestStatusEnum.OK, "SUCCESS", "");
        }
 
        @RequestMapping(value = { "/portalApi/microservices" }, method = RequestMethod.GET)
        public List<MicroserviceData> getMicroservice(HttpServletRequest request, HttpServletResponse response)
                        throws Exception {
-               List<MicroserviceData> list = microserviceService.getMicroserviceData();
-               return list;
+               return microserviceService.getMicroserviceData();
        }
 
        @RequestMapping(value = { "/portalApi/microservices/{serviceId}" }, method = RequestMethod.PUT)
        public PortalRestResponse<String> 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<String>(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<ConstraintViolation<MicroserviceData>> 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<String>(PortalRestStatusEnum.ERROR, "FAILURE", e.getMessage());
+                       return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "FAILURE", e.getMessage());
                }
-               return new PortalRestResponse<String>(PortalRestStatusEnum.OK, "SUCCESS", "");
+               return new PortalRestResponse<>(PortalRestStatusEnum.OK, "SUCCESS", "");
        }
        
        @RequestMapping(value = { "/portalApi/microservices/{serviceId}" }, method = RequestMethod.DELETE)
        public PortalRestResponse<String> deleteMicroservice(HttpServletRequest request, HttpServletResponse response,
-                       @PathVariable("serviceId") long serviceId) throws Exception {
+                       @PathVariable("serviceId") long serviceId) {
                try {
                        ParameterizedTypeReference<List<WidgetCatalog>> typeRef = new ParameterizedTypeReference<List<WidgetCatalog>>() {
                        };
                        // If this service is assoicated with widgets, cannnot be deleted
-                       ResponseEntity<List<WidgetCatalog>> ans = (ResponseEntity<List<WidgetCatalog>>) template.exchange(
+                       ResponseEntity<List<WidgetCatalog>> 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<String>(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<String>(PortalRestStatusEnum.ERROR, "FAILURE", e.getMessage());
+                       return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "FAILURE", e.getMessage());
                }
-               return new PortalRestResponse<String>(PortalRestStatusEnum.OK, "SUCCESS", "");
+               return new PortalRestResponse<>(PortalRestStatusEnum.OK, "SUCCESS", "");
        }
 
 }
index f62b892..b8f79d0 100644 (file)
@@ -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<MicroserviceParameter> parameterList;
 
        public Long getId() {
index 0c64571..848c6a2 100644 (file)
@@ -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() {
index 21d0cf7..81e1f8b 100644 (file)
@@ -96,7 +96,7 @@ public class MicroserviceControllerTest extends MockitoTestSuite{
 
        @SuppressWarnings("rawtypes")
        @Mock
-       ResponseEntity<List<WidgetCatalog>> ans = new ResponseEntity<List<WidgetCatalog>>(HttpStatus.OK);
+       ResponseEntity<List<WidgetCatalog>> 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<String> expectedportalRestResponse = new PortalRestResponse<String>();
+               PortalRestResponse<String> 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<String> actualportalRestResponse = microserviceController.createMicroservice(mockedRequest,
                                mockedResponse, microserviceData);
@@ -127,23 +126,35 @@ public class MicroserviceControllerTest extends MockitoTestSuite{
 
        @Test
        public void createMicroserviceTest() throws Exception {
-               PortalRestResponse<String> expectedportalRestResponse = new PortalRestResponse<String>();
+               PortalRestResponse<String> expectedportalRestResponse = new PortalRestResponse<>();
                expectedportalRestResponse.setMessage("SUCCESS");
                expectedportalRestResponse.setResponse("");
-               PortalRestStatusEnum portalRestStatusEnum = null;
-               expectedportalRestResponse.setStatus(portalRestStatusEnum.OK);
+               expectedportalRestResponse.setStatus(PortalRestStatusEnum.OK);
                PortalRestResponse<String> actualportalRestResponse = microserviceController.createMicroservice(mockedRequest,
                                mockedResponse, microserviceData);
                assertEquals(actualportalRestResponse, expectedportalRestResponse);
        }
 
+       @Test
+       public void createMicroserviceXSSTest() throws Exception {
+               PortalRestResponse<String> expectedportalRestResponse = new PortalRestResponse<>();
+               expectedportalRestResponse.setMessage("ERROR");
+               expectedportalRestResponse.setResponse("MicroserviceData is not valid");
+               expectedportalRestResponse.setStatus(PortalRestStatusEnum.ERROR);
+               MicroserviceData XSSMicroserviceData = new MicroserviceData();
+               XSSMicroserviceData.setActive("<script>alert(123);</script>");
+               XSSMicroserviceData.setName("<script>alert(/XSS”)</script>");
+               PortalRestResponse<String> actualportalRestResponse = microserviceController.createMicroservice(mockedRequest,
+                       mockedResponse, XSSMicroserviceData);
+               assertEquals(expectedportalRestResponse, actualportalRestResponse);
+       }
+
        @Test
        public void createMicroserviceExceptionTest() throws Exception {
-               PortalRestResponse<String> expectedportalRestResponse = new PortalRestResponse<String>();
+               PortalRestResponse<String> 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<String> actualportalRestResponse = microserviceController.createMicroservice(mockedRequest,
@@ -159,12 +170,11 @@ public class MicroserviceControllerTest extends MockitoTestSuite{
        }
 
        @Test
-       public void updateMicroserviceIfServiceISNullTest() throws Exception {
-               PortalRestResponse<String> expectedportalRestResponse = new PortalRestResponse<String>();
+       public void updateMicroserviceIfServiceISNullTest() {
+               PortalRestResponse<String> 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<String> actualportalRestResponse = microserviceController.updateMicroservice(mockedRequest,
                                mockedResponse, 1, microserviceData);
@@ -172,24 +182,36 @@ public class MicroserviceControllerTest extends MockitoTestSuite{
        }
 
        @Test
-       public void updateMicroserviceTest() throws Exception {
-               PortalRestResponse<String> expectedportalRestResponse = new PortalRestResponse<String>();
+       public void updateMicroserviceTest() {
+               PortalRestResponse<String> expectedportalRestResponse = new PortalRestResponse<>();
                expectedportalRestResponse.setMessage("SUCCESS");
                expectedportalRestResponse.setResponse("");
-               PortalRestStatusEnum portalRestStatusEnum = null;
-               expectedportalRestResponse.setStatus(portalRestStatusEnum.OK);
+               expectedportalRestResponse.setStatus(PortalRestStatusEnum.OK);
                PortalRestResponse<String> actualportalRestResponse = microserviceController.updateMicroservice(mockedRequest,
-                               mockedResponse, 1, microserviceData);
+                       mockedResponse, 1, microserviceData);
                assertEquals(actualportalRestResponse, expectedportalRestResponse);
        }
 
        @Test
-       public void updateMicroserviceExceptionTest() throws Exception {
-               PortalRestResponse<String> expectedportalRestResponse = new PortalRestResponse<String>();
+       public void updateMicroserviceXSSTest() {
+               PortalRestResponse<String> expectedportalRestResponse = new PortalRestResponse<>();
+               expectedportalRestResponse.setMessage("ERROR");
+               expectedportalRestResponse.setResponse("MicroserviceData is not valid");
+               expectedportalRestResponse.setStatus(PortalRestStatusEnum.ERROR);
+               MicroserviceData XSSMicroserviceData = new MicroserviceData();
+               XSSMicroserviceData.setActive("<script>alert(123);</script>");
+               XSSMicroserviceData.setName("<script>alert(/XSS”)</script>");
+               PortalRestResponse<String> actualportalRestResponse = microserviceController.updateMicroservice(mockedRequest,
+                       mockedResponse, 1, XSSMicroserviceData);
+               assertEquals(expectedportalRestResponse, actualportalRestResponse);
+       }
+
+       @Test
+       public void updateMicroserviceExceptionTest() {
+               PortalRestResponse<String> 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<String> actualportalRestResponse = microserviceController.updateMicroservice(mockedRequest,
@@ -198,14 +220,14 @@ public class MicroserviceControllerTest extends MockitoTestSuite{
        }
 
        @Test
-       public void deleteMicroserviceExceptionTest() throws Exception {
-               PortalRestResponse<String> expectedportalRestResponse = new PortalRestResponse<String>();
+       public void deleteMicroserviceExceptionTest() {
+               PortalRestResponse<String> 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<String> 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<String> expectedportalRestResponse = new PortalRestResponse<String>();
+               PortalRestResponse<String> expectedportalRestResponse = new PortalRestResponse<>();
                expectedportalRestResponse.setMessage("SOME WIDGETS ASSOICATE WITH THIS SERVICE");
                expectedportalRestResponse.setResponse("'null' ,'null' ");
-               PortalRestStatusEnum portalRestStatusEnum = null;
-               expectedportalRestResponse.setStatus(portalRestStatusEnum.WARN);
-               List<WidgetCatalog> List = new ArrayList<WidgetCatalog>();
+               expectedportalRestResponse.setStatus(PortalRestStatusEnum.WARN);
+               List<WidgetCatalog> List = new ArrayList<>();
                WidgetCatalog widgetCatalog = new WidgetCatalog();
                widgetCatalog.setId(1);
                WidgetCatalog widgetCatalog1 = new WidgetCatalog();
@@ -236,7 +256,7 @@ public class MicroserviceControllerTest extends MockitoTestSuite{
                ParameterizedTypeReference<List<WidgetCatalog>> typeRef = new ParameterizedTypeReference<List<WidgetCatalog>>() {
                };
                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<String> expectedportalRestResponse = new PortalRestResponse<String>();
+               PortalRestResponse<String> expectedportalRestResponse = new PortalRestResponse<>();
                expectedportalRestResponse.setMessage("SUCCESS");
                expectedportalRestResponse.setResponse("");
-               PortalRestStatusEnum portalRestStatusEnum = null;
-               expectedportalRestResponse.setStatus(portalRestStatusEnum.OK);
-               List<WidgetCatalog> List = new ArrayList<WidgetCatalog>();
+               expectedportalRestResponse.setStatus(PortalRestStatusEnum.OK);
+               List<WidgetCatalog> 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<List<WidgetCatalog>> typeRef = new ParameterizedTypeReference<List<WidgetCatalog>>() {
                };
                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<String> actuaPportalRestResponse = microserviceController.deleteMicroservice(mockedRequest,