From 24a865a6c97b6cade208d8ae2b61ed3ff665d668 Mon Sep 17 00:00:00 2001 From: kamiljarmusik Date: Mon, 15 May 2023 22:10:49 +0200 Subject: [PATCH] #2538 Added permissions for report preventPurge function --- WebContent/WEB-INF/spring-security.xml | 5 +-- scadalts-ui/src/store/reports/index.js | 4 +-- .../dao/report/ReportInstanceDAO.java | 28 ++++++++++++++++ .../scada_lts/mango/adapter/MangoReport.java | 14 ++++++-- .../mango/service/ReportService.java | 32 ++++++++++++++++--- .../service/GetReportInstancesWithAccess.java | 24 ++++++++------ src/org/scada_lts/web/mvc/api/ReportsAPI.java | 4 +-- .../web/mvc/api/ReportsApiService.java | 17 ++++------ .../scada_lts/web/mvc/api/security/Guard.java | 12 +++++++ .../web/mvc/api/security/GuardUtils.java | 2 +- .../mvc/api/security/WithIdentifierGuard.java | 25 ++++++++++----- 11 files changed, 126 insertions(+), 41 deletions(-) diff --git a/WebContent/WEB-INF/spring-security.xml b/WebContent/WEB-INF/spring-security.xml index 7eef359618..808d7229da 100644 --- a/WebContent/WEB-INF/spring-security.xml +++ b/WebContent/WEB-INF/spring-security.xml @@ -116,8 +116,9 @@ - - + + + diff --git a/scadalts-ui/src/store/reports/index.js b/scadalts-ui/src/store/reports/index.js index 8e5855bc9c..886731557e 100644 --- a/scadalts-ui/src/store/reports/index.js +++ b/scadalts-ui/src/store/reports/index.js @@ -49,9 +49,9 @@ const storeReports = { .then((r) => { commit(SET_REPORT_TEMPLATES, r)}) .catch(() => { dispatch('showErrorNotification', 'Reports not loaded')}); }, - setPreventPurge({ dispatch }, payload) { + setPreventPurge({ commit, dispatch }, payload) { dispatch('requestGet', `/reports/instances/${payload.id}/preventPurge/${payload.preventPurge}`) - .then(() => { commit(TOGGLE_PURGE, payload.id)}) + .then((r) => { commit(TOGGLE_PURGE, r)}) .catch(() => { dispatch('showErrorNotification', 'Failed to save this property')}); }, diff --git a/src/org/scada_lts/dao/report/ReportInstanceDAO.java b/src/org/scada_lts/dao/report/ReportInstanceDAO.java index f450428158..c0e481564f 100644 --- a/src/org/scada_lts/dao/report/ReportInstanceDAO.java +++ b/src/org/scada_lts/dao/report/ReportInstanceDAO.java @@ -139,6 +139,12 @@ public class ReportInstanceDAO { + "order by " + COLUMN_NAME_RUN_START_TIME + " " + "desc "; + + private static final String REPORT_INSTANCE_UPDATE_PREVENT_PURGE_BY_ID = "" + + "update reportInstances set " + + COLUMN_NAME_PREVENT_PURGE + "=? " + + "where " + + COLUMN_NAME_ID + "=? "; // @formatter:on private class ReportInstanceRowMapper implements RowMapper { @@ -310,6 +316,28 @@ public List getReportInstanceEvents(int instanceId) { return DAO.getInstance().getJdbcTemp().query(REPORT_INSTANCE_EVENT_SELECT, new Object[] {instanceId}, new ReportEventRowMapper()); } + public List getReportInstances() { + + if (LOG.isTraceEnabled()) { + LOG.trace("getReportInstances()"); + } + + return DAO.getInstance().getJdbcTemp().query(REPORT_INSTANCE_SELECT, new ReportInstanceRowMapper()); + } + + @Transactional(readOnly = false,propagation= Propagation.REQUIRES_NEW,isolation= Isolation.READ_COMMITTED,rollbackFor=SQLException.class) + public void updatePreventPurge(int id, boolean preventPurge) { + + if (LOG.isTraceEnabled()) { + LOG.trace("updatePreventPurge(int id, boolean preventPurge) id:" + id + ", preventPurge:" + preventPurge); + } + + DAO.getInstance().getJdbcTemp().update(REPORT_INSTANCE_UPDATE_PREVENT_PURGE_BY_ID, new Object[]{ + DAO.boolToChar(preventPurge), + id + }); + } + private static class ReportEventRowMapper implements RowMapper { public EventInstance mapRow(ResultSet rs, int rowNum) throws SQLException { diff --git a/src/org/scada_lts/mango/adapter/MangoReport.java b/src/org/scada_lts/mango/adapter/MangoReport.java index 36a3f4c654..de5d61d29e 100644 --- a/src/org/scada_lts/mango/adapter/MangoReport.java +++ b/src/org/scada_lts/mango/adapter/MangoReport.java @@ -50,7 +50,9 @@ public interface MangoReport { List getReportInstances(int userId); - ReportInstance getReportInstance(int id); + List getReportInstances(); + + ReportInstance getReportInstance(int id); void deleteReportInstance(int id, int userId); @@ -58,7 +60,9 @@ public interface MangoReport { void setReportInstancePreventPurge(int id, boolean preventPurge, int userId); - void saveReportInstance(ReportInstance instance); + void setReportInstancePreventPurge(int id, boolean preventPurge); + + void saveReportInstance(ReportInstance instance); int runReport(final ReportInstance instance, List points, ResourceBundle bundle); @@ -72,13 +76,19 @@ public interface MangoReport { boolean hasReportReadPermission(User user, ReportVO report); + boolean hasReportSetPermission(User user, ReportVO report); + boolean hasReportOwnerPermission(User user, ReportVO report); boolean hasReportInstanceReadPermission(User user, ReportInstance report); + boolean hasReportInstanceSetPermission(User user, ReportInstance report); + boolean hasReportInstanceOwnerPermission(User user, ReportInstance report); boolean hasReportInstanceReadPermission(User user, int reportInstanceId); + boolean hasReportInstanceSetPermission(User user, int reportInstanceId); + boolean hasReportInstanceOwnerPermission(User user, int reportInstanceId); } diff --git a/src/org/scada_lts/mango/service/ReportService.java b/src/org/scada_lts/mango/service/ReportService.java index 4d5234e26a..d22e07069b 100644 --- a/src/org/scada_lts/mango/service/ReportService.java +++ b/src/org/scada_lts/mango/service/ReportService.java @@ -142,6 +142,11 @@ public List getReportInstances(int userId) { return reportInstanceDAO.getReportInstances(userId); } + @Override + public List getReportInstances() { + return reportInstanceDAO.getReportInstances(); + } + @Override public ReportInstance getReportInstance(int id) { return reportInstanceDAO.getReportInstance(id); @@ -162,6 +167,11 @@ public void setReportInstancePreventPurge(int id, boolean preventPurge, int user reportInstanceDAO.updatePreventPurge(id, preventPurge, userId); } + @Override + public void setReportInstancePreventPurge(int id, boolean preventPurge) { + reportInstanceDAO.updatePreventPurge(id, preventPurge); + } + /** * This method should only be called by the ReportWorkItem. */ @@ -335,6 +345,11 @@ public boolean hasReportReadPermission(User user, ReportVO report) { return getReportsWithAccess.hasReadPermission(user, report); } + @Override + public boolean hasReportSetPermission(User user, ReportVO report) { + return getReportsWithAccess.hasSetPermission(user, report); + } + @Override public boolean hasReportOwnerPermission(User user, ReportVO report) { return getReportsWithAccess.hasOwnerPermission(user, report); @@ -345,6 +360,11 @@ public boolean hasReportInstanceReadPermission(User user, ReportInstance report) return getReportInstancesWithAccess.hasReadPermission(user, report); } + @Override + public boolean hasReportInstanceSetPermission(User user, ReportInstance report) { + return getReportInstancesWithAccess.hasSetPermission(user, report); + } + @Override public boolean hasReportInstanceOwnerPermission(User user, ReportInstance report) { return getReportInstancesWithAccess.hasOwnerPermission(user, report); @@ -352,15 +372,19 @@ public boolean hasReportInstanceOwnerPermission(User user, ReportInstance report @Override public boolean hasReportInstanceReadPermission(User user, int reportInstanceId) { - ReportInstance reportInstance = new ReportInstance(); - reportInstance.setId(reportInstanceId); + ReportInstance reportInstance = getReportInstance(reportInstanceId); return getReportInstancesWithAccess.hasReadPermission(user, reportInstance); } + @Override + public boolean hasReportInstanceSetPermission(User user, int reportInstanceId) { + ReportInstance reportInstance = getReportInstance(reportInstanceId); + return getReportInstancesWithAccess.hasSetPermission(user, reportInstance); + } + @Override public boolean hasReportInstanceOwnerPermission(User user, int reportInstanceId) { - ReportInstance reportInstance = new ReportInstance(); - reportInstance.setId(reportInstanceId); + ReportInstance reportInstance = getReportInstance(reportInstanceId); return getReportInstancesWithAccess.hasOwnerPermission(user, reportInstance); } diff --git a/src/org/scada_lts/permissions/service/GetReportInstancesWithAccess.java b/src/org/scada_lts/permissions/service/GetReportInstancesWithAccess.java index 7b9fda95f0..0550a275ae 100644 --- a/src/org/scada_lts/permissions/service/GetReportInstancesWithAccess.java +++ b/src/org/scada_lts/permissions/service/GetReportInstancesWithAccess.java @@ -27,7 +27,11 @@ public List getObjectsWithAccess(User user) { LOG.warn("user is null"); return Collections.emptyList(); } - return reportInstanceDAO.getReportInstances(user.getId()); + if(user.isAdmin()) + return reportInstanceDAO.getReportInstances(); + return reportInstanceDAO.getReportInstances().stream() + .filter(a -> hasReportInstanceReadPermission(user, a)) + .collect(Collectors.toList()); } @Override @@ -56,39 +60,39 @@ public boolean hasOwnerPermission(User user, ReportInstance object) { return GetReportInstancesWithAccess.hasReportInstanceOwnerPermission(user, object); } - public static boolean hasReportInstanceReadPermission(User user, ReportInstance report) { + public static boolean hasReportInstanceReadPermission(User user, ReportInstance reportInstance) { if(user == null) { LOG.warn("user is null"); return false; } - if(report == null) { + if(reportInstance == null) { LOG.warn("report is null"); return false; } - return user.isAdmin() || report.getUserId() == user.getId(); + return user.isAdmin() || reportInstance.getUserId() == user.getId(); } - public static boolean hasReportInstanceSetPermission(User user, ReportInstance report) { + public static boolean hasReportInstanceSetPermission(User user, ReportInstance reportInstance) { if(user == null) { LOG.warn("user is null"); return false; } - if(report == null) { + if(reportInstance == null) { LOG.warn("report is null"); return false; } - return user.isAdmin() || report.getUserId() == user.getId(); + return user.isAdmin() || reportInstance.getUserId() == user.getId(); } - public static boolean hasReportInstanceOwnerPermission(User user, ReportInstance report) { + public static boolean hasReportInstanceOwnerPermission(User user, ReportInstance reportInstance) { if(user == null) { LOG.warn("user is null"); return false; } - if(report == null) { + if(reportInstance == null) { LOG.warn("report is null"); return false; } - return user.isAdmin() || report.getUserId() == user.getId(); + return user.isAdmin() || reportInstance.getUserId() == user.getId(); } } diff --git a/src/org/scada_lts/web/mvc/api/ReportsAPI.java b/src/org/scada_lts/web/mvc/api/ReportsAPI.java index 8664f10c20..e31498499a 100644 --- a/src/org/scada_lts/web/mvc/api/ReportsAPI.java +++ b/src/org/scada_lts/web/mvc/api/ReportsAPI.java @@ -97,9 +97,9 @@ public ResponseEntity runReport(@PathVariable("id") Integer id, HttpServ * List */ @GetMapping(value = "/instances") - public ResponseEntity> getInstances(HttpServletRequest request) { + public ResponseEntity> getReportInstances(HttpServletRequest request) { LOG.info("GET::/api/reports/instances"); - List reportInstances = reportsApiService.getInstances(request); + List reportInstances = reportsApiService.getReportInstances(request); return new ResponseEntity<>(reportInstances, HttpStatus.OK); } diff --git a/src/org/scada_lts/web/mvc/api/ReportsApiService.java b/src/org/scada_lts/web/mvc/api/ReportsApiService.java index 1c9a0f747a..7143bc0a55 100644 --- a/src/org/scada_lts/web/mvc/api/ReportsApiService.java +++ b/src/org/scada_lts/web/mvc/api/ReportsApiService.java @@ -18,9 +18,6 @@ import org.scada_lts.web.mvc.api.exceptions.BadRequestException; import org.scada_lts.web.mvc.api.exceptions.InternalServerErrorException; import org.scada_lts.web.mvc.api.exceptions.UnauthorizedException; -import org.springframework.http.HttpEntity; -import org.springframework.http.HttpStatus; -import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; import javax.servlet.http.HttpServletRequest; @@ -174,7 +171,7 @@ public void workFail(Exception exception) { public void runReport(HttpServletRequest request, String xid, Integer id) { ReportVO report = read(request, xid, id); User user = Common.getUser(request); - if(!reportService.hasReportOwnerPermission(user, report)) + if(!reportService.hasReportSetPermission(user, report)) throw new UnauthorizedException(request.getRequestURI()); DwrResponseI18n response = new DwrResponseI18n(); report.validate(response, user); @@ -187,11 +184,11 @@ public void runReport(HttpServletRequest request, String xid, Integer id) { } } - public List getInstances(HttpServletRequest request) { + public List getReportInstances(HttpServletRequest request) { User user = Common.getUser(request); List reportInstances; try { - reportInstances = reportService.getReportInstances(user.getId()).stream() + reportInstances = reportService.getReportInstances().stream() .filter(a -> reportService.hasReportInstanceReadPermission(user, a)) .collect(Collectors.toList()); } catch (Exception ex) { @@ -200,17 +197,17 @@ public List getInstances(HttpServletRequest request) { return reportInstances; } - public HttpEntity setReportInstancePreventPurge(HttpServletRequest request, Integer id, Boolean preventPurge) { + public Integer setReportInstancePreventPurge(HttpServletRequest request, Integer id, Boolean preventPurge) { checkArgsIfTwoEmptyThenBadRequest(request, "Id and preventPurge cannot be null.", id, preventPurge); User user = Common.getUser(request); - if(!reportService.hasReportInstanceOwnerPermission(user, id)) + if(!reportService.hasReportInstanceSetPermission(user, id)) throw new UnauthorizedException(request.getRequestURI()); try { - reportService.setReportInstancePreventPurge(id, preventPurge, user.getId()); + reportService.setReportInstancePreventPurge(id, preventPurge); } catch (Exception ex) { throw new InternalServerErrorException(ex, request.getRequestURI()); } - return new ResponseEntity<>(id, HttpStatus.OK); + return id; } public ReportVO toReport(HttpServletRequest request, ReportDTO query) { diff --git a/src/org/scada_lts/web/mvc/api/security/Guard.java b/src/org/scada_lts/web/mvc/api/security/Guard.java index 2f63f5d6ac..8d8aba9e45 100644 --- a/src/org/scada_lts/web/mvc/api/security/Guard.java +++ b/src/org/scada_lts/web/mvc/api/security/Guard.java @@ -221,6 +221,18 @@ public boolean hasReportInstanceSetPermission(HttpServletRequest request, String return withIdentifierGuard.hasReportInstanceSetPermission(request, id, isXid); } + public boolean hasReportInstanceOwnerPermission(HttpServletRequest request, String id) { + return withIdentifierGuard.hasReportInstanceOwnerPermission(request, id); + } + + public boolean hasReportInstanceReadPermission(HttpServletRequest request, String id) { + return withIdentifierGuard.hasReportInstanceReadPermission(request, id); + } + + public boolean hasReportInstanceSetPermission(HttpServletRequest request, String id) { + return withIdentifierGuard.hasReportInstanceSetPermission(request, id); + } + public boolean hasReportInstanceOwnerPermission(HttpServletRequest request) { return getIdentifierFromHttpParameterGuard.hasReportInstanceOwnerPermission(request); } diff --git a/src/org/scada_lts/web/mvc/api/security/GuardUtils.java b/src/org/scada_lts/web/mvc/api/security/GuardUtils.java index 00a62e3c21..7496096cd9 100644 --- a/src/org/scada_lts/web/mvc/api/security/GuardUtils.java +++ b/src/org/scada_lts/web/mvc/api/security/GuardUtils.java @@ -55,7 +55,7 @@ private static int converter(String value) { try { return Integer.parseInt(value); } catch (Exception ex) { - LOG.warn(ex.getMessage(), ex); + LOG.warn("Trying to convert the value of " + value + " to int, failed. Exception: " + ex.getMessage(), ex); return Common.NEW_ID; } } diff --git a/src/org/scada_lts/web/mvc/api/security/WithIdentifierGuard.java b/src/org/scada_lts/web/mvc/api/security/WithIdentifierGuard.java index b6c0eb3795..3c0c488ce1 100644 --- a/src/org/scada_lts/web/mvc/api/security/WithIdentifierGuard.java +++ b/src/org/scada_lts/web/mvc/api/security/WithIdentifierGuard.java @@ -77,25 +77,34 @@ public boolean hasReportSetPermission(HttpServletRequest request, String id, boo public boolean hasReportInstanceOwnerPermission(HttpServletRequest request, String id, boolean isXid) { if(isXid) { - LOG.warn(ARG_IS_XID_IS_NOT_SUPPORTED); - return false; + throw new IllegalArgumentException(ARG_IS_XID_IS_NOT_SUPPORTED); } - return doHasPermission(request, hasPermissionOperations::hasReportInstanceOwnerPermission, (a,b) -> false, id, false); + return doHasPermission(request, hasPermissionOperations::hasReportInstanceOwnerPermission, (a,b) -> false, id, isXid); } public boolean hasReportInstanceReadPermission(HttpServletRequest request, String id, boolean isXid) { if(isXid) { - LOG.warn(ARG_IS_XID_IS_NOT_SUPPORTED); - return false; + throw new IllegalArgumentException(ARG_IS_XID_IS_NOT_SUPPORTED); } - return doHasPermission(request, hasPermissionOperations::hasReportInstanceReadPermission, (a,b) -> false, id, false); + return doHasPermission(request, hasPermissionOperations::hasReportInstanceReadPermission, (a,b) -> false, id, isXid); } public boolean hasReportInstanceSetPermission(HttpServletRequest request, String id, boolean isXid) { if(isXid) { - LOG.warn(ARG_IS_XID_IS_NOT_SUPPORTED); - return false; + throw new IllegalArgumentException(ARG_IS_XID_IS_NOT_SUPPORTED); } + return doHasPermission(request, hasPermissionOperations::hasReportInstanceSetPermission, (a,b) -> false, id, isXid); + } + + public boolean hasReportInstanceOwnerPermission(HttpServletRequest request, String id) { + return doHasPermission(request, hasPermissionOperations::hasReportInstanceOwnerPermission, (a,b) -> false, id, false); + } + + public boolean hasReportInstanceReadPermission(HttpServletRequest request, String id) { + return doHasPermission(request, hasPermissionOperations::hasReportInstanceReadPermission, (a,b) -> false, id, false); + } + + public boolean hasReportInstanceSetPermission(HttpServletRequest request, String id) { return doHasPermission(request, hasPermissionOperations::hasReportInstanceSetPermission, (a,b) -> false, id, false); } } \ No newline at end of file