From 4a6877c3982a82d4ebd651a5a3ed89eb2280c1c5 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 21 Mar 2025 14:07:45 -0700 Subject: [PATCH 1/2] Issue 52614: AssertionError deleting site group from its details page --- .../org/labkey/api/audit/AuditTypeEvent.java | 30 ++--- .../api/audit/ClientApiAuditProvider.java | 3 +- .../api/audit/DetailedAuditTypeEvent.java | 5 - .../api/audit/ExperimentAuditEvent.java | 4 +- .../api/audit/SampleTimelineAuditEvent.java | 2 +- .../api/audit/TransactionAuditProvider.java | 3 +- .../provider/FileSystemAuditProvider.java | 3 +- .../audit/provider/GroupAuditProvider.java | 37 +++++- .../audit/provider/MessageAuditProvider.java | 3 +- .../provider/SiteSettingsAuditProvider.java | 3 +- .../org/labkey/api/data/ContainerManager.java | 5 +- .../api/data/SelectQueryAuditEvent.java | 2 +- .../api/exp/property/DomainAuditProvider.java | 2 +- .../property/DomainPropertyAuditProvider.java | 3 +- .../labkey/api/premium/AntiVirusService.java | 2 +- .../api/query/AbstractQueryUpdateService.java | 2 +- .../api/security/AuthenticationManager.java | 2 +- ...thenticationSettingsAuditTypeProvider.java | 2 +- .../org/labkey/api/security/GroupManager.java | 43 +++---- .../labkey/api/security/NestedGroupsTest.java | 14 +- .../labkey/api/security/SecurityManager.java | 120 +++++++++++------- .../api/security/SecurityPolicyManager.java | 12 +- .../org/labkey/api/security/UserManager.java | 12 +- .../GroupImpersonationContextFactory.java | 4 +- .../RoleImpersonationContextFactory.java | 6 +- .../UserImpersonationContextFactory.java | 8 +- .../settings/AbstractCustomLabelProvider.java | 5 +- .../AbstractWriteableSettingsGroup.java | 6 +- .../org/labkey/api/util/ExceptionUtil.java | 14 +- api/src/org/labkey/api/util/MailHelper.java | 2 +- .../api/webdav/AbstractWebdavResource.java | 2 +- .../audit/query/AuditLogUpdateService.java | 2 +- .../labkey/core/CoreContainerListener.java | 2 +- .../admin/AbstractFileSiteSettingsAction.java | 2 +- .../labkey/core/admin/AdminController.java | 11 +- .../core/admin/UpdateFilePathsAction.java | 2 +- .../SecurityGroupImporterFactory.java | 2 +- .../attachment/AttachmentServiceImpl.java | 8 +- .../core/query/AttachmentAuditProvider.java | 2 +- .../src/org/labkey/core/query/UsersTable.java | 2 +- .../core/security/SecurityApiActions.java | 73 +++-------- .../core/security/SecurityController.java | 53 ++------ .../experiment/SampleTypeAuditProvider.java | 3 +- .../experiment/api/ExpQCFlagTableImpl.java | 6 +- .../experiment/api/ExperimentServiceImpl.java | 5 +- .../experiment/api/SampleTypeServiceImpl.java | 11 +- .../api/SampleTypeUpdateServiceDI.java | 2 +- .../experiment/api/property/DomainImpl.java | 7 +- .../labkey/list/model/ListAuditProvider.java | 6 +- .../org/labkey/list/model/ListManager.java | 21 +-- query/src/org/labkey/query/MultiValueTest.jsp | 4 +- .../org/labkey/query/QueryServiceImpl.java | 9 +- .../query/audit/QueryExportAuditProvider.java | 3 +- .../query/audit/QueryUpdateAuditProvider.java | 2 +- .../query/controllers/QueryController.java | 3 +- .../org/labkey/search/SearchController.java | 2 +- .../search/audit/SearchAuditProvider.java | 3 +- .../specimen/SpecimenCommentAuditEvent.java | 3 +- .../org/labkey/specimen/SpecimenManager.java | 2 +- .../labkey/specimen/SpecimenServiceImpl.java | 2 +- .../actions/ShowGroupMembersAction.java | 4 +- .../specimen/actions/SpecimenController.java | 2 +- .../importer/SpecimenSettingsImporter.java | 6 +- .../specimen/requirements/DefaultActor.java | 12 +- .../DefaultRequirementProvider.java | 12 +- .../requirements/RequirementActor.java | 4 +- .../requirements/RequirementProvider.java | 2 +- .../study/assay/StudyPublishManager.java | 8 +- .../assay/query/PublishAuditProvider.java | 3 +- .../study/dataset/DatasetAuditProvider.java | 4 +- .../labkey/study/model/DatasetDefinition.java | 17 +-- .../org/labkey/study/model/StudyManager.java | 6 +- 72 files changed, 323 insertions(+), 371 deletions(-) diff --git a/api/src/org/labkey/api/audit/AuditTypeEvent.java b/api/src/org/labkey/api/audit/AuditTypeEvent.java index ad71db7a059..361afb6b49a 100644 --- a/api/src/org/labkey/api/audit/AuditTypeEvent.java +++ b/api/src/org/labkey/api/audit/AuditTypeEvent.java @@ -19,7 +19,6 @@ import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.security.Group; -import org.labkey.api.security.SecurityManager; import org.labkey.api.security.User; import org.labkey.api.security.UserManager; @@ -56,16 +55,16 @@ public class AuditTypeEvent private String userComment; private Long _transactionId; - public AuditTypeEvent(String eventType, Container container, String comment) - { - this(eventType, container.getId(), comment); - } - - public AuditTypeEvent(String eventType, String container, String comment) + public AuditTypeEvent(String eventType, @NotNull Container container, String comment) { _eventType = eventType; - _container = container; + _container = container.getId(); _comment = comment; + Container project = container.getProject(); + if (project != null) + { + _projectId = project.getId(); + } } public AuditTypeEvent(){} @@ -199,7 +198,12 @@ protected String getContainerMessageElement(@NotNull String containerId) return value; } - protected String getUserMessageElement(@NotNull Integer userId) + protected String getUserMessageElement(@NotNull User user) + { + return user.getEmail() + " (" + user.getUserId() + ")"; + } + + protected String getUserMessageElement(int userId) { String value = " (" + userId + ")"; User user = UserManager.getUser(userId); @@ -208,13 +212,9 @@ protected String getUserMessageElement(@NotNull Integer userId) return value; } - protected String getGroupMessageElement(@NotNull Integer groupId) + protected String getGroupMessageElement(@NotNull Group group) { - String value = " (" + groupId + ")"; - Group group = SecurityManager.getGroup(groupId); - if (group != null) - value = group.getName() + value; - return value; + return group.getName() + " (" + group.getUserId() + ")"; } public Map getAuditLogMessageElements() diff --git a/api/src/org/labkey/api/audit/ClientApiAuditProvider.java b/api/src/org/labkey/api/audit/ClientApiAuditProvider.java index e5680de24b8..48e1735ee52 100644 --- a/api/src/org/labkey/api/audit/ClientApiAuditProvider.java +++ b/api/src/org/labkey/api/audit/ClientApiAuditProvider.java @@ -18,6 +18,7 @@ import org.jetbrains.annotations.NotNull; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; +import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.PropertyStorageSpec.Index; import org.labkey.api.data.TableInfo; @@ -165,7 +166,7 @@ public ClientApiAuditEvent() super(); } - public ClientApiAuditEvent(String container, String comment) + public ClientApiAuditEvent(Container container, String comment) { super(EVENT_TYPE, container, comment); } diff --git a/api/src/org/labkey/api/audit/DetailedAuditTypeEvent.java b/api/src/org/labkey/api/audit/DetailedAuditTypeEvent.java index 8b7ab0cbc95..0ccb580e6f1 100644 --- a/api/src/org/labkey/api/audit/DetailedAuditTypeEvent.java +++ b/api/src/org/labkey/api/audit/DetailedAuditTypeEvent.java @@ -12,11 +12,6 @@ public DetailedAuditTypeEvent() } public DetailedAuditTypeEvent(String eventType, Container container, String comment) - { - super(eventType, container.getId(), comment); - } - - public DetailedAuditTypeEvent(String eventType, String container, String comment) { super(eventType, container, comment); } diff --git a/api/src/org/labkey/api/audit/ExperimentAuditEvent.java b/api/src/org/labkey/api/audit/ExperimentAuditEvent.java index bdb9cd7530f..8a1d94465f7 100644 --- a/api/src/org/labkey/api/audit/ExperimentAuditEvent.java +++ b/api/src/org/labkey/api/audit/ExperimentAuditEvent.java @@ -1,5 +1,7 @@ package org.labkey.api.audit; +import org.labkey.api.data.Container; + import java.util.LinkedHashMap; import java.util.Map; @@ -19,7 +21,7 @@ public ExperimentAuditEvent() super(); } - public ExperimentAuditEvent(String container, String comment) + public ExperimentAuditEvent(Container container, String comment) { super(EVENT_TYPE, container, comment); } diff --git a/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java b/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java index 0a4dae040f2..4c5c1cd5262 100644 --- a/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java +++ b/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java @@ -93,7 +93,7 @@ public SampleTimelineAuditEvent() super(); } - public SampleTimelineAuditEvent(String container, String comment) + public SampleTimelineAuditEvent(Container container, String comment) { super(EVENT_TYPE, container, comment); } diff --git a/api/src/org/labkey/api/audit/TransactionAuditProvider.java b/api/src/org/labkey/api/audit/TransactionAuditProvider.java index 134334881b5..a8be5907012 100644 --- a/api/src/org/labkey/api/audit/TransactionAuditProvider.java +++ b/api/src/org/labkey/api/audit/TransactionAuditProvider.java @@ -4,6 +4,7 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; +import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.JdbcType; import org.labkey.api.data.MutableColumnInfo; @@ -111,7 +112,7 @@ public TransactionAuditEvent() super(); } - public TransactionAuditEvent(String container, QueryService.AuditAction auditAction, long transactionId) + public TransactionAuditEvent(Container container, QueryService.AuditAction auditAction, long transactionId) { super(EVENT_TYPE, container, auditAction.getDefaultCommentSummary()); _auditAction = auditAction; diff --git a/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java b/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java index fbe6c8903ca..6f40484caf0 100644 --- a/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java +++ b/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java @@ -19,6 +19,7 @@ import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.AuditTypeProvider; import org.labkey.api.audit.query.AbstractAuditDomainKind; +import org.labkey.api.data.Container; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.PropertyType; import org.labkey.api.query.FieldKey; @@ -111,7 +112,7 @@ public FileSystemAuditEvent() super(); } - public FileSystemAuditEvent(String container, String comment) + public FileSystemAuditEvent(Container container, String comment) { super(EVENT_TYPE, container, comment); } diff --git a/api/src/org/labkey/api/audit/provider/GroupAuditProvider.java b/api/src/org/labkey/api/audit/provider/GroupAuditProvider.java index 9e33fff8ad4..b4042163783 100644 --- a/api/src/org/labkey/api/audit/provider/GroupAuditProvider.java +++ b/api/src/org/labkey/api/audit/provider/GroupAuditProvider.java @@ -58,6 +58,7 @@ import java.io.Writer; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -221,15 +222,43 @@ public static class GroupAuditEvent extends AuditTypeEvent Integer _group; String _resourceEntityId; + private final Map _messageElements = new HashMap<>(); + public GroupAuditEvent() { super(); } - public GroupAuditEvent(String container, String comment) + public GroupAuditEvent(Container container, String comment) { super(GroupManager.GROUP_AUDIT_EVENT, container, comment); } + public GroupAuditEvent(Container c, String comment, Group group) + { + this(c, comment, group, null); + } + public GroupAuditEvent(Container c, String comment, Group group, UserPrincipal userOrGroup) + { + this(c, comment); + if (group != null) + { + _messageElements.put("group", getGroupMessageElement(group)); + _group = group.getUserId(); + } + if (userOrGroup != null) + { + if (userOrGroup instanceof User u) + { + _messageElements.put("user", getUserMessageElement(u)); + } + _user = userOrGroup.getUserId(); + } + } + + public GroupAuditEvent(Container c, String comment, UserPrincipal userOrGroup) + { + this(c, comment, null, userOrGroup); + } public Integer getUser() { @@ -264,11 +293,7 @@ public void setResourceEntityId(String resourceEntityId) @Override public Map getAuditLogMessageElements() { - Map elements = new LinkedHashMap<>(); - if (getUser() != null) - elements.put("user", getUserMessageElement(getUser())); - if (getGroup() != null) - elements.put("group", getGroupMessageElement(getGroup())); + Map elements = new LinkedHashMap<>(_messageElements); elements.putAll(super.getAuditLogMessageElements()); return elements; } diff --git a/api/src/org/labkey/api/audit/provider/MessageAuditProvider.java b/api/src/org/labkey/api/audit/provider/MessageAuditProvider.java index 50f731b7266..2f831f54011 100644 --- a/api/src/org/labkey/api/audit/provider/MessageAuditProvider.java +++ b/api/src/org/labkey/api/audit/provider/MessageAuditProvider.java @@ -19,6 +19,7 @@ import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.AuditTypeProvider; import org.labkey.api.audit.query.AbstractAuditDomainKind; +import org.labkey.api.data.Container; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.PropertyType; import org.labkey.api.query.FieldKey; @@ -111,7 +112,7 @@ public MessageAuditEvent() super(); } - public MessageAuditEvent(String container, String comment) + public MessageAuditEvent(Container container, String comment) { super(MailHelper.MESSAGE_AUDIT_EVENT, container, comment); } diff --git a/api/src/org/labkey/api/audit/provider/SiteSettingsAuditProvider.java b/api/src/org/labkey/api/audit/provider/SiteSettingsAuditProvider.java index fb3fd37339a..0a818749984 100644 --- a/api/src/org/labkey/api/audit/provider/SiteSettingsAuditProvider.java +++ b/api/src/org/labkey/api/audit/provider/SiteSettingsAuditProvider.java @@ -21,6 +21,7 @@ import org.labkey.api.audit.AuditTypeProvider; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; +import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.HtmlDisplayColumnFactory; import org.labkey.api.data.TableInfo; @@ -117,7 +118,7 @@ public SiteSettingsAuditEvent() super(); } - public SiteSettingsAuditEvent(String container, String comment) + public SiteSettingsAuditEvent(Container container, String comment) { super(AUDIT_EVENT_TYPE, container, comment); } diff --git a/api/src/org/labkey/api/data/ContainerManager.java b/api/src/org/labkey/api/data/ContainerManager.java index 96f7a59c0e4..59ac691e80e 100644 --- a/api/src/org/labkey/api/data/ContainerManager.java +++ b/api/src/org/labkey/api/data/ContainerManager.java @@ -2027,10 +2027,7 @@ private static void addAuditEvent(User user, Container c, String comment) { if (user != null) { - AuditTypeEvent event = new AuditTypeEvent(ContainerAuditProvider.CONTAINER_AUDIT_EVENT, c.getId(), comment); - if (c.getProject() != null) - event.setProjectId(c.getProject().getId()); - + AuditTypeEvent event = new AuditTypeEvent(ContainerAuditProvider.CONTAINER_AUDIT_EVENT, c, comment); AuditLogService.get().addEvent(user, event); } } diff --git a/api/src/org/labkey/api/data/SelectQueryAuditEvent.java b/api/src/org/labkey/api/data/SelectQueryAuditEvent.java index be0fc8a4d4d..6f594b7c045 100644 --- a/api/src/org/labkey/api/data/SelectQueryAuditEvent.java +++ b/api/src/org/labkey/api/data/SelectQueryAuditEvent.java @@ -40,7 +40,7 @@ public SelectQueryAuditEvent() public SelectQueryAuditEvent(Container container, String comment) { - super(SelectQueryAuditProvider.EVENT_NAME, container.getId(), comment); + super(SelectQueryAuditProvider.EVENT_NAME, container, comment); } public SelectQueryAuditEvent(QueryLogging queryLogging) diff --git a/api/src/org/labkey/api/exp/property/DomainAuditProvider.java b/api/src/org/labkey/api/exp/property/DomainAuditProvider.java index 65ad2b74cbb..efa4423e0fc 100644 --- a/api/src/org/labkey/api/exp/property/DomainAuditProvider.java +++ b/api/src/org/labkey/api/exp/property/DomainAuditProvider.java @@ -145,7 +145,7 @@ public DomainAuditEvent() super(); } - public DomainAuditEvent(String container, String comment) + public DomainAuditEvent(Container container, String comment) { super(EVENT_TYPE, container, comment); } diff --git a/api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java b/api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java index b57ab251e2d..b7b5fa8f648 100644 --- a/api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java +++ b/api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java @@ -19,6 +19,7 @@ import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; +import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.MutableColumnInfo; @@ -190,7 +191,7 @@ public DomainPropertyAuditEvent() super(); } - public DomainPropertyAuditEvent(String container, String propertyUri, String propertyName, String action, + public DomainPropertyAuditEvent(Container container, String propertyUri, String propertyName, String action, Long domainEventId, String domainName, String comment) { super(EVENT_NAME, container, comment); diff --git a/api/src/org/labkey/api/premium/AntiVirusService.java b/api/src/org/labkey/api/premium/AntiVirusService.java index c51bd96d374..06f21056dc1 100644 --- a/api/src/org/labkey/api/premium/AntiVirusService.java +++ b/api/src/org/labkey/api/premium/AntiVirusService.java @@ -103,7 +103,7 @@ default void warnAndAuditLog(Logger log, String logmessage, ViewBackgroundInfo i { log.warn((null != info.getUser() ? info.getUser().getEmail() + " " : "") + logmessage); FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent( - info.getContainer().getId(), logmessage + info.getContainer(), logmessage ); if (null != info.getURL()) event.setDirectory(info.getURL().getPath()); diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index ae83d74f498..ffa062dfe58 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -218,7 +218,7 @@ public boolean hasExistingRowsInOtherContainers(Container container, Map(), 1); + return copyGroupToContainer(g, c, user, new HashMap<>(), 1); } - private static Group copyGroupToContainer(Group g, Container c, HashMap groupMap) + private static Group copyGroupToContainer(Group g, Container c, User user, Map groupMap) { - return copyGroupToContainer(g, c, groupMap, 1); + return copyGroupToContainer(g, c, user, groupMap, 1); } - private static Group copyGroupToContainer(Group g, Container c, HashMap groupMap, int suffix) + private static Group copyGroupToContainer(Group g, Container c, User user, Map groupMap, int suffix) { if (!g.isProjectGroup()) { @@ -644,9 +641,9 @@ private static Group copyGroupToContainer(Group g, Container c, HashMap errors = SecurityManager.addMembers(newGroup, translatedMembers); @@ -688,14 +685,14 @@ else if (m instanceof Group && ((Group) m).isProjectGroup()) return newGroup; } - public static HashMap copyGroupsToContainer(Container source, Container target) + public static HashMap copyGroupsToContainer(Container source, Container target, User user) { //copy all project groups to new project. returns a map between old groups and new groups //note: site-groups are not copied, but the map will contain them anyway HashMap groupMap = new HashMap<>(); for (Group g : SecurityManager.getGroups(source, false)) { - groupMap.put(g, GroupManager.copyGroupToContainer(g, target, groupMap)); + groupMap.put(g, GroupManager.copyGroupToContainer(g, target, user, groupMap)); } return groupMap; diff --git a/api/src/org/labkey/api/security/NestedGroupsTest.java b/api/src/org/labkey/api/security/NestedGroupsTest.java index 92acab1ea87..14915f8b17c 100644 --- a/api/src/org/labkey/api/security/NestedGroupsTest.java +++ b/api/src/org/labkey/api/security/NestedGroupsTest.java @@ -79,7 +79,7 @@ public void cleanup() throws ValidEmail.InvalidEmailException, SecurityManager.U Integer groupId = SecurityManager.getGroupId(project, groupName, false); if (null != groupId) - SecurityManager.deleteGroup(groupId); + SecurityManager.deleteGroup(groupId, TestContext.get().getUser()); } for (String groupName : SITE_GROUP_NAMES) @@ -87,7 +87,7 @@ public void cleanup() throws ValidEmail.InvalidEmailException, SecurityManager.U Integer groupId = SecurityManager.getGroupId(ContainerManager.getRoot(), groupName, false); if (null != groupId) - SecurityManager.deleteGroup(groupId); + SecurityManager.deleteGroup(groupId, TestContext.get().getUser()); } ValidEmail email = new ValidEmail("junit_test_user@test.com"); @@ -115,10 +115,10 @@ public void test() throws Throwable Group writers = create(WRITERS); Group projectX = create(PROJECT_X); final Group cycleTest = create(CONCURRENCY_TEST_GROUP); - Group siteGroup1 = SecurityManager.createGroup(ContainerManager.getRoot(), SITE_GROUP_1); - assertTrue(!siteGroup1.isProjectGroup()); - Group siteGroup2 = SecurityManager.createGroup(ContainerManager.getRoot(), SITE_GROUP_2); - assertTrue(!siteGroup2.isProjectGroup()); + Group siteGroup1 = SecurityManager.createGroup(ContainerManager.getRoot(), SITE_GROUP_1, TestContext.get().getUser()); + assertFalse(siteGroup1.isProjectGroup()); + Group siteGroup2 = SecurityManager.createGroup(ContainerManager.getRoot(), SITE_GROUP_2, TestContext.get().getUser()); + assertFalse(siteGroup2.isProjectGroup()); addMember(projectX, user); addMember(projectX, _testUser); @@ -277,7 +277,7 @@ public void test() throws Throwable private Group create(String name) { - Group group = SecurityManager.createGroup(_project, name); + Group group = SecurityManager.createGroup(_project, name, TestContext.get().getUser()); assertTrue(group.isProjectGroup()); return group; diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index ef80aaa8827..75e412dc8b9 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -396,7 +396,7 @@ private static class SecurityContainerListener extends ContainerManager.Abstract @Override public void containerDeleted(Container c, User user) { - deleteGroups(c, null); + deleteGroups(c, null, user); } } @@ -1166,12 +1166,14 @@ private static MimeMessage createMessage(Container c, User fromUser, SecurityMes } } - public static Group createGroup(Container c, String name) + /** @param c assumed to be the root container if none is specified */ + public static Group createGroup(@Nullable Container c, String name, @Nullable User user) { - return createGroup(c, name, PrincipalType.GROUP); + return createGroup(c, name, user, PrincipalType.GROUP); } - public static Group createGroup(Container c, String name, PrincipalType type) + /** @param c assumed to be the root container if none is specified */ + public static Group createGroup(@Nullable Container c, String name, @Nullable User user, PrincipalType type) { // Consider: add validation rules to enum if (type != PrincipalType.GROUP && type != PrincipalType.MODULE) @@ -1192,16 +1194,17 @@ public static Group createGroup(Container c, String name, PrincipalType type) throw new IllegalStateException("Security groups can only be associated with a project or the root"); } - return createGroup(c, name, type, ownerId); + return createGroup(c, name, user, type, ownerId); } - public static Group createGroup(Container c, String name, PrincipalType type, String ownerId) + /** @param c assumed to be the root container if none is specified */ + public static Group createGroup(@Nullable Container c, String name, @Nullable User user, PrincipalType type, String ownerId) { - String containerId = (null == c || c.isRoot()) ? null : c.getId(); + c = c == null ? ContainerManager.getRoot() : c; Group group = new Group(type); group.setName(StringUtils.trimToNull(name)); group.setOwnerId(ownerId); - group.setContainer(containerId); + group.setContainer(c.isRoot() ? null : c.getId()); if (null == group.getName()) throw new IllegalArgumentException("Group can not have blank name"); @@ -1213,8 +1216,19 @@ public static Group createGroup(Container c, String name, PrincipalType type, St if (groupExists(c, group.getName(), group.getOwnerId())) throw new IllegalArgumentException("Group '" + group.getName() + "' already exists"); - Table.insert(null, core.getTableInfoPrincipals(), group); - ProjectAndSiteGroupsCache.uncache(c); + try (DbScope.Transaction t = core.getSchema().getScope().ensureTransaction()) + { + group = Table.insert(null, core.getTableInfoPrincipals(), group); + GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(c, + "A new security group named " + name + " was created.", + group); + AuditLogService.get().addEvent(user, event); + t.commit(); + } + finally + { + ProjectAndSiteGroupsCache.uncache(c); + } return group; } @@ -1227,27 +1241,41 @@ private static boolean groupExists(Container c, String groupName, String ownerId public static void renameGroup(Group group, String newName, User currentUser) { - if (group.isSystemGroup()) - throw new IllegalArgumentException("System groups may not be renamed!"); - Container c = null == group.getContainer() ? null : ContainerManager.getForId(group.getContainer()); - if (StringUtils.isEmpty(newName)) - throw new IllegalArgumentException("Name is required (may not be blank)"); - String valid = UserManager.validGroupName(newName, group.getPrincipalType()); - if (null != valid) - throw new IllegalArgumentException(valid); - if (null != getGroupId(c, newName, false)) - throw new IllegalArgumentException("Cannot rename group '" + group.getName() + "' to '" + newName + "' because that name is already used by another group!"); + try (DbScope.Transaction t = core.getSchema().getScope().ensureTransaction()) + { + String oldName = group.getName(); + if (group.isSystemGroup()) + throw new IllegalArgumentException("System groups may not be renamed!"); + Container c = null == group.getContainer() ? null : ContainerManager.getForId(group.getContainer()); + if (StringUtils.isEmpty(newName)) + throw new IllegalArgumentException("Name is required (may not be blank)"); + String valid = UserManager.validGroupName(newName, group.getPrincipalType()); + if (null != valid) + throw new IllegalArgumentException(valid); + if (null != getGroupId(c, newName, false)) + throw new IllegalArgumentException("Cannot rename group '" + group.getName() + "' to '" + newName + "' because that name is already used by another group!"); - Table.update(currentUser, core.getTableInfoPrincipals(), Collections.singletonMap("name", newName), group.getUserId()); - GroupCache.uncache(group.getUserId()); + Table.update(currentUser, core.getTableInfoPrincipals(), Collections.singletonMap("name", newName), group.getUserId()); + + GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(c == null ? ContainerManager.getRoot(): c, + "The security group named '" + oldName + "' was renamed to '" + newName + "'.", + group); + AuditLogService.get().addEvent(currentUser, event); + + t.commit(); + } + finally + { + GroupCache.uncache(group.getUserId()); + } } - public static void deleteGroup(Group group) + public static void deleteGroup(@NotNull Group group, @NotNull User user) { - deleteGroup(group.getUserId()); + deleteGroup(group.getUserId(), user); } - static void deleteGroup(int groupId) + static void deleteGroup(int groupId, User user) { Group group = getGroup(groupId); if (null == group) @@ -1302,11 +1330,17 @@ static void deleteGroup(int groupId) if (!group.isProjectGroup()) ensureAtLeastOneRootAdminExists(); + GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent( + c == null ? ContainerManager.getRoot() : c, + "The security group named " + group.getName() + " was deleted.", + group); + AuditLogService.get().addEvent(user, event); + transaction.commit(); } } - public static void deleteGroups(Container c, @Nullable PrincipalType type) + public static void deleteGroups(Container c, @Nullable PrincipalType type, User user) { if (!(null == type || type == PrincipalType.GROUP || type == PrincipalType.MODULE)) throw new IllegalArgumentException("Illegal group type: " + type); @@ -1317,7 +1351,7 @@ public static void deleteGroups(Container c, @Nullable PrincipalType type) String typeString = (null == type ? "%" : String.valueOf(type.getTypeChar())); new SqlSelector(core.getSchema(), "SELECT UserId FROM " + core.getTableInfoPrincipals() + "\tWHERE Container=? AND Type LIKE ?", c, typeString).stream(Integer.class) - .forEach(SecurityManager::deleteGroup); + .forEach((g) -> SecurityManager.deleteGroup(g, user)); } public static void deleteMembers(Group group, Collection membersToDelete) @@ -2436,7 +2470,7 @@ public static void createNewProjectGroups(Container project, User user) // Create default groups //Note: we are no longer creating the project-level Administrators group - Group userGroup = createGroup(project, "Users"); + Group userGroup = createGroup(project, "Users", user); // Set default permissions Role noPermsRole = RoleManager.getRole(NoPermissionsRole.class); @@ -2493,18 +2527,14 @@ public static void setNewSubfoldersInheritPermissions(Container project, User us WritablePropertyMap props = PropertyManager.getWritableProperties(project, SUBFOLDERS_INHERIT_PERMISSIONS_NAME, true); props.put(SUBFOLDERS_INHERIT_PERMISSIONS_NAME, Boolean.toString(inherit)); props.save(); - addAuditEvent(project, user, String.format("Container %s was updated so that new subfolders would " + (inherit ? "" : "not ") + "inherit security permissions", project.getName()), 0); + addAuditEvent(project, user, String.format("Container %s was updated so that new subfolders would " + (inherit ? "" : "not ") + "inherit security permissions", project.getName()), null); } - public static void addAuditEvent(Container c, User user, String comment, int groupId) + public static void addAuditEvent(Container c, User user, String comment, Group group) { if (user != null) { - GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(c.getId(), comment); - event.setGroup(groupId); - if (c.getProject() != null) - event.setProjectId(c.getProject().getId()); - + GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(c, comment, group); AuditLogService.get().addEvent(user, event); } } @@ -2520,7 +2550,7 @@ public static void changeProject(Container c, Container oldProject, Container ne /* when demoting a project to a regular folder, delete the project groups */ if (oldProject == c) { - deleteGroups(c, PrincipalType.GROUP); + deleteGroups(c, PrincipalType.GROUP, user); } /* @@ -2788,7 +2818,7 @@ public void handle(Collection entries) { try { - group = createGroup(rootContainer, groupName, PrincipalType.GROUP); + group = createGroup(rootContainer, groupName, user, PrincipalType.GROUP); } catch (IllegalArgumentException e) { @@ -2856,7 +2886,7 @@ public void handle(Collection entries) { try { - group = createGroup(rootContainer, prop.getName(), PrincipalType.GROUP); + group = createGroup(rootContainer, prop.getName(), null, PrincipalType.GROUP); } catch (IllegalArgumentException e) { @@ -3140,15 +3170,15 @@ public static class TestCase extends Assert public void setUp() { project = JunitUtil.getTestContainer().getProject(); - groupA = createGroup(project, "a"); - groupB = createGroup(project, "b"); + groupA = createGroup(project, "a", TestContext.get().getUser()); + groupB = createGroup(project, "b", TestContext.get().getUser()); } // try{tearDown();} catch(Exception e){}; @After public void tearDown() { - deleteGroup(groupA); - try{deleteGroup(groupB);}catch(Exception ignored){} + deleteGroup(groupA, TestContext.get().getUser()); + try{deleteGroup(groupB, TestContext.get().getUser());}catch(Exception ignored){} } @Test @@ -3219,7 +3249,7 @@ public void testCircularGroupMembership() throws InvalidGroupMembershipException while(count++ < maxLoop) { - Group newGroup = createGroup(project, "testGroup" + count); + Group newGroup = createGroup(project, "testGroup" + count, TestContext.get().getUser()); addMember(groups.getLast(), newGroup); groups.add(newGroup); try @@ -3238,7 +3268,7 @@ public void testCircularGroupMembership() throws InvalidGroupMembershipException deleteMember(groupA, groupB); while(groups.size()>1)//groupB will be deleted by the tearDown { - deleteGroup(groups.removeLast()); + deleteGroup(groups.removeLast(), TestContext.get().getUser()); } } } @@ -3574,7 +3604,7 @@ public boolean performChecks() assertTrue("The group defined in the startup properties: " + TEST_GROUP_1_NAME + " did not have the specified role: " + TEST_GROUP_1_ROLE_NAME, roles.contains(role)); // delete the test group that was added - deleteGroup(group); + deleteGroup(group, TestContext.get().getUser()); } /** diff --git a/api/src/org/labkey/api/security/SecurityPolicyManager.java b/api/src/org/labkey/api/security/SecurityPolicyManager.java index 1ec5a7560cc..0ebac52a05f 100644 --- a/api/src/org/labkey/api/security/SecurityPolicyManager.java +++ b/api/src/org/labkey/api/security/SecurityPolicyManager.java @@ -330,7 +330,7 @@ private static GroupAuditProvider.GroupAuditEvent getGroupAuditEvent(Container c { SecurableResource parent = resource.getParentResource(); String parentName = parent != null ? parent.getResourceName() : "root"; - GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(c.getId(), + GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(c, "A new security policy was established for " + resource.getResourceName() + ". It will no longer inherit permissions from " + parentName); @@ -353,12 +353,10 @@ protected static void writeAuditEvent(Container c, User user, int principalId, R sb.append(role.getName()); sb.append("."); - GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(c.getId(), sb.toString()); - event.setProjectId(c.getProject() != null ? c.getProject().getId() : null); - if (principal.getPrincipalType() == PrincipalType.USER) - event.setUser(principal.getUserId()); - else - event.setGroup(principal.getUserId()); + GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(c, + sb.toString(), + principal instanceof Group g ? g : null, + principal instanceof User u ? u : null); event.setResourceEntityId(resource.getResourceId()); AuditLogService.get().addEvent(user, event); diff --git a/api/src/org/labkey/api/security/UserManager.java b/api/src/org/labkey/api/security/UserManager.java index fd712817e7e..fc6272ee8be 100644 --- a/api/src/org/labkey/api/security/UserManager.java +++ b/api/src/org/labkey/api/security/UserManager.java @@ -1102,17 +1102,22 @@ public static class UserAuditEvent extends AuditTypeEvent int _user; + private final Map _messageElements = new LinkedHashMap<>(); + public UserAuditEvent() { super(); } - public UserAuditEvent(String container, String comment, User modifiedUser) + public UserAuditEvent(Container container, String comment, User modifiedUser) { super(UserManager.USER_AUDIT_EVENT, container, comment); if (modifiedUser != null) + { _user = modifiedUser.getUserId(); + _messageElements.put("user", getUserMessageElement(modifiedUser)); + } } public int getUser() @@ -1128,8 +1133,7 @@ public void setUser(int user) @Override public Map getAuditLogMessageElements() { - Map elements = new LinkedHashMap<>(); - elements.put("user", getUserMessageElement(getUser())); + Map elements = new LinkedHashMap<>(_messageElements); elements.putAll(super.getAuditLogMessageElements()); return elements; } @@ -1141,7 +1145,7 @@ public Map getAuditLogMessageElements() */ public static void addAuditEvent(User user, Container c, @Nullable User modifiedUser, String msg) { - UserAuditEvent event = new UserAuditEvent(c.getId(), msg, modifiedUser); + UserAuditEvent event = new UserAuditEvent(c, msg, modifiedUser); AuditLogService.get().addEvent(user, event); } diff --git a/api/src/org/labkey/api/security/impersonation/GroupImpersonationContextFactory.java b/api/src/org/labkey/api/security/impersonation/GroupImpersonationContextFactory.java index 985a6c385bc..75f7f0dd08b 100644 --- a/api/src/org/labkey/api/security/impersonation/GroupImpersonationContextFactory.java +++ b/api/src/org/labkey/api/security/impersonation/GroupImpersonationContextFactory.java @@ -97,7 +97,7 @@ public void startImpersonating(ViewContext context) if (group != null) { User adminUser = getAdminUser(); - UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(context.getContainer().getId(), + UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(context.getContainer(), adminUser.getEmail() + " impersonated group: " + group.getName() + ".", adminUser); AuditLogService.get().addEvent(adminUser, event); } @@ -113,7 +113,7 @@ public void stopImpersonating(HttpServletRequest request) { User adminUser = getAdminUser(); Container project = null == _projectId ? ContainerManager.getRoot() : ContainerManager.getForId(_projectId); - UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(project.getId(), + UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(project, adminUser.getEmail() + " stopped impersonating group: " + group.getName() + ".", adminUser); AuditLogService.get().addEvent(adminUser, event); } diff --git a/api/src/org/labkey/api/security/impersonation/RoleImpersonationContextFactory.java b/api/src/org/labkey/api/security/impersonation/RoleImpersonationContextFactory.java index eca8ab26106..caa50dbad12 100644 --- a/api/src/org/labkey/api/security/impersonation/RoleImpersonationContextFactory.java +++ b/api/src/org/labkey/api/security/impersonation/RoleImpersonationContextFactory.java @@ -122,12 +122,12 @@ public void startImpersonating(ViewContext context) if (!_previousRoles.isEmpty()) { - UserManager.UserAuditEvent stopEvent = new UserManager.UserAuditEvent(context.getContainer().getId(), + UserManager.UserAuditEvent stopEvent = new UserManager.UserAuditEvent(context.getContainer(), adminUser.getEmail() + " stopped impersonating role" + getRolesDisplayString(_previousRoles), adminUser); AuditLogService.get().addEvent(adminUser, stopEvent); } - UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(context.getContainer().getId(), + UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(context.getContainer(), adminUser.getEmail() + " impersonated role" + getRolesDisplayString(_roles), adminUser); AuditLogService.get().addEvent(adminUser, event); } @@ -165,7 +165,7 @@ public void stopImpersonating(HttpServletRequest request) User adminUser = getAdminUser(); Container project = null == _projectId ? ContainerManager.getRoot() : ContainerManager.getForId(_projectId); - UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(project.getId(),adminUser.getEmail() + UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(project,adminUser.getEmail() + " stopped impersonating role" + getRolesDisplayString(_roles), adminUser); AuditLogService.get().addEvent(adminUser, event); } diff --git a/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java b/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java index d35723f1c2f..1a1142b7228 100644 --- a/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java +++ b/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java @@ -103,11 +103,11 @@ public void startImpersonating(ViewContext context) SecurityManager.setAuthenticatedUser(request, null, impersonatedUser, false); User adminUser = getAdminUser(); - UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(context.getContainer().getId(), + UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(context.getContainer(), adminUser.getEmail() + " impersonated " + impersonatedUser.getEmail(), adminUser); AuditLogService.get().addEvent(adminUser, event); - UserManager.UserAuditEvent event2 = new UserManager.UserAuditEvent(context.getContainer().getId(), + UserManager.UserAuditEvent event2 = new UserManager.UserAuditEvent(context.getContainer(), impersonatedUser.getEmail() + " was impersonated by " + adminUser.getEmail(), impersonatedUser); AuditLogService.get().addEvent(adminUser, event2); } @@ -125,11 +125,11 @@ public void stopImpersonating(HttpServletRequest request) User adminUser = getAdminUser(); Container project = null == _projectId ? ContainerManager.getRoot() : ContainerManager.getForId(_projectId); - UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(project.getId(), + UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(project, impersonatedUser.getEmail() + " was no longer impersonated by " + adminUser.getEmail(), impersonatedUser); AuditLogService.get().addEvent(adminUser, event); - UserManager.UserAuditEvent event2 = new UserManager.UserAuditEvent(project.getId(), + UserManager.UserAuditEvent event2 = new UserManager.UserAuditEvent(project, adminUser.getEmail() + " stopped impersonating " + impersonatedUser.getEmail(), adminUser); AuditLogService.get().addEvent(adminUser, event2); } diff --git a/api/src/org/labkey/api/settings/AbstractCustomLabelProvider.java b/api/src/org/labkey/api/settings/AbstractCustomLabelProvider.java index c523f3458e1..dd213adb8c4 100644 --- a/api/src/org/labkey/api/settings/AbstractCustomLabelProvider.java +++ b/api/src/org/labkey/api/settings/AbstractCustomLabelProvider.java @@ -6,6 +6,7 @@ import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.provider.SiteSettingsAuditProvider; import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerManager; import org.labkey.api.data.PropertyManager; import org.labkey.api.data.PropertyManager.WritablePropertyMap; import org.labkey.api.data.PropertyStore; @@ -129,7 +130,7 @@ public void resetLabels(@Nullable Container container, @Nullable User auditUser) if (auditUser != null) { - SiteSettingsAuditProvider.SiteSettingsAuditEvent event = new SiteSettingsAuditProvider.SiteSettingsAuditEvent(labelContainer.getId(), getProviderLabel() + " labels have been reset to default."); + SiteSettingsAuditProvider.SiteSettingsAuditEvent event = new SiteSettingsAuditProvider.SiteSettingsAuditEvent(labelContainer == null ? ContainerManager.getRoot() : labelContainer, getProviderLabel() + " labels have been reset to default."); AuditLogService.get().addEvent(auditUser, event); } } @@ -167,7 +168,7 @@ private AuditTypeEvent getUpdateLabelEvent(Container labelContainer, Map"); - SiteSettingsAuditProvider.SiteSettingsAuditEvent event = new SiteSettingsAuditProvider.SiteSettingsAuditEvent(labelContainer.getId(), getProviderLabel() + " labels have been updated."); + SiteSettingsAuditProvider.SiteSettingsAuditEvent event = new SiteSettingsAuditProvider.SiteSettingsAuditEvent(labelContainer, getProviderLabel() + " labels have been updated."); event.setChanges(html.toString()); return event; diff --git a/api/src/org/labkey/api/settings/AbstractWriteableSettingsGroup.java b/api/src/org/labkey/api/settings/AbstractWriteableSettingsGroup.java index 507edb7da43..e45142a88a5 100644 --- a/api/src/org/labkey/api/settings/AbstractWriteableSettingsGroup.java +++ b/api/src/org/labkey/api/settings/AbstractWriteableSettingsGroup.java @@ -116,12 +116,8 @@ public void writeAuditLogEvent(Container c, User user) if (null != diff) { - SiteSettingsAuditEvent event = new SiteSettingsAuditEvent(c.getId(), "The " + getType() + " were changed (see details)."); - - if (c.getProject() != null) - event.setProjectId(c.getProject().getId()); + SiteSettingsAuditEvent event = new SiteSettingsAuditEvent(c, "The " + getType() + " were changed (see details)."); event.setChanges(diff); - AuditLogService.get().addEvent(user, event); } } diff --git a/api/src/org/labkey/api/util/ExceptionUtil.java b/api/src/org/labkey/api/util/ExceptionUtil.java index 5c4bcf56a23..fd76f4780b8 100644 --- a/api/src/org/labkey/api/util/ExceptionUtil.java +++ b/api/src/org/labkey/api/util/ExceptionUtil.java @@ -312,30 +312,30 @@ public static String logExceptionToMothership(@Nullable HttpServletRequest reque String message = "Exception detected"; if (null != errorCode) message += " and logged to mothership with error code " + errorCode; - LOG.error(message, ex); String decorations = getExtendedMessage(ex); if (!extraInfo.isBlank() || !decorations.isBlank()) { - String logMessage = "Additional exception info:"; + message += "\nAdditional exception info:"; if (!extraInfo.isBlank()) { - logMessage += "\n" + extraInfo; + message += "\n" + extraInfo; } if (!decorations.isBlank()) { - logMessage += "\n" + decorations; + message += "\n" + decorations; } if (HttpView.hasCurrentView()) { ViewContext viewContext = HttpView.currentContext(); - logMessage += "\nCurrent URL: " + viewContext.getActionURL(); + message += "\nCurrent URL: " + viewContext.getActionURL(); if (null != viewContext.getUser()) { - logMessage += "\nCurrent user: " + (viewContext.getUser().isGuest() ? "Guest" : viewContext.getUser().getEmail()); + message += "\nCurrent user: " + (viewContext.getUser().isGuest() ? "Guest" : viewContext.getUser().getEmail()); } } - LOG.error(logMessage); + + LOG.error(message, ex); } } } diff --git a/api/src/org/labkey/api/util/MailHelper.java b/api/src/org/labkey/api/util/MailHelper.java index c0881f92d61..b492d5e40ea 100644 --- a/api/src/org/labkey/api/util/MailHelper.java +++ b/api/src/org/labkey/api/util/MailHelper.java @@ -255,7 +255,7 @@ public static void send(Message m, @Nullable User user, Container c) private static void addAuditEvent(@Nullable User user, @Nullable Container c, Message m) throws MessagingException { - MessageAuditProvider.MessageAuditEvent event = new MessageAuditProvider.MessageAuditEvent(c != null ? c.getId() : ContainerManager.getRoot().getId(), + MessageAuditProvider.MessageAuditEvent event = new MessageAuditProvider.MessageAuditEvent(c != null ? c : ContainerManager.getRoot(), "The Email Message: (" + m.getSubject() + ") was sent"); try diff --git a/api/src/org/labkey/api/webdav/AbstractWebdavResource.java b/api/src/org/labkey/api/webdav/AbstractWebdavResource.java index 7b9fa49f19b..9f4ac97cbf3 100644 --- a/api/src/org/labkey/api/webdav/AbstractWebdavResource.java +++ b/api/src/org/labkey/api/webdav/AbstractWebdavResource.java @@ -653,7 +653,7 @@ else if ("dirDeleteFailed".equalsIgnoreCase(message)) // String subject = "File Management Tool notification: " + message; - FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(c.getId(), message); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(c, message); event.setDirectory(dir); event.setFile(name); diff --git a/audit/src/org/labkey/audit/query/AuditLogUpdateService.java b/audit/src/org/labkey/audit/query/AuditLogUpdateService.java index 814a59f89b9..06c7075d4d9 100644 --- a/audit/src/org/labkey/audit/query/AuditLogUpdateService.java +++ b/audit/src/org/labkey/audit/query/AuditLogUpdateService.java @@ -57,7 +57,7 @@ protected Map getRow(User user, Container container, Map insertRow(User user, Container container, Map row) throws ValidationException { - ClientApiAuditProvider.ClientApiAuditEvent event = new ClientApiAuditProvider.ClientApiAuditEvent(container.getId(), getString(row, "Comment")); + ClientApiAuditProvider.ClientApiAuditEvent event = new ClientApiAuditProvider.ClientApiAuditEvent(container, getString(row, "Comment")); if (row.get("EventType") != null) { String eventType = row.get("EventType").toString(); diff --git a/core/src/org/labkey/core/CoreContainerListener.java b/core/src/org/labkey/core/CoreContainerListener.java index 59d1780441a..8263970e2fe 100644 --- a/core/src/org/labkey/core/CoreContainerListener.java +++ b/core/src/org/labkey/core/CoreContainerListener.java @@ -95,7 +95,7 @@ private void addAuditEvent(User user, Container c, String comment) { if (user != null) { - AuditTypeEvent event = new AuditTypeEvent(ContainerAuditProvider.CONTAINER_AUDIT_EVENT, c.getId(), comment); + AuditTypeEvent event = new AuditTypeEvent(ContainerAuditProvider.CONTAINER_AUDIT_EVENT, c, comment); AuditLogService.get().addEvent(user, event); } } diff --git a/core/src/org/labkey/core/admin/AbstractFileSiteSettingsAction.java b/core/src/org/labkey/core/admin/AbstractFileSiteSettingsAction.java index 6115e42bc19..dae17998f2f 100644 --- a/core/src/org/labkey/core/admin/AbstractFileSiteSettingsAction.java +++ b/core/src/org/labkey/core/admin/AbstractFileSiteSettingsAction.java @@ -124,7 +124,7 @@ private String getDisableFileUploadDiff(String title, boolean before, boolean af private void saveFileUploadDisabledSetting(FormType form, User user) { - SiteSettingsAuditProvider.SiteSettingsAuditEvent event = new SiteSettingsAuditProvider.SiteSettingsAuditEvent(ContainerManager.getRoot().getId(), "The setting for disable file upload was changed (see details)."); + SiteSettingsAuditProvider.SiteSettingsAuditEvent event = new SiteSettingsAuditProvider.SiteSettingsAuditEvent(ContainerManager.getRoot(), "The setting for disable file upload was changed (see details)."); WriteableAppProps props = AppProps.getWriteableInstance(); boolean hasChange = false; diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index 9a707575128..67b669cbcde 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -5187,9 +5187,7 @@ public boolean handlePost(ExportFolderForm form, BindException errors) throws Ex form.getFormat(), form.isIncludeSubfolders(), form.getExportPhiLevel(), form.isShiftDates(), form.isAlternateIds(), form.isMaskClinic(), new StaticLoggerGetter(FolderWriterImpl.LOG)); - AuditTypeEvent event = new AuditTypeEvent(ContainerAuditProvider.CONTAINER_AUDIT_EVENT, container.getId(), "Folder export initiated to " + exportOption.getDescription() + " " + (form.isIncludeSubfolders() ? "including" : "excluding") + " subfolders."); - if (container.getProject() != null) - event.setProjectId(container.getProject().getId()); + AuditTypeEvent event = new AuditTypeEvent(ContainerAuditProvider.CONTAINER_AUDIT_EVENT, container, "Folder export initiated to " + exportOption.getDescription() + " " + (form.isIncludeSubfolders() ? "including" : "excluding") + " subfolders."); AuditLogService.get().addEvent(getUser(), event); _successURL = exportOption.initiateExport(container, errors, writer, ctx, getViewContext().getResponse()); @@ -6409,10 +6407,7 @@ else if (form.isCloudFileRoot()) { setFormAndConfirmMessage(ctx.getContainer(), form, true, false, migrateFilesOption.name()); String comment = (ctx.getContainer().isProject() ? "Project " : "Folder ") + ctx.getContainer().getPath() + ": " + form.getConfirmMessage(); - AuditTypeEvent event = new AuditTypeEvent(ContainerAuditProvider.CONTAINER_AUDIT_EVENT, ctx.getContainer().getId(), comment); - if (ctx.getContainer().getProject() != null) - event.setProjectId(ctx.getContainer().getProject().getId()); - + AuditTypeEvent event = new AuditTypeEvent(ContainerAuditProvider.CONTAINER_AUDIT_EVENT, ctx.getContainer(), comment); AuditLogService.get().addEvent(ctx.getUser(), event); } } @@ -8046,7 +8041,7 @@ public boolean handlePost(SetFolderPermissionsForm form, BindException errors) { throw new NotFoundException("An unknown project was specified to copy permissions from: " + targetProject); } - Map groupMap = GroupManager.copyGroupsToContainer(source, c); + Map groupMap = GroupManager.copyGroupsToContainer(source, c, getUser()); //copy role assignments SecurityPolicy op = SecurityPolicyManager.getPolicy(source); diff --git a/core/src/org/labkey/core/admin/UpdateFilePathsAction.java b/core/src/org/labkey/core/admin/UpdateFilePathsAction.java index 5694f4cd80c..e5ca365eec0 100644 --- a/core/src/org/labkey/core/admin/UpdateFilePathsAction.java +++ b/core/src/org/labkey/core/admin/UpdateFilePathsAction.java @@ -120,7 +120,7 @@ public boolean handlePost(UpdateFilePathsForm form, BindException errors) throws int rows = FileContentService.get().fireFileMoveEvent(source, target, getUser(), null); SiteSettingsAuditProvider.SiteSettingsAuditEvent event = new SiteSettingsAuditProvider.SiteSettingsAuditEvent( - ContainerManager.getRoot().getId(), + ContainerManager.getRoot(), "Updated site-wide file paths from " + source + " to " + target); event.setChanges(rows + " row(s) updated in database tables"); AuditLogService.get().addEvent(getUser(), event); diff --git a/core/src/org/labkey/core/admin/importer/SecurityGroupImporterFactory.java b/core/src/org/labkey/core/admin/importer/SecurityGroupImporterFactory.java index d5b505bd3b1..ca3b47f02e2 100644 --- a/core/src/org/labkey/core/admin/importer/SecurityGroupImporterFactory.java +++ b/core/src/org/labkey/core/admin/importer/SecurityGroupImporterFactory.java @@ -75,7 +75,7 @@ public void process(@Nullable PipelineJob job, FolderImportContext ctx, VirtualF { Integer groupId = SecurityManager.getGroupId(ctx.getContainer(), xmlGroupType.getName(), false); if (groupId == null) - SecurityManager.createGroup(ctx.getContainer(), xmlGroupType.getName()); + SecurityManager.createGroup(ctx.getContainer(), xmlGroupType.getName(), ctx.getUser()); } // now populate the groups with their members diff --git a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java index 95c9bc7d6d9..b827ebbf5f0 100644 --- a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java +++ b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java @@ -197,19 +197,19 @@ public void addAuditEvent(User user, AttachmentParent parent, String filename, S if (parent != null) { Container c = ContainerManager.getForId(parent.getContainerId()); - AttachmentAuditProvider.AttachmentAuditEvent attachmentEvent = new AttachmentAuditProvider.AttachmentAuditEvent(c != null ? c.getId() : null, comment); + AttachmentAuditProvider.AttachmentAuditEvent attachmentEvent = new AttachmentAuditProvider.AttachmentAuditEvent(c == null ? ContainerManager.getRoot() : c, comment); attachmentEvent.setAttachmentParentEntityId(parent.getEntityId()); attachmentEvent.setAttachment(filename); AuditLogService.get().addEvent(user, attachmentEvent); - if (parent instanceof AttachmentDirectory) + if (parent instanceof AttachmentDirectory adParent) { - FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(c != null ? c.getId() : null, comment); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(c, comment); try { - event.setDirectory(((AttachmentDirectory)parent).getFileSystemDirectory().getPath()); + event.setDirectory(adParent.getFileSystemDirectory().getPath()); } catch (MissingRootDirectoryException ex) { diff --git a/core/src/org/labkey/core/query/AttachmentAuditProvider.java b/core/src/org/labkey/core/query/AttachmentAuditProvider.java index d8f54e2d704..5d143d0c597 100644 --- a/core/src/org/labkey/core/query/AttachmentAuditProvider.java +++ b/core/src/org/labkey/core/query/AttachmentAuditProvider.java @@ -118,7 +118,7 @@ public AttachmentAuditEvent() super(); } - public AttachmentAuditEvent(String container, String comment) + public AttachmentAuditEvent(Container container, String comment) { super(AttachmentService.ATTACHMENT_AUDIT_EVENT, container, comment); } diff --git a/core/src/org/labkey/core/query/UsersTable.java b/core/src/org/labkey/core/query/UsersTable.java index 36549643aea..440a4bad1d4 100644 --- a/core/src/org/labkey/core/query/UsersTable.java +++ b/core/src/org/labkey/core/query/UsersTable.java @@ -656,7 +656,7 @@ else if (oldExpirationDate.compareTo(newExpirationDate) != 0) else return; - UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(c.getId(), message, userToUpdate); + UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(c, message, userToUpdate); AuditLogService.get().addEvent(editingUser, event); } } diff --git a/core/src/org/labkey/core/security/SecurityApiActions.java b/core/src/org/labkey/core/security/SecurityApiActions.java index 755af3a8ee9..c780ec52b97 100644 --- a/core/src/org/labkey/core/security/SecurityApiActions.java +++ b/core/src/org/labkey/core/security/SecurityApiActions.java @@ -31,6 +31,8 @@ import org.labkey.api.audit.AuditLogService; import org.labkey.api.audit.provider.GroupAuditProvider; import org.labkey.api.data.Container; +import org.labkey.api.data.CoreSchema; +import org.labkey.api.data.DbScope; import org.labkey.api.exceptions.OptimisticConflictException; import org.labkey.api.security.ActionNames; import org.labkey.api.security.Group; @@ -78,7 +80,6 @@ import org.labkey.api.view.NavTree; import org.labkey.api.view.NotFoundException; import org.labkey.api.view.UnauthorizedException; -import org.labkey.api.view.ViewContext; import org.labkey.core.security.SecurityController.UserForm; import org.springframework.validation.BindException; import org.springframework.validation.Errors; @@ -740,27 +741,24 @@ public ApiResponse execute(PolicyIdForm form, BindException errors) if (!resource.hasPermission(user, AdminPermission.class)) throw new IllegalArgumentException("You do not have permission to delete the security policy for this resource!"); - SecurityPolicyManager.deletePolicy(resource); + try (DbScope.Transaction t = CoreSchema.getInstance().getSchema().getScope().ensureTransaction()) + { + SecurityPolicyManager.deletePolicy(resource); - //audit log - writeToAuditLog(resource); + String parentResource = resource.getParentResource() != null ? resource.getParentResource().getResourceName() : "root"; + GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(resource.getResourceContainer(), + "The security policy for " + resource.getResourceName() + + " was deleted. It will now inherit the security policy of " + + parentResource); + event.setResourceEntityId(resource.getResourceId()); + AuditLogService.get().addEvent(getUser(), event); - return new ApiSimpleResponse("success", true); - } + t.commit(); + } - protected void writeToAuditLog(SecurableResource resource) - { - String parentResource = resource.getParentResource() != null ? resource.getParentResource().getResourceName() : "root"; - GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(resource.getResourceContainer().getId(), - "The security policy for " + resource.getResourceName() - + " was deleted. It will now inherit the security policy of " + - parentResource); - event.setResourceEntityId(resource.getResourceId()); - if (null != resource.getResourceContainer().getProject()) - event.setProjectId(resource.getResourceContainer().getProject().getId()); - AuditLogService.get().addEvent(getUser(), event); + return new ApiSimpleResponse("success", true); } } @@ -999,22 +997,13 @@ public ApiResponse execute(NameForm form, BindException errors) if (null == name) throw new IllegalArgumentException("You must specify a name parameter!"); - Group newGroup = SecurityManager.createGroup(getContainer().getProject(), name); - writeToAuditLog(newGroup); + Group newGroup = SecurityManager.createGroup(getContainer().getProject(), name, getUser()); ApiSimpleResponse resp = new ApiSimpleResponse(); resp.put("id", newGroup.getUserId()); resp.put("name", newGroup.getName()); return resp; } - - protected void writeToAuditLog(Group newGroup) - { - GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(getContainer().getId(), "A new security group named " + newGroup.getName() + " was created."); - event.setGroup(newGroup.getUserId()); - - AuditLogService.get().addEvent(getUser(), event); - } } @RequiresPermission(AdminPermission.class) @@ -1089,8 +1078,7 @@ public ApiResponse execute(GroupForm form, BindException errors) if (_group == null && form.getCreateGroup()) { - _group = SecurityManager.createGroup(getContainer().getProject(), form.getGroupName()); - writeToAuditLog(_group, this.getViewContext()); + _group = SecurityManager.createGroup(getContainer().getProject(), form.getGroupName(), getUser()); } if (_group.getContainer() == null && !getUser().hasRootPermission(UpdateUserPermission.class)) @@ -1167,13 +1155,6 @@ public ApiResponse execute(GroupForm form, BindException errors) } - protected void writeToAuditLog(Group newGroup, ViewContext viewContext) - { - GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(getContainer().getId(), "A new security group named " + newGroup.getName() + " was created."); - event.setGroup(newGroup.getUserId()); - AuditLogService.get().addEvent(viewContext.getUser(), event); - } - private void removeMembers(GroupForm form, Map> members, Map memberErrors) { members.put("removed", new ArrayList<>()); @@ -1547,19 +1528,10 @@ public ApiResponse execute(IdForm form, BindException errors) throw new IllegalArgumentException("Group id " + form.getId() + " does not exist within " + containerInfo); } - SecurityManager.deleteGroup(group); - writeToAuditLog(group); + SecurityManager.deleteGroup(group, getUser()); return new ApiSimpleResponse("deleted", form.getId()); } - - protected void writeToAuditLog(Group group) - { - GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(getContainer().getId(), "The security group named " + group.getName() + " was deleted."); - event.setGroup(group.getUserId()); - - AuditLogService.get().addEvent(getUser(), event); - } } @RequiresPermission(DeleteUserPermission.class) @@ -1667,7 +1639,6 @@ public ApiResponse execute(RenameForm form, BindException errors) { SecurityManager.renameGroup(group, form.getNewName(), getUser()); group.setName(form.getNewName()); - writeToAuditLog(group, oldName); } catch (IllegalArgumentException x) { @@ -1688,14 +1659,6 @@ public ApiResponse execute(RenameForm form, BindException errors) resp.put("newName", group.getName()); return resp; } - - public void writeToAuditLog(Group group, String oldName) - { - GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(getContainer().getId(), "The security group named '" + oldName + "' was renamed to '" + group.getName() + "'."); - event.setGroup(group.getUserId()); - - AuditLogService.get().addEvent(getUser(), event); - } } public static class GroupMemberForm diff --git a/core/src/org/labkey/core/security/SecurityController.java b/core/src/org/labkey/core/security/SecurityController.java index b6b3c9aa248..5a0a6590481 100644 --- a/core/src/org/labkey/core/security/SecurityController.java +++ b/core/src/org/labkey/core/security/SecurityController.java @@ -556,20 +556,13 @@ public void validateCommand(GroupForm form, Errors errors) {} @Override public boolean handlePost(GroupForm form, BindException errors) { - try + Group group = form.getGroupFor(getContainer()); + // Issue 13837: if someone else already deleted the group, no need to throw exception + if (group != null) { - Group group = form.getGroupFor(getContainer()); - ensureGroupInContainer(group,getContainer()); + ensureGroupInContainer(group, getContainer()); ensureGroupUserAccess(group, getUser()); - if (group != null) - { - SecurityManager.deleteGroup(group); - addGroupAuditEvent(getViewContext(), group, "The group: " + group.getPath() + " was deleted."); - } - } - catch(NotFoundException e) - { - // Issue 13837: if someone else already deleted the group, no need to throw exception + SecurityManager.deleteGroup(group, getUser()); } return true; } @@ -618,22 +611,6 @@ public void setObjectId(String objectId) } } - - private void addGroupAuditEvent(ContainerUser context, Group group, String message) - { - GroupAuditEvent event = new GroupAuditEvent(group.getContainer(), message); - - event.setGroup(group.getUserId()); - Container c = null==group.getContainer() ? ContainerManager.getRoot() : ContainerManager.getForId(group.getContainer()); - if (c != null && c.getProject() != null) - event.setProjectId(c.getProject().getId()); - else - event.setProjectId(ContainerManager.getRoot().getId()); - - AuditLogService.get().addEvent(context.getUser(), event); - } - - public static class UpdateMembersForm extends GroupForm { private String names; @@ -1238,10 +1215,10 @@ public class UpdatePermissionsAction extends FormHandlerAction @Override public void validateCommand(Object target, Errors errors) {} - private void addAuditEvent(User user, String comment, int groupId) + private void addAuditEvent(User user, String comment, Group group) { if (user != null) - SecurityManager.addAuditEvent(getContainer(), user, comment, groupId); + SecurityManager.addAuditEvent(getContainer(), user, comment, group); } // UNDONE move to SecurityManager @@ -1267,15 +1244,15 @@ private void addAuditEvent(Group group, SecurityPolicy newPolicy, SecurityPolicy { case explicit: addAuditEvent(getUser(), String.format("The permissions for group %s were changed from %s to %s", - group.getName(), oldRole.getName(), newRole.getName()), group.getUserId()); + group.getName(), oldRole.getName(), newRole.getName()), group); break; case fromInherited: addAuditEvent(getUser(), String.format("The permissions for group %s were changed from %s (inherited) to %s", - group.getName(), oldRole.getName(), newRole.getName()), group.getUserId()); + group.getName(), oldRole.getName(), newRole.getName()), group); break; case toInherited: addAuditEvent(getUser(), String.format("The permissions for group %s were changed from %s to %s (inherited)", - group.getName(), oldRole.getName(), newRole.getName()), group.getUserId()); + group.getName(), oldRole.getName(), newRole.getName()), group); break; } } @@ -1303,7 +1280,7 @@ public boolean handlePost(Object o, BindException errors) if (inherit) { - addAuditEvent(getUser(), String.format("Container %s was updated to inherit security permissions", c.getName()), 0); + addAuditEvent(getUser(), String.format("Container %s was updated to inherit security permissions", c.getName()), null); //get existing policy specifically for this container SecurityPolicy oldPolicy = SecurityPolicyManager.getPolicy(c, false); @@ -1479,7 +1456,7 @@ public boolean handlePost(AddUsersForm form, BindException errors) throws Except User userToClone = null; final String sourceUser = form.getCloneUser(); - if (sourceUser != null && sourceUser.length() > 0) + if (sourceUser != null && !sourceUser.isEmpty()) { try { @@ -1503,7 +1480,7 @@ public boolean handlePost(AddUsersForm form, BindException errors) throws Except for (String rawEmail : invalidEmails) { // Ignore lines of all whitespace, otherwise show an error. - if (!"".equals(rawEmail.trim())) + if (!rawEmail.trim().isEmpty()) errors.addError(new LabKeyError("Failed to create user " + rawEmail.trim() + ": Invalid email address")); } @@ -1567,9 +1544,7 @@ public ActionURL getSuccessURL(AddUsersForm addUsersForm) private void audit(User targetUser, String message) { - GroupAuditEvent event = new GroupAuditEvent(ContainerManager.getRoot().getId(), message); - event.setUser(targetUser.getUserId()); - event.setProjectId(ContainerManager.getRoot().getId()); + GroupAuditEvent event = new GroupAuditEvent(ContainerManager.getRoot(), message, targetUser); AuditLogService.get().addEvent(getUser(), event); } diff --git a/experiment/src/org/labkey/experiment/SampleTypeAuditProvider.java b/experiment/src/org/labkey/experiment/SampleTypeAuditProvider.java index cd1be34a761..013d85d3886 100644 --- a/experiment/src/org/labkey/experiment/SampleTypeAuditProvider.java +++ b/experiment/src/org/labkey/experiment/SampleTypeAuditProvider.java @@ -20,6 +20,7 @@ import org.labkey.api.audit.AuditTypeProvider; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; +import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.MutableColumnInfo; import org.labkey.api.data.TableInfo; @@ -124,7 +125,7 @@ public SampleTypeAuditEvent() super(); } - public SampleTypeAuditEvent(String container, String comment) + public SampleTypeAuditEvent(Container container, String comment) { super(EVENT_TYPE, container, comment); } diff --git a/experiment/src/org/labkey/experiment/api/ExpQCFlagTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpQCFlagTableImpl.java index 9b81a394c3f..ea71e96bc93 100644 --- a/experiment/src/org/labkey/experiment/api/ExpQCFlagTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpQCFlagTableImpl.java @@ -61,7 +61,7 @@ public class ExpQCFlagTableImpl extends ExpTableImpl impl private AssayProvider _provider; private ExpProtocol _assayProtocol; - private Map _columnMapping = new CaseInsensitiveHashMap<>(); + private final Map _columnMapping = new CaseInsensitiveHashMap<>(); public ExpQCFlagTableImpl(String name, UserSchema schema, ContainerFilter cf) { @@ -258,7 +258,7 @@ private void addAuditEvent(Container container, @Nullable User user, int action, DataState state = qcId != null ? DataStateManager.getInstance().getStateForRowId(run.getProtocol().getContainer(), qcId) : null; if (state != null) { - ExperimentAuditEvent event = new ExperimentAuditEvent(container.getId(), comment); + ExperimentAuditEvent event = new ExperimentAuditEvent(container, comment); event.setProtocolLsid(run.getProtocol().getLSID()); event.setRunLsid(run.getLSID()); @@ -283,7 +283,7 @@ private void addAuditEvent(Container container, @Nullable User user, int action, } catch (ConversionException e) { - LOG.warn("Unable to log audit event for QC flag changes: " + e.getMessage()); + LOG.warn("Unable to log audit event for QC flag changes: {}", e.getMessage()); } } } diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 71fb67c635c..757b98aeb29 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -420,9 +420,8 @@ public void auditRunEvent(User user, ExpProtocol protocol, ExpRun run, @Nullable public void auditRunEvent(User user, ExpProtocol protocol, ExpRun run, @Nullable ExpExperiment runGroup, String comment, String userComment) { Container c = run != null ? run.getContainer() : protocol.getContainer(); - ExperimentAuditEvent event = new ExperimentAuditEvent(c.getId(), comment); + ExperimentAuditEvent event = new ExperimentAuditEvent(c, comment); event.setUserComment(userComment); - event.setProjectId(c.getProject() == null ? null : c.getProject().getId()); if (runGroup != null) event.setRunGroup(runGroup.getRowId()); event.setProtocolLsid(protocol.getLSID()); @@ -8911,7 +8910,7 @@ private void addAuditEventForDataTypeContainerUpdate(DataTypeForExclusion type, Set exclusions = _getContainerDataTypeExclusions(type, containerId); String auditMsg = ("Data exclusion for folder " + container.getName() + " was updated.\n") + getDisabledDataTypeAuditMsg(type, exclusions.stream().toList(), true); - AuditTypeEvent event = new AuditTypeEvent(ContainerAuditProvider.CONTAINER_AUDIT_EVENT, container.getId(), auditMsg); + AuditTypeEvent event = new AuditTypeEvent(ContainerAuditProvider.CONTAINER_AUDIT_EVENT, container, auditMsg); AuditLogService.get().addEvent(user, event); } } diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index e2f701edde0..b96d08e7677 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -653,7 +653,7 @@ private void addSampleTypeDeletedAuditEvent(User user, Container c, ExpSampleTyp private void addSampleTypeAuditEvent(User user, Container c, ExpSampleType sampleType, Long txAuditId, String comment, String auditUserComment, String insertUpdateChoice) { - SampleTypeAuditProvider.SampleTypeAuditEvent event = new SampleTypeAuditProvider.SampleTypeAuditEvent(c.getId(), comment); + SampleTypeAuditProvider.SampleTypeAuditEvent event = new SampleTypeAuditProvider.SampleTypeAuditEvent(c, comment); event.setUserComment(auditUserComment); if (txAuditId != null) @@ -1176,15 +1176,12 @@ private boolean isInputFieldKey(String fieldKey) private SampleTimelineAuditEvent createAuditRecord(Container c, String comment, String userComment, @Nullable QueryService.AuditAction action, @Nullable Map row, @Nullable Map existingRow) { - SampleTimelineAuditEvent event = new SampleTimelineAuditEvent(c.getId(), comment); + SampleTimelineAuditEvent event = new SampleTimelineAuditEvent(c, comment); event.setUserComment(userComment); var tx = getExpSchema().getScope().getCurrentTransaction(); if (tx != null) event.setTransactionId(tx.getAuditId()); - if (c.getProject() != null) - event.setProjectId(c.getProject().getId()); - var staticsRow = existingRow != null && !existingRow.isEmpty() ? existingRow : row; if (row != null) { @@ -1242,11 +1239,9 @@ else if (event.getSampleLsid() != null) private SampleTimelineAuditEvent createAuditRecord(Container container, String comment, String userComment, ExpMaterial sample, @Nullable Map metadata) { - SampleTimelineAuditEvent event = new SampleTimelineAuditEvent(container.getId(), comment); + SampleTimelineAuditEvent event = new SampleTimelineAuditEvent(container, comment); if (getExpSchema().getScope().getCurrentTransaction() != null) event.setTransactionId(getExpSchema().getScope().getCurrentTransaction().getAuditId()); - if (container.getProject() != null) - event.setProjectId(container.getProject().getId()); event.setSampleName(sample.getName()); event.setSampleLsid(sample.getLSID()); event.setSampleId(sample.getRowId()); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 41de777ac24..56984c637f0 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -1396,7 +1396,7 @@ void audit(QueryService.AuditAction auditAction) { assert _sampleType != null || auditAction == QueryService.AuditAction.DELETE : "SampleType required for insert/update, but not required for read/delete"; SampleTypeAuditProvider.SampleTypeAuditEvent event = new SampleTypeAuditProvider.SampleTypeAuditEvent( - getContainer().getId(), "Samples " + auditAction.getVerbPastTense() + " in: " + (_sampleType == null ? "" : _sampleType.getName())); + getContainer(), "Samples " + auditAction.getVerbPastTense() + " in: " + (_sampleType == null ? "" : _sampleType.getName())); var tx = getSchema().getDbSchema().getScope().getCurrentTransaction(); if (tx != null) event.setTransactionId(tx.getAuditId()); diff --git a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java index 90e54518bba..4b974d211f8 100644 --- a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java @@ -932,12 +932,9 @@ private Long addAuditEvent(@Nullable User user, String comment, @Nullable String { if (user != null) { - DomainAuditProvider.DomainAuditEvent event = new DomainAuditProvider.DomainAuditEvent(getContainer().getId(), comment); + DomainAuditProvider.DomainAuditEvent event = new DomainAuditProvider.DomainAuditEvent(getContainer(), comment); event.setUserComment(auditUserComment); - if (_dd.getProject() != null) - event.setProjectId(_dd.getProject().getId()); - event.setDomainUri(getTypeURI()); event.setDomainName(getName()); @@ -950,7 +947,7 @@ private Long addAuditEvent(@Nullable User user, String comment, @Nullable String private void addPropertyAuditEvent(@Nullable User user, DomainProperty prop, String action, Long domainEventId, String domainName, String comment) { DomainPropertyAuditProvider.DomainPropertyAuditEvent event = - new DomainPropertyAuditProvider.DomainPropertyAuditEvent(getContainer().getId(), prop.getPropertyURI(), prop.getName(), + new DomainPropertyAuditProvider.DomainPropertyAuditEvent(getContainer(), prop.getPropertyURI(), prop.getName(), action, domainEventId, domainName, comment); AuditLogService.get().addEvent(user, event); } diff --git a/list/src/org/labkey/list/model/ListAuditProvider.java b/list/src/org/labkey/list/model/ListAuditProvider.java index c4b9abfdce0..c60d0969bc3 100644 --- a/list/src/org/labkey/list/model/ListAuditProvider.java +++ b/list/src/org/labkey/list/model/ListAuditProvider.java @@ -22,6 +22,7 @@ import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.DisplayColumn; import org.labkey.api.data.DisplayColumnFactory; @@ -157,9 +158,12 @@ public ListAuditEvent() super(); } - public ListAuditEvent(String container, String comment) + public ListAuditEvent(Container container, String comment, ListDefinitionImpl list) { super(ListManager.LIST_AUDIT_EVENT, container, comment); + setListDomainUri(list.getDomain().getTypeURI()); + setListId(list.getListId()); + setListName(list.getName()); } public int getListId() diff --git a/list/src/org/labkey/list/model/ListManager.java b/list/src/org/labkey/list/model/ListManager.java index 4fa5601de0d..269a0909946 100644 --- a/list/src/org/labkey/list/model/ListManager.java +++ b/list/src/org/labkey/list/model/ListManager.java @@ -1157,16 +1157,7 @@ void addAuditEvent(ListDefinitionImpl list, User user, String comment) { if (null != user) { - ListAuditProvider.ListAuditEvent event = new ListAuditProvider.ListAuditEvent(list.getContainer().getId(), comment); - - Container c = list.getContainer(); - if (c.getProject() != null) - event.setProjectId(c.getProject().getId()); - - event.setListDomainUri(list.getDomain().getTypeURI()); - event.setListId(list.getListId()); - event.setListName(list.getName()); - + ListAuditProvider.ListAuditEvent event = new ListAuditProvider.ListAuditEvent(list.getContainer(), comment, list); AuditLogService.get().addEvent(user, event); } } @@ -1176,17 +1167,9 @@ void addAuditEvent(ListDefinitionImpl list, User user, String comment) */ void addAuditEvent(ListDefinitionImpl list, User user, Container c, String comment, String entityId, @Nullable String oldRecord, @Nullable String newRecord) { - ListAuditProvider.ListAuditEvent event = new ListAuditProvider.ListAuditEvent(c.getId(), comment); + ListAuditProvider.ListAuditEvent event = new ListAuditProvider.ListAuditEvent(c, comment, list); - Container project = c.getProject(); - if (null != project) - event.setProjectId(project.getId()); - - event.setListDomainUri(list.getDomain().getTypeURI()); - event.setListId(list.getListId()); event.setListItemEntityId(entityId); - event.setListName(list.getName()); - if (oldRecord != null) event.setOldRecordMap(oldRecord); if (newRecord != null) event.setNewRecordMap(newRecord); diff --git a/query/src/org/labkey/query/MultiValueTest.jsp b/query/src/org/labkey/query/MultiValueTest.jsp index 15e282f8880..57ffad11d09 100644 --- a/query/src/org/labkey/query/MultiValueTest.jsp +++ b/query/src/org/labkey/query/MultiValueTest.jsp @@ -81,10 +81,10 @@ public void testCoreGroups() throws Exception { // Add a user to multiple groups - Group g1 = SecurityManager.createGroup(c, "group1"); + Group g1 = SecurityManager.createGroup(c, "group1", getUser()); SecurityManager.addMember(g1, getUser()); - Group g2 = SecurityManager.createGroup(c, "group2"); + Group g2 = SecurityManager.createGroup(c, "group2", getUser()); SecurityManager.addMember(g2, getUser()); ActionURL url = new ActionURL(QueryController.SelectRowsAction.class, c); diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index 6165c6657fd..72855690f45 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -3130,13 +3130,11 @@ protected DetailedAuditTypeEvent createDetailedAuditRecord(User user, Container private QueryUpdateAuditProvider.QueryUpdateAuditEvent createAuditRecord(Container c, AuditConfigurable tinfo, String comment, @Nullable Map row, @Nullable Map existingRow) { - QueryUpdateAuditProvider.QueryUpdateAuditEvent event = new QueryUpdateAuditProvider.QueryUpdateAuditEvent(c.getId(), comment); + QueryUpdateAuditProvider.QueryUpdateAuditEvent event = new QueryUpdateAuditProvider.QueryUpdateAuditEvent(c, comment); DbScope.Transaction tx = tinfo.getSchema().getScope().getCurrentTransaction(); if (tx != null) event.setTransactionId(tx.getAuditId()); - if (c.getProject() != null) - event.setProjectId(c.getProject().getId()); event.setSchemaName(tinfo.getPublicSchemaName()); event.setQueryName(tinfo.getPublicName()); @@ -3177,10 +3175,7 @@ public void addAuditEvent(QueryView queryView, String comment, @Nullable Integer @Override public void addAuditEvent(User user, Container c, String schemaName, String queryName, ActionURL sortFilter, String comment, @Nullable Integer dataRowCount) { - QueryExportAuditProvider.QueryExportAuditEvent event = new QueryExportAuditProvider.QueryExportAuditEvent(c.getId(), comment); - - if (c.getProject() != null) - event.setProjectId(c.getProject().getId()); + QueryExportAuditProvider.QueryExportAuditEvent event = new QueryExportAuditProvider.QueryExportAuditEvent(c, comment); event.setSchemaName(schemaName); event.setQueryName(queryName); if (dataRowCount != null) diff --git a/query/src/org/labkey/query/audit/QueryExportAuditProvider.java b/query/src/org/labkey/query/audit/QueryExportAuditProvider.java index 8cd4d7847d5..5b0706ef97e 100644 --- a/query/src/org/labkey/query/audit/QueryExportAuditProvider.java +++ b/query/src/org/labkey/query/audit/QueryExportAuditProvider.java @@ -20,6 +20,7 @@ import org.labkey.api.audit.AuditTypeProvider; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; +import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.MutableColumnInfo; import org.labkey.api.data.TableInfo; @@ -159,7 +160,7 @@ public QueryExportAuditEvent() super(); } - public QueryExportAuditEvent(String container, String comment) + public QueryExportAuditEvent(Container container, String comment) { super(QUERY_AUDIT_EVENT, container, comment); } diff --git a/query/src/org/labkey/query/audit/QueryUpdateAuditProvider.java b/query/src/org/labkey/query/audit/QueryUpdateAuditProvider.java index ad241f0f6b9..9cc0613b831 100644 --- a/query/src/org/labkey/query/audit/QueryUpdateAuditProvider.java +++ b/query/src/org/labkey/query/audit/QueryUpdateAuditProvider.java @@ -218,7 +218,7 @@ public QueryUpdateAuditEvent() super(); } - public QueryUpdateAuditEvent(String container, String comment) + public QueryUpdateAuditEvent(Container container, String comment) { super(QUERY_UPDATE_AUDIT_EVENT, container, comment); } diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index 71f22d80479..24f1eec86e4 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -8489,8 +8489,7 @@ public Object execute(QueryImportTemplateForm form, BindException errors) throws QueryManager.get().update(user, queryDef); } - DomainAuditProvider.DomainAuditEvent event = new DomainAuditProvider.DomainAuditEvent(getContainer().getId(), "Import templates updated."); - event.setProjectId(container.getId()); + DomainAuditProvider.DomainAuditEvent event = new DomainAuditProvider.DomainAuditEvent(getContainer(), "Import templates updated."); event.setDomainUri(_domain.getTypeURI()); event.setDomainName(_domain.getName()); AuditLogService.get().addEvent(user, event); diff --git a/search/src/org/labkey/search/SearchController.java b/search/src/org/labkey/search/SearchController.java index f7ce30e173e..6fd0180e2de 100644 --- a/search/src/org/labkey/search/SearchController.java +++ b/search/src/org/labkey/search/SearchController.java @@ -1146,7 +1146,7 @@ public static void audit(@Nullable User user, @Nullable Container c, String quer if (query.length() > 200) query = query.substring(0, 197) + "..."; - SearchAuditProvider.SearchAuditEvent event = new SearchAuditProvider.SearchAuditEvent(c.getId(), comment); + SearchAuditProvider.SearchAuditEvent event = new SearchAuditProvider.SearchAuditEvent(c, comment); event.setQuery(query); AuditLogService.get().addEvent(user, event); diff --git a/search/src/org/labkey/search/audit/SearchAuditProvider.java b/search/src/org/labkey/search/audit/SearchAuditProvider.java index fb06efbe423..8e897d20d3b 100644 --- a/search/src/org/labkey/search/audit/SearchAuditProvider.java +++ b/search/src/org/labkey/search/audit/SearchAuditProvider.java @@ -19,6 +19,7 @@ import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.AuditTypeProvider; import org.labkey.api.audit.query.AbstractAuditDomainKind; +import org.labkey.api.data.Container; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.PropertyType; import org.labkey.api.query.FieldKey; @@ -107,7 +108,7 @@ public SearchAuditEvent() super(); } - public SearchAuditEvent(String container, String comment) + public SearchAuditEvent(Container container, String comment) { super(EVENT_TYPE, container, comment); } diff --git a/specimen/src/org/labkey/specimen/SpecimenCommentAuditEvent.java b/specimen/src/org/labkey/specimen/SpecimenCommentAuditEvent.java index daaa29661a5..994eebd8f14 100644 --- a/specimen/src/org/labkey/specimen/SpecimenCommentAuditEvent.java +++ b/specimen/src/org/labkey/specimen/SpecimenCommentAuditEvent.java @@ -1,6 +1,7 @@ package org.labkey.specimen; import org.labkey.api.audit.AuditTypeEvent; +import org.labkey.api.data.Container; import java.util.LinkedHashMap; import java.util.Map; @@ -16,7 +17,7 @@ public SpecimenCommentAuditEvent() super(); } - public SpecimenCommentAuditEvent(String container, String comment) + public SpecimenCommentAuditEvent(Container container, String comment) { super(SPECIMEN_COMMENT_EVENT, container, comment); } diff --git a/specimen/src/org/labkey/specimen/SpecimenManager.java b/specimen/src/org/labkey/specimen/SpecimenManager.java index 749ab6214c2..33c4ba5e2ea 100644 --- a/specimen/src/org/labkey/specimen/SpecimenManager.java +++ b/specimen/src/org/labkey/specimen/SpecimenManager.java @@ -340,7 +340,7 @@ else if (newComment == null) message += "New value: " + newConflictState + "\n"; } - SpecimenCommentAuditEvent event = new SpecimenCommentAuditEvent(vial.getContainer().getId(), message); + SpecimenCommentAuditEvent event = new SpecimenCommentAuditEvent(vial.getContainer(), message); event.setVialId(vial.getGlobalUniqueId()); AuditLogService.get().addEvent(user, event); diff --git a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java index c722380f8d6..38d50984089 100644 --- a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java +++ b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java @@ -483,7 +483,7 @@ public void deleteAllSpecimenData(Container c, Set set, User user) Table.delete(SpecimenSchema.get().getTableInfoSampleAvailabilityRule(), containerFilter); assert set.add(SpecimenSchema.get().getTableInfoSampleAvailabilityRule()); - SpecimenRequestRequirementProvider.get().purgeContainer(c); + SpecimenRequestRequirementProvider.get().purgeContainer(c, user); assert set.add(SpecimenSchema.get().getTableInfoSampleRequestRequirement()); assert set.add(SpecimenSchema.get().getTableInfoSampleRequestActor()); diff --git a/specimen/src/org/labkey/specimen/actions/ShowGroupMembersAction.java b/specimen/src/org/labkey/specimen/actions/ShowGroupMembersAction.java index 0817384bab9..ddf84246704 100644 --- a/specimen/src/org/labkey/specimen/actions/ShowGroupMembersAction.java +++ b/specimen/src/org/labkey/specimen/actions/ShowGroupMembersAction.java @@ -121,7 +121,7 @@ public boolean handlePost(UpdateGroupForm form, BindException errors) throws Exc for (String rawEmail : invalidEmails) { // Ignore lines of all whitespace, otherwise show an error. - if (!"".equals(rawEmail.trim())) + if (!rawEmail.trim().isEmpty()) { errors.reject(SpringActionController.ERROR_MSG, "Could not add user " + rawEmail.trim() + ": Invalid email address"); } @@ -151,7 +151,7 @@ public boolean handlePost(UpdateGroupForm form, BindException errors) throws Exc } } - actor.addMembers(location, newMembers.toArray(new User[newMembers.size()])); + actor.addMembers(location, newMembers.toArray(new User[0])); } return !errors.hasErrors(); diff --git a/specimen/src/org/labkey/specimen/actions/SpecimenController.java b/specimen/src/org/labkey/specimen/actions/SpecimenController.java index b894b54ce0f..e7e044e7977 100644 --- a/specimen/src/org/labkey/specimen/actions/SpecimenController.java +++ b/specimen/src/org/labkey/specimen/actions/SpecimenController.java @@ -2880,7 +2880,7 @@ public boolean handlePost(IdForm form, BindException errors) { SpecimenRequestActor actor = SpecimenRequestRequirementProvider.get().getActor(getContainer(), form.getId()); if (actor != null) - actor.delete(); + actor.delete(getUser()); return true; } diff --git a/specimen/src/org/labkey/specimen/importer/SpecimenSettingsImporter.java b/specimen/src/org/labkey/specimen/importer/SpecimenSettingsImporter.java index 998c0fbdd7b..a302f91a83a 100644 --- a/specimen/src/org/labkey/specimen/importer/SpecimenSettingsImporter.java +++ b/specimen/src/org/labkey/specimen/importer/SpecimenSettingsImporter.java @@ -250,7 +250,7 @@ private void importRequestActors(SimpleStudyImportContext ctx, SpecimenSettingsT for (SpecimenRequestActor existingActor : SpecimenRequestRequirementProvider.get().getActors(ctx.getContainer())) { if (!inUseActorIds.contains(existingActor.getRowId())) - existingActor.delete(); + existingActor.delete(ctx.getUser()); else inUseActors.put(existingActor.getLabel(), existingActor); } @@ -328,7 +328,7 @@ private void importDefaultRequirements(SimpleStudyImportContext ctx, SpecimenSet { SpecimenRequestManager.get().deleteRequestRequirement(ctx.getUser(), existingReq, false); } - catch(AttachmentService.DuplicateFilenameException e) + catch(AttachmentService.DuplicateFilenameException ignored) {} // no op, this would only occur with deleteRequestRequirement when createEvent is true } @@ -428,7 +428,7 @@ private void importRequestForm(SimpleStudyImportContext ctx, SpecimenSettingsTyp ctx.getLogger().info("There is currently a form with the same title: " + form.getTitle() + ", skipping this from import"); } inputs.sort(Comparator.comparingInt(SpecimenRequestInput::getDisplayOrder)); - SpecimenRequestManager.get().saveNewSpecimenRequestInputs(ctx.getContainer(), inputs.toArray(new SpecimenRequestInput[inputs.size()])); + SpecimenRequestManager.get().saveNewSpecimenRequestInputs(ctx.getContainer(), inputs.toArray(new SpecimenRequestInput[0])); } } } diff --git a/specimen/src/org/labkey/specimen/requirements/DefaultActor.java b/specimen/src/org/labkey/specimen/requirements/DefaultActor.java index 2b9ab97030e..62be745ac37 100644 --- a/specimen/src/org/labkey/specimen/requirements/DefaultActor.java +++ b/specimen/src/org/labkey/specimen/requirements/DefaultActor.java @@ -122,19 +122,19 @@ public Integer getGroupId(@Nullable Location location, boolean createIfMissing) { groupId = SecurityManager.getGroupId(getContainer(), groupName, location.getEntityId(), false); if (groupId == null && createIfMissing) - groupId = SecurityManager.createGroup(getContainer(), groupName, PrincipalType.MODULE, location.getEntityId()).getUserId(); + groupId = SecurityManager.createGroup(getContainer(), groupName, null, PrincipalType.MODULE, location.getEntityId()).getUserId(); } else { groupId = SecurityManager.getGroupId(getContainer(), groupName, false); if (groupId == null && createIfMissing) - groupId = SecurityManager.createGroup(getContainer(), groupName, PrincipalType.MODULE).getUserId(); + groupId = SecurityManager.createGroup(getContainer(), groupName, null, PrincipalType.MODULE).getUserId(); } return groupId; } @Override - public void deleteAllGroups() + public void deleteAllGroups(User user) { List groupsToDelete = new ArrayList<>(); List locations = LocationManager.get().getLocations(getContainer()); @@ -152,7 +152,7 @@ public void deleteAllGroups() { Group group = SecurityManager.getGroup(groupId); if (group != null) - SecurityManager.deleteGroup(group); + SecurityManager.deleteGroup(group, user); } } @@ -171,12 +171,12 @@ public A update(User user) } @Override - public void delete() + public void delete(User user) { DbScope scope = SpecimenSchema.get().getScope(); try (DbScope.Transaction transaction = scope.ensureTransaction()) { - deleteAllGroups(); + deleteAllGroups(user); Table.delete(getTableInfo(), getPrimaryKey()); transaction.commit(); diff --git a/specimen/src/org/labkey/specimen/requirements/DefaultRequirementProvider.java b/specimen/src/org/labkey/specimen/requirements/DefaultRequirementProvider.java index cc007f149ac..803e98fd4b5 100644 --- a/specimen/src/org/labkey/specimen/requirements/DefaultRequirementProvider.java +++ b/specimen/src/org/labkey/specimen/requirements/DefaultRequirementProvider.java @@ -147,15 +147,15 @@ public List getActorsByLabel(Container c, String label) @Override public Collection getActorsInUse(Container c) { - Requirement[] requirements = getRequirements(c); + Requirement[] requirements = getRequirements(c); Map actors = new HashMap<>(); - for (Requirement requirement : requirements) + for (Requirement requirement : requirements) { A actor = getActor(c, requirement.getActorPrimaryKey()); actors.put(actor.getPrimaryKey(), actor); } - Requirement[] defaultRequirements = getDefaultRequirements(c); - for (Requirement requirement : defaultRequirements) + Requirement[] defaultRequirements = getDefaultRequirements(c); + for (Requirement requirement : defaultRequirements) { A actor = getActor(c, requirement.getActorPrimaryKey()); actors.put(actor.getPrimaryKey(), actor); @@ -164,7 +164,7 @@ public Collection getActorsInUse(Container c) } @Override - public void purgeContainer(Container c) + public void purgeContainer(Container c, User user) { R[] requirements = getRequirements(c); for (R requirement : requirements) @@ -172,7 +172,7 @@ public void purgeContainer(Container c) A[] actors = getActors(c); for (A actor : actors) - actor.delete(); + actor.delete(user); } diff --git a/specimen/src/org/labkey/specimen/requirements/RequirementActor.java b/specimen/src/org/labkey/specimen/requirements/RequirementActor.java index 4073c3d5126..a6a1e481cf8 100644 --- a/specimen/src/org/labkey/specimen/requirements/RequirementActor.java +++ b/specimen/src/org/labkey/specimen/requirements/RequirementActor.java @@ -43,11 +43,11 @@ public interface RequirementActor void removeMembers(Location location, User... members); - void deleteAllGroups(); + void deleteAllGroups(User user); A create(User user); A update(User user); - void delete(); + void delete(User user); } \ No newline at end of file diff --git a/specimen/src/org/labkey/specimen/requirements/RequirementProvider.java b/specimen/src/org/labkey/specimen/requirements/RequirementProvider.java index 52ae69ec077..d8c90276923 100644 --- a/specimen/src/org/labkey/specimen/requirements/RequirementProvider.java +++ b/specimen/src/org/labkey/specimen/requirements/RequirementProvider.java @@ -44,7 +44,7 @@ public interface RequirementProvider, void generateDefaultRequirements(User user, RequirementOwner owner); - void purgeContainer(Container c); + void purgeContainer(Container c, User user); R createDefaultRequirement(User user, R requirement, RequirementType type); diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index b35400a5389..9cd723bec63 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -628,7 +628,7 @@ private void logPublishEvent(Dataset.PublishSource publishSource, ExpObject sour int recordCount = rows.size(); String auditMessage = publishSource.getLinkToStudyAuditMessage(source, recordCount); - PublishAuditProvider.AuditEvent event = new PublishAuditProvider.AuditEvent(sourceContainer.getId(), auditMessage, publishSource, source, sourceLsid); + PublishAuditProvider.AuditEvent event = new PublishAuditProvider.AuditEvent(sourceContainer, auditMessage, publishSource, source, sourceLsid); event.setTargetStudy(targetContainer.getId()); event.setDatasetId(dataset.getDatasetId()); @@ -653,7 +653,7 @@ private void logPublishEvent(Dataset.PublishSource publishSource, ExpObject sour String sampleName = sample.getName(); String sampleLsid = sample.getLSID(); - SampleTimelineAuditEvent timelineEvent = new SampleTimelineAuditEvent(sourceContainer.getId(), timelineEventType.getComment()); + SampleTimelineAuditEvent timelineEvent = new SampleTimelineAuditEvent(sourceContainer, timelineEventType.getComment()); timelineEvent.setSampleType(source.getName()); timelineEvent.setSampleTypeId(source.getRowId()); timelineEvent.setSampleId(sampleId); @@ -1583,7 +1583,7 @@ public void addRecallAuditEvent(Container sourceContainer, User user, Dataset de sourceName = source.getName(); String auditMessage = sourceType.getRecallFromStudyAuditMessage(sourceName, rowCount); - PublishAuditProvider.AuditEvent event = new PublishAuditProvider.AuditEvent(sourceContainer.getId(), auditMessage, sourceType, source, null); + PublishAuditProvider.AuditEvent event = new PublishAuditProvider.AuditEvent(sourceContainer, auditMessage, sourceType, source, null); event.setTargetStudy(def.getStudy().getContainer().getId()); event.setDatasetId(def.getDatasetId()); @@ -1608,7 +1608,7 @@ public void addRecallAuditEvent(Container sourceContainer, User user, Dataset de String sampleName = sample.getName(); String sampleLsid = sample.getLSID(); - SampleTimelineAuditEvent timelineEvent = new SampleTimelineAuditEvent(sourceContainer.getId(), timelineEventType.getComment()); + SampleTimelineAuditEvent timelineEvent = new SampleTimelineAuditEvent(sourceContainer, timelineEventType.getComment()); timelineEvent.setSampleType(sourceName); if (source != null) timelineEvent.setSampleTypeId(source.getRowId()); diff --git a/study/src/org/labkey/study/assay/query/PublishAuditProvider.java b/study/src/org/labkey/study/assay/query/PublishAuditProvider.java index 30f5f2ad142..64d9f58dd6c 100644 --- a/study/src/org/labkey/study/assay/query/PublishAuditProvider.java +++ b/study/src/org/labkey/study/assay/query/PublishAuditProvider.java @@ -24,6 +24,7 @@ import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.MutableColumnInfo; import org.labkey.api.data.PropertyStorageSpec.Index; @@ -216,7 +217,7 @@ public AuditEvent() super(); } - public AuditEvent(String container, String comment, Dataset.PublishSource sourceType, @Nullable ExpObject source, @Nullable String sourceLsid) + public AuditEvent(Container container, String comment, Dataset.PublishSource sourceType, @Nullable ExpObject source, @Nullable String sourceLsid) { super(PUBLISH_AUDIT_EVENT, container, comment); _sourceType = sourceType.name(); diff --git a/study/src/org/labkey/study/dataset/DatasetAuditProvider.java b/study/src/org/labkey/study/dataset/DatasetAuditProvider.java index ad722ae8f28..66e4a9ccf5d 100644 --- a/study/src/org/labkey/study/dataset/DatasetAuditProvider.java +++ b/study/src/org/labkey/study/dataset/DatasetAuditProvider.java @@ -21,6 +21,7 @@ import org.labkey.api.audit.DetailedAuditTypeEvent; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; +import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.MutableColumnInfo; import org.labkey.api.data.PropertyStorageSpec; @@ -198,9 +199,10 @@ public DatasetAuditEvent() super(); } - public DatasetAuditEvent(String container, String comment) + public DatasetAuditEvent(Container container, String comment, int datasetId) { super(DATASET_AUDIT_EVENT, container, comment); + setDatasetId(datasetId); } public int getDatasetId() diff --git a/study/src/org/labkey/study/model/DatasetDefinition.java b/study/src/org/labkey/study/model/DatasetDefinition.java index eb613c11025..ece7e9f93ae 100644 --- a/study/src/org/labkey/study/model/DatasetDefinition.java +++ b/study/src/org/labkey/study/model/DatasetDefinition.java @@ -332,8 +332,9 @@ public void savePolicy(MutableSecurityPolicy policy, User user) SecurityPolicy existingPolicy = SecurityPolicyManager.getPolicy(this); if (!existingPolicy.getAssignments().equals(policy.getAssignments())) { - DatasetAuditProvider.DatasetAuditEvent event = new DatasetAuditProvider.DatasetAuditEvent(getContainer().getId(), getPolicyChangeSummary(policy, existingPolicy, baseDescription, removalDescription, additionDescription)); - event.setDatasetId(getDatasetId()); + DatasetAuditProvider.DatasetAuditEvent event = new DatasetAuditProvider.DatasetAuditEvent(getContainer(), + getPolicyChangeSummary(policy, existingPolicy, baseDescription, removalDescription, additionDescription), + getDatasetId()); AuditLogService.get().addEvent(user, event); } super.savePolicy(policy, user); @@ -1820,11 +1821,7 @@ else if (existingRecord != null && existingRecord.size() > 0) newRecordString = DatasetAuditProvider.encodeForDataMap(c, record); } - DatasetAuditProvider.DatasetAuditEvent event = new DatasetAuditProvider.DatasetAuditEvent(c.getId(), auditComment); - - if (c.getProject() != null) - event.setProjectId(c.getProject().getId()); - event.setDatasetId(_dataset.getDatasetId()); + DatasetAuditProvider.DatasetAuditEvent event = new DatasetAuditProvider.DatasetAuditEvent(c, auditComment, _dataset.getDatasetId()); event.setHasDetails(true); event.setLsid(lsid == null ? null : lsid.toString()); @@ -1847,11 +1844,7 @@ public void addAuditEvent(User user, Container c, AuditBehaviorType requiredAudi if (table != null && table.getAuditBehavior((AuditBehaviorType)null) != requiredAuditType) return; - DatasetAuditProvider.DatasetAuditEvent event = new DatasetAuditProvider.DatasetAuditEvent(c.getId(), comment); - - if (c.getProject() != null) - event.setProjectId(c.getProject().getId()); - event.setDatasetId(_dataset.getDatasetId()); + DatasetAuditProvider.DatasetAuditEvent event = new DatasetAuditProvider.DatasetAuditEvent(c, comment, _dataset.getDatasetId()); if (ul != null) { event.setLsid(ul.getFilePath()); diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index cc902f30441..2f695c956cf 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -2004,11 +2004,7 @@ public void updateDataQCState(Container container, User user, int datasetId, Col String auditComment = "QC state was changed for " + updateLsids.size() + " record" + (updateLsids.size() == 1 ? "" : "s") + ". User comment: " + comments; - DatasetAuditProvider.DatasetAuditEvent event = new DatasetAuditProvider.DatasetAuditEvent(container.getId(), auditComment); - - if (container.getProject() != null) - event.setProjectId(container.getProject().getId()); - event.setDatasetId(datasetId); + DatasetAuditProvider.DatasetAuditEvent event = new DatasetAuditProvider.DatasetAuditEvent(container, auditComment, datasetId); event.setHasDetails(true); event.setOldRecordMap(AbstractAuditTypeProvider.encodeForDataMap(container, oldQCStates)); event.setNewRecordMap(AbstractAuditTypeProvider.encodeForDataMap(container, newQCStates)); From 39a2e1926650e238ae2751e9df78e842e10d746e Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Mon, 24 Mar 2025 14:57:32 -0700 Subject: [PATCH 2/2] Switch to use Container for member variables too --- .../org/labkey/api/audit/AuditTypeEvent.java | 44 ++++++++++--------- .../org/labkey/api/security/GroupManager.java | 3 -- .../SecurityEscalationAuditProvider.java | 7 ++- .../api/study/security/SecurityEscalator.java | 16 +++---- .../StudySecurityEscalationAuditProvider.java | 7 +++ .../security/StudySecurityEscalator.java | 4 +- audit/src/org/labkey/audit/AuditLogImpl.java | 13 +----- .../org/labkey/audit/model/LogManager.java | 31 ++++++------- .../study/controllers/DatasetController.java | 11 ++--- 9 files changed, 58 insertions(+), 78 deletions(-) diff --git a/api/src/org/labkey/api/audit/AuditTypeEvent.java b/api/src/org/labkey/api/audit/AuditTypeEvent.java index 361afb6b49a..b0f80d4edac 100644 --- a/api/src/org/labkey/api/audit/AuditTypeEvent.java +++ b/api/src/org/labkey/api/audit/AuditTypeEvent.java @@ -15,12 +15,17 @@ */ package org.labkey.api.audit; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.security.Group; import org.labkey.api.security.User; import org.labkey.api.security.UserManager; +import org.labkey.api.util.ExceptionUtil; +import org.labkey.api.util.logging.LogHelper; +import org.slf4j.LoggerFactory; import java.util.Date; import java.util.LinkedHashMap; @@ -45,8 +50,8 @@ public class AuditTypeEvent private long _rowId; private Integer _impersonatedBy; private String _comment; - private String _projectId; - private String _container; + private Container _projectId; + private Container _container; private String _eventType; private Date _created; private User _createdBy; @@ -55,16 +60,16 @@ public class AuditTypeEvent private String userComment; private Long _transactionId; - public AuditTypeEvent(String eventType, @NotNull Container container, String comment) + public AuditTypeEvent(@NotNull String eventType, @NotNull Container container, @Nullable String comment) { _eventType = eventType; - _container = container.getId(); - _comment = comment; - Container project = container.getProject(); - if (project != null) + if (container == null) { - _projectId = project.getId(); + ExceptionUtil.logExceptionToMothership(null, new IllegalStateException("Audit event container is null")); } + _container = container; + _comment = comment; + _projectId = container.getProject(); } public AuditTypeEvent(){} @@ -99,22 +104,22 @@ public void setComment(String comment) _comment = comment; } - public String getProjectId() + public Container getProjectId() { return _projectId; } - public void setProjectId(String projectId) + public void setProjectId(Container projectId) { _projectId = projectId; } - public String getContainer() + public Container getContainer() { return _container; } - public void setContainer(String container) + public void setContainer(Container container) { _container = container; } @@ -189,12 +194,10 @@ public void setTransactionId(Long transactionId) _transactionId = transactionId; } - protected String getContainerMessageElement(@NotNull String containerId) + protected String getContainerMessageElement(@NotNull Container container) { - String value = " (" + containerId + ")"; - Container container = ContainerManager.getForId(containerId); - if (container != null) - value = container.getPath() + value; + String value = " (" + container.getId() + ")"; + value = container.getPath() + value; return value; } @@ -230,10 +233,9 @@ public Map getAuditLogMessageElements() Integer impersonatorId = getImpersonatedBy(); if (impersonatorId != null) elements.put(IMPERSONATED_BY_KEY, getUserMessageElement(impersonatorId)); - String containerId = getContainer(); - if (containerId != null) - elements.put(CONTAINER_KEY, getContainerMessageElement(containerId)); - String projectId = getProjectId(); + Container container = getContainer(); + elements.put(CONTAINER_KEY, getContainerMessageElement(container)); + Container projectId = getProjectId(); if (projectId != null) elements.put(PROJECT_KEY, getContainerMessageElement(projectId)); if (getComment() != null) diff --git a/api/src/org/labkey/api/security/GroupManager.java b/api/src/org/labkey/api/security/GroupManager.java index 9574ef907b8..d75bf028cca 100644 --- a/api/src/org/labkey/api/security/GroupManager.java +++ b/api/src/org/labkey/api/security/GroupManager.java @@ -362,9 +362,6 @@ private void addAuditEvent(Group group, UserPrincipal principal, String message) c = c == null ? ContainerManager.getRoot() : c; GroupAuditProvider.GroupAuditEvent event = new GroupAuditProvider.GroupAuditEvent(c, message, group, principal); - if (c == null) - event.setContainer(ContainerManager.getRoot().getId()); - AuditLogService.get().addEvent(user, event); } diff --git a/api/src/org/labkey/api/study/security/SecurityEscalationAuditProvider.java b/api/src/org/labkey/api/study/security/SecurityEscalationAuditProvider.java index b7e3223f836..b22fcb37337 100644 --- a/api/src/org/labkey/api/study/security/SecurityEscalationAuditProvider.java +++ b/api/src/org/labkey/api/study/security/SecurityEscalationAuditProvider.java @@ -214,10 +214,9 @@ public abstract static class SecurityEscalationEvent extends AuditTypeEvent private int level; // It is essential that you set the container and event type, otherwise the Audit Log will fail at - // runtime (silently in the case of the "eventType" and noisely in the case of the container). - public SecurityEscalationEvent() { - super(); - this.setEventType(this.getEventType()); + // runtime (silently in the case of the "eventType" and noisily in the case of the container). + public SecurityEscalationEvent(String eventType, Container container, String comment) { + super(eventType, container, comment); } /** diff --git a/api/src/org/labkey/api/study/security/SecurityEscalator.java b/api/src/org/labkey/api/study/security/SecurityEscalator.java index db1096df2bb..e2242df516d 100644 --- a/api/src/org/labkey/api/study/security/SecurityEscalator.java +++ b/api/src/org/labkey/api/study/security/SecurityEscalator.java @@ -75,23 +75,19 @@ public abstract class SecurityEscalator implements AutoCloseable /** * Returns a blank, new {@link SecurityEscalationAuditProvider.SecurityEscalationEvent} that will * be filled with data about the escalation. You should only add values to the event that are specific - * to your SecurityEscalator implementation, as this class will override all of the values tracked by + * to your SecurityEscalator implementation, as this class will override all the values tracked by * the base class. * * @return A blank, new {@link SecurityEscalationAuditProvider.SecurityEscalationEvent}. */ - abstract protected SecurityEscalationAuditProvider.SecurityEscalationEvent getNewSecurityEvent(); + abstract protected SecurityEscalationAuditProvider.SecurityEscalationEvent getNewSecurityEvent(Container container, String comment); - private SecurityEscalationAuditProvider.SecurityEscalationEvent _event; - private User user; + private final SecurityEscalationAuditProvider.SecurityEscalationEvent _event; + private final User user; /** * Constructor to return a new escalator object. This increments the escalation level, and prepares * an Audit Message (but doesn't add it just yet). - * - * @param user - * @param container - * @param comment */ public SecurityEscalator(User user, Container container, String comment) { this.user = user; @@ -130,12 +126,10 @@ public SecurityEscalator(User user, Container container, String comment) { String relevantStackTrace = Joiner.on("\n").join(relevantStackTraceElements); // Create an audit entry, but don't submit it yet. - _event = this.getNewSecurityEvent(); - _event.setContainer(container.getId()); + _event = this.getNewSecurityEvent(container, comment); _event.setStartTime(new Date()); _event.setServiceName(serviceName); _event.setStackTrace(relevantStackTrace); - _event.setComment(comment); _event.setLevel(getEscalationLevel()); _event.setEscalatingUser(user.getUserId()); diff --git a/api/src/org/labkey/api/study/security/StudySecurityEscalationAuditProvider.java b/api/src/org/labkey/api/study/security/StudySecurityEscalationAuditProvider.java index f8f3d34f3d7..d29ab0f84f9 100644 --- a/api/src/org/labkey/api/study/security/StudySecurityEscalationAuditProvider.java +++ b/api/src/org/labkey/api/study/security/StudySecurityEscalationAuditProvider.java @@ -15,6 +15,8 @@ */ package org.labkey.api.study.security; +import org.labkey.api.data.Container; + /** * @see SecurityEscalationAuditProvider */ @@ -50,6 +52,11 @@ public StudySecurityEscalationAuditProvider() public static class StudySecurityEscalationEvent extends SecurityEscalationEvent { + public StudySecurityEscalationEvent(Container container, String comment) + { + super(EVENT_TYPE, container, comment); + } + @Override public String getEventType() { return EVENT_TYPE; diff --git a/api/src/org/labkey/api/study/security/StudySecurityEscalator.java b/api/src/org/labkey/api/study/security/StudySecurityEscalator.java index 4257c94c23f..3d6f3b86645 100644 --- a/api/src/org/labkey/api/study/security/StudySecurityEscalator.java +++ b/api/src/org/labkey/api/study/security/StudySecurityEscalator.java @@ -62,8 +62,8 @@ protected ThreadLocal getEscalationLevelTracker() { * @return A blank new {@link SecurityEscalationAuditProvider.SecurityEscalationEvent}. */ @Override - protected SecurityEscalationAuditProvider.SecurityEscalationEvent getNewSecurityEvent() { - return new StudySecurityEscalationAuditProvider.StudySecurityEscalationEvent(); + protected SecurityEscalationAuditProvider.SecurityEscalationEvent getNewSecurityEvent(Container container, String comment) { + return new StudySecurityEscalationAuditProvider.StudySecurityEscalationEvent(container, comment); } /** diff --git a/audit/src/org/labkey/audit/AuditLogImpl.java b/audit/src/org/labkey/audit/AuditLogImpl.java index c969252072e..003f12f1e28 100644 --- a/audit/src/org/labkey/audit/AuditLogImpl.java +++ b/audit/src/org/labkey/audit/AuditLogImpl.java @@ -154,17 +154,10 @@ private K _addEvents(@Nullable User user, List eve { assert event.getContainer() != null : "Container cannot be null"; - if (event.getContainer() == null) - { - _log.warn("container was not specified for event type " + event.getEventType() + "; defaulting to root container."); - Container root = ContainerManager.getRoot(); - event.setContainer(root.getId()); - } - if (user == null) { if (HttpView.hasCurrentView() && HttpView.currentContext() != null) - _log.warn("user was not specified for event type " + event.getEventType() + " in container " + ContainerManager.getForId(event.getContainer()).getPath() + "; defaulting to guest user."); + _log.warn("user was not specified for event type " + event.getEventType() + " in container " + event.getContainer() + "; defaulting to guest user."); user = UserManager.getGuestUser(); } if (event.getTransactionId() != null && useTransactionAuditCache) @@ -179,10 +172,6 @@ private K _addEvents(@Nullable User user, List eve if (event.getCreatedBy() == null) event.setCreatedBy(user); - Container c = ContainerManager.getForId(event.getContainer()); - if (event.getProjectId() == null && c != null && c.getProject() != null) - event.setProjectId(c.getProject().getId()); - if (event.getImpersonatedBy() == null && user.isImpersonated()) { User impersonatingUser = user.getImpersonatingUser(); diff --git a/audit/src/org/labkey/audit/model/LogManager.java b/audit/src/org/labkey/audit/model/LogManager.java index d773dbbd7ca..18c4c60894b 100644 --- a/audit/src/org/labkey/audit/model/LogManager.java +++ b/audit/src/org/labkey/audit/model/LogManager.java @@ -79,14 +79,14 @@ public DbSchema getSchema() /** There are a few places that depend on the reselect behavior. e.g. to get the rowid of the event */ public K insertEvent(User user, K type) { - Logger auditLogger = org.apache.logging.log4j.LogManager.getLogger("org.labkey.audit.event." + type.getEventType().replaceAll(" ", "")); + Logger auditLogger = getAuditLogger(type); auditLogger.info(type.getAuditLogMessage()); AuditTypeProvider provider = AuditLogService.get().getAuditProvider(type.getEventType()); if (provider != null) { - Container c = ContainerManager.getForId(type.getContainer()); + Container c = type.getContainer(); UserSchema schema = AuditLogService.getAuditLogSchema(user, c != null ? c : ContainerManager.getRoot()); @@ -107,6 +107,11 @@ public K insertEvent(User user, K type) return null; } + private static Logger getAuditLogger(K type) + { + return org.apache.logging.log4j.LogManager.getLogger("org.labkey.audit.event." + type.getEventType().replaceAll(" ", "")); + } + /** all events must be of same event type and container, for optimized code path */ public void insertEvents(User user, List events) { @@ -123,7 +128,7 @@ public void insertEvents(User user, List events) { // make sure all events are the same type final String expectedEventType = type.getEventType(); - final String expectedContainer = type.getContainer(); + final Container expectedContainer = type.getContainer(); Optional problemEvent = events.stream() .filter(event -> !Objects.equals(expectedEventType, event.getEventType()) || !Objects.equals(expectedContainer, event.getContainer())) .findAny(); @@ -141,12 +146,12 @@ public void insertEvents(User user, List events) AuditTypeProvider provider = AuditLogService.get().getAuditProvider(type.getEventType()); if (null == provider) return; - Container c = ContainerManager.getForId(type.getContainer()); + Container c = type.getContainer(); UserSchema schema = AuditLogService.getAuditLogSchema(user, c != null ? c : ContainerManager.getRoot()); TableInfo table = null==schema ? null : schema.getTable(provider.getEventName(), false); TableInfo dbTable = table instanceof DefaultAuditTypeTable ? ((DefaultAuditTypeTable) table).getRealTable() : null; - Logger auditLogger = org.apache.logging.log4j.LogManager.getLogger("org.labkey.audit.event." + type.getEventType().replaceAll(" ", "")); + Logger auditLogger = getAuditLogger(type); SQLException sqlx = null; if (null != dbTable) @@ -195,7 +200,7 @@ public K getAuditEvent(User user, String eventType, i TableInfo table = schema.getTable(provider.getEventName(), cf); TableSelector selector = new TableSelector(table, null, null); - return (K)selector.getObject(rowId, provider.getEventClass()); + return selector.getObject(rowId, provider.getEventClass()); } } return null; @@ -223,29 +228,19 @@ public List getAuditEvents(Container container, Us TableInfo table = schema.getTable(provider.getEventName(), cf); TableSelector selector = new TableSelector(table, filter, sort); - return (List)selector.getArrayList(provider.getEventClass()); + return selector.getArrayList(provider.getEventClass()); } } return Collections.emptyList(); } - private String ensureMaxLength(String input, int max) - { - if (input != null && input.length() > max) - { - _log.warn("Audit field input : \n" + input + "\nexceeded the maximum length : " + max); - return input.substring(0, max-3) + "..."; - } - return input; - } - /** * Ensure that the string properties don't exceed the length of the provisioned columns. * Values will be trimmed to the max length. */ private K validateFields(@NotNull AuditTypeProvider provider, @NotNull K type) { - ObjectFactory factory = ObjectFactory.Registry.getFactory((Class)type.getClass()); + ObjectFactory factory = ObjectFactory.Registry.getFactory((Class)type.getClass()); Map values = new CaseInsensitiveHashMap<>(); factory.toMap(type, values); diff --git a/study/src/org/labkey/study/controllers/DatasetController.java b/study/src/org/labkey/study/controllers/DatasetController.java index b177fd1075a..2b901dcfd52 100644 --- a/study/src/org/labkey/study/controllers/DatasetController.java +++ b/study/src/org/labkey/study/controllers/DatasetController.java @@ -23,7 +23,6 @@ import org.labkey.api.audit.permissions.CanSeeAuditLogPermission; import org.labkey.api.audit.view.AuditChangesView; import org.labkey.api.data.Container; -import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DataRegion; import org.labkey.api.data.DbScope; import org.labkey.api.security.RequiresPermission; @@ -64,7 +63,7 @@ public DatasetController() } @RequiresPermission(ReadPermission.class) - public static class UpdateAction extends InsertUpdateAction + public static class UpdateAction extends InsertUpdateAction { public UpdateAction() { @@ -85,7 +84,7 @@ protected void addExtraNavTrail(NavTree root) } @RequiresPermission(ReadPermission.class) - public static class InsertAction extends InsertUpdateAction + public static class InsertAction extends InsertUpdateAction { public InsertAction() { @@ -115,7 +114,6 @@ public ModelAndView getView(DatasetAuditHistoryForm form, BindException errors) String comment = null; String oldRecord = null; String newRecord = null; - int datasetId = -1; Container eventContainer = null; VBox view = new VBox(); @@ -126,8 +124,7 @@ public ModelAndView getView(DatasetAuditHistoryForm form, BindException errors) comment = event.getComment(); oldRecord = event.getOldRecordMap(); newRecord = event.getNewRecordMap(); - datasetId = event.getDatasetId(); - eventContainer = ContainerManager.getForId(event.getContainer()); + eventContainer = event.getContainer(); } Map oldData = null; @@ -167,7 +164,7 @@ public ModelAndView getView(DatasetAuditHistoryForm form, BindException errors) return view; } - private static class NoRecordView extends HttpView + private static class NoRecordView extends HttpView { @Override protected void renderInternal(Object model, PrintWriter out)