XSS Vulnerability fix in PortalAdminController

Custom data validator used to fix this issue.

Issue-ID: OJSI-15
Change-Id: I224887d31e4e2d7301544194ef44ba38e66e047d
Signed-off-by: Dominik Mizyn <d.mizyn@samsung.com>

diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/PortalAdminController.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/PortalAdminController.java
index 1186f44..32b28c7 100644 (file)
--- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/PortalAdminController.java
+++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/PortalAdminController.java
@@ -56,12 +56,15 @@ import org.onap.portalapp.portal.transport.PortalAdmin;
 import org.onap.portalapp.portal.utils.EPCommonSystemProperties;
 import org.onap.portalapp.portal.utils.EcompPortalUtils;
 import org.onap.portalapp.util.EPUserUtils;
+import org.onap.portalapp.validation.DataValidator;
+import org.onap.portalapp.validation.SecureString;
 import org.onap.portalsdk.core.domain.AuditLog;
 import org.onap.portalsdk.core.logging.logic.EELFLoggerDelegate;
 import org.onap.portalsdk.core.service.AuditService;
 import org.onap.portalsdk.core.util.SystemProperties;
 import org.slf4j.MDC;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.EnableAspectJAutoProxy;
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.RequestBody;
@@ -70,18 +73,24 @@ import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.RestController;
 
 @RestController
-@org.springframework.context.annotation.Configuration
+@Configuration
 @EnableAspectJAutoProxy
 @EPAuditLog
 public class PortalAdminController extends EPRestrictedBaseController {
-       @Autowired
-       PortalAdminService portalAdminService;
-       @Autowired
-       AdminRolesService adminRolesService;
-       @Autowired
-       AuditService auditService;
+       private static final EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(PortalAdminController.class);
+       private static final DataValidator DATA_VALIDATOR = new DataValidator();
 
-       EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(PortalAdminController.class);
+       private PortalAdminService portalAdminService;
+       private AdminRolesService adminRolesService;
+       private AuditService auditService;
+
+       @Autowired
+       public PortalAdminController(PortalAdminService portalAdminService,
+               AdminRolesService adminRolesService, AuditService auditService){
+               this.portalAdminService = portalAdminService;
+               this.adminRolesService = adminRolesService;
+               this.auditService = auditService;
+       }
 
        @RequestMapping(value = { "/portalApi/portalAdmins" }, method = RequestMethod.GET, produces = "application/json")
        public List<PortalAdmin> getPortalAdmins(HttpServletRequest request, HttpServletResponse response) {
@@ -116,7 +125,10 @@ public class PortalAdminController extends EPRestrictedBaseController {
                        HttpServletResponse response) {
                EPUser user = EPUserUtils.getUserSession(request);
                FieldsValidator fieldsValidator = null;
-               if (user == null) {
+               if(!DATA_VALIDATOR.isValid(new SecureString(userId))){
+                       logger.debug(EELFLoggerDelegate.debugLogger, "PortalAdminController.createPortalAdmin not valid userId");
+                       EcompPortalUtils.setBadPermissions(user, response, "createPortalAdmin");
+               }else if (user == null) {
                        logger.debug(EELFLoggerDelegate.debugLogger, "PortalAdminController.createPortalAdmin, null user");
                        EcompPortalUtils.setBadPermissions(user, response, "createPortalAdmin");
                } else if (!adminRolesService.isSuperAdmin(user)) {
@@ -158,6 +170,12 @@ public class PortalAdminController extends EPRestrictedBaseController {
        @RequestMapping(value = { "/portalApi/portalAdmin/{userInfo}" }, method = RequestMethod.DELETE)
        public FieldsValidator deletePortalAdmin(HttpServletRequest request, @PathVariable("userInfo") String userInfo,
                        HttpServletResponse response) {
+
+               if(!DATA_VALIDATOR.isValid(new SecureString(userInfo))){
+                       logger.debug(EELFLoggerDelegate.debugLogger, "PortalAdminController.deletePortalAdmin not valid userId");
+                       return null;
+               }
+
                int userIdIdx = userInfo.indexOf("-");
                Long userId = null;
                String sbcid = null;

diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/PortalAdminControllerTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/PortalAdminControllerTest.java
index 20bb3e8..bd8d155 100644 (file)
--- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/PortalAdminControllerTest.java
+++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/PortalAdminControllerTest.java
@@ -42,22 +42,17 @@ import static org.junit.Assert.assertNull;
 
 import java.util.ArrayList;
 import java.util.List;
-
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.InjectMocks;
-import org.mockito.Matchers;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
-import org.onap.portalapp.portal.controller.PortalAdminController;
 import org.onap.portalapp.portal.core.MockEPUser;
 import org.onap.portalapp.portal.domain.EPRole;
 import org.onap.portalapp.portal.domain.EPUser;
-import org.onap.portalapp.portal.exceptions.NoHealthyServiceException;
 import org.onap.portalapp.portal.framework.MockitoTestSuite;
 import org.onap.portalapp.portal.service.AdminRolesService;
 import org.onap.portalapp.portal.service.AdminRolesServiceImpl;
@@ -73,7 +68,7 @@ import org.onap.portalsdk.core.service.AuditServiceImpl;
 public class PortalAdminControllerTest extends MockitoTestSuite{
 
        @InjectMocks
-       PortalAdminController portalAdminController = new PortalAdminController();
+       PortalAdminController portalAdminController;
 
        @Mock
        AdminRolesService adminRolesService = new AdminRolesServiceImpl();
@@ -168,9 +163,22 @@ public class PortalAdminControllerTest extends MockitoTestSuite{
         assertEquals(actualFieldValidator,expectedFieldValidator);
 
        }
-       
 
-               
+       @Test
+       public void createPortalAdminXSSTest()
+       {
+               EPUser user = mockUser.mockEPUser();
+               Mockito.when(EPUserUtils.getUserSession(mockedRequest)).thenReturn(user);
+               FieldsValidator expectedFieldValidator = null;
+               FieldsValidator actualFieldValidator;
+               String userId = "<IMG SRC=jAVasCrIPt:alert(‘XSS’)>";
+               Mockito.when(adminRolesService.isSuperAdmin(user)).thenReturn(true);
+               Mockito.when(portalAdminService.createPortalAdmin(userId)).thenReturn(expectedFieldValidator);
+               actualFieldValidator = portalAdminController.createPortalAdmin(mockedRequest, userId, mockedResponse);
+               assertEquals(expectedFieldValidator, actualFieldValidator);
+
+       }
+
        @Test
        public void createPortalAdminIfUserIsNullTest()
        {
@@ -204,6 +212,17 @@ public class PortalAdminControllerTest extends MockitoTestSuite{
          assertNull(actualPortalAdminsList);
 
        }
+
+       @Test
+       public void deletePortalAdminXSSTest()
+       {
+               EPUser user = mockUser.mockEPUser();
+               Mockito.when(EPUserUtils.getUserSession(mockedRequest)).thenReturn(user);
+               Mockito.when(adminRolesService.isSuperAdmin(user)).thenReturn(true);
+               FieldsValidator actualFieldValidator = portalAdminController.deletePortalAdmin(mockedRequest,"<img src=xss onerror=alert(1)>" , mockedResponse);
+               assertNull(actualFieldValidator);
+
+       }
        
        @Test
        public void deletePortalAdminTest1()