From 48cdccd52f684c5864026ef84e5a3037c99e9539 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Fri, 24 Oct 2025 05:36:07 -0700 Subject: [PATCH 1/2] Audit EHR notification events --- LDK/src/org/labkey/ldk/LDKController.java | 27 ++---- .../notification/NotificationServiceImpl.java | 96 ++++++++++++++++--- 2 files changed, 95 insertions(+), 28 deletions(-) diff --git a/LDK/src/org/labkey/ldk/LDKController.java b/LDK/src/org/labkey/ldk/LDKController.java index 7fb264d3..ede5d1fc 100644 --- a/LDK/src/org/labkey/ldk/LDKController.java +++ b/LDK/src/org/labkey/ldk/LDKController.java @@ -21,26 +21,20 @@ import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.jetbrains.annotations.NotNull; import org.json.JSONArray; import org.json.JSONObject; import org.labkey.api.action.ApiResponse; import org.labkey.api.action.ApiSimpleResponse; -import org.labkey.api.action.ConfirmAction; import org.labkey.api.action.ExportAction; import org.labkey.api.action.HasAllowBindParameter; import org.labkey.api.action.MutatingApiAction; import org.labkey.api.action.ReadOnlyApiAction; -import org.labkey.api.action.SimpleErrorView; import org.labkey.api.action.SimpleViewAction; import org.labkey.api.action.SpringActionController; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; -import org.labkey.api.data.CoreSchema; -import org.labkey.api.data.DbSequenceManager; import org.labkey.api.data.PropertyManager; import org.labkey.api.data.PropertyManager.WritablePropertyMap; -import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.SqlScriptRunner; import org.labkey.api.data.TableInfo; import org.labkey.api.ldk.LDKService; @@ -70,7 +64,7 @@ import org.labkey.api.settings.AppProps; import org.labkey.api.settings.LookAndFeelProperties; import org.labkey.api.util.ConfigurationException; -import org.labkey.api.util.GUID; +import org.labkey.api.util.HtmlString; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Path; import org.labkey.api.util.StringUtilsLabKey; @@ -86,7 +80,6 @@ import org.labkey.ldk.notification.NotificationServiceImpl; import org.labkey.ldk.sql.LDKNaturalizeInstallationManager; import org.springframework.validation.BindException; -import org.springframework.validation.Errors; import org.springframework.web.servlet.ModelAndView; import java.net.URISyntaxException; @@ -371,7 +364,7 @@ public ModelAndView getView(RunNotificationForm form, BindException errors) thro if (!n.isAvailable(getContainer())) { - return new HtmlView("The notification " + form.getKey() + " is not available in this container"); + return new HtmlView(HtmlString.of("The notification " + form.getKey() + " is not available in this container")); } _title = n.getName(); @@ -392,7 +385,7 @@ public ModelAndView getView(RunNotificationForm form, BindException errors) thro sb.append(msg == null ? "The notification did not produce a message" : msg); } - return new HtmlView(sb.toString()); + return new HtmlView(HtmlString.of(sb)); } @Override @@ -448,7 +441,7 @@ public ModelAndView getView(Object form, BindException errors) throws Exception String sb = "This page is designed to inspect all registered container scoped tables and report any tables with duplicate keys in the same container. This should be enforced by the user schema; however, direct DB inserts will bypass this check.

