Skip to content

Commit

Permalink
#2544 Fixed 'not logged in' after logged - removed V2_7_5_4__EnabledA…
Browse files Browse the repository at this point in the history
…nonymousUser.java; refactor spring-security.xml; secure methods from ReportsDwr: deleteReport, deleteReportInstance, secure methods from ViewDwr: addUpdateSharedUser, removeSharedUser, deleteViewShare, addComponent, secure methods from ViewEditController: showForm, handleImage, save, cancel, delete; ViewsController change Permissions.ensureViewPermission on GetViewsWithAccess.ensureViewReadPermission; Added methods in GetReportInstancesWithAccess: ensureReportInstanceReadPermission, ensureReportInstanceSetPermission, ensureReportInstanceOwnerPermission, Added methods in GetReportsWithAccess: ensureReportReadPermission, ensureReportSetPermission, ensureReportOwnerPermission, Added methods in GetViewsWithAccess: ensureViewReadPermission, ensureViewSetPermission, ensureViewOwnerPermission
  • Loading branch information
Limraj committed Jun 21, 2023
1 parent c989e3c commit 0713faf
Show file tree
Hide file tree
Showing 9 changed files with 232 additions and 138 deletions.
236 changes: 149 additions & 87 deletions WebContent/WEB-INF/spring-security.xml

Large diffs are not rendered by default.

18 changes: 12 additions & 6 deletions src/com/serotonin/mango/web/dwr/ReportsDwr.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import com.serotonin.web.i18n.LocalizableMessage;
import org.scada_lts.mango.adapter.MangoReport;
import org.scada_lts.mango.service.*;
import org.scada_lts.permissions.service.GetReportInstancesWithAccess;
import org.scada_lts.permissions.service.GetReportsWithAccess;

import java.util.List;
import java.util.ResourceBundle;
Expand Down Expand Up @@ -200,20 +202,24 @@ public DwrResponseI18n runReport(String name, List<ReportPointVO> points, int in
}

public void deleteReport(int id) {
MangoReport reportDao = new ReportService();
MangoReport reportService = new ReportService();

ReportVO report = reportDao.getReport(id);
ReportVO report = reportService.getReport(id);
if (report != null) {
Permissions.ensureReportPermission(Common.getUser(), report);
GetReportsWithAccess.ensureReportOwnerPermission(Common.getUser(), report);
ReportJob.unscheduleReportJob(report);
reportDao.deleteReport(id);
reportService.deleteReport(id);
}
}

public List<ReportInstance> deleteReportInstance(int instanceId) {
User user = Common.getUser();
MangoReport reportDao = new ReportService();
reportDao.deleteReportInstance(instanceId, user.getId());
MangoReport reportService = new ReportService();
ReportInstance reportInstance = reportService.getReportInstance(instanceId);
if(reportInstance != null) {
GetReportInstancesWithAccess.ensureReportInstanceOwnerPermission(user, reportInstance);
reportService.deleteReportInstance(instanceId, user.getId());
}
return getReportInstances(user);
}

