diff options
4 files changed, 161 insertions, 77 deletions
diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/DashboardController.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/DashboardController.java index 727d190d..6137aec9 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/DashboardController.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/DashboardController.java @@ -66,6 +66,8 @@ import org.onap.portalapp.portal.utils.EPCommonSystemProperties; import org.onap.portalapp.portal.utils.EcompPortalUtils; import org.onap.portalapp.portal.utils.PortalConstants; 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.domain.support.CollaborateList; import org.onap.portalsdk.core.logging.logic.EELFLoggerDelegate; @@ -87,19 +89,23 @@ import org.springframework.web.bind.annotation.RestController; @RestController @RequestMapping("/portalApi/dashboard") public class DashboardController extends EPRestrictedBaseController { + private static final DataValidator DATA_VALIDATOR = new DataValidator(); + private static final EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(DashboardController.class); - private static EELFLoggerDelegate logger = EELFLoggerDelegate.getLogger(DashboardController.class); - - @Autowired private DashboardSearchService searchService; - @Autowired private AuditService auditService; - - @Autowired private AdminRolesService adminRolesService; - + + @Autowired + public DashboardController(DashboardSearchService searchService, + AuditService auditService, AdminRolesService adminRolesService) { + this.searchService = searchService; + this.auditService = auditService; + this.adminRolesService = adminRolesService; + } + public enum WidgetCategory { - EVENTS, NEWS, IMPORTANTRESOURCES; + EVENTS, NEWS, IMPORTANTRESOURCES } /** @@ -129,11 +135,15 @@ public class DashboardController extends EPRestrictedBaseController { @RequestMapping(value = "/widgetData", method = RequestMethod.GET, produces = "application/json") public PortalRestResponse<CommonWidgetMeta> getWidgetData(HttpServletRequest request, @RequestParam String resourceType) { - if (!isValidResourceType(resourceType)) - return new PortalRestResponse<CommonWidgetMeta>(PortalRestStatusEnum.ERROR, - "Unexpected resource type " + resourceType, null); - return new PortalRestResponse<CommonWidgetMeta>(PortalRestStatusEnum.OK, "success", - searchService.getWidgetData(resourceType)); + if (!isValidResourceType(resourceType)) { + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, + "Unexpected resource type " + resourceType, null); + }else if (!DATA_VALIDATOR.isValid(new SecureString(resourceType))){ + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, + "Unsafe resource type " + resourceType, null); + } + return new PortalRestResponse<>(PortalRestStatusEnum.OK, "success", + searchService.getWidgetData(resourceType)); } @@ -147,20 +157,23 @@ public class DashboardController extends EPRestrictedBaseController { @RequestMapping(value = "/widgetDataBulk", method = RequestMethod.POST, produces = "application/json") public PortalRestResponse<String> saveWidgetDataBulk(@RequestBody CommonWidgetMeta commonWidgetMeta) { logger.debug(EELFLoggerDelegate.debugLogger, "saveWidgetDataBulk: argument is {}", commonWidgetMeta); - if (commonWidgetMeta.getCategory() == null || commonWidgetMeta.getCategory().trim().equals("")) - return new PortalRestResponse<String>(PortalRestStatusEnum.ERROR, "ERROR", - "Category cannot be null or empty"); - if (!isValidResourceType(commonWidgetMeta.getCategory())) - return new PortalRestResponse<String>(PortalRestStatusEnum.ERROR, - "Unexpected resource type " + commonWidgetMeta.getCategory(), null); - // validate dates + if (!DATA_VALIDATOR.isValid(commonWidgetMeta)){ + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, + "Unsafe resource type " + commonWidgetMeta, "ERROR"); + }else if (commonWidgetMeta.getCategory() == null || commonWidgetMeta.getCategory().trim().equals("")) { + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "ERROR", + "Category cannot be null or empty"); + }else if (!isValidResourceType(commonWidgetMeta.getCategory())) { + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, + "Unexpected resource type " + commonWidgetMeta.getCategory(), null); + } for (CommonWidget cw : commonWidgetMeta.getItems()) { String err = validateCommonWidget(cw); if (err != null) - return new PortalRestResponse<String>(PortalRestStatusEnum.ERROR, err, null); + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, err, null); } - return new PortalRestResponse<String>(PortalRestStatusEnum.OK, "success", - searchService.saveWidgetDataBulk(commonWidgetMeta)); + return new PortalRestResponse<>(PortalRestStatusEnum.OK, "success", + searchService.saveWidgetDataBulk(commonWidgetMeta)); } /** @@ -175,17 +188,21 @@ public class DashboardController extends EPRestrictedBaseController { logger.debug(EELFLoggerDelegate.debugLogger, "saveWidgetData: argument is {}", commonWidget); EPUser user = EPUserUtils.getUserSession(request); if (adminRolesService.isSuperAdmin(user)) { - if (commonWidget.getCategory() == null || commonWidget.getCategory().trim().isEmpty()) - return new PortalRestResponse<String>(PortalRestStatusEnum.ERROR, "ERROR", - "Category cannot be null or empty"); + if (commonWidget.getCategory() == null || commonWidget.getCategory().trim().isEmpty()) { + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "ERROR", + "Category cannot be null or empty"); + }else if (!DATA_VALIDATOR.isValid(commonWidget)){ + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, + "Unsafe resource type " + commonWidget, "ERROR"); + } String err = validateCommonWidget(commonWidget); if (err != null) - return new PortalRestResponse<String>(PortalRestStatusEnum.ERROR, err, null); - return new PortalRestResponse<String>(PortalRestStatusEnum.OK, "success", - searchService.saveWidgetData(commonWidget)); + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, err, null); + return new PortalRestResponse<>(PortalRestStatusEnum.OK, "success", + searchService.saveWidgetData(commonWidget)); } else { EcompPortalUtils.setBadPermissions(user, response, "saveWidgetData"); - return new PortalRestResponse<String>(PortalRestStatusEnum.ERROR, "Failed", null); + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "Failed", null); } } @@ -235,8 +252,12 @@ public class DashboardController extends EPRestrictedBaseController { @RequestMapping(value = "/deleteData", method = RequestMethod.POST, produces = "application/json") public PortalRestResponse<String> deleteWidgetData(@RequestBody CommonWidget commonWidget) { logger.debug(EELFLoggerDelegate.debugLogger, "deleteWidgetData: argument is {}", commonWidget); - return new PortalRestResponse<String>(PortalRestStatusEnum.OK, "success", - searchService.deleteWidgetData(commonWidget)); + if (!DATA_VALIDATOR.isValid(commonWidget)){ + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, + "Unsafe resource type " + commonWidget, "ERROR"); + } + return new PortalRestResponse<>(PortalRestStatusEnum.OK, "success", + searchService.deleteWidgetData(commonWidget)); } /** @@ -251,7 +272,10 @@ public class DashboardController extends EPRestrictedBaseController { @RequestMapping(value = "/search", method = RequestMethod.GET, produces = "application/json") public PortalRestResponse<Map<String, List<SearchResultItem>>> searchPortal(HttpServletRequest request, @RequestParam String searchString) { - + if (!DATA_VALIDATOR.isValid(new SecureString(searchString))){ + return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "searchPortal: String string is not safe", + new HashMap<>()); + } if (searchString != null) searchString = searchString.trim(); EPUser user = EPUserUtils.getUserSession(request); @@ -259,10 +283,10 @@ public class DashboardController extends EPRestrictedBaseController { if (user == null) { return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "searchPortal: User object is null? - check logs", - new HashMap<String, List<SearchResultItem>>()); + new HashMap<>()); } else if (searchString == null || searchString.length() == 0) { return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "searchPortal: String string is null", - new HashMap<String, List<SearchResultItem>>()); + new HashMap<>()); } else { logger.debug(EELFLoggerDelegate.debugLogger, "searchPortal: user {}, search string '{}'", user.getLoginId(), searchString); @@ -294,7 +318,7 @@ public class DashboardController extends EPRestrictedBaseController { MDC.put(EPCommonSystemProperties.STATUS_CODE, "ERROR"); MDC.remove(EPCommonSystemProperties.STATUS_CODE); return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, e.getMessage() + " - check logs.", - new HashMap<String, List<SearchResultItem>>()); + new HashMap<>()); } } @@ -308,7 +332,7 @@ public class DashboardController extends EPRestrictedBaseController { */ @RequestMapping(value = "/activeUsers", method = RequestMethod.GET, produces = "application/json") public List<String> getActiveUsers(HttpServletRequest request) { - List<String> activeUsers = null; + List<String> activeUsers; List<String> onlineUsers = new ArrayList<>(); try { EPUser user = EPUserUtils.getUserSession(request); @@ -341,7 +365,7 @@ public class DashboardController extends EPRestrictedBaseController { String updateDuration = SystemProperties.getProperty(EPCommonSystemProperties.ONLINE_USER_UPDATE_DURATION); Integer rateInMiliSec = Integer.valueOf(updateRate)*1000; Integer durationInMiliSec = Integer.valueOf(updateDuration)*1000; - Map<String, String> results = new HashMap<String,String>(); + Map<String, String> results = new HashMap<>(); results.put("onlineUserUpdateRate", String.valueOf(rateInMiliSec)); results.put("onlineUserUpdateDuration", String.valueOf(durationInMiliSec)); return new PortalRestResponse<>(PortalRestStatusEnum.OK, "success", results); @@ -362,7 +386,7 @@ public class DashboardController extends EPRestrictedBaseController { try { String windowWidthString = SystemProperties.getProperty(EPCommonSystemProperties.WINDOW_WIDTH_THRESHOLD_RIGHT_MENU); Integer windowWidth = Integer.valueOf(windowWidthString); - Map<String, String> results = new HashMap<String,String>(); + Map<String, String> results = new HashMap<>(); results.put("windowWidth", String.valueOf(windowWidth)); return new PortalRestResponse<>(PortalRestStatusEnum.OK, "success", results); } catch (Exception e) { @@ -383,7 +407,7 @@ public class DashboardController extends EPRestrictedBaseController { try { String windowWidthString = SystemProperties.getProperty(EPCommonSystemProperties.WINDOW_WIDTH_THRESHOLD_LEFT_MENU); Integer windowWidth = Integer.valueOf(windowWidthString); - Map<String, String> results = new HashMap<String,String>(); + Map<String, String> results = new HashMap<>(); results.put("windowWidth", String.valueOf(windowWidth)); return new PortalRestResponse<>(PortalRestStatusEnum.OK, "success", results); } catch (Exception e) { diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/CommonWidget.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/CommonWidget.java index 90277877..e9d720e3 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/CommonWidget.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/CommonWidget.java @@ -49,6 +49,7 @@ import javax.validation.constraints.Size; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; +import lombok.ToString; import org.hibernate.validator.constraints.SafeHtml; import org.onap.portalsdk.core.domain.support.DomainVo; import com.fasterxml.jackson.annotation.JsonInclude; @@ -62,6 +63,7 @@ import com.fasterxml.jackson.annotation.JsonInclude; @NoArgsConstructor @Getter @Setter +@ToString public class CommonWidget extends DomainVo{ private static final long serialVersionUID = 7897021982887364557L; diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/CommonWidgetMeta.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/CommonWidgetMeta.java index 51a02652..0a999495 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/CommonWidgetMeta.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/transport/CommonWidgetMeta.java @@ -39,33 +39,21 @@ package org.onap.portalapp.portal.transport; import java.util.List; import javax.validation.Valid; +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; +import lombok.ToString; import org.hibernate.validator.constraints.SafeHtml; +@NoArgsConstructor +@AllArgsConstructor +@Getter +@Setter +@ToString public class CommonWidgetMeta { @SafeHtml private String category; @Valid private List<CommonWidget> items; - - public CommonWidgetMeta(){ - - } - - public CommonWidgetMeta(String category, List<CommonWidget> items){ - this.category = category; - this.items = items; - } - - public String getCategory() { - return category; - } - public void setCategory(String category) { - this.category = category; - } - public List<CommonWidget> getItems() { - return items; - } - public void setItems(List<CommonWidget> items) { - this.items = items; - } } diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/DashboardControllerTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/DashboardControllerTest.java index 417568da..cd130e9f 100644 --- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/DashboardControllerTest.java +++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/DashboardControllerTest.java @@ -57,10 +57,8 @@ import org.mockito.Matchers; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; -import org.onap.portalapp.portal.controller.DashboardController; import org.onap.portalapp.portal.core.MockEPUser; import org.onap.portalapp.portal.domain.EPUser; -import org.onap.portalapp.portal.domain.EcompAuditLog; import org.onap.portalapp.portal.ecomp.model.PortalRestResponse; import org.onap.portalapp.portal.ecomp.model.PortalRestStatusEnum; import org.onap.portalapp.portal.ecomp.model.SearchResultItem; @@ -72,13 +70,10 @@ import org.onap.portalapp.portal.service.DashboardSearchServiceImpl; import org.onap.portalapp.portal.transport.CommonWidget; import org.onap.portalapp.portal.transport.CommonWidgetMeta; import org.onap.portalapp.portal.utils.EPCommonSystemProperties; -import org.onap.portalapp.portal.utils.EcompPortalUtils; -import org.onap.portalapp.portal.utils.PortalConstants; import org.onap.portalapp.util.EPUserUtils; import org.onap.portalsdk.core.domain.AuditLog; import org.onap.portalsdk.core.domain.support.CollaborateList; import org.onap.portalsdk.core.service.AuditService; -import org.onap.portalsdk.core.service.AuditServiceImpl; import org.onap.portalsdk.core.util.SystemProperties; import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PrepareForTest; @@ -92,12 +87,9 @@ public class DashboardControllerTest { @Mock DashboardSearchService searchService = new DashboardSearchServiceImpl(); - - /*@Mock - AuditService auditService = new AuditServiceImpl();*/ - + @InjectMocks - DashboardController dashboardController = new DashboardController(); + DashboardController dashboardController; @Mock AdminRolesService adminRolesService = new AdminRolesServiceImpl(); @@ -129,7 +121,7 @@ public class DashboardControllerTest { commonWidget.setHref("testhref"); commonWidget.setTitle("testTitle"); commonWidget.setContent("testcontent"); - commonWidget.setEventDate("testDate"); + commonWidget.setEventDate("2017-03-24"); commonWidget.setSortOrder(1); widgetList.add(commonWidget); commonWidgetMeta.setItems(widgetList); @@ -163,8 +155,21 @@ public class DashboardControllerTest { PortalRestResponse<CommonWidgetMeta> actualResponse = dashboardController.getWidgetData(mockedRequest, resourceType); assertEquals(expectedData,actualResponse); - } - + } + + @Test + public void getWidgetDataTestXSS() { + + String resourceType = "“><script>alert(“XSS”)</script>"; + PortalRestResponse<CommonWidgetMeta> expectedData = new PortalRestResponse<>(); + expectedData.setStatus(PortalRestStatusEnum.ERROR); + expectedData.setMessage("Unexpected resource type “><script>alert(“XSS”)</script>"); + expectedData.setResponse(null); + + PortalRestResponse<CommonWidgetMeta> actualResponse = dashboardController.getWidgetData(mockedRequest, resourceType); + assertEquals(expectedData, actualResponse); + } + @Test public void getWidgetDataWithValidResourceTest() throws IOException { String resourceType = "EVENTS"; @@ -194,6 +199,20 @@ public class DashboardControllerTest { PortalRestResponse<String> actualResponse = dashboardController.saveWidgetDataBulk(commonWidgetMeta); assertEquals(expectedData,actualResponse); } + + @Test + public void saveWidgetDataBulkXSSTest() { + CommonWidgetMeta commonWidgetMeta= mockCommonWidgetMeta(); + commonWidgetMeta.setCategory("<script>alert(‘XSS’)</script>"); + + PortalRestResponse<String> expectedData = new PortalRestResponse<>(); + expectedData.setStatus(PortalRestStatusEnum.ERROR); + expectedData.setResponse("ERROR"); + expectedData.setMessage("Unsafe resource type " + commonWidgetMeta.toString()); + + PortalRestResponse<String> actualResponse = dashboardController.saveWidgetDataBulk(commonWidgetMeta); + assertEquals(expectedData,actualResponse); + } @Test public void saveWidgetUnexpectedDataBulkTest() throws IOException { @@ -261,6 +280,24 @@ public class DashboardControllerTest { assertEquals(expectedData,actualResponse); } + + @Test + public void saveWidgetDataXSSTest() { + + CommonWidget commonWidget = mockCommonWidget(); + commonWidget.setId((long)1); + commonWidget.setContent("test"); + commonWidget.setCategory("<form><a href=\"javascript:\\u0061lert(1)\">X"); + PortalRestResponse<String> expectedData = new PortalRestResponse<String>(); + expectedData.setStatus(PortalRestStatusEnum.ERROR); + expectedData.setResponse("ERROR"); + expectedData.setMessage("Unsafe resource type " + commonWidget.toString()); + + Mockito.when(adminRolesService.isSuperAdmin(Matchers.anyObject())).thenReturn(true); + PortalRestResponse<String> actualResponse = dashboardController.saveWidgetData(commonWidget, mockedRequest, mockedResponse); + assertEquals(expectedData,actualResponse); + + } @Test public void saveWidgetDataTitleTest() throws IOException { @@ -268,6 +305,7 @@ public class DashboardControllerTest { commonWidget.setId((long)1); commonWidget.setContent("test"); commonWidget.setTitle("test"); + commonWidget.setEventDate("2017-05-06"); PortalRestResponse<String> expectedData = new PortalRestResponse<String>(); expectedData.setStatus(PortalRestStatusEnum.ERROR); expectedData.setMessage("Invalid category: test"); @@ -280,7 +318,8 @@ public class DashboardControllerTest { @Test public void saveWidgetDataErrorTest() throws IOException { - CommonWidget commonWidget = mockCommonWidget(); + CommonWidget commonWidget = mockCommonWidget(); + commonWidget.setEventDate("2017-03-05"); PortalRestResponse<String> expectedData = new PortalRestResponse<String>(); expectedData.setStatus(PortalRestStatusEnum.ERROR); expectedData.setMessage("Invalid category: test"); @@ -323,7 +362,7 @@ public class DashboardControllerTest { public void deleteWidgetDataTest() throws IOException { CommonWidget commonWidget = mockCommonWidget(); - + commonWidget.setEventDate("2017-03-25"); PortalRestResponse<String> expectedData = new PortalRestResponse<String>(); expectedData.setStatus(PortalRestStatusEnum.OK); expectedData.setMessage("success"); @@ -335,6 +374,20 @@ public class DashboardControllerTest { assertEquals(expectedData,actualResponse); } + + @Test + public void deleteWidgetDataXSSTest() { + + CommonWidget commonWidget = mockCommonWidget(); + commonWidget.setCategory("<svg><script x:href='https://dl.dropbox.com/u/13018058/js.js' {Opera}"); + PortalRestResponse<String> expectedData = new PortalRestResponse<>(); + expectedData.setStatus(PortalRestStatusEnum.ERROR); + expectedData.setMessage("Unsafe resource type " + commonWidget.toString()); + expectedData.setResponse("ERROR"); + PortalRestResponse<String> actualResponse = dashboardController.deleteWidgetData(commonWidget); + assertEquals(expectedData,actualResponse); + + } @Test public void getActiveUsersTest(){ @@ -541,6 +594,23 @@ public class DashboardControllerTest { PortalRestResponse<Map<String, List<SearchResultItem>>> actualResponse = dashboardController.searchPortal(mockedRequest, null); assertTrue(actualResponse.getStatus().compareTo(PortalRestStatusEnum.ERROR) == 0); } + + @Test + public void searchPortalXSSTest(){ + EPUser user = null; + String searchString = "\n" + + "<form><textarea onkeyup='\\u0061\\u006C\\u0065\\u0072\\u0074(1)'>"; + PowerMockito.mockStatic(EPUserUtils.class); + Mockito.when(EPUserUtils.getUserSession(mockedRequest)).thenReturn(user); + PortalRestResponse<Map<String, List<SearchResultItem>>> expectedResult = new PortalRestResponse<>(); + expectedResult.setMessage("searchPortal: String string is not safe"); + expectedResult.setResponse(new HashMap<>()); + expectedResult.setStatus(PortalRestStatusEnum.ERROR); + + PortalRestResponse<Map<String, List<SearchResultItem>>> actualResponse = dashboardController.searchPortal(mockedRequest, searchString); + assertEquals(expectedResult, actualResponse); + } + @Test public void searchPortalTestWithException(){ EPUser user = mockUser.mockEPUser(); |