" + StringUtils.join(messages, "
"); - return new HtmlView(sb); + return new HtmlView(HtmlString.of(sb)); } @Override @@ -520,7 +513,7 @@ public ApiResponse execute(NotificationSettingsForm form, BindException errors) { if (form.getEnabled() != null) { - NotificationServiceImpl.get().setServiceEnabled(form.getEnabled()); + NotificationServiceImpl.get().setServiceEnabled(getUser(), form.getEnabled()); } } @@ -529,7 +522,7 @@ public ApiResponse execute(NotificationSettingsForm form, BindException errors) try { ValidEmail e = new ValidEmail(form.getReplyEmail()); - NotificationServiceImpl.get().setReturnEmail(getContainer(), e.getEmailAddress()); + NotificationServiceImpl.get().setReturnEmail(getContainer(), getUser(), e.getEmailAddress()); } catch (ValidEmail.InvalidEmailException e) { @@ -555,7 +548,7 @@ public ApiResponse execute(NotificationSettingsForm form, BindException errors) return null; } - NotificationServiceImpl.get().setUser(getContainer(), u.getUserId()); + NotificationServiceImpl.get().setUser(getContainer(), getUser(), u.getUserId()); } catch (ValidEmail.InvalidEmailException e) { @@ -576,7 +569,7 @@ public ApiResponse execute(NotificationSettingsForm form, BindException errors) return null; } - NotificationServiceImpl.get().setActive(n, getContainer(), notifications.getBoolean(key)); + NotificationServiceImpl.get().setActive(n, getContainer(), getUser(), notifications.getBoolean(key)); } } } @@ -904,7 +897,7 @@ public ModelAndView getView(Object form, BindException errors) throws Exception String urlString = PropertyManager.getProperties(getContainer(), REDIRECT_URL_DOMAIN).get(REDIRECT_URL_PROP); if (urlString == null) { - return new HtmlView("This folder is only visible to admins"); + return new HtmlView(HtmlString.of("This folder is only visible to admins")); } else { @@ -919,7 +912,7 @@ public ModelAndView getView(Object form, BindException errors) throws Exception } catch (URISyntaxException e) { - return new HtmlView("Invalid redirect URL set: " + urlString); + return new HtmlView(HtmlString.of("Invalid redirect URL set: " + urlString)); } } } diff --git a/LDK/src/org/labkey/ldk/notification/NotificationServiceImpl.java b/LDK/src/org/labkey/ldk/notification/NotificationServiceImpl.java index 4c7a95c9..5c8ee32d 100644 --- a/LDK/src/org/labkey/ldk/notification/NotificationServiceImpl.java +++ b/LDK/src/org/labkey/ldk/notification/NotificationServiceImpl.java @@ -26,6 +26,8 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.json.JSONObject; +import org.labkey.api.audit.AuditLogService; +import org.labkey.api.audit.provider.SiteSettingsAuditProvider.SiteSettingsAuditEvent; import org.labkey.api.data.CompareType; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; @@ -168,11 +170,19 @@ public boolean isServiceEnabled() return Boolean.parseBoolean(prop); } - public void setServiceEnabled(Boolean status) + public void setServiceEnabled(User u, Boolean status) { + boolean oldStatus = isServiceEnabled(); WritablePropertyMap pm = PropertyManager.getWritableProperties(ContainerManager.getRoot(), NotificationServiceImpl.CONFIG_PROPERTY_DOMAIN, true); pm.put(ENABLED_PROP, status.toString()); pm.save(); + + if (status != oldStatus) + { + SiteSettingsAuditEvent event = new SiteSettingsAuditEvent(ContainerManager.getRoot(), + "Notification service has been " + (status ? "enabled" : "disabled") + "."); + AuditLogService.get().addEvent(u, event); + } } @Override @@ -219,17 +229,28 @@ public Address getReturnEmail(Container c) return email == null ? null : getAddress(email); } - public void setReturnEmail(Container c, String returnEmail) + public void setReturnEmail(Container c, User u, String returnEmail) { try { if(returnEmail != null) { ValidEmail email = new ValidEmail(returnEmail); + String previous = getConfigProperty(c, RETURN_EMAIL); WritablePropertyMap pm = PropertyManager.getWritableProperties(c, NotificationServiceImpl.CONFIG_PROPERTY_DOMAIN, true); pm.put(RETURN_EMAIL, email.getEmailAddress()); pm.save(); + + if(!returnEmail.equals(previous)) + { + SiteSettingsAuditEvent event = new SiteSettingsAuditEvent(c, "Notification reply-to email updated."); + String before = previous == null ? "" : previous; + String after = email.getEmailAddress(); + String html = "
Reply-to email" + PageFlowUtil.filter(before) + " » " + PageFlowUtil.filter(after) + "
"; + event.setChanges(html); + AuditLogService.get().addEvent(u, event); + } } } catch (ValidEmail.InvalidEmailException e) @@ -247,13 +268,25 @@ public User getUser(Container c) return UserManager.getUser(Integer.parseInt(user)); } - public void setUser(Container c, Integer userId) + public void setUser(Container c, User u, Integer userId) { if(userId != null) { + User previousUser = getUser(c); WritablePropertyMap pm = PropertyManager.getWritableProperties(c, NotificationServiceImpl.CONFIG_PROPERTY_DOMAIN, true); pm.put(USER_PROP, String.valueOf(userId)); pm.save(); + + if (previousUser == null || (previousUser.getUserId() != userId)) + { + User newUser = UserManager.getUser(userId); + String before = previousUser == null ? "" : previousUser.getEmail(); + String after = newUser == null ? "" : newUser.getEmail(); + SiteSettingsAuditEvent event = new SiteSettingsAuditEvent(c, "Notification service user updated."); + String html = "
Service user" + PageFlowUtil.filter(before) + " » " + PageFlowUtil.filter(after) + "
"; + event.setChanges(html); + AuditLogService.get().addEvent(u, event); + } } } @@ -362,11 +395,20 @@ public boolean isActive(Notification n, Container c) return false; } - public void setActive(Notification n, Container c, boolean active) + public void setActive(Notification n, Container c, User u, boolean active) { + boolean old = isActive(n, c); WritablePropertyMap pm = PropertyManager.getWritableProperties(c, NotificationServiceImpl.STATUS_PROPERTY_DOMAIN, true); pm.put(getKey(n), active ? String.valueOf(active) : null); pm.save(); + + if (old != active) + { + String action = active ? "enabled" : "disabled"; + String comment = "Notification '" + n.getName() + "' has been " + action + "."; + SiteSettingsAuditEvent event = new SiteSettingsAuditEvent(c, comment); + AuditLogService.get().addEvent(u, event); + } } @Override @@ -530,6 +572,9 @@ public void updateSubscriptions(Container c, User u, Notification n, @Nullable L { TableInfo ti = LDKSchema.getTable(LDKSchema.TABLE_NOTIFICATION_RECIPIENTS); + List actuallyAdded = new ArrayList<>(); + List actuallyRemoved = new ArrayList<>(); + if (toAdd != null) { for (UserPrincipal up : toAdd) @@ -548,20 +593,49 @@ public void updateSubscriptions(Container c, User u, Notification n, @Nullable L row.put("createdby", u.getUserId()); row.put("created", new Date()); Table.insert(u, ti, row); + actuallyAdded.add(up); } } + } - if (toRemove != null) + if (toRemove != null) + { + for (UserPrincipal up : toRemove) { - for (UserPrincipal up : toRemove) - { - SimpleFilter filter = new SimpleFilter(FieldKey.fromString("container"), c.getId(), CompareType.EQUAL); - filter.addCondition(FieldKey.fromString("recipient"), up.getUserId()); - filter.addCondition(FieldKey.fromString("notificationtype"), n.getName()); + SimpleFilter filter = new SimpleFilter(FieldKey.fromString("container"), c.getId(), CompareType.EQUAL); + filter.addCondition(FieldKey.fromString("recipient"), up.getUserId()); + filter.addCondition(FieldKey.fromString("notificationtype"), n.getName()); + + Table.delete(ti, filter); + actuallyRemoved.add(up); + } + } - Table.delete(ti, filter); + // Audit event + if ((!actuallyAdded.isEmpty() || !actuallyRemoved.isEmpty()) && u != null) + { + StringBuilder html = new StringBuilder(); + if (!actuallyAdded.isEmpty()) + { + html.append("

