From 549e5beb52b92ed1b212a752da47b3b282ea9ec4 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 20 Jun 2026 10:00:35 -0700 Subject: [PATCH 1/2] Configuration option to remember terms-of-use acceptance --- .../api/security/AuthenticationManager.java | 2 +- .../api/security/WikiTermsOfUseProvider.java | 196 +++++++++++------- api/src/org/labkey/api/settings/AppProps.java | 7 + .../org/labkey/api/settings/AppPropsImpl.java | 6 + .../api/settings/SiteSettingsProperties.java | 8 + .../api/settings/WriteableAppProps.java | 7 + .../org/labkey/api/util/SessionHelper.java | 6 - api/src/org/labkey/api/wiki/WikiService.java | 3 + .../labkey/core/admin/AdminController.java | 13 ++ .../org/labkey/core/admin/customizeSite.jsp | 44 +++- .../labkey/core/login/LoginController.java | 21 +- core/webapp/login.js | 2 +- wiki/src/org/labkey/wiki/WikiManager.java | 12 +- 13 files changed, 231 insertions(+), 96 deletions(-) diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index c6051d89850..f1aefd5a46f 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -483,7 +483,7 @@ public static Map getLoginPageConfiguration(Project project) { Map config = new HashMap<>(); config.put("registrationEnabled", isRegistrationEnabled()); - config.put("requiresTermsOfUse", WikiTermsOfUseProvider.isTermsOfUseRequired(project)); + config.put("requiresTermsOfUse", WikiTermsOfUseProvider.isTermsOfUseConfigured(project) && AppProps.getInstance().getTermsOfUseFrequencySeconds() == 0); config.put("hasOtherLoginMechanisms", hasSSOAuthenticationConfiguration()); return config; } diff --git a/api/src/org/labkey/api/security/WikiTermsOfUseProvider.java b/api/src/org/labkey/api/security/WikiTermsOfUseProvider.java index 5a707ae4ac2..30b5c71c1bb 100644 --- a/api/src/org/labkey/api/security/WikiTermsOfUseProvider.java +++ b/api/src/org/labkey/api/security/WikiTermsOfUseProvider.java @@ -15,34 +15,40 @@ */ package org.labkey.api.security; +import jakarta.servlet.http.HttpSession; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.labkey.api.action.SpringActionController; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.Project; +import org.labkey.api.data.PropertyManager; +import org.labkey.api.data.PropertyManager.WritablePropertyMap; import org.labkey.api.module.ModuleLoader; import org.labkey.api.security.SecurityManager.TermsOfUseProvider; +import org.labkey.api.settings.AppProps; import org.labkey.api.util.HtmlString; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.SafeToRenderEnum; import org.labkey.api.util.SessionHelper; +import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ActionURL; import org.labkey.api.view.RedirectException; import org.labkey.api.view.ViewContext; import org.labkey.api.wiki.WikiService; -import jakarta.servlet.http.HttpSession; +import java.time.Instant; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.Callable; -/** - * Created by adam on 1/13/2016. - */ public class WikiTermsOfUseProvider implements TermsOfUseProvider { public static final String TERMS_OF_USE_WIKI_NAME = "_termsOfUse"; public static final String TERMS_APPROVED_KEY = "TERMS_APPROVED_KEY"; + private static final Logger LOG = LogHelper.getLogger(WikiTermsOfUseProvider.class, "Terms of use workflow"); private static final TermsOfUse NO_TERMS = new TermsOfUse(TermsOfUseType.NONE, null); @Override @@ -56,7 +62,8 @@ public void verifyTermsOfUse(ViewContext context, boolean isBasicAuth) throws Re } } - private static boolean isTermsOfUseRequired(ViewContext ctx) + // Are terms required in this container and the user hasn't approved them yet + public static boolean isTermsOfUseRequired(ViewContext ctx) { Container proj = ctx.getContainer().getProject(); @@ -66,107 +73,152 @@ private static boolean isTermsOfUseRequired(ViewContext ctx) project = new Project(proj); } - // not required if user has already approved at the project level - if (isTermsOfUseApproved(ctx, project)) - return false; + return isTermsOfUseRequired(ctx, project); + } - TermsOfUse termsOfUse = getTermsOfUse(project); - boolean required; - switch (termsOfUse.getType()) - { - case SITE_WIDE: - // if we don't require project-level and have approved site-wide level, not required to ask again, - // but we don't cache for the project in case we set project-level terms later - required = !isTermsOfUseApproved(ctx, null); - break; - case PROJECT_LEVEL: // we already checked if the project-level terms were approved, so we know that they are required here - required = true; - break; - default: - required = false; - } + // Are terms required in this container and the user hasn't approved them yet + public static boolean isTermsOfUseRequired(ViewContext ctx, @Nullable Project project) + { + // First, quick check for whether terms are ever needed for this project + TermsOfUseConfiguration config = getTermsOfUseConfiguration(project); + if (config.type() == TermsOfUseType.NONE) + return false; - return required; + // Terms are needed, so check if user has already approved + return !isTermsOfUseApproved(ctx, config.termsContainer()); } - public static boolean isTermsOfUseApproved(ViewContext ctx, @Nullable Project project) + private static final String LAST_TERMS_ACCEPTANCE = "lastTermsAcceptance"; + private static final String DATE = "date"; + + // termsContainer is guaranteed to have a terms-of-use wiki + public static boolean isTermsOfUseApproved(ViewContext ctx, @NotNull Container termsContainer) { HttpSession session = ctx.getRequest().getSession(false); if (null == session) return false; - @Nullable Set termsApproved = getApprovedTerms(session); - return null != termsApproved && termsApproved.contains(project); + @NotNull Set termsApproved = getApprovedTerms(session); + assert WikiService.get().hasTermsOfUseWiki(termsContainer); // TODO: Temporary check + boolean approved = termsApproved.contains(termsContainer); + if (!approved) + LOG.info("Approved terms did not include {} for {}", termsContainer, ctx.getUser()); + if (!approved) + { + User user = ctx.getUser(); + if (!user.isGuest()) + { + int frequencySeconds = AppProps.getInstance().getTermsOfUseFrequencySeconds(); + if (frequencySeconds > 0) + { + String isoDateString = PropertyManager.getProperties(user, termsContainer, LAST_TERMS_ACCEPTANCE).get(DATE); + if (isoDateString != null) + { + Instant lastAccepted = Instant.parse(isoDateString); + approved = Instant.now().isBefore(lastAccepted.plusSeconds(frequencySeconds)); + // On first terms check at the site or each project, if last acceptance hasn't expired, stash + // an "approval" into session. This short-circuits future requests (so we don't run through + // this code block on every request). It also ensures the terms dialogs don't randomly pop up + // later in the session (when acceptance expires). + if (approved) + { + try (var ignored = SpringActionController.ignoreSqlUpdates()) + { + setTermsOfUseApprovedInSession(ctx, termsContainer); + } + } + } + } + } + } + return approved; } - public static @Nullable Set getApprovedTerms(@NotNull HttpSession session) + public static @NotNull Container getTermsContainer(@Nullable Project project) { - synchronized (SessionHelper.getSessionLock(session)) - { - return (Set) session.getAttribute(TERMS_APPROVED_KEY); - } + return project != null ? project.getContainer() : ContainerManager.getRoot(); } - public static boolean isTermsOfUseRequired(@Nullable Project project) + public static @NotNull Set getApprovedTerms(@NotNull HttpSession session) { - //TODO: Should do this more efficiently, but no efficient public wiki api for this yet - TermsOfUse terms = getTermsOfUse(project); - return terms.getType() != TermsOfUseType.NONE; + return SessionHelper.getAttribute(session, TERMS_APPROVED_KEY, (Callable>) HashSet::new); } - @NotNull - public static TermsOfUse getTermsOfUse(@Nullable Project project) + public static boolean isTermsOfUseConfigured(@Nullable Project project) { - if (!ModuleLoader.getInstance().isStartupComplete()) - return NO_TERMS; + // This should be fairly efficient. We could consider caching a project -> terms configuration map. + return getTermsOfUseConfiguration(project).type() != TermsOfUseType.NONE; + } - WikiService service = WikiService.get(); - //No wiki service. Wiki module most not be present. Don't do terms here... - if (null == service) - return NO_TERMS; + public record TermsOfUseConfiguration(TermsOfUseType type, /* Null only if type is NONE */ Container termsContainer){} + public static final TermsOfUseConfiguration NO_TERMS_CONFIGURATION = new TermsOfUseConfiguration(TermsOfUseType.NONE, null); - HtmlString termsString; - if (null != project) // find project-level terms of use, if any + public static TermsOfUseConfiguration getTermsOfUseConfiguration(@Nullable Project project) + { + if (ModuleLoader.getInstance().isStartupComplete()) { - termsString = service.getHtml(project.getContainer(), TERMS_OF_USE_WIKI_NAME); - if (null != termsString) + WikiService service = WikiService.get(); + + // Need the wiki service to do terms + if (null != service) { - return new TermsOfUse(TermsOfUseType.PROJECT_LEVEL, termsString); + // Project terms override root terms + if (project != null) + { + Container c = project.getContainer(); + if (service.hasTermsOfUseWiki(c)) + return new TermsOfUseConfiguration(TermsOfUseType.PROJECT_LEVEL, c); + } + + // Now check the root + Container c = ContainerManager.getRoot(); + if (service.hasTermsOfUseWiki(c)) + return new TermsOfUseConfiguration(TermsOfUseType.SITE_WIDE, c); } } - // now check if we have site-wide terms of use - termsString = service.getHtml(ContainerManager.getRoot(), TERMS_OF_USE_WIKI_NAME); - if (null != termsString) + return NO_TERMS_CONFIGURATION; + } + + @NotNull + public static TermsOfUse getTermsOfUse(@Nullable Project project) + { + TermsOfUseConfiguration config = getTermsOfUseConfiguration(project); + + if (config.type() != TermsOfUseType.NONE) { - return new TermsOfUse(TermsOfUseType.SITE_WIDE, termsString); + // Check above guarantees that wiki service is present and getContainer is non-null + @SuppressWarnings("DataFlowIssue") + HtmlString termsString = WikiService.get().getHtml(config.termsContainer(), TERMS_OF_USE_WIKI_NAME); + if (null != termsString) + { + return new TermsOfUse(config.type(), termsString); + } } return NO_TERMS; } - public static void setTermsOfUseApproved(ViewContext ctx, @Nullable Project project, boolean approved) + public static void setTermsOfUseApproved(ViewContext ctx, @NotNull Container termsContainer) { - HttpSession session = ctx.getRequest().getSession(false); - if (null == session && !approved) - return; - session = ctx.getRequest().getSession(true); + setTermsOfUseApprovedInSession(ctx, termsContainer); + User user = ctx.getUser(); + if (!user.isGuest() && AppProps.getInstance().getTermsOfUseFrequencySeconds() > 0) + { + WritablePropertyMap map = PropertyManager.getWritableProperties(ctx.getUser(), termsContainer, LAST_TERMS_ACCEPTANCE, true); + map.put(DATE, Instant.now().toString()); + map.save(); + LOG.info("Saving terms acceptance timestamp for {} in {}", ctx.getUser(), termsContainer); + } + } + private static void setTermsOfUseApprovedInSession(ViewContext ctx, @NotNull Container termsContainer) + { + HttpSession session = ctx.getRequest().getSession(true); synchronized (SessionHelper.getSessionLock(session)) { - Set termsApproved = (Set) session.getAttribute(TERMS_APPROVED_KEY); - if (null == termsApproved) - { - termsApproved = new HashSet<>(); - session.setAttribute(TERMS_APPROVED_KEY, termsApproved); - } - if (approved) - { - termsApproved.add(project); - } - else - { - termsApproved.remove(project); - } + Set termsApproved = getApprovedTerms(session); + termsApproved.add(termsContainer); } + LOG.info("Stashing terms acceptance in session for {} in {}", ctx.getUser(), termsContainer); } public enum TermsOfUseType implements SafeToRenderEnum diff --git a/api/src/org/labkey/api/settings/AppProps.java b/api/src/org/labkey/api/settings/AppProps.java index 35ee6f6aa5d..1687f2cc748 100644 --- a/api/src/org/labkey/api/settings/AppProps.java +++ b/api/src/org/labkey/api/settings/AppProps.java @@ -229,6 +229,13 @@ static WriteableAppProps getWriteableInstance() /** @return whether the server should include its name and version as a header in HTTP responses */ boolean isIncludeServerHttpHeader(); + /** + * Returns the terms-of-use re-acceptance frequency in seconds. 0 means require acceptance on every sign-in, which + * is the default. Positive value means acceptance is valid for that many seconds before the user is prompted again + * after next login. + */ + int getTermsOfUseFrequencySeconds(); + /** * @return List of configured external redirect hosts */ diff --git a/api/src/org/labkey/api/settings/AppPropsImpl.java b/api/src/org/labkey/api/settings/AppPropsImpl.java index c3458f11262..424b5ed7f3e 100644 --- a/api/src/org/labkey/api/settings/AppPropsImpl.java +++ b/api/src/org/labkey/api/settings/AppPropsImpl.java @@ -644,6 +644,12 @@ public boolean isIncludeServerHttpHeader() return lookupBooleanValue(includeServerHttpHeader, true); } + @Override + public int getTermsOfUseFrequencySeconds() + { + return lookupIntValue(termsOfUseFrequencySeconds, 0); + } + @Override @NotNull public List getExternalRedirectHosts() diff --git a/api/src/org/labkey/api/settings/SiteSettingsProperties.java b/api/src/org/labkey/api/settings/SiteSettingsProperties.java index ce450b74436..0eb0a639a33 100644 --- a/api/src/org/labkey/api/settings/SiteSettingsProperties.java +++ b/api/src/org/labkey/api/settings/SiteSettingsProperties.java @@ -201,6 +201,14 @@ public void setValue(WriteableAppProps writeable, String value) { writeable.setIncludeServerHttpHeader(Boolean.parseBoolean(value)); } + }, + termsOfUseFrequencySeconds("Require terms-of-use acceptance frequency in seconds. 0 = every sign-in, or positive number of seconds between required re-acceptance.") + { + @Override + public void setValue(WriteableAppProps writeable, String value) + { + writeable.setTermsOfUseFrequencySeconds(Integer.parseInt(value)); + } }; private final static Logger LOG = LogHelper.getLogger(SiteSettingsProperties.class, "Warnings about setting properties"); diff --git a/api/src/org/labkey/api/settings/WriteableAppProps.java b/api/src/org/labkey/api/settings/WriteableAppProps.java index f7cbf248cdd..e83ad70dfb6 100644 --- a/api/src/org/labkey/api/settings/WriteableAppProps.java +++ b/api/src/org/labkey/api/settings/WriteableAppProps.java @@ -186,6 +186,13 @@ public void setIncludeServerHttpHeader(boolean b) storeBooleanValue(includeServerHttpHeader, b); } + public void setTermsOfUseFrequencySeconds(int seconds) + { + if (seconds < 0) + throw new IllegalArgumentException("termsOfUseFrequencySeconds must be >= 0"); + storeIntValue(termsOfUseFrequencySeconds, seconds); + } + public void setAdministratorContactEmail(String email) { storeStringValue(administratorContactEmail, email); diff --git a/api/src/org/labkey/api/util/SessionHelper.java b/api/src/org/labkey/api/util/SessionHelper.java index 2714c027eac..1f08d86719a 100644 --- a/api/src/org/labkey/api/util/SessionHelper.java +++ b/api/src/org/labkey/api/util/SessionHelper.java @@ -32,12 +32,6 @@ import java.util.Set; import java.util.concurrent.Callable; - -/** - * User: matthewb - * Date: 2012-01-24 - * Time: 3:03 PM - */ public class SessionHelper { private static final LockManager LOCK_MANAGER = new LockManager<>(); diff --git a/api/src/org/labkey/api/wiki/WikiService.java b/api/src/org/labkey/api/wiki/WikiService.java index 0fa9e448ae5..40d6a1210d7 100644 --- a/api/src/org/labkey/api/wiki/WikiService.java +++ b/api/src/org/labkey/api/wiki/WikiService.java @@ -50,6 +50,9 @@ record RenderedWiki (String name, String title, HtmlString html, String entityId RenderedWiki getRenderedWiki(Container c, String name); + // Quick check for a terms-of-use wiki in the provided container. Helps optimize the every-request terms check. + boolean hasTermsOfUseWiki(Container c); + default HtmlString getHtml(Container c, String name) { var wiki = getRenderedWiki(c, name); diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index 38a6f22b2db..62af41c9d3c 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -1478,6 +1478,8 @@ public boolean handlePost(SiteSettingsForm form, BindException errors) throws Ex props.setXFrameOption(frameOption); props.setIncludeServerHttpHeader(form.isIncludeServerHttpHeader()); + props.setTermsOfUseFrequencySeconds(form.getTermsOfUseFrequencySeconds()); + props.save(getViewContext().getUser()); UsageReportingLevel.reportNow(); if (sslSettingChanged) @@ -2369,6 +2371,7 @@ public static class SiteSettingsForm private String _XFrameOption; private boolean _includeServerHttpHeader; + private int _termsOfUseFrequencySeconds; public String getPipelineToolsDirectory() { @@ -2599,6 +2602,16 @@ public void setIncludeServerHttpHeader(boolean includeServerHttpHeader) { _includeServerHttpHeader = includeServerHttpHeader; } + + public int getTermsOfUseFrequencySeconds() + { + return _termsOfUseFrequencySeconds; + } + + public void setTermsOfUseFrequencySeconds(int termsOfUseFrequencySeconds) + { + _termsOfUseFrequencySeconds = termsOfUseFrequencySeconds; + } } diff --git a/core/src/org/labkey/core/admin/customizeSite.jsp b/core/src/org/labkey/core/admin/customizeSite.jsp index 9efb83ab14e..5fa3ea1fcfd 100644 --- a/core/src/org/labkey/core/admin/customizeSite.jsp +++ b/core/src/org/labkey/core/admin/customizeSite.jsp @@ -126,11 +126,11 @@ Click the Save button at any time to accept the current settings and continue. <%=getTroubleshooterWarning(hasAdminOpsPerms, HtmlString.unsafe(""" -   - - - """), HtmlString.unsafe("\n" + - " "))%> +   + + + """), HtmlString.unsafe("\n" + + ""))%>   @@ -376,6 +376,40 @@ Click the Save button at any time to accept the current settings and continue. + +   + + + Customize terms-of-use frequency (<%=bean.getSiteSettingsHelpLink("terms")%>) + + + + +<% + final int currentTermsFrequency = AppProps.getInstance().getTermsOfUseFrequencySeconds(); + Map termsFrequencyOptions = new TreeMap<>(Comparator.comparing(key -> key)); + termsFrequencyOptions.put(0, "Every signin"); + termsFrequencyOptions.put(60, "Once a minute"); + termsFrequencyOptions.put(SECONDS_PER_DAY, "Once a day"); + termsFrequencyOptions.put(7 * SECONDS_PER_DAY, "Every 7 days"); + termsFrequencyOptions.put(30 * SECONDS_PER_DAY, "Every 30 days"); + + // If current value is non-standard (perhaps set by a startup property) then add it, formatting label as a duration + if (!termsFrequencyOptions.containsKey(currentTermsFrequency)) + termsFrequencyOptions.put(currentTermsFrequency, DateUtil.formatDuration(1000L * currentTermsFrequency)); +%> + + <%= + select() + .name(termsOfUseFrequencySeconds.name()) + .id(termsOfUseFrequencySeconds.name()) + .addOptions(termsFrequencyOptions) + .selected(currentTermsFrequency) + .className(null) + %> + + +   diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index 5e1e40e98d5..e72f16313ca 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -665,7 +665,7 @@ public Object execute(LoginForm form, BindException errors) Project termsProject = getTermsOfUseProject(form); boolean isGuest = getUser().isGuest(); - if (!isTermsOfUseApproved(form) && !form.isApprovedTermsOfUse()) + if (!isTermsOfUseApproved(form) && AppProps.getInstance().getTermsOfUseFrequencySeconds() == 0) { if (null != termsProject) { @@ -699,9 +699,9 @@ public Object execute(LoginForm form, BindException errors) if (form.isApprovedTermsOfUse()) { if (form.getTermsOfUseType() == TermsOfUseType.PROJECT_LEVEL) - WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), termsProject, true); + WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), termsContainer(termsProject)); else if (form.getTermsOfUseType() == TermsOfUseType.SITE_WIDE) - WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), null, true); + WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), ContainerManager.getRoot()); response.put("approvedTermsOfUse", true); } @@ -755,6 +755,11 @@ else if (!errors.hasErrors()) } } + private static Container termsContainer(@Nullable Project project) + { + return WikiTermsOfUseProvider.getTermsContainer(project); + } + @SuppressWarnings("unused") @RequiresNoPermission @IgnoresTermsOfUse @@ -996,9 +1001,9 @@ public Object execute(AgreeToTermsForm form, BindException errors) return false; } if (form.getTermsOfUseType() == TermsOfUseType.PROJECT_LEVEL) - WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), project, true); + WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), termsContainer(project)); else if (form.getTermsOfUseType() == TermsOfUseType.SITE_WIDE) - WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), null, true); + WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), ContainerManager.getRoot()); else { errors.reject(ERROR_MSG, "Unable to determine the terms of use type from the information submitted on the form."); @@ -1263,9 +1268,9 @@ public boolean handlePost(AgreeToTermsForm form, BindException errors) } if (form.getTermsOfUseType() == TermsOfUseType.PROJECT_LEVEL) - WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), project, true); + WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), termsContainer(project)); else if (form.getTermsOfUseType() == TermsOfUseType.SITE_WIDE) - WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), null, true); + WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), ContainerManager.getRoot()); return true; } @@ -1331,7 +1336,7 @@ private Project getTermsOfUseProject(AgreeToTermsForm form) private boolean isTermsOfUseApproved(AgreeToTermsForm form) { Project termsProject = getTermsOfUseProject(form); - return form.isApprovedTermsOfUse() || !WikiTermsOfUseProvider.isTermsOfUseRequired(termsProject) || WikiTermsOfUseProvider.isTermsOfUseApproved(getViewContext(), termsProject); + return form.isApprovedTermsOfUse() || !WikiTermsOfUseProvider.isTermsOfUseRequired(getViewContext(), termsProject); } private static abstract class AbstractLoginForm extends ReturnUrlForm diff --git a/core/webapp/login.js b/core/webapp/login.js index e2534ede023..057334e847e 100644 --- a/core/webapp/login.js +++ b/core/webapp/login.js @@ -172,7 +172,7 @@ } else { - termsSecitons[0].hidden = true; + termsSections[0].hidden = true; } } if (document.getElementById('termsOfUseType') && response && response.termsOfUseType) diff --git a/wiki/src/org/labkey/wiki/WikiManager.java b/wiki/src/org/labkey/wiki/WikiManager.java index ef2695d8645..8a880270223 100644 --- a/wiki/src/org/labkey/wiki/WikiManager.java +++ b/wiki/src/org/labkey/wiki/WikiManager.java @@ -27,9 +27,9 @@ import org.labkey.api.attachments.Attachment; import org.labkey.api.attachments.AttachmentFile; import org.labkey.api.attachments.AttachmentParent; +import org.labkey.api.attachments.AttachmentParentType; import org.labkey.api.attachments.AttachmentService; import org.labkey.api.attachments.AttachmentService.DuplicateFilenameException; -import org.labkey.api.attachments.AttachmentParentType; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerService; import org.labkey.api.data.CoreSchema; @@ -46,7 +46,6 @@ import org.labkey.api.query.QueryService; import org.labkey.api.search.SearchService; import org.labkey.api.security.User; -import org.labkey.api.security.WikiTermsOfUseProvider; import org.labkey.api.util.ContainerUtil; import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.HtmlString; @@ -94,6 +93,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import static org.labkey.api.action.SpringActionController.ERROR_MSG; +import static org.labkey.api.security.WikiTermsOfUseProvider.TERMS_OF_USE_WIKI_NAME; public class WikiManager implements WikiService { @@ -745,7 +745,7 @@ private void indexWikiContainerFast(SearchService.TaskIndexingQueue queue, @Null wikiName = rs.getString("name"); assert null != wikiName; - if (WikiTermsOfUseProvider.TERMS_OF_USE_WIKI_NAME.equals(wikiName)) + if (TERMS_OF_USE_WIKI_NAME.equals(wikiName)) continue; if (!rs.getBoolean("shouldindex")) @@ -869,6 +869,12 @@ public RenderedWiki getRenderedWiki(Container c, String name) } } + @Override + public boolean hasTermsOfUseWiki(Container c) + { + return (c.isRoot() || c.isProject()) && null != WikiSelectManager.getWiki(c, TERMS_OF_USE_WIKI_NAME); + } + @Override public void insertWiki(User user, Container c, String name, String body, WikiRendererType renderType, String title) { From 075a38fb383116a77b60f35547c4a596e889c2c4 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 20 Jun 2026 16:14:06 -0700 Subject: [PATCH 2/2] Address feedback --- .../api/security/WikiTermsOfUseProvider.java | 18 +++++++++++------- .../org/labkey/core/admin/customizeSite.jsp | 3 ++- .../org/labkey/core/login/LoginController.java | 11 +++-------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/api/src/org/labkey/api/security/WikiTermsOfUseProvider.java b/api/src/org/labkey/api/security/WikiTermsOfUseProvider.java index 30b5c71c1bb..68bf7ba9c7d 100644 --- a/api/src/org/labkey/api/security/WikiTermsOfUseProvider.java +++ b/api/src/org/labkey/api/security/WikiTermsOfUseProvider.java @@ -85,6 +85,7 @@ public static boolean isTermsOfUseRequired(ViewContext ctx, @Nullable Project pr return false; // Terms are needed, so check if user has already approved + //noinspection DataFlowIssue - termsContainer() is null only in the NONE case (see above) return !isTermsOfUseApproved(ctx, config.termsContainer()); } @@ -97,11 +98,14 @@ public static boolean isTermsOfUseApproved(ViewContext ctx, @NotNull Container t HttpSession session = ctx.getRequest().getSession(false); if (null == session) return false; - @NotNull Set termsApproved = getApprovedTerms(session); - assert WikiService.get().hasTermsOfUseWiki(termsContainer); // TODO: Temporary check - boolean approved = termsApproved.contains(termsContainer); + boolean approved; + synchronized (SessionHelper.getSessionLock(session)) + { + @NotNull Set termsApproved = getApprovedTerms(session); + approved = termsApproved.contains(termsContainer); + } if (!approved) - LOG.info("Approved terms did not include {} for {}", termsContainer, ctx.getUser()); + LOG.debug("Approved terms did not include {} for {}", termsContainer, ctx.getUser()); if (!approved) { User user = ctx.getUser(); @@ -149,7 +153,7 @@ public static boolean isTermsOfUseConfigured(@Nullable Project project) return getTermsOfUseConfiguration(project).type() != TermsOfUseType.NONE; } - public record TermsOfUseConfiguration(TermsOfUseType type, /* Null only if type is NONE */ Container termsContainer){} + public record TermsOfUseConfiguration(TermsOfUseType type, /* Null only if type is NONE */ @Nullable Container termsContainer){} public static final TermsOfUseConfiguration NO_TERMS_CONFIGURATION = new TermsOfUseConfiguration(TermsOfUseType.NONE, null); public static TermsOfUseConfiguration getTermsOfUseConfiguration(@Nullable Project project) @@ -206,7 +210,7 @@ public static void setTermsOfUseApproved(ViewContext ctx, @NotNull Container ter WritablePropertyMap map = PropertyManager.getWritableProperties(ctx.getUser(), termsContainer, LAST_TERMS_ACCEPTANCE, true); map.put(DATE, Instant.now().toString()); map.save(); - LOG.info("Saving terms acceptance timestamp for {} in {}", ctx.getUser(), termsContainer); + LOG.debug("Saving terms acceptance timestamp for {} in {}", ctx.getUser(), termsContainer); } } @@ -218,7 +222,7 @@ private static void setTermsOfUseApprovedInSession(ViewContext ctx, @NotNull Con Set termsApproved = getApprovedTerms(session); termsApproved.add(termsContainer); } - LOG.info("Stashing terms acceptance in session for {} in {}", ctx.getUser(), termsContainer); + LOG.debug("Stashing terms acceptance in session for {} in {}", ctx.getUser(), termsContainer); } public enum TermsOfUseType implements SafeToRenderEnum diff --git a/core/src/org/labkey/core/admin/customizeSite.jsp b/core/src/org/labkey/core/admin/customizeSite.jsp index 5fa3ea1fcfd..473bed8b635 100644 --- a/core/src/org/labkey/core/admin/customizeSite.jsp +++ b/core/src/org/labkey/core/admin/customizeSite.jsp @@ -389,7 +389,8 @@ Click the Save button at any time to accept the current settings and continue. termsFrequencyOptions = new TreeMap<>(Comparator.comparing(key -> key)); termsFrequencyOptions.put(0, "Every signin"); - termsFrequencyOptions.put(60, "Once a minute"); + if (appProps.isDevMode()) + termsFrequencyOptions.put(60, "Once a minute"); // For testing termsFrequencyOptions.put(SECONDS_PER_DAY, "Once a day"); termsFrequencyOptions.put(7 * SECONDS_PER_DAY, "Every 7 days"); termsFrequencyOptions.put(30 * SECONDS_PER_DAY, "Every 30 days"); diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index e72f16313ca..a706c9f7e4b 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -699,7 +699,7 @@ public Object execute(LoginForm form, BindException errors) if (form.isApprovedTermsOfUse()) { if (form.getTermsOfUseType() == TermsOfUseType.PROJECT_LEVEL) - WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), termsContainer(termsProject)); + WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), WikiTermsOfUseProvider.getTermsContainer(termsProject)); else if (form.getTermsOfUseType() == TermsOfUseType.SITE_WIDE) WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), ContainerManager.getRoot()); response.put("approvedTermsOfUse", true); @@ -755,11 +755,6 @@ else if (!errors.hasErrors()) } } - private static Container termsContainer(@Nullable Project project) - { - return WikiTermsOfUseProvider.getTermsContainer(project); - } - @SuppressWarnings("unused") @RequiresNoPermission @IgnoresTermsOfUse @@ -1001,7 +996,7 @@ public Object execute(AgreeToTermsForm form, BindException errors) return false; } if (form.getTermsOfUseType() == TermsOfUseType.PROJECT_LEVEL) - WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), termsContainer(project)); + WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), WikiTermsOfUseProvider.getTermsContainer(project)); else if (form.getTermsOfUseType() == TermsOfUseType.SITE_WIDE) WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), ContainerManager.getRoot()); else @@ -1268,7 +1263,7 @@ public boolean handlePost(AgreeToTermsForm form, BindException errors) } if (form.getTermsOfUseType() == TermsOfUseType.PROJECT_LEVEL) - WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), termsContainer(project)); + WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), WikiTermsOfUseProvider.getTermsContainer(project)); else if (form.getTermsOfUseType() == TermsOfUseType.SITE_WIDE) WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), ContainerManager.getRoot());