From 604bf4f45cf1f1726f1b8129963627ffb90b5f4c Mon Sep 17 00:00:00 2001 From: Dominik Mizyn Date: Mon, 21 Oct 2019 13:46:35 +0200 Subject: [PATCH] Persistent XSS vulnerability in basicAuthAccount form fix javax.validation.Validator used to fix this vulnerability issue. Issue-ID: OJSI-20 Change-Id: I2e8188d9dabf634fcaf41b8d42d0f7160cc0886d Signed-off-by: Dominik Mizyn --- .../controller/BasicAuthAccountController.java | 17 ++++++++++++++++- .../portal/domain/BasicAuthCredentials.java | 11 +++++++---- .../onap/portalapp/portal/domain/EPEndpoint.java | 2 ++ .../service/BasicAuthAccountServiceImpl.java | 7 ++++++- .../controller/BasicAuthAccountControllerTest.java | 22 ++++++++++++++++++++++ .../service/BasicAuthAccountServiceImplTest.java | 9 +++++++++ 6 files changed, 62 insertions(+), 6 deletions(-) diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/BasicAuthAccountController.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/BasicAuthAccountController.java index 9024570c..f655d352 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/BasicAuthAccountController.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/BasicAuthAccountController.java @@ -53,6 +53,7 @@ import org.onap.portalapp.portal.logging.aop.EPAuditLog; import org.onap.portalapp.portal.service.AdminRolesService; import org.onap.portalapp.portal.service.BasicAuthAccountService; import org.onap.portalapp.util.EPUserUtils; +import org.onap.portalapp.validation.DataValidator; import org.onap.portalsdk.core.logging.logic.EELFLoggerDelegate; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.EnableAspectJAutoProxy; @@ -74,6 +75,7 @@ public class BasicAuthAccountController extends EPRestrictedBaseController { private static final String ADMIN_ONLY_OPERATIONS = "Admin Only Operation! "; private static final EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(BasicAuthAccountController.class); + private final DataValidator dataValidator = new DataValidator(); @Autowired private BasicAuthAccountService basicAuthAccountService; @@ -98,6 +100,8 @@ public class BasicAuthAccountController extends EPRestrictedBaseController { public PortalRestResponse createBasicAuthAccount(HttpServletRequest request, HttpServletResponse response, @RequestBody BasicAuthCredentials newBasicAuthAccount) throws Exception { + + EPUser user = EPUserUtils.getUserSession(request); if (!adminRolesService.isSuperAdmin(user)) { return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, AUTHORIZATION_REQUIRED, @@ -108,7 +112,18 @@ public class BasicAuthAccountController extends EPRestrictedBaseController { return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, FAILURE, "newBasicAuthAccount cannot be null or empty"); } - long accountId = basicAuthAccountService.saveBasicAuthAccount(newBasicAuthAccount); + + if(!dataValidator.isValid(newBasicAuthAccount)){ + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "createBasicAuthAccount() failed, new credential are not safe", + ""); + } + + long accountId; + try { + accountId = basicAuthAccountService.saveBasicAuthAccount(newBasicAuthAccount); + } catch (Exception e){ + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, FAILURE, e.getMessage()); + } List endpointIdList = new ArrayList<>(); try { diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/BasicAuthCredentials.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/BasicAuthCredentials.java index f0e93bcb..6d8a3f87 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/BasicAuthCredentials.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/BasicAuthCredentials.java @@ -39,21 +39,24 @@ package org.onap.portalapp.portal.domain; import java.util.List; +import javax.validation.Valid; +import org.hibernate.validator.constraints.SafeHtml; import org.onap.portalsdk.core.domain.support.DomainVo; public class BasicAuthCredentials extends DomainVo { private static final long serialVersionUID = 1L; - public BasicAuthCredentials() { - - } - private Long id; + @SafeHtml private String applicationName; + @SafeHtml private String username; + @SafeHtml private String password; + @SafeHtml private String isActive; + @Valid private List endpoints; public Long getId() { diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPEndpoint.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPEndpoint.java index 92c8572b..97ecbcbe 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPEndpoint.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPEndpoint.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 EPEndpoint extends DomainVo { @@ -48,6 +49,7 @@ public class EPEndpoint extends DomainVo { } private Long id; + @SafeHtml private String name; public Long getId() { diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/service/BasicAuthAccountServiceImpl.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/service/BasicAuthAccountServiceImpl.java index 74cf1726..98b0f127 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/service/BasicAuthAccountServiceImpl.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/service/BasicAuthAccountServiceImpl.java @@ -49,6 +49,7 @@ import org.onap.portalapp.portal.domain.EPEndpoint; import org.onap.portalapp.portal.domain.EPEndpointAccount; import org.onap.portalapp.portal.logging.aop.EPMetricsLog; import org.onap.portalapp.portal.utils.EPCommonSystemProperties; +import org.onap.portalapp.validation.DataValidator; import org.onap.portalsdk.core.logging.logic.EELFLoggerDelegate; import org.onap.portalsdk.core.onboarding.util.CipherUtil; import org.onap.portalsdk.core.service.DataAccessService; @@ -62,12 +63,16 @@ import org.springframework.stereotype.Service; @EPMetricsLog public class BasicAuthAccountServiceImpl implements BasicAuthAccountService{ EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(MicroserviceServiceImpl.class); - + private final DataValidator dataValidator = new DataValidator(); @Autowired private DataAccessService dataAccessService; @Override public Long saveBasicAuthAccount(BasicAuthCredentials newCredential) throws Exception { + + if(!dataValidator.isValid(newCredential)){ + throw new Exception("saveBasicAuthAccount() failed, new credential are not safe"); + } if (newCredential.getPassword() != null) newCredential.setPassword(encryptedPassword(newCredential.getPassword())); try{ diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/BasicAuthAccountControllerTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/BasicAuthAccountControllerTest.java index c9d3c2fd..ff056d0d 100644 --- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/BasicAuthAccountControllerTest.java +++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/BasicAuthAccountControllerTest.java @@ -134,6 +134,28 @@ public class BasicAuthAccountControllerTest extends MockitoTestSuite { assertEquals(actualResponse, expectedResponse); } + @Test + public void createBasicAuthAccountXSSTest() throws Exception { + BasicAuthCredentials basicAuthCredentials = basicAuthCredentials(); + basicAuthCredentials.setPassword(""); + + EPUser user = mockUser.mockEPUser(); + Mockito.when(EPUserUtils.getUserSession(mockedRequest)).thenReturn(user); + Mockito.when(adminRolesService.isSuperAdmin(user)).thenReturn(true); + PortalRestResponse expectedResponse = new PortalRestResponse(); + expectedResponse.setMessage("createBasicAuthAccount() failed, new credential are not safe"); + expectedResponse.setResponse(""); + PortalRestStatusEnum portalRestStatusEnum = null; + expectedResponse.setStatus(portalRestStatusEnum.ERROR); + long accountd = 1; + + Mockito.when(basicAuthAccountService.saveBasicAuthAccount(basicAuthCredentials)).thenReturn(accountd); + + PortalRestResponse actualResponse = basicAuthAccountController.createBasicAuthAccount(mockedRequest, + mockedResponse, basicAuthCredentials); + assertEquals(actualResponse, expectedResponse); + } + @Test public void createBasicAuthAccountAdminTest() throws Exception { BasicAuthCredentials basicAuthCredentials = basicAuthCredentials(); diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/service/BasicAuthAccountServiceImplTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/service/BasicAuthAccountServiceImplTest.java index 4409a4fc..6382bef4 100644 --- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/service/BasicAuthAccountServiceImplTest.java +++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/service/BasicAuthAccountServiceImplTest.java @@ -78,6 +78,15 @@ public class BasicAuthAccountServiceImplTest { Mockito.doNothing().when(dataAccessService).saveDomainObject(basicAuthCredentials, null); basicAuthAccountServiceImpl.saveBasicAuthAccount(basicAuthCredentials); + } + + @Test(expected= Exception.class) + public void saveBasicAuthAccountValidTest() throws Exception { + BasicAuthCredentials basicAuthCredentials = new BasicAuthCredentials(); + basicAuthCredentials.setPassword(""); + Mockito.doNothing().when(dataAccessService).saveDomainObject(basicAuthCredentials, null); + basicAuthAccountServiceImpl.saveBasicAuthAccount(basicAuthCredentials); + } @Test -- 2.16.6