Added recipients
"); + } + if (!actuallyRemoved.isEmpty()) + { + html.append("
Removed recipients
"); } + SiteSettingsAuditEvent event = new SiteSettingsAuditEvent(c, "Updated notification subscriptions for '" + n.getName() + "'."); + event.setChanges(html.toString()); + AuditLogService.get().addEvent(u, event); } } From 4410e67b7958f699acfdca8fee9295d4a7f2f607 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Tue, 28 Oct 2025 07:17:40 -0700 Subject: [PATCH 2/2] Don't double encode --- LDK/src/org/labkey/ldk/LDKController.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/LDK/src/org/labkey/ldk/LDKController.java b/LDK/src/org/labkey/ldk/LDKController.java index ede5d1fc..6709803f 100644 --- a/LDK/src/org/labkey/ldk/LDKController.java +++ b/LDK/src/org/labkey/ldk/LDKController.java @@ -385,7 +385,7 @@ public ModelAndView getView(RunNotificationForm form, BindException errors) thro sb.append(msg == null ? "The notification did not produce a message" : msg); } - return new HtmlView(HtmlString.of(sb)); + return new HtmlView(HtmlString.unsafe(sb.toString())); } @Override @@ -912,7 +912,7 @@ public ModelAndView getView(Object form, BindException errors) throws Exception } catch (URISyntaxException e) { - return new HtmlView(HtmlString.of("Invalid redirect URL set: " + urlString)); + return new HtmlView(HtmlString.unsafe("Invalid redirect URL set: " + urlString)); } } }