Expand Down
12 changes: 6 additions & 6 deletions src/com/serotonin/mango/web/dwr/ViewDwr.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
import com.serotonin.mango.view.component.ThumbnailComponent;
import com.serotonin.mango.view.component.ViewComponent;
import com.serotonin.mango.view.text.TextRenderer;
import com.serotonin.mango.vo.AnonymousUser;
import com.serotonin.mango.vo.DataPointExtendedNameComparator;
import com.serotonin.mango.vo.DataPointVO;
import com.serotonin.mango.vo.User;
Expand Down Expand Up @@ -328,6 +327,7 @@ private void addCustomComponentState(ViewComponent viewComponent, RuntimeManager

public List<ShareUser> addUpdateSharedUser(int userId, int accessType, int viewId) {
View view = getView(viewId, WebContextFactory.get().getHttpServletRequest(), new ViewService(), true);
GetViewsWithAccess.ensureViewOwnerPermission(Common.getUser(), view);
boolean found = false;
for (ShareUser su : view.getViewUsers()) {
if (su.getUserId() == userId) {
Expand All @@ -350,7 +350,7 @@ public List<ShareUser> addUpdateSharedUser(int userId, int accessType, int viewI

public List<ShareUser> removeSharedUser(int userId, int viewId) {
View view = getView(viewId, WebContextFactory.get().getHttpServletRequest(), new ViewService(), true);

GetViewsWithAccess.ensureViewOwnerPermission(Common.getUser(), view);
for (ShareUser su : view.getViewUsers()) {
if (su.getUserId() == userId) {
view.getViewUsers().remove(su);
Expand All @@ -365,6 +365,7 @@ public List<ShareUser> removeSharedUser(int userId, int viewId) {
public void deleteViewShare(int viewId) {
User user = Common.getUser();
View view = getView(viewId, WebContextFactory.get().getHttpServletRequest(), new ViewService(), true);
GetViewsWithAccess.ensureViewOwnerPermission(Common.getUser(), view);
new ViewService().removeUserFromView(view.getId(), user.getId());
}

Expand Down Expand Up @@ -416,7 +417,7 @@ public ViewComponent addComponent(String componentName, int viewId) {

User user = Common.getUser();
View view = getView(viewId, WebContextFactory.get().getHttpServletRequest(), new ViewService(), true);

GetViewsWithAccess.ensureViewOwnerPermission(Common.getUser(), view);
view.addViewComponent(viewComponent);
viewComponent.validateDataPoint(user, view.getUserAccess(user) == ShareUser.ACCESS_READ);
return viewComponent;
Expand All @@ -440,6 +441,7 @@ public int getViewComponentZIndex(String viewComponentId, int viewId) {

public void deleteViewComponent(String viewComponentId, int viewId) {
View view = getView(viewId, WebContextFactory.get().getHttpServletRequest(), new ViewService(), true);
GetViewsWithAccess.ensureViewOwnerPermission(Common.getUser(), view);
view.removeViewComponent(getViewComponent(view, viewComponentId));
}

Expand Down Expand Up @@ -491,9 +493,7 @@ public String setViewPoint(String viewComponentId, String valueStr, int viewId)

if (point != null) {
// Check that setting is allowed.
int access = view.getUserAccess(user);
if (!(access == ShareUser.ACCESS_OWNER || access == ShareUser.ACCESS_SET))
throw new PermissionException("Not allowed to set this point", user);
GetViewsWithAccess.ensureViewSetPermission(user, view);

// Try setting the point.
setPointImpl(point, valueStr, user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.serotonin.mango.view.ShareUser;
import com.serotonin.mango.view.View;
import com.serotonin.mango.vo.User;
import com.serotonin.mango.vo.permission.Permissions;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.scada_lts.dao.IViewDAO;
Expand Down Expand Up @@ -93,7 +92,7 @@ protected ModelAndView handleRequestInternal(HttpServletRequest request,

if (currentView != null) {
if (!user.isAdmin())
Permissions.ensureViewPermission(user, currentView);
GetViewsWithAccess.ensureViewReadPermission(user, currentView);

// Make sure the owner still has permission to all of the points in
// the view, and that components are
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.scada_lts.permissions.service;

import com.serotonin.mango.vo.User;
import com.serotonin.mango.vo.permission.PermissionException;
import com.serotonin.mango.vo.report.ReportInstance;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
Expand Down Expand Up @@ -95,4 +96,22 @@ public static boolean hasReportInstanceOwnerPermission(User user, ReportInstance
}
return user.isAdmin() || reportInstance.getUserId() == user.getId();
}

public static void ensureReportInstanceReadPermission(User user, ReportInstance reportInstance) {
if(!hasReportInstanceReadPermission(user, reportInstance)) {
throw new PermissionException("User does not have permission to access the report instance", user);
}
}

public static void ensureReportInstanceSetPermission(User user, ReportInstance reportInstance) {
if(!hasReportInstanceSetPermission(user, reportInstance)) {
throw new PermissionException("User does not have permission to access the report instance", user);
}
}

public static void ensureReportInstanceOwnerPermission(User user, ReportInstance reportInstance) {
if(!hasReportInstanceOwnerPermission(user, reportInstance)) {
throw new PermissionException("User does not have permission to access the report instance", user);
}
}
}
19 changes: 19 additions & 0 deletions src/org/scada_lts/permissions/service/GetReportsWithAccess.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.scada_lts.permissions.service;

import com.serotonin.mango.vo.User;
import com.serotonin.mango.vo.permission.PermissionException;
import com.serotonin.mango.vo.report.ReportVO;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
Expand Down Expand Up @@ -95,4 +96,22 @@ public static boolean hasReportOwnerPermission(User user, ReportVO report) {
}
return user.isAdmin() || report.getUserId() == user.getId();
}

public static void ensureReportReadPermission(User user, ReportVO report) {
if(!hasReportReadPermission(user, report)) {
throw new PermissionException("User does not have permission to access the report", user);
}
}

public static void ensureReportSetPermission(User user, ReportVO report) {
if(!hasReportSetPermission(user, report)) {
throw new PermissionException("User does not have permission to access the report", user);
}
}

public static void ensureReportOwnerPermission(User user, ReportVO report) {
if(!hasReportOwnerPermission(user, report)) {
throw new PermissionException("User does not have permission to access the report", user);
}
}
}
19 changes: 19 additions & 0 deletions src/org/scada_lts/permissions/service/GetViewsWithAccess.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.serotonin.mango.view.ShareUser;
import com.serotonin.mango.view.View;
import com.serotonin.mango.vo.User;
import com.serotonin.mango.vo.permission.PermissionException;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.scada_lts.dao.IViewDAO;
Expand Down Expand Up @@ -93,4 +94,22 @@ public static boolean hasViewOwnerPermission(User user, View view) {
}
return user.isAdmin() || view.getUserId() == user.getId() || view.getUserAccess(user) >= ShareUser.ACCESS_OWNER;
}

public static void ensureViewReadPermission(User user, View view) {
if(!hasViewReadPermission(user, view)) {
throw new PermissionException("User does not have permission to access the report", user);
}
}

public static void ensureViewSetPermission(User user, View view) {
if(!hasViewSetPermission(user, view)) {
throw new PermissionException("User does not have permission to access the report", user);
}
}

public static void ensureViewOwnerPermission(User user, View view) {
if(!hasViewOwnerPermission(user, view)) {
throw new PermissionException("User does not have permission to access the report", user);
}
}
}
14 changes: 7 additions & 7 deletions src/org/scada_lts/web/mvc/controller/ViewEditController.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.apache.commons.logging.LogFactory;
import org.scada_lts.mango.service.UsersProfileService;
import org.scada_lts.mango.service.ViewService;
import org.scada_lts.permissions.service.GetViewsWithAccess;
import org.scada_lts.web.mvc.form.ViewEditForm;
import org.scada_lts.web.mvc.validator.ViewEditValidator;
import org.springframework.stereotype.Controller;
Expand All @@ -49,7 +50,6 @@
import com.serotonin.mango.Common;
import com.serotonin.mango.view.View;
import com.serotonin.mango.vo.User;
import com.serotonin.mango.vo.permission.Permissions;

import static com.serotonin.mango.util.ViewControllerUtils.*;
import static org.scada_lts.utils.PathSecureUtils.toSecurePath;
Expand Down Expand Up @@ -102,7 +102,7 @@ protected ModelAndView showForm(HttpServletRequest request) throws Exception {

if (view != null) {
// An existing view.
Permissions.ensureViewEditPermission(user, view);
GetViewsWithAccess.ensureViewOwnerPermission(user, view);
view.validateViewComponents(false);
request.getSession().setAttribute("view_" + view.getId(), view);
request.getSession().setAttribute("view_" + view.getXid(), view);
Expand Down Expand Up @@ -137,7 +137,7 @@ protected ModelAndView handleImage(HttpServletRequest request, HttpServletRespon
if (WebUtils.hasSubmitParameter(request, SUBMIT_CLEAR_IMAGE)) {
User user = Common.getUser(request);
view = getOrEmptyView(request, viewService, true);
Permissions.ensureViewPermission(user, view);
GetViewsWithAccess.ensureViewOwnerPermission(user, view);

form.setView(view);
view.setBackgroundFilename(null);
Expand All @@ -146,7 +146,7 @@ protected ModelAndView handleImage(HttpServletRequest request, HttpServletRespon
if (WebUtils.hasSubmitParameter(request, SUBMIT_UPLOAD)) {
User user = Common.getUser(request);
view = getOrEmptyView(request, viewService, true);
Permissions.ensureViewPermission(user, view);
GetViewsWithAccess.ensureViewOwnerPermission(user, view);

form.setView(view);
uploadFile(request, form, errors);
Expand All @@ -165,7 +165,7 @@ protected ModelAndView save(HttpServletRequest request, @ModelAttribute(FORM_OBJ
LOG.debug("ViewEditController:save");
User user = Common.getUser();
View view = getOrEmptyView(request, viewService, true);
Permissions.ensureViewPermission(user, view);
GetViewsWithAccess.ensureViewOwnerPermission(user, view);
copyViewProperties(view, form.getView());
form.setView(view);

Expand Down Expand Up @@ -196,7 +196,7 @@ protected ModelAndView cancel(HttpServletRequest request, @ModelAttribute(FORM_O
LOG.debug("ViewEditController:cancel");
User user = Common.getUser(request);
View view = getOrEmptyView(request, viewService, false);
Permissions.ensureViewPermission(user, view);
GetViewsWithAccess.ensureViewReadPermission(user, view);
form.setView(view);
request.getSession().removeAttribute(EMPTY_VIEW_KEY);
request.getSession().removeAttribute("view_" + view.getId());
Expand All @@ -209,7 +209,7 @@ protected ModelAndView delete(HttpServletRequest request, @ModelAttribute(FORM_O
LOG.debug("ViewEditController:delete");
User user = Common.getUser(request);
View view = getOrEmptyView(request, viewService, true);
Permissions.ensureViewPermission(user, view);
GetViewsWithAccess.ensureViewOwnerPermission(user, view);
form.setView(view);

new ViewService().removeView(form.getView().getId());
Expand Down

0 comments on commit 0713faf

Please sign in to comment.