From 6fb5b257a327c64eb3e3f8df65db835ca6cb38aa Mon Sep 17 00:00:00 2001 From: Dominik Mizyn Date: Fri, 5 Jul 2019 14:33:53 +0200 Subject: [PATCH] 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 --- .../portal/controller/PortalAdminController.java | 36 ++++++++++++++++------ .../controller/PortalAdminControllerTest.java | 35 ++++++++++++++++----- 2 files changed, 54 insertions(+), 17 deletions(-) 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 1186f444..32b28c7d 100644 --- 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 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 20bb3e8b..bd8d1551 100644 --- 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 = ""; + 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,"" , mockedResponse); + assertNull(actualFieldValidator); + + } @Test public void deletePortalAdminTest1() -- 2.16.6