Merge pull request #2539 from SCADA-LTS/feature/#2538_Added_permissio…

…ns_for_report_preventPurge_function #2538 Added permissions for report preventPurge function
SCADA-LTS · May 15, 2023 · fc5654e · fc5654e
2 parents 7178d51 + 24a865a
commit fc5654e
Show file tree

Hide file tree

Showing 11 changed files with 126 additions and 41 deletions.
diff --git a/WebContent/WEB-INF/spring-security.xml b/WebContent/WEB-INF/spring-security.xml
@@ -116,8 +116,9 @@
         <intercept-url pattern="/api/reports/search" access="hasAnyRole('ROLE_ADMIN', 'ROLE_USER')" method="POST" />
         <intercept-url pattern="/api/reports/sendTestEmails" access="hasAnyRole('ROLE_ADMIN', 'ROLE_USER')" method="POST" />
         <intercept-url pattern="/api/reports/instances" access="hasAnyRole('ROLE_ADMIN', 'ROLE_USER')" method="GET" />
-        <intercept-url pattern="/api/reports/instances/{id}" access="@guard.hasReportInstanceOwnerPermission(request,#id,false)" method="DELETE" />
-        <intercept-url pattern="/api/reports/run/{id}" access="@guard.hasReportOwnerPermission(request,#id,false)" method="GET" />
+        <intercept-url pattern="/api/reports/instances/{id}/preventPurge/{preventPurge}" access="@guard.hasReportInstanceSetPermission(request,#id)" method="GET" />
+        <intercept-url pattern="/api/reports/instances/{id}" access="@guard.hasReportInstanceOwnerPermission(request,#id)" method="DELETE" />
+        <intercept-url pattern="/api/reports/run/{id}" access="@guard.hasReportSetPermission(request,#id,false)" method="GET" />
         <intercept-url pattern="/api/reports/{id}" access="@guard.hasReportOwnerPermission(request,#id,false)" method="DELETE" />
 
         <!-- User PUT/GET -->

diff --git 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
@@ -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<ReportInstance> {
@@ -310,6 +316,28 @@ public List<EventInstance> getReportInstanceEvents(int instanceId) {
 		return DAO.getInstance().getJdbcTemp().query(REPORT_INSTANCE_EVENT_SELECT, new Object[] {instanceId}, new ReportEventRowMapper());
 	}
 
+	public List<ReportInstance> 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<EventInstance> {
 		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
@@ -50,15 +50,19 @@ public interface MangoReport {
 
 	List<ReportInstance> getReportInstances(int userId);
 
-	ReportInstance getReportInstance(int id);
+    List<ReportInstance> getReportInstances();
+
+    ReportInstance getReportInstance(int id);
 
 	void deleteReportInstance(int id, int userId);
 
 	int purgeReportsBefore(final long time);
 
 	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<ReportInstancePointDAO.PointInfo> 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
@@ -142,6 +142,11 @@ public List<ReportInstance> getReportInstances(int userId) {
 		return reportInstanceDAO.getReportInstances(userId);
 	}
 
+	@Override
+	public List<ReportInstance> 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,22 +360,31 @@ 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);
 	}
 
 	@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
@@ -27,7 +27,11 @@ public List<ReportInstance> 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
@@ -97,9 +97,9 @@ public ResponseEntity<String> runReport(@PathVariable("id") Integer id, HttpServ
      * List<ReportInstance>
      */
     @GetMapping(value = "/instances")
-    public ResponseEntity<List<ReportInstance>> getInstances(HttpServletRequest request) {
+    public ResponseEntity<List<ReportInstance>> getReportInstances(HttpServletRequest request) {
         LOG.info("GET::/api/reports/instances");
-        List<ReportInstance> reportInstances = reportsApiService.getInstances(request);
+        List<ReportInstance> 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
@@ -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<ReportInstance> getInstances(HttpServletRequest request) {
+    public List<ReportInstance> getReportInstances(HttpServletRequest request) {
         User user = Common.getUser(request);
         List<ReportInstance> 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<ReportInstance> getInstances(HttpServletRequest request) {
         return reportInstances;
     }
 
-    public HttpEntity<Integer> 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
@@ -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
@@ -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
@@ -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);
     }
 }