diff --git a/announcements/src/org/labkey/announcements/AnnouncementsController.java b/announcements/src/org/labkey/announcements/AnnouncementsController.java index a52c4c4278e..d14ba2e4332 100644 --- a/announcements/src/org/labkey/announcements/AnnouncementsController.java +++ b/announcements/src/org/labkey/announcements/AnnouncementsController.java @@ -26,6 +26,7 @@ import org.jetbrains.annotations.Nullable; import org.json.JSONObject; import org.labkey.announcements.model.AnnouncementDigestProvider; +import org.labkey.announcements.model.AnnouncementFullModel; import org.labkey.announcements.model.AnnouncementManager; import org.labkey.announcements.model.AnnouncementModel; import org.labkey.announcements.model.DailyDigestEmailPrefsSelector; @@ -107,11 +108,11 @@ import org.labkey.api.util.DateUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.HtmlString; +import org.labkey.api.util.OptionBuilder; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; -import org.labkey.api.util.URLHelper; -import org.labkey.api.util.OptionBuilder; import org.labkey.api.util.SelectBuilder; +import org.labkey.api.util.URLHelper; import org.labkey.api.view.ActionURL; import org.labkey.api.view.AjaxCompletion; import org.labkey.api.view.AlwaysAvailableWebPartFactory; @@ -137,7 +138,6 @@ import org.springframework.web.servlet.ModelAndView; import java.io.IOException; -import java.io.PrintWriter; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; @@ -218,7 +218,6 @@ public static ActionURL getBeginURL(Container c) public static AnnouncementModel copyEditableProps(AnnouncementModel target, AnnouncementModel source, boolean isInsert) { - if (source.getApproved() != null) target.setApproved(source.getApproved()); if (source.getAssignedTo() != null) target.setAssignedTo(source.getAssignedTo()); if (source.getBody() != null) target.setBody(source.getBody()); if (source.getExpires() != null) target.setExpires(source.getExpires()); @@ -940,7 +939,7 @@ public BindException bindParameters(PropertyValues m) throws Exception public ModelAndView getInsertUpdateView(AnnouncementForm form, boolean reshow, BindException errors) { Permissions perm = getPermissions(); - AnnouncementModel parent = null; + AnnouncementFullModel parent = null; Container c = getContainer(); if (null != form.getParentId()) @@ -2308,7 +2307,7 @@ protected DataRegion getDataRegion(Permissions perm, Settings settings) public static class ThreadViewBean { - public AnnouncementModel announcementModel; + public AnnouncementFullModel announcementModel; public String message = ""; public Permissions perm = null; public boolean isResponse = false; @@ -2336,7 +2335,7 @@ public ThreadView(Container c, URLHelper currentURL, User user, String rowId, St init(c, findThread(c, rowId, entityId), currentURL, getPermissions(c, user, getSettings(c)), false, false); } - public ThreadView(Container c, ActionURL url, AnnouncementModel ann, Permissions perm) + public ThreadView(Container c, ActionURL url, AnnouncementFullModel ann, Permissions perm) { this(); init(c, ann, url, perm, true, false); @@ -2345,11 +2344,11 @@ public ThreadView(Container c, ActionURL url, AnnouncementModel ann, Permissions public ThreadView(AnnouncementForm form, Container c, ActionURL url, Permissions perm, boolean print) { this(); - AnnouncementModel ann = findThread(c, form.get("rowId"), form.get("entityId")); + AnnouncementFullModel ann = findThread(c, form.get("rowId"), form.get("entityId")); init(c, ann, url, perm, false, print); } - protected void init(Container c, AnnouncementModel ann, URLHelper currentURL, Permissions perm, boolean isResponse, boolean print) + protected void init(Container c, AnnouncementFullModel ann, URLHelper currentURL, Permissions perm, boolean isResponse, boolean print) { if (null == c || !perm.allowRead(ann)) { @@ -2454,7 +2453,7 @@ public AnnouncementModel getAnnouncement() } - private static @Nullable AnnouncementModel findThread(Container c, String rowIdVal, String entityId) + private static @Nullable AnnouncementFullModel findThread(Container c, String rowIdVal, String entityId) { int rowId = 0; if (rowIdVal != null) diff --git a/announcements/src/org/labkey/announcements/announcementThread.jsp b/announcements/src/org/labkey/announcements/announcementThread.jsp index 78ddd926c94..de02d2b148f 100644 --- a/announcements/src/org/labkey/announcements/announcementThread.jsp +++ b/announcements/src/org/labkey/announcements/announcementThread.jsp @@ -20,6 +20,7 @@ <%@ page import="org.labkey.announcements.AnnouncementsController.RespondAction" %> <%@ page import="org.labkey.announcements.AnnouncementsController.ThreadView" %> <%@ page import="org.labkey.announcements.AnnouncementsController.ThreadViewBean" %> +<%@ page import="org.labkey.announcements.model.AnnouncementFullModel" %> <%@ page import="org.labkey.announcements.model.AnnouncementManager" %> <%@ page import="org.labkey.announcements.model.AnnouncementModel" %> <%@ page import="org.labkey.announcements.model.DiscussionServiceImpl" %> @@ -40,7 +41,7 @@ Container c = getContainer(); User user = getUser(); ThreadViewBean bean = me.getModelBean(); - AnnouncementModel announcementModel = bean.announcementModel; + AnnouncementFullModel announcementModel = bean.announcementModel; DiscussionService.Settings settings = bean.settings; if (null == announcementModel) diff --git a/announcements/src/org/labkey/announcements/model/AnnouncementFullModel.java b/announcements/src/org/labkey/announcements/model/AnnouncementFullModel.java new file mode 100644 index 00000000000..fb6c5e07ccd --- /dev/null +++ b/announcements/src/org/labkey/announcements/model/AnnouncementFullModel.java @@ -0,0 +1,23 @@ +package org.labkey.announcements.model; + +import java.util.Date; + +public class AnnouncementFullModel extends AnnouncementModel +{ + private Date _approved = null; + + public Date getApproved() + { + return _approved; + } + + public void setApproved(Date approved) + { + _approved = approved; + } + + public boolean isSpam() + { + return AnnouncementManager.SPAM_MAGIC_DATE.equals(getApproved()); + } +} diff --git a/announcements/src/org/labkey/announcements/model/AnnouncementManager.java b/announcements/src/org/labkey/announcements/model/AnnouncementManager.java index 7c50d985c2a..b1053019ba2 100644 --- a/announcements/src/org/labkey/announcements/model/AnnouncementManager.java +++ b/announcements/src/org/labkey/announcements/model/AnnouncementManager.java @@ -124,20 +124,20 @@ private AnnouncementManager() { } - private static @Nullable AnnouncementModel getAnnouncement(@Nullable Container c, @NotNull SimpleFilter filter) + private static @Nullable AnnouncementFullModel getAnnouncement(@Nullable Container c, @NotNull SimpleFilter filter) { if (c != null) filter.addCondition(FieldKey.fromParts("Container"), c); - return new TableSelector(_comm.getTableInfoAnnouncements(), filter, null).getObject(AnnouncementModel.class); + return new TableSelector(_comm.getTableInfoAnnouncements(), filter, null).getObject(AnnouncementFullModel.class); } - public static @Nullable AnnouncementModel getAnnouncement(@Nullable Container c, int rowId) + public static @Nullable AnnouncementFullModel getAnnouncement(@Nullable Container c, int rowId) { return getAnnouncement(c, new SimpleFilter(FieldKey.fromParts("RowId"), rowId)); } - public static @Nullable AnnouncementModel getAnnouncement(@Nullable Container c, String entityId) + public static @Nullable AnnouncementFullModel getAnnouncement(@Nullable Container c, String entityId) { try { @@ -531,7 +531,7 @@ private static AnnouncementModel validateModelWithSideEffects(AnnouncementModel } // Magic date value used to mark an announcement that a moderator has reviewed and marked as spam - private static final Date SPAM_MAGIC_DATE = new Date(0); + static final Date SPAM_MAGIC_DATE = new Date(0); // Standard filters for retrieving specific classes of messages (approved, spam, needs review) public static final SimpleFilter IS_APPROVED_FILTER = new SimpleFilter(FieldKey.fromParts("Approved"), AnnouncementManager.SPAM_MAGIC_DATE, CompareType.GT); @@ -543,11 +543,6 @@ public static void markAsSpam(Container c, AnnouncementModel ann) updateApproved(c, ann, SPAM_MAGIC_DATE); } - public static boolean isSpam(AnnouncementModel ann) - { - return SPAM_MAGIC_DATE.equals(ann.getApproved()); - } - // Execute direct SQL (not Table.update())... I don't think we want to change Modified or ModifiedBy. Could consider adding column for Moderator, though. // Returns true if an update was made, false if not (e.g., message was already reviewed). private static boolean updateApproved(Container c, AnnouncementModel ann, Date date) diff --git a/announcements/src/org/labkey/announcements/model/AnnouncementModel.java b/announcements/src/org/labkey/announcements/model/AnnouncementModel.java index 3a1e2ad5960..46bcc85abdb 100644 --- a/announcements/src/org/labkey/announcements/model/AnnouncementModel.java +++ b/announcements/src/org/labkey/announcements/model/AnnouncementModel.java @@ -81,7 +81,6 @@ public class AnnouncementModel extends Entity implements Serializable private Collection _responses = null; private Set _authors; - private Date _approved = null; /** * Standard constructor. @@ -429,21 +428,5 @@ public AttachmentParent getAttachmentParent() { return new AnnouncementAttachmentParent(this); } - - public Date getApproved() - { - return _approved; - } - - public void setApproved(Date approved) - { - _approved = approved; - } - - @JsonIgnore - public boolean isSpam() - { - return AnnouncementManager.isSpam(this); - } } diff --git a/api/src/org/labkey/api/action/ApiJsonWriter.java b/api/src/org/labkey/api/action/ApiJsonWriter.java index f98a4e23ade..6ce20dbc676 100644 --- a/api/src/org/labkey/api/action/ApiJsonWriter.java +++ b/api/src/org/labkey/api/action/ApiJsonWriter.java @@ -475,7 +475,9 @@ public void testExceptionNotCommitted() throws IOException var responseText = ((MockHttpServletResponse)writer.getResponse()).getContentAsString(); var json = new JSONObject(responseText); assertEquals("throwing up", json.getString("exception")); - assertTrue(json.has("stackTrace")); + assertFalse(json.getBoolean("success")); + assertEquals("java.lang.IllegalStateException", json.get("exceptionClass")); + assertFalse(json.has("stackTrace")); assertFalse(json.has("schemaName")); } diff --git a/api/src/org/labkey/api/action/ApiResponseWriter.java b/api/src/org/labkey/api/action/ApiResponseWriter.java index 3d202d3b298..f85716c02c2 100644 --- a/api/src/org/labkey/api/action/ApiResponseWriter.java +++ b/api/src/org/labkey/api/action/ApiResponseWriter.java @@ -446,7 +446,6 @@ public JSONObject toJSON(Throwable e) JSONObject json = new JSONObject(); json.put("exception", e.getMessage() != null ? e.getMessage() : e.getClass().getName()); json.put("exceptionClass", e.getClass().getName()); - json.put("stackTrace", e.getStackTrace()); return json; } diff --git a/api/src/org/labkey/api/defaults/DefaultValuesAction.java b/api/src/org/labkey/api/defaults/DefaultValuesAction.java index 0d1c34c01ad..ab70b1de541 100644 --- a/api/src/org/labkey/api/defaults/DefaultValuesAction.java +++ b/api/src/org/labkey/api/defaults/DefaultValuesAction.java @@ -37,7 +37,7 @@ public DefaultValuesAction(Class formClass) protected Domain getDomain(FormType domainIdForm) { Domain domain = PropertyService.get().getDomain(domainIdForm.getDomainId()); - if (domain == null) + if (domain == null || !domain.getContainer().equals(getContainer())) { throw new NotFoundException(); } diff --git a/api/src/org/labkey/api/qc/AbstractManageQCStatesAction.java b/api/src/org/labkey/api/qc/AbstractManageQCStatesAction.java index 0731902e111..ca68b7d4158 100644 --- a/api/src/org/labkey/api/qc/AbstractManageQCStatesAction.java +++ b/api/src/org/labkey/api/qc/AbstractManageQCStatesAction.java @@ -192,5 +192,4 @@ protected HtmlString getQcStateHtml(Container container, DataStateHandler qcStat return qcStateHtml.getHtmlString(); } - } diff --git a/core/src/org/labkey/core/CoreController.java b/core/src/org/labkey/core/CoreController.java index 9f70b100609..10d71253623 100644 --- a/core/src/org/labkey/core/CoreController.java +++ b/core/src/org/labkey/core/CoreController.java @@ -29,6 +29,7 @@ import org.jetbrains.annotations.Nullable; import org.json.JSONArray; import org.json.JSONObject; +import org.junit.Assert; import org.junit.Test; import org.labkey.api.action.ApiResponse; import org.labkey.api.action.ApiSimpleResponse; @@ -107,10 +108,16 @@ import org.labkey.api.reports.LabKeyScriptEngineManager; import org.labkey.api.security.AdminConsoleAction; import org.labkey.api.security.IgnoresTermsOfUse; +import org.labkey.api.security.MutableSecurityPolicy; import org.labkey.api.security.RequiresLogin; import org.labkey.api.security.RequiresNoPermission; import org.labkey.api.security.RequiresPermission; +import org.labkey.api.security.SecurityManager; +import org.labkey.api.security.SecurityManager.NewUserStatus; +import org.labkey.api.security.SecurityPolicyManager; import org.labkey.api.security.User; +import org.labkey.api.security.UserManager; +import org.labkey.api.security.ValidEmail; import org.labkey.api.security.permissions.AbstractActionPermissionTest; import org.labkey.api.security.permissions.AdminOperationsPermission; import org.labkey.api.security.permissions.AdminPermission; @@ -119,6 +126,9 @@ import org.labkey.api.security.permissions.Permission; import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.permissions.UpdatePermission; +import org.labkey.api.security.roles.FolderAdminRole; +import org.labkey.api.security.roles.ProjectAdminRole; +import org.labkey.api.security.roles.ReaderRole; import org.labkey.api.security.roles.RoleManager; import org.labkey.api.services.ServiceRegistry; import org.labkey.api.settings.AdminConsole; @@ -135,6 +145,7 @@ import org.labkey.api.util.HtmlString; import org.labkey.api.util.HtmlStringBuilder; import org.labkey.api.util.JsonUtil; +import org.labkey.api.util.JunitUtil; import org.labkey.api.util.MimeMap; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.PageFlowUtil.Content; @@ -156,6 +167,7 @@ import org.labkey.api.view.RedirectException; import org.labkey.api.view.UnauthorizedException; import org.labkey.api.view.ViewContext; +import org.labkey.api.view.ViewServlet; import org.labkey.api.view.template.ClientDependency; import org.labkey.api.view.template.WarningService; import org.labkey.api.view.template.Warnings; @@ -176,8 +188,10 @@ import org.labkey.core.workbook.MoveWorkbooksBean; import org.labkey.core.workbook.WorkbookFolderType; import org.labkey.folder.xml.FolderDocument; +import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.validation.BindException; import org.springframework.validation.Errors; +import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.mvc.Controller; @@ -205,10 +219,6 @@ import static org.labkey.api.view.template.WarningService.SESSION_WARNINGS_BANNER_KEY; -/** - * User: jeckels - * Date: Jan 4, 2007 - */ public class CoreController extends SpringActionController { private static final Map _customStylesheetCache = new ConcurrentHashMap<>(); @@ -863,36 +873,11 @@ public static class MoveContainerAction extends MutatingApiAction children = ContainerManager.getAllChildren(target, getUser()); // assumes read permission if (children.contains(parent)) { - errors.reject(ERROR_MSG, "The container '" + parentIdentifier + "' is not a valid parent folder."); - return; + errors.reject(ERROR_MSG, "The container '" + json.get("parent") + "' is not a valid parent folder."); + } + } + + private @Nullable Container validateContainer(JSONObject json, String key, String description, Errors errors) + { + String identifier = json.optString(key, null); + + if (null == identifier) + { + errors.reject(ERROR_MSG, description + " container must be specified for move operation."); + return null; + } + + Path path = Path.parse(identifier); + Container c = ContainerManager.getForPath(path); + + if (null == c) + { + c = ContainerManager.getForId(identifier); + } + + // Treat non-existent and bad permissions equivalently to not leak any info + if (null == c || !c.hasPermission(getUser(), AdminPermission.class)) + { + throw new NotFoundException(description + " not found"); } + + return c; } @Override @@ -949,7 +956,7 @@ public ApiResponse execute(SimpleApiJsonForm form, BindException errors) throws // Prepare aliases JSONObject object = form.getJsonObject(); - Boolean addAlias = (Boolean) object.get("addAlias"); + boolean addAlias = object.optBoolean("addAlias"); // optional, false by default List aliasList = new ArrayList<>(ContainerManager.getAliasesForContainer(target)); aliasList.add(target.getPath()); @@ -2914,4 +2921,106 @@ public void setProvider(String provider) } + public static class MoveContainerTestCase extends Assert + { + private static final String FOLDER_NAME = "MoveContainerFolder"; + private static final String NEW_PARENT = "NewParent"; + private static final ValidEmail TEST_EMAIL; + + static + { + try + { + TEST_EMAIL = new ValidEmail("testMove@myDomain.com"); + } + catch (ValidEmail.InvalidEmailException e) + { + throw new RuntimeException(e); + } + } + + @Test + public void testMoveContainer() throws Exception + { + doCleanup(); + + User adminUser = TestContext.get().getUser(); + Container junit = JunitUtil.getTestContainer(); + Container folder = ContainerManager.createContainer(junit, FOLDER_NAME, adminUser); + Container newParent = ContainerManager.createContainer(junit, NEW_PARENT, adminUser); + + NewUserStatus newUserStatus = SecurityManager.addUser(TEST_EMAIL, null); + User nonAdminUser = newUserStatus.getUser(); + // Create and save a new, non-empty policy for the folder so it doesn't inherit permissions + MutableSecurityPolicy policy = new MutableSecurityPolicy(folder.getPolicy()); + policy.addRoleAssignment(nonAdminUser, ReaderRole.class); + SecurityPolicyManager.savePolicyForTests(policy, adminUser); + + // Admin permissions nowhere... should fail + moveFolder(nonAdminUser, folder, newParent, HttpServletResponse.SC_FORBIDDEN); + + // Give nonAdminUser admin permission in just the folder being moved and try again (should fail) + policy = new MutableSecurityPolicy(folder.getPolicy()); + policy.addRoleAssignment(nonAdminUser, FolderAdminRole.class); + SecurityPolicyManager.savePolicyForTests(policy, adminUser); + moveFolder(nonAdminUser, folder, newParent, HttpServletResponse.SC_FORBIDDEN); + + // Give nonAdminUser admin permission in the new parent as well and try again (should still fail) + MutableSecurityPolicy newParentPolicy = new MutableSecurityPolicy(newParent); + newParentPolicy.addRoleAssignment(nonAdminUser, FolderAdminRole.class); + SecurityPolicyManager.savePolicyForTests(newParentPolicy, adminUser); + moveFolder(nonAdminUser, folder, newParent, HttpServletResponse.SC_FORBIDDEN); + + // Give nonAdminUser admin permission in the folder's current parent and try again (should now succeed) + policy = new MutableSecurityPolicy(folder.getParent().getPolicy()); + policy.addRoleAssignment(nonAdminUser, FolderAdminRole.class); + SecurityPolicyManager.savePolicyForTests(policy, adminUser); + moveFolder(nonAdminUser, folder, newParent, HttpServletResponse.SC_OK); + folder = ContainerManager.getForId(folder.getId()); // Refresh its path + assertNotNull(folder); + assertEquals(junit.getPath() + "/" + NEW_PARENT + "/" + FOLDER_NAME, folder.getPath()); + // Should be able to move it back + moveFolder(nonAdminUser, folder, junit, HttpServletResponse.SC_OK); + folder = ContainerManager.getForId(folder.getId()); // Refresh its path + assertNotNull(folder); + assertEquals(junit.getPath() + "/" + FOLDER_NAME, folder.getPath()); + + // Happy path -- admin user should be able to move folder to new parent and back + moveFolder(adminUser, folder, newParent, HttpServletResponse.SC_OK); + folder = ContainerManager.getForId(folder.getId()); // Refresh its path + assertNotNull(folder); + assertEquals(junit.getPath() + "/" + NEW_PARENT + "/" + FOLDER_NAME, folder.getPath()); + moveFolder(adminUser, folder, junit, HttpServletResponse.SC_OK); + folder = ContainerManager.getForId(folder.getId()); // Refresh its path + assertNotNull(folder); + assertEquals(junit.getPath() + "/" + FOLDER_NAME, folder.getPath()); + } + + private void moveFolder(User user, Container folder, Container newParent, int expectedResponseCode) throws Exception + { + JSONObject json = new JSONObject().put("container", folder.getId()).put("parent", newParent.getId()); + HttpServletRequest request = ViewServlet.mockRequest(RequestMethod.POST.name(), new ActionURL(MoveContainerAction.class, folder), user, Map.of("Content-Type", "application/json"), json.toString()); + MockHttpServletResponse response = ViewServlet.mockDispatch(request, null); + assertEquals("Unexpected response code", expectedResponseCode, response.getStatus()); + } + + private static void doCleanup() throws Exception + { + Container folder = ContainerManager.getForPath(JunitUtil.getTestContainer().getPath() + "/" + FOLDER_NAME); + if (folder != null) + { + ContainerManager.deleteAll(folder, TestContext.get().getUser()); + } + + Container newParent = ContainerManager.getForPath(JunitUtil.getTestContainer().getPath() + "/" + NEW_PARENT); + if (newParent != null) + { + ContainerManager.deleteAll(newParent, TestContext.get().getUser()); + } + + User u = UserManager.getUser(TEST_EMAIL); + if (u != null) + UserManager.deleteUser(u.getUserId()); + } + } } diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index bd342ae5afa..3c862288961 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -1398,6 +1398,7 @@ public Set getIntegrationTests() AllowListType.TestCase.class, AttachmentServiceImpl.TestCase.class, CoreController.TestCase.class, + CoreController.MoveContainerTestCase.class, DataRegion.TestCase.class, DavController.TestCase.class, DavController.MoveActionContainerScopingTestCase.class, diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index cc2ffd6445f..33bfd5a917d 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -2415,7 +2415,8 @@ public static class CheckDataFileAction extends MutatingApiAction public void validateForm(DataFileForm form, Errors errors) { _data = form.lookupData(); - if (_data == null) + // Not using ensureCorrectContainer() because we don't redirect API actions + if (_data == null || !getContainer().equals(_data.getContainer())) { errors.reject(ERROR_MSG, "No ExpData found for id: " + form.getRowId()); } @@ -8387,7 +8388,17 @@ public void testDataClassAttachmentContainerScoping() throws Exception .addParameter("lsid", lsid) .addParameter("name", attachmentName); assertStatus(HttpServletResponse.SC_OK, get(ownUrl, admin)); + + ActionURL checkDataFileUrl = new ActionURL(CheckDataFileAction.class, folderB) + .addParameter("rowId", data.getRowId()); + assertStatus(HttpServletResponse.SC_OK, post(checkDataFileUrl, admin)); + assertStatus(HttpServletResponse.SC_FORBIDDEN, post(checkDataFileUrl, readerA)); // No perms + checkDataFileUrl.setContainer(folderA); + assertStatus(HttpServletResponse.SC_FORBIDDEN, post(checkDataFileUrl, readerA)); // Has read in folder A, but not admin + resp = post(checkDataFileUrl, admin); // Wrong container. Not found. + assertStatus(HttpServletResponse.SC_BAD_REQUEST, resp); + JSONObject json = new JSONObject(resp.getContentAsString()); + assertEquals("No ExpData found for id: " + data.getRowId(), json.get("exception")); } } - } diff --git a/filecontent/src/org/labkey/filecontent/FileContentController.java b/filecontent/src/org/labkey/filecontent/FileContentController.java index 10e144ca207..3f01703bb95 100644 --- a/filecontent/src/org/labkey/filecontent/FileContentController.java +++ b/filecontent/src/org/labkey/filecontent/FileContentController.java @@ -16,10 +16,13 @@ package org.labkey.filecontent; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import org.json.JSONArray; import org.json.JSONObject; +import org.junit.Test; import org.labkey.api.action.ApiJsonWriter; import org.labkey.api.action.ApiResponse; import org.labkey.api.action.ApiResponseWriter; @@ -79,19 +82,27 @@ import org.labkey.api.query.UserSchema; import org.labkey.api.query.ValidationError; import org.labkey.api.reader.Readers; +import org.labkey.api.security.MutableSecurityPolicy; import org.labkey.api.security.RequiresLogin; import org.labkey.api.security.RequiresNoPermission; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.RequiresSiteAdmin; +import org.labkey.api.security.SecurityManager; +import org.labkey.api.security.SecurityManager.NewUserStatus; +import org.labkey.api.security.SecurityPolicyManager; import org.labkey.api.security.User; +import org.labkey.api.security.UserManager; +import org.labkey.api.security.ValidEmail; import org.labkey.api.security.permissions.AbstractActionPermissionTest; import org.labkey.api.security.permissions.AdminOperationsPermission; import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.security.permissions.InsertPermission; import org.labkey.api.security.permissions.ReadPermission; +import org.labkey.api.security.roles.ReaderRole; import org.labkey.api.util.DOM; import org.labkey.api.util.FileUtil; import org.labkey.api.util.HtmlString; +import org.labkey.api.util.JunitUtil; import org.labkey.api.util.LinkBuilder; import org.labkey.api.util.MimeMap; import org.labkey.api.util.NetworkDrive; @@ -108,6 +119,7 @@ import org.labkey.api.view.Portal; import org.labkey.api.view.UnauthorizedException; import org.labkey.api.view.ViewContext; +import org.labkey.api.view.ViewServlet; import org.labkey.api.view.WebPartView; import org.labkey.api.view.template.PageConfig; import org.labkey.api.webdav.WebdavResource; @@ -115,10 +127,13 @@ import org.labkey.api.writer.HtmlWriter; import org.labkey.filecontent.message.FileEmailConfig; import org.labkey.filecontent.message.ShortMessageDigest; +import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.validation.BindException; import org.springframework.validation.Errors; import org.springframework.validation.ObjectError; +import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.servlet.ModelAndView; +import org.springframework.web.servlet.mvc.Controller; import java.io.BufferedReader; import java.io.File; @@ -720,6 +735,9 @@ public Set> getChildren(NodeForm form, BindException errors) if (c == null) c = ContainerManager.getRoot(); + if (!c.hasPermission(getUser(), ReadPermission.class)) + throw new UnauthorizedException("You do not have permission to read this summary."); + ActionURL browse = new ActionURL(BeginAction.class, c); Set> children = FileContentServiceImpl.getInstance().getNodes(form.isShowOverridesOnly(), browse.getEncodedLocalURIString(), c); @@ -756,6 +774,9 @@ protected Set> getChildren(NodeForm form, BindException erro if (c == null) c = ContainerManager.getRoot(); + if (!c.hasPermission(getUser(), ReadPermission.class)) + throw new UnauthorizedException("You do not have permission to read this project summary."); + Set> children = new LinkedHashSet<>(); FileContentService svc = FileContentService.get(); @@ -1605,5 +1626,35 @@ public void testActionPermissions() new SendShortDigestAction() ); } + + @Test + public void testSummaryActions() throws Exception + { + Container folder = JunitUtil.getTestContainer(); + User adminUser = TestContext.get().getUser(); + + // Happy path -- admin should be able to invoke summary actions in root + testAction(FileContentSummaryAction.class, folder, adminUser, HttpServletResponse.SC_OK); + testAction(FileContentProjectSummaryAction.class, folder, adminUser, HttpServletResponse.SC_OK); + + NewUserStatus newUserStatus = SecurityManager.addUser(new ValidEmail("testSummaryActions@myDomain.com"), null); + User nonAdminUser = newUserStatus.getUser(); + MutableSecurityPolicy policy = new MutableSecurityPolicy(folder.getPolicy()); + policy.addRoleAssignment(nonAdminUser, ReaderRole.class); + SecurityPolicyManager.savePolicyForTests(policy, adminUser); + + // Non-admin user should be forbidden + testAction(FileContentSummaryAction.class, folder, nonAdminUser, HttpServletResponse.SC_FORBIDDEN); + testAction(FileContentProjectSummaryAction.class, folder, nonAdminUser, HttpServletResponse.SC_FORBIDDEN); + + UserManager.deleteUser(nonAdminUser.getUserId()); + } + + private void testAction(Class actionClass, Container folder, User user, int expectedResponseCode) throws Exception + { + HttpServletRequest request = ViewServlet.mockRequest(RequestMethod.POST.name(), new ActionURL(actionClass, folder), user, Map.of("Content-Type", "application/json"), null); + MockHttpServletResponse response = ViewServlet.mockDispatch(request, null); + assertEquals("Unexpected response code", expectedResponseCode, response.getStatus()); + } } } diff --git a/mothership/src/org/labkey/mothership/MothershipController.java b/mothership/src/org/labkey/mothership/MothershipController.java index 398dec07b60..d5b8579530c 100644 --- a/mothership/src/org/labkey/mothership/MothershipController.java +++ b/mothership/src/org/labkey/mothership/MothershipController.java @@ -363,12 +363,21 @@ public static class ShowServerSessionDetailAction extends SimpleViewAction { - public ServerInstallationForm(ServerInstallation installation) - { - this(); - setBean(installation); - } - public ServerInstallationForm() { super(ServerInstallation.class, MothershipManager.get().getTableInfoServerInstallation()); } } - public static class ServerSessionForm extends BeanViewForm + public static class ServerSessionForm { - public ServerSessionForm(ServerSession session) + private Integer _serverSessionId = null; + + public Integer getServerSessionId() { - this(); - setBean(session); + return _serverSessionId; } - public ServerSessionForm() + public void setServerSessionId(Integer serverSessionId) { - super(ServerSession.class, MothershipManager.get().getTableInfoServerSession()); + _serverSessionId = serverSessionId; } } diff --git a/mothership/src/org/labkey/mothership/MothershipManager.java b/mothership/src/org/labkey/mothership/MothershipManager.java index f5a9c22f104..26eede9da92 100644 --- a/mothership/src/org/labkey/mothership/MothershipManager.java +++ b/mothership/src/org/labkey/mothership/MothershipManager.java @@ -226,6 +226,13 @@ public ServerSession getServerSession(String serverSessionGUID, Container c) return new TableSelector(getTableInfoServerSession(), filter, null).getObject(ServerSession.class); } + public ServerSession getServerSession(int serverSessionId, Container c) + { + SimpleFilter filter = SimpleFilter.createContainerFilter(c); + filter.addCondition(FieldKey.fromString("ServerSessionId"), serverSessionId); + return new TableSelector(getTableInfoServerSession(), filter, null).getObject(ServerSession.class); + } + public ExceptionStackTrace getExceptionStackTrace(String stackTraceHash, String containerId) { SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Container"), containerId); diff --git a/mothership/src/org/labkey/mothership/query/MothershipSchema.java b/mothership/src/org/labkey/mothership/query/MothershipSchema.java index 39aead99dbf..1845a0e7731 100644 --- a/mothership/src/org/labkey/mothership/query/MothershipSchema.java +++ b/mothership/src/org/labkey/mothership/query/MothershipSchema.java @@ -27,6 +27,7 @@ import org.labkey.api.data.DataColumn; import org.labkey.api.data.ForeignKey; import org.labkey.api.data.JdbcType; +import org.labkey.api.data.RenderContext; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.TableInfo; import org.labkey.api.data.dialect.SqlDialect; @@ -405,18 +406,25 @@ public FilteredTable createExceptionReportTable(ContainerFilte FilteredTable result = new FilteredTable<>(MothershipManager.get().getTableInfoExceptionReport(), this, cf); result.setDetailsURL(AbstractTableInfo.LINK_DISABLER); result.wrapAllColumns(true); + // Reports are submitted by anonymous users, so untrusted. Don't render these two URLs as links. result.getMutableColumnOrThrow("URL").setDisplayColumnFactory(colInfo -> - { - DataColumn result1 = new DataColumn(colInfo); - result1.setURLExpression(StringExpressionFactory.create("${URL}", false)); - return result1; - }); + new DataColumn(colInfo) + { + @Override + protected String renderURLorValueURL(RenderContext ctx) + { + return null; + } + }); result.getMutableColumnOrThrow("ReferrerURL").setDisplayColumnFactory(colInfo -> - { - DataColumn result12 = new DataColumn(colInfo); - result12.setURLExpression(StringExpressionFactory.create("${ReferrerURL}", false)); - return result12; - }); + new DataColumn(colInfo) + { + @Override + protected String renderURLorValueURL(RenderContext ctx) + { + return null; + } + }); // Container column is on another table so join to it to filter appropriately SQLFragment containerSQL = new SQLFragment("ExceptionStackTraceId IN (SELECT ExceptionStackTraceId FROM "); diff --git a/specimen/src/org/labkey/specimen/actions/SpecimenController.java b/specimen/src/org/labkey/specimen/actions/SpecimenController.java index f3d79169d7e..9d48a30e7c7 100644 --- a/specimen/src/org/labkey/specimen/actions/SpecimenController.java +++ b/specimen/src/org/labkey/specimen/actions/SpecimenController.java @@ -5546,6 +5546,8 @@ public void addNavTrail(NavTree root) } } + public record PtidVisit(String ptid, String visit){} + @RequiresPermission(ReadPermission.class) public class SelectedSpecimensAction extends QueryViewAction { @@ -5561,26 +5563,26 @@ public SelectedSpecimensAction() protected ModelAndView getHtmlView(SpecimenViewTypeForm form, BindException errors) throws Exception { Study study = getStudyRedirectIfNull(); - Set> ptidVisits = new HashSet<>(); + Set ptidVisits = new HashSet<>(); for (ParticipantDataset pd : getFilterPds()) { if (pd.getSequenceNum() == null) { - ptidVisits.add(new Pair<>(pd.getParticipantId(), null)); + ptidVisits.add(new PtidVisit(pd.getParticipantId(), null)); } else if (study.getTimepointType() != TimepointType.VISIT && pd.getVisitDate() != null) { - ptidVisits.add(new Pair<>(pd.getParticipantId(), DateUtil.formatDate(pd.getContainer(), pd.getVisitDate()))); + ptidVisits.add(new PtidVisit(pd.getParticipantId(), DateUtil.formatDate(pd.getContainer(), pd.getVisitDate()))); } else { Visit visit = pd.getSequenceNum() != null ? StudyInternalService.get().getVisitForSequence(study, pd.getSequenceNum()) : null; - ptidVisits.add(new Pair<>(pd.getParticipantId(), visit != null ? visit.getLabel() : "" + StudyInternalService.get().formatSequenceNum(pd.getSequenceNum()))); + ptidVisits.add(new PtidVisit(pd.getParticipantId(), visit != null ? visit.getLabel() : StudyInternalService.get().formatSequenceNum(pd.getSequenceNum()))); } } SpecimenQueryView view = createInitializedQueryView(form, errors, form.getExportType() != null, null); JspView header = new JspView<>("/org/labkey/specimen/view/specimenHeader.jsp", - new SpecimenHeaderBean(getViewContext(), view, ptidVisits)); + new SpecimenHeaderBean(getViewContext(), view, ptidVisits)); return new VBox(header, view); } diff --git a/specimen/src/org/labkey/specimen/actions/SpecimenHeaderBean.java b/specimen/src/org/labkey/specimen/actions/SpecimenHeaderBean.java index fe32d12dc24..fd0cbd575df 100644 --- a/specimen/src/org/labkey/specimen/actions/SpecimenHeaderBean.java +++ b/specimen/src/org/labkey/specimen/actions/SpecimenHeaderBean.java @@ -5,13 +5,13 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.query.QueryService; import org.labkey.api.specimen.SpecimenQuerySchema; -import org.labkey.specimen.query.SpecimenQueryView; import org.labkey.api.study.Study; import org.labkey.api.study.StudyService; -import org.labkey.api.util.Pair; import org.labkey.api.view.ActionURL; import org.labkey.api.view.NotFoundException; import org.labkey.api.view.ViewContext; +import org.labkey.specimen.actions.SpecimenController.PtidVisit; +import org.labkey.specimen.query.SpecimenQueryView; import java.util.Collections; import java.util.Iterator; @@ -24,7 +24,7 @@ public final class SpecimenHeaderBean private final ActionURL _otherViewURL; private final ViewContext _viewContext; private final boolean _showingVials; - private final Set> _filteredPtidVisits; + private final Set _ptidVisits; private Integer _selectedRequest; @@ -33,7 +33,7 @@ public SpecimenHeaderBean(ViewContext context, SpecimenQueryView view) this(context, view, Collections.emptySet()); } - public SpecimenHeaderBean(ViewContext context, SpecimenQueryView view, Set> filteredPtidVisits) throws RuntimeException + public SpecimenHeaderBean(ViewContext context, SpecimenQueryView view, Set ptidVisits) throws RuntimeException { Map params = context.getRequest().getParameterMap(); @@ -90,11 +90,11 @@ public SpecimenHeaderBean(ViewContext context, SpecimenQueryView view, Set> getFilteredPtidVisits() + public Set getPtidVisits() { - return _filteredPtidVisits; + return _ptidVisits; } public boolean isSingleVisitFilter() { - if (getFilteredPtidVisits().isEmpty()) + if (getPtidVisits().isEmpty()) return false; - Iterator> visitIt = getFilteredPtidVisits().iterator(); - String firstVisit = visitIt.next().getValue(); - while (visitIt.hasNext()) + Iterator ptidVisit = getPtidVisits().iterator(); + String firstVisit = ptidVisit.next().visit(); + while (ptidVisit.hasNext()) { - if (!Objects.equals(firstVisit, visitIt.next().getValue())) + if (!Objects.equals(firstVisit, ptidVisit.next().visit())) return false; } return true; diff --git a/specimen/src/org/labkey/specimen/view/manageRequirement.jsp b/specimen/src/org/labkey/specimen/view/manageRequirement.jsp index 68109262af7..6721bbc61de 100644 --- a/specimen/src/org/labkey/specimen/view/manageRequirement.jsp +++ b/specimen/src/org/labkey/specimen/view/manageRequirement.jsp @@ -84,7 +84,7 @@ Description - <%= unsafe(requirement.getDescription()) %> + <%= h(requirement.getDescription()) %> <% if (!bean.isRequestManager()) diff --git a/specimen/src/org/labkey/specimen/view/specimenHeader.jsp b/specimen/src/org/labkey/specimen/view/specimenHeader.jsp index b0599197eab..b1e2c7b5089 100644 --- a/specimen/src/org/labkey/specimen/view/specimenHeader.jsp +++ b/specimen/src/org/labkey/specimen/view/specimenHeader.jsp @@ -15,16 +15,18 @@ * limitations under the License. */ %> -<%@ page import="org.labkey.api.security.permissions.AdminPermission"%> -<%@ page import="org.labkey.api.study.StudyService"%> -<%@ page import="org.labkey.api.study.StudyUrls"%> -<%@ page import="org.labkey.api.util.Pair"%> +<%@ page import="org.apache.commons.lang3.StringUtils" %> +<%@ page import="org.labkey.api.security.permissions.AdminPermission" %> +<%@ page import="org.labkey.api.study.StudyService" %> +<%@ page import="org.labkey.api.study.StudyUrls" %> +<%@ page import="org.labkey.api.util.HtmlStringBuilder" %> <%@ page import="org.labkey.api.view.ActionURL" %> <%@ page import="org.labkey.api.view.HttpView" %> <%@ page import="org.labkey.api.view.JspView" %> <%@ page import="org.labkey.api.view.template.ClientDependencies" %> <%@ page import="org.labkey.specimen.actions.ShowSearchAction" %> <%@ page import="org.labkey.specimen.actions.SpecimenController" %> +<%@ page import="org.labkey.specimen.actions.SpecimenController.PtidVisit" %> <%@ page import="org.labkey.specimen.actions.SpecimenController.SpecimensAction" %> <%@ page import="org.labkey.specimen.actions.SpecimenHeaderBean" %> <%@ page import="java.util.Iterator" %> @@ -74,47 +76,57 @@ <%=link("Search", ShowSearchAction.getShowSearchURL(getContainer(), bean.isShowingVials()))%>  <%=link("Reports", urlFor(SpecimenController.AutoReportListAction.class)) %> <% - if (!bean.getFilteredPtidVisits().isEmpty()) + if (!bean.getPtidVisits().isEmpty()) { // get the first visit label: - StringBuilder filterString = new StringBuilder(); - filterString.append("This view is displaying specimens only from "); - boolean usePlural = bean.getFilteredPtidVisits().size() != 1; + HtmlStringBuilder builder = HtmlStringBuilder.of() + .unsafeAppend("") + .append("This view is displaying specimens only from "); + boolean usePlural = bean.getPtidVisits().size() != 1; if (bean.isSingleVisitFilter()) { - filterString.append(h((usePlural?subjectNounPlural:subjectNounSingle).toLowerCase())).append(" "); - for (Iterator> it = bean.getFilteredPtidVisits().iterator(); it.hasNext();) + builder.append((usePlural ? subjectNounPlural : subjectNounSingle).toLowerCase()) + .append(" "); + for (Iterator it = bean.getPtidVisits().iterator(); it.hasNext();) { - String ptid = it.next().getKey(); - filterString.append(ptid); + String ptid = it.next().ptid(); + builder.append(ptid); if (it.hasNext()) - filterString.append(", "); + builder.append(", "); } - String visit = bean.getFilteredPtidVisits().iterator().next().getValue(); + String visit = bean.getPtidVisits().iterator().next().visit(); if (visit != null) - filterString.append(" at visit ").append(visit); - filterString.append(".
"); + builder.append(" at visit ").append(visit); + + builder.append(".") + .unsafeAppend("

"); } else { - filterString.append(" the following ").append(h(subjectNounSingle.toLowerCase())).append("/visit ").append(usePlural?"pairs":"pair").append(":
"); - for (Iterator> it = bean.getFilteredPtidVisits().iterator(); it.hasNext();) + builder.append(" the following ") + .append(subjectNounSingle.toLowerCase()) + .append("/visit ").append(usePlural ? "pairs" : "pair") + .append(":") + .unsafeAppend("
"); + for (Iterator it = bean.getPtidVisits().iterator(); it.hasNext();) { - Pair ptidVisit = it.next(); - filterString.append(ptidVisit.getKey()).append("/").append(ptidVisit.getValue()); + PtidVisit ptidVisit = it.next(); + builder.append(ptidVisit.ptid()) + .append("/") + .append(StringUtils.trimToEmpty(ptidVisit.visit())); if (it.hasNext()) - filterString.append(", "); + builder.append(", "); } - filterString.append("."); + builder.append("."); } - ActionURL noFitlerUrl = getViewContext().cloneActionURL().setAction(SpecimensAction.class); + ActionURL noFilterUrl = getViewContext().cloneActionURL().setAction(SpecimensAction.class); %>

- +
<%= unsafe(filterString.toString()) %>
<%=builder%>

-<%= link("Remove " + subjectNounSingle + "/Visit Filter", noFitlerUrl)%><% +<%= link("Remove " + subjectNounSingle + "/Visit Filter", noFilterUrl)%><% } %>
diff --git a/study/src/org/labkey/study/StudyModule.java b/study/src/org/labkey/study/StudyModule.java index 60bb3b9d968..08210a05e20 100644 --- a/study/src/org/labkey/study/StudyModule.java +++ b/study/src/org/labkey/study/StudyModule.java @@ -737,6 +737,7 @@ public Set getUnitTests() DatasetDataWriter.TestCase.class, DefaultStudyDesignWriter.TestCase.class, ParticipantIdImportHelper.ParticipantIdTest.class, + ReportsController.TestCase.class, SequenceNumImportHelper.SequenceNumTest.class, StudyImpl.DateMathTestCase.class ); diff --git a/study/src/org/labkey/study/controllers/reports/ReportsController.java b/study/src/org/labkey/study/controllers/reports/ReportsController.java index e09c6265b59..f2b1b06dae4 100644 --- a/study/src/org/labkey/study/controllers/reports/ReportsController.java +++ b/study/src/org/labkey/study/controllers/reports/ReportsController.java @@ -22,6 +22,7 @@ import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; +import org.junit.Test; import org.labkey.api.action.Action; import org.labkey.api.action.ActionType; import org.labkey.api.action.ApiResponse; @@ -49,9 +50,9 @@ import org.labkey.api.reports.report.view.ReportUtil; import org.labkey.api.reports.report.view.ScriptReportBean; import org.labkey.api.security.RequiresLogin; -import org.labkey.api.security.RequiresNoPermission; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.User; +import org.labkey.api.security.permissions.AbstractActionPermissionTest; import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.security.permissions.InsertPermission; import org.labkey.api.security.permissions.ReadPermission; @@ -63,6 +64,7 @@ import org.labkey.api.study.reports.CrosstabReportDescriptor; import org.labkey.api.util.ImageUtil; import org.labkey.api.util.PageFlowUtil; +import org.labkey.api.util.TestContext; import org.labkey.api.util.URLHelper; import org.labkey.api.util.UniqueID; import org.labkey.api.view.ActionURL; @@ -533,7 +535,7 @@ public void addNavTrail(NavTree root) } } - @RequiresNoPermission + @RequiresPermission(ReadPermission.class) public class CreateCrosstabReportAction extends SimpleViewAction { @Override @@ -661,13 +663,22 @@ public int getReportId() return reportId; } + @SuppressWarnings("unused") public void setReportId(int reportId) { this.reportId = reportId; } - public void setReportView(String label){_reportView = label;} - public String getReportView(){return _reportView;} + public String getReportView() + { + return _reportView; + } + + @SuppressWarnings("unused") + public void setReportView(String label) + { + _reportView = label; + } } public static class SaveReportForm extends ViewForm @@ -765,13 +776,35 @@ public void setShowWithDataset(int showWithDataset) this.showWithDataset = showWithDataset; } - public void setRedirectToDataset(Integer dataset){redirectToDataset = dataset;} - public Integer getRedirectToDataset(){return redirectToDataset;} + public Integer getRedirectToDataset() + { + return redirectToDataset; + } + + public void setRedirectToDataset(Integer dataset) + { + redirectToDataset = dataset; + } + + public String getDescription() + { + return this.description; + } + + public void setDescription(String description) + { + this.description = description; + } + + public BindException getErrors() + { + return _errors; + } - public void setDescription(String description){this.description = description;} - public String getDescription(){return this.description;} - public void setErrors(BindException errors){_errors = errors;} - public BindException getErrors(){return _errors;} + public void setErrors(BindException errors) + { + _errors = errors; + } public String getDataRegionName() { @@ -829,19 +862,57 @@ public Report getReport(ContainerUser cu) return null; } - public void setShareReport(boolean shareReport){_shareReport = shareReport;} - public boolean getShareReport(){return _shareReport;} + public void setShareReport(boolean shareReport) + { + _shareReport = shareReport; + } + + public boolean getShareReport() + { + return _shareReport; + } + + public void setSchemaName(String schemaName) + { + _schemaName = schemaName; + } + + public String getSchemaName() + { + return _schemaName; + } + + public void setQueryName(String queryName) + { + _queryName = queryName; + } + + public String getQueryName() + { + return _queryName; + } + + public void setViewName(String viewName) + { + _viewName = viewName; + } + + public String getViewName() + { + return _viewName; + } - public void setSchemaName(String schemaName){_schemaName = schemaName;} - public String getSchemaName(){return _schemaName;} - public void setQueryName(String queryName){_queryName = queryName;} - public String getQueryName(){return _queryName;} - public void setViewName(String viewName){_viewName = viewName;} - public String getViewName(){return _viewName;} @Override - public void setDataRegionName(String dataRegionName){_dataRegionName = dataRegionName;} + public void setDataRegionName(String dataRegionName) + { + _dataRegionName = dataRegionName; + } + @Override - public String getDataRegionName(){return _dataRegionName;} + public String getDataRegionName() + { + return _dataRegionName; + } public String getRedirectUrl() { @@ -969,7 +1040,9 @@ private void _addNavTrail(NavTree root, String name) if (getContainer().hasPermission(getUser(), AdminPermission.class)) root.addChild("Manage Views", urlProvider(ReportUrls.class).urlManageViews(getContainer())); } - catch (Exception ignored) {} + catch (Exception ignored) + { + } root.addChild(name); } @@ -1217,4 +1290,36 @@ public void bindJson(JSONObject json) } } } + + public static class TestCase extends AbstractActionPermissionTest + { + @Override + @Test + public void testActionPermissions() + { + User user = TestContext.get().getUser(); + assertTrue(user.hasSiteAdminPermission()); + + ReportsController controller = new ReportsController(); + + // @RequiresPermission(ReadPermission.class) + assertForReadPermission(user, false, + controller.new BeginAction(), + new StreamFileAction(), + controller.new SaveReportAction(), + controller.new SaveReportViewAction(), + new ShowReportAction(), + new ParticipantCrosstabAction(), + controller.new CreateCrosstabReportAction(), + controller.new RunRReportAction(), + controller.new ParticipantReportAction(), + new SaveParticipantReportAction() + ); + + // @RequiresPermission(AdminPermission.class) + assertForAdminPermission(user, + controller.new CreateQueryReportAction() + ); + } + } }