From 73cf89e10ba0d50c119cbd82b3aa4f46154c4b9f Mon Sep 17 00:00:00 2001 From: Dominik Mizyn Date: Thu, 30 May 2019 15:29:24 +0200 Subject: [PATCH] XSS Vulnerability fix in AppsControllerExternalRequest @SafeHtml annotation is used to fix this problem. This patch also fix some minor issues: * isAuxRESTfulCall() method delete. Method was nowhere used. * '.length() == 0' changed to '.isEmpty()' Issue-ID: PORTAL-604 Change-Id: Ib7091622081f507812654b50275ad7ac4c97bfc3 Signed-off-by: Dominik Mizyn --- .../controller/AppsControllerExternalRequest.java | 49 ++++++++++--- .../onap/portalapp/portal/domain/AppContactUs.java | 6 ++ .../org/onap/portalapp/portal/domain/EPApp.java | 19 ++++- .../org/onap/portalapp/portal/domain/EPRole.java | 5 +- .../org/onap/portalapp/portal/domain/EPUser.java | 42 +++++++++++- .../onap/portalapp/portal/domain/EPUserApp.java | 3 + .../portalapp/portal/transport/OnboardingApp.java | 36 +++++----- .../AppsControllerExternalRequestTest.java | 80 ++++++++++++++++++++++ 8 files changed, 210 insertions(+), 30 deletions(-) diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/AppsControllerExternalRequest.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/AppsControllerExternalRequest.java index cef5fa74..fe029e0e 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/AppsControllerExternalRequest.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/AppsControllerExternalRequest.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.portal.domain.EPApp; import org.onap.portalapp.portal.domain.EPUser; import org.onap.portalapp.portal.ecomp.model.PortalRestResponse; @@ -88,16 +94,12 @@ import io.swagger.annotations.ApiOperation; @EnableAspectJAutoProxy @EPAuditLog public class AppsControllerExternalRequest implements BasicAuthenticationController { + private static final ValidatorFactory VALIDATOR_FACTORY = Validation.buildDefaultValidatorFactory(); private EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(AppsControllerExternalRequest.class); private static final String ONBOARD_APP = "/onboardApp"; - // Where is this used? - public boolean isAuxRESTfulCall() { - return true; - } - /** * For testing whether a user is a superadmin. */ @@ -145,10 +147,20 @@ public class AppsControllerExternalRequest implements BasicAuthenticationControl @RequestMapping(value = "/portalAdmin", method = RequestMethod.POST, produces = "application/json") @ResponseBody public PortalRestResponse postPortalAdmin(HttpServletRequest request, HttpServletResponse response, - @RequestBody EPUser epUser) { + @Valid @RequestBody EPUser epUser) { EcompPortalUtils.logAndSerializeObject(logger, "postPortalAdmin", "request", epUser); PortalRestResponse portalResponse = new PortalRestResponse<>(); + if (epUser!=null){ + Validator validator = VALIDATOR_FACTORY.getValidator(); + Set> constraintViolations = validator.validate(epUser); + if (!constraintViolations.isEmpty()){ + portalResponse.setStatus(PortalRestStatusEnum.ERROR); + portalResponse.setMessage("Data is not valid"); + return portalResponse; + } + } + // Check mandatory fields. if (epUser.getEmail() == null || epUser.getEmail().trim().length() == 0 // || epUser.getLoginId() == null || epUser.getLoginId().trim().length() == 0 // @@ -248,10 +260,18 @@ public class AppsControllerExternalRequest implements BasicAuthenticationControl @RequestMapping(value = { ONBOARD_APP }, method = RequestMethod.POST, produces = "application/json") @ResponseBody public PortalRestResponse postOnboardAppExternal(HttpServletRequest request, HttpServletResponse response, - @RequestBody OnboardingApp newOnboardApp) { + @Valid @RequestBody OnboardingApp newOnboardApp) { EcompPortalUtils.logAndSerializeObject(logger, "postOnboardAppExternal", "request", newOnboardApp); PortalRestResponse portalResponse = new PortalRestResponse<>(); - + if (newOnboardApp != null){ + Validator validator = VALIDATOR_FACTORY.getValidator(); + Set> constraintViolations = validator.validate(newOnboardApp); + if (!constraintViolations.isEmpty()){ + portalResponse.setStatus(PortalRestStatusEnum.ERROR); + portalResponse.setMessage("Data is not valid"); + return portalResponse; + } + } // Validate fields if (newOnboardApp.id != null) { portalResponse.setStatus(PortalRestStatusEnum.ERROR); @@ -335,9 +355,20 @@ public class AppsControllerExternalRequest implements BasicAuthenticationControl @RequestMapping(value = { ONBOARD_APP + "/{appId}" }, method = RequestMethod.PUT, produces = "application/json") @ResponseBody public PortalRestResponse putOnboardAppExternal(HttpServletRequest request, HttpServletResponse response, - @PathVariable("appId") Long appId, @RequestBody OnboardingApp oldOnboardApp) { + @PathVariable("appId") Long appId, @Valid @RequestBody OnboardingApp oldOnboardApp) { EcompPortalUtils.logAndSerializeObject(logger, "putOnboardAppExternal", "request", oldOnboardApp); PortalRestResponse portalResponse = new PortalRestResponse<>(); + + if (oldOnboardApp != null){ + Validator validator = VALIDATOR_FACTORY.getValidator(); + Set> constraintViolations = validator.validate(oldOnboardApp); + if (!constraintViolations.isEmpty()){ + portalResponse.setStatus(PortalRestStatusEnum.ERROR); + portalResponse.setMessage("Data is not valid"); + return portalResponse; + } + } + // Validate fields. if (oldOnboardApp.id == null || !appId.equals(oldOnboardApp.id)) { portalResponse.setStatus(PortalRestStatusEnum.ERROR); diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/AppContactUs.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/AppContactUs.java index 6cf2ea79..0fe8a351 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/AppContactUs.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/AppContactUs.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; import com.fasterxml.jackson.annotation.JsonBackReference; @@ -46,10 +47,15 @@ public class AppContactUs extends DomainVo { private static final long serialVersionUID = -2742197830465055134L; @JsonBackReference private EPApp app; + @SafeHtml private String description; + @SafeHtml private String contactEmail; + @SafeHtml private String contactName; + @SafeHtml private String url; + @SafeHtml private String activeYN; public EPApp getApp() { diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPApp.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPApp.java index 6e77e747..8227d9ab 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPApp.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPApp.java @@ -41,7 +41,9 @@ import java.util.Arrays; import javax.persistence.Lob; +import javax.validation.Valid; import org.apache.commons.lang.StringUtils; +import org.hibernate.validator.constraints.SafeHtml; import org.onap.portalsdk.core.domain.support.DomainVo; /** @@ -50,29 +52,44 @@ import org.onap.portalsdk.core.domain.support.DomainVo; public class EPApp extends DomainVo { private static final long serialVersionUID = 1L; - + @SafeHtml private String name; + @SafeHtml private String imageUrl; + @SafeHtml private String description; + @SafeHtml private String notes; + @SafeHtml private String url; + @SafeHtml private String alternateUrl; + @SafeHtml private String appRestEndpoint; + @SafeHtml private String mlAppName; + @SafeHtml private String mlAppAdminId; private Long motsId; + @SafeHtml private String username; + @SafeHtml private String appPassword; @Lob private byte[] thumbnail; private Boolean open; private Boolean enabled; + @SafeHtml private String uebTopicName; + @SafeHtml private String uebKey; + @SafeHtml private String uebSecret; private Integer appType; + @Valid private AppContactUs contactUs; private Boolean centralAuth; + @SafeHtml private String nameSpace; public EPApp() { diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPRole.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPRole.java index f9ff97d1..55f7e0cc 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPRole.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPRole.java @@ -41,6 +41,8 @@ import java.util.Iterator; import java.util.SortedSet; import java.util.TreeSet; +import javax.validation.Valid; +import org.hibernate.validator.constraints.SafeHtml; import org.onap.portalsdk.core.domain.RoleFunction; import org.onap.portalsdk.core.domain.support.DomainVo; import com.fasterxml.jackson.annotation.JsonIgnore; @@ -48,6 +50,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore; public class EPRole extends DomainVo { private static final long serialVersionUID = 1L; + @SafeHtml private String name; private boolean active; private Integer priority; @@ -57,7 +60,7 @@ public class EPRole extends DomainVo { private Long appRoleId; // used by ONAP only private SortedSet roleFunctions = new TreeSet(); - + @Valid private SortedSet childRoles = new TreeSet(); @JsonIgnore diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPUser.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPUser.java index ce7495f7..dff5601b 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPUser.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPUser.java @@ -42,6 +42,8 @@ import java.util.Iterator; import java.util.SortedSet; import java.util.TreeSet; +import javax.validation.Valid; +import org.hibernate.validator.constraints.SafeHtml; import org.onap.portalapp.portal.utils.PortalConstants; import org.onap.portalsdk.core.domain.User; import org.onap.portalsdk.core.logging.logic.EELFLoggerDelegate; @@ -52,44 +54,78 @@ public class EPUser extends User { private Long orgId; private Long managerId; + @SafeHtml private String firstName; + @SafeHtml private String middleInitial; + @SafeHtml private String lastName; + @SafeHtml private String phone; + @SafeHtml private String fax; + @SafeHtml private String cellular; + @SafeHtml private String email; private Long addressId; + @SafeHtml private String alertMethodCd; + @SafeHtml private String hrid; + @SafeHtml private String orgUserId; + @SafeHtml private String orgCode; + @SafeHtml private String address1; + @SafeHtml private String address2; + @SafeHtml private String city; + @SafeHtml private String state; + @SafeHtml private String zipCode; + @SafeHtml private String country; + @SafeHtml private String orgManagerUserId; + @SafeHtml private String locationClli; + @SafeHtml private String businessCountryCode; + @SafeHtml private String businessCountryName; + @SafeHtml private String businessUnit; + @SafeHtml private String businessUnitName; + @SafeHtml private String department; + @SafeHtml private String departmentName; + @SafeHtml private String companyCode; + @SafeHtml private String company; + @SafeHtml private String zipCodeSuffix; + @SafeHtml private String jobTitle; + @SafeHtml private String commandChain; + @SafeHtml private String siloStatus; + @SafeHtml private String costCenter; + @SafeHtml private String financialLocCode; - + @SafeHtml private String loginId; + @SafeHtml private String loginPwd; private Date lastLoginDate; private boolean active; @@ -97,6 +133,7 @@ public class EPUser extends User { private Long selectedProfileId; private Long timeZoneId; private boolean online; + @SafeHtml private String chatId; private Integer languageId; private static final long serialVersionUID = 1L; @@ -104,8 +141,9 @@ public class EPUser extends User { private static EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(EPUser.class); private static final String ECOMP_PORTAL_NAME = "ECOMP"; private boolean isGuest = false; - + @Valid private SortedSet userApps = new TreeSet(); + @Valid private SortedSet pseudoRoles = new TreeSet(); public EPUser() {} diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPUserApp.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPUserApp.java index 3470a9e3..424a9152 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPUserApp.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/domain/EPUserApp.java @@ -37,6 +37,7 @@ */ package org.onap.portalapp.portal.domain; +import javax.validation.Valid; import org.onap.portalsdk.core.domain.support.DomainVo; @SuppressWarnings("rawtypes") @@ -45,7 +46,9 @@ public class EPUserApp extends DomainVo implements java.io.Serializable, Compara private static final long serialVersionUID = 1L; private Long userId; + @Valid private EPApp app; + @Valid private EPRole role; private Integer priority; diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/OnboardingApp.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/OnboardingApp.java index f2503b42..37ad5add 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/OnboardingApp.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/OnboardingApp.java @@ -37,6 +37,8 @@ */ package org.onap.portalapp.portal.transport; +import org.hibernate.validator.constraints.SafeHtml; + /** * Model of rows in the fn_app table; serialized as a message add or update an * on-boarded application. @@ -44,21 +46,21 @@ package org.onap.portalapp.portal.transport; public class OnboardingApp { public Long id; - + @SafeHtml public String name; - + @SafeHtml public String imageUrl; - + @SafeHtml public String imageLink; - + @SafeHtml public String description; - + @SafeHtml public String notes; - + @SafeHtml public String url; - + @SafeHtml public String alternateUrl; - + @SafeHtml public String restUrl; public Boolean isOpen; @@ -66,27 +68,27 @@ public class OnboardingApp { public Boolean isEnabled; public Long motsId; - + @SafeHtml public String myLoginsAppName; - + @SafeHtml public String myLoginsAppOwner; - + @SafeHtml public String username; - + @SafeHtml public String appPassword; - + @SafeHtml public String thumbnail; - + @SafeHtml public String uebTopicName; - + @SafeHtml public String uebKey; - + @SafeHtml public String uebSecret; public Boolean restrictedApp; public Boolean isCentralAuth; - + @SafeHtml public String nameSpace; /** diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/AppsControllerExternalRequestTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/AppsControllerExternalRequestTest.java index 847d4744..9d3c7785 100644 --- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/AppsControllerExternalRequestTest.java +++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/AppsControllerExternalRequestTest.java @@ -132,6 +132,24 @@ public class AppsControllerExternalRequestTest extends MockitoTestSuite { assertEquals(actualPortalRestResponse, expectedportalRestResponse); } + @Test + public void postPortalAdminXSSTest() { + PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); + expectedportalRestResponse.setMessage("Data is not valid"); + expectedportalRestResponse.setResponse(null); + PortalRestStatusEnum portalRestStatusEnum = null; + expectedportalRestResponse.setStatus(portalRestStatusEnum.ERROR); + EPUser user = mockUser.mockEPUser(); + user.setEmail("“>"); + user.setLoginPwd("pwd"); + user.setLoginId("Test"); + Mockito.when(EPUserUtils.getUserSession(mockedRequest)).thenReturn(user); + Mockito.when(userService.getUserByUserId(user.getOrgUserId())).thenThrow(nullPointerException); + PortalRestResponse actualPortalRestResponse = appsControllerExternalRequest + .postPortalAdmin(mockedRequest, mockedResponse, user); + assertEquals(expectedportalRestResponse, actualPortalRestResponse); + } + @Test public void postPortalAdminCreateUserIfNotFoundTest() throws Exception { PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); @@ -276,6 +294,36 @@ public class AppsControllerExternalRequestTest extends MockitoTestSuite { } + @Test + public void postOnboardAppExternalXSSTest() { + PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); + expectedportalRestResponse.setMessage( + "Data is not valid"); + expectedportalRestResponse.setResponse(null); + PortalRestStatusEnum portalRestStatusEnum = null; + expectedportalRestResponse.setStatus(portalRestStatusEnum.ERROR); + + OnboardingApp expectedOnboardingApp = new OnboardingApp();; + expectedOnboardingApp.name = "test"; + expectedOnboardingApp.url="test.com"; + expectedOnboardingApp.restUrl=""; + expectedOnboardingApp.myLoginsAppOwner="testUser"; + expectedOnboardingApp.restrictedApp=false; + expectedOnboardingApp.isOpen=true; + expectedOnboardingApp.isEnabled=true; + EPUser user = mockUser.mockEPUser(); + user.setEmail("guestT@test.portal.onap.org"); + user.setLoginPwd("pwd"); + user.setLoginId("Test"); + List expectedList = new ArrayList(); + expectedList.add(user); + + PortalRestResponse actualPortalRestResponse = appsControllerExternalRequest + .postOnboardAppExternal(mockedRequest, mockedResponse, expectedOnboardingApp); + assertEquals(expectedportalRestResponse, actualPortalRestResponse); + + } + @Test public void putOnboardAppExternalifAppNullTest() { PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); @@ -292,6 +340,38 @@ public class AppsControllerExternalRequestTest extends MockitoTestSuite { assertEquals(actualPortalRestResponse, expectedportalRestResponse); } + @Test + public void putOnboardAppExternalXSSTest() { + PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); + expectedportalRestResponse.setMessage( + "Data is not valid"); + expectedportalRestResponse.setResponse(null); + PortalRestStatusEnum portalRestStatusEnum = null; + expectedportalRestResponse.setStatus(portalRestStatusEnum.ERROR); + + OnboardingApp expectedOnboardingApp = new OnboardingApp();; + expectedOnboardingApp.name = "test"; + expectedOnboardingApp.url="test.com"; + expectedOnboardingApp.restUrl=""; + expectedOnboardingApp.myLoginsAppOwner="testUser"; + expectedOnboardingApp.restrictedApp=false; + expectedOnboardingApp.isOpen=true; + expectedOnboardingApp.isEnabled=true; + EPUser user = mockUser.mockEPUser(); + user.setEmail("guestT@test.portal.onap.org"); + user.setLoginPwd("pwd"); + user.setLoginId("Test"); + List expectedList = new ArrayList(); + expectedList.add(user); + + Long appId = (long) 1; + + PortalRestResponse actualPortalRestResponse = appsControllerExternalRequest + .putOnboardAppExternal(mockedRequest, mockedResponse, appId, expectedOnboardingApp); + assertEquals(expectedportalRestResponse, actualPortalRestResponse); + + } + @Test public void putOnboardAppExternalIfOnboardingAppDetailsNullTest() { PortalRestResponse expectedportalRestResponse = new PortalRestResponse(); -- 2.16.6