Merge remote-tracking branch 'origin/develop' into fb_deriveActions

Author: labkey-susanh

api/src/org/labkey/api/admin/AdminUrls.java

@@ -65,7 +65,9 @@ public interface AdminUrls extends UrlProvider
     ActionURL getSessionLoggingURL();
     ActionURL getTrackedAllocationsViewerURL();
     ActionURL getSystemMaintenanceURL();
-    ActionURL getCspReportToURL(String cspVersion);
+    ActionURL getCspReportToURL();
+
+    ActionURL getAllowedExternalRedirectHostsURL();
 
     /**
      * Simply adds an "Admin Console" link to nav trail if invoked in the root container. Otherwise, root is unchanged.

api/src/org/labkey/api/exp/api/ExpProtocol.java

@@ -99,7 +99,7 @@ enum Status
     String getContact();
 
     List<? extends ExpProtocol> getChildProtocols();
-    List<? extends ExpExperiment> getBatches();
+    List<? extends ExpExperiment> getBatches(@Nullable Container c);
 
     void setEntityId(String entityId);
     String getEntityId();

api/src/org/labkey/api/reports/ReportService.java

@@ -47,6 +47,8 @@
 
 public interface ReportService
 {
+    String R_REPORT_CUSTOM_SHARING = "rReportCustomSharing";
+
     // this logger is to enable all report loggers in the admin ui (org.labkey.api.reports.*)
     @SuppressWarnings({"UnusedDeclaration", "SSBasedInspection"})
     Logger packageLogger = LogManager.getLogger(ReportService.class.getPackageName());

api/src/org/labkey/api/reports/report/r/RReport.java

@@ -46,6 +46,7 @@
 import org.labkey.api.rstudio.RStudioService;
 import org.labkey.api.security.SecurityManager;
 import org.labkey.api.security.User;
+import org.labkey.api.settings.OptionalFeatureService;
 import org.labkey.api.thumbnail.Thumbnail;
 import org.labkey.api.util.FileUtil;
 import org.labkey.api.util.PageFlowUtil;
@@ -74,6 +75,8 @@
 import java.util.Map;
 import java.util.Set;
 
+import static org.labkey.api.reports.ReportService.R_REPORT_CUSTOM_SHARING;
+
 public class RReport extends ExternalScriptEngineReport
 {
     public static final String TYPE = "ReportService.rReport";
@@ -925,8 +928,12 @@ public String getEditAreaSyntax()
     @Override
     public boolean allowShareButton(User user, Container container)
     {
-        // allow sharing if this R report is a DB report and the user canShare
-        return !getDescriptor().isModuleBased() && canShare(user, container);
+        if (OptionalFeatureService.get().isFeatureEnabled(R_REPORT_CUSTOM_SHARING))
+        {
+            // allow sharing if this R report is a DB report and the user canShare
+            return !getDescriptor().isModuleBased() && canShare(user, container);
+        }
+        return false;
     }
 
     public static class TestCase extends Assert

api/src/org/labkey/filters/ContentSecurityPolicyFilter.java

@@ -10,6 +10,7 @@
 import jakarta.servlet.http.HttpServletResponse;
 import org.apache.commons.collections4.SetValuedMap;
 import org.apache.commons.collections4.multimap.HashSetValuedHashMap;
+import org.apache.commons.lang3.EnumUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.Strings;
 import org.apache.logging.log4j.Logger;
@@ -19,7 +20,6 @@
 import org.junit.Test;
 import org.labkey.api.admin.AdminUrls;
 import org.labkey.api.collections.CopyOnWriteHashMap;
-import org.labkey.api.collections.LabKeyCollectors;
 import org.labkey.api.security.Directive;
 import org.labkey.api.settings.AppProps;
 import org.labkey.api.settings.OptionalFeatureService;
@@ -42,6 +42,8 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 
@@ -95,6 +97,11 @@ public String getHeaderName()
         {
             return _headerName;
         }
+
+        private static @Nullable ContentSecurityPolicyType get(String disposition)
+        {
+            return EnumUtils.getEnumIgnoreCase(ContentSecurityPolicyType.class, disposition);
+        }
     }
 
     static
@@ -119,8 +126,9 @@ public void init(FilterConfig filterConfig) throws ServletException
             String paramValue = filterConfig.getInitParameter(paramName);
             if ("policy".equalsIgnoreCase(paramName))
             {
+                // Extract before filtering since CSP version is in a comment
+                extractCspVersion(paramValue);
                 _stashedTemplate = filterPolicy(paramValue);
-                extractCspVersion(_stashedTemplate);
             }
             else if ("disposition".equalsIgnoreCase(paramName))
             {
@@ -139,12 +147,12 @@ else if ("disposition".equalsIgnoreCase(paramName))
         if (CSP_FILTERS.put(getType(), this) != null)
             throw new ServletException("ContentSecurityPolicyFilter is misconfigured, duplicate policies of type: " + getType());
 
-        // configure a different endpoint for each type to convey the correct csp version (eXX vs. rXX)
+        // configure a different endpoint for each type. TODO: We only need one CSP violation reporting endpoint now, so one header would do
         _reportToEndpointName = "csp-" + getType().name().toLowerCase();
     }
 
     /** Filter out block comments and replace special characters in the provided policy */
-    public static String filterPolicy(String policy)
+    private static String filterPolicy(String policy)
     {
         String s = policy.trim();
         s = s.replace( '\n', ' ' );
@@ -164,40 +172,24 @@ public static String filterPolicy(String policy)
         return s;
     }
 
+    private static final Pattern CSP_VERSION_PATTERN = Pattern.compile("cspVersion\\s*=\\s*(\\w+)");
+
     /**
-     * Extract the cspVersion parameter value from the report-uri directive, if possible. Otherwise, cspVersion is left
-     * as "Unknown". This value is reported as part of usage metrics.
+     * Extract the cspVersion value from a comment in the CSP, if it exists. Otherwise, cspVersion is left as "Unknown".
+     * This value is reported as part of usage metrics and included in violation reports that are logged and forwarded.
      */
     private void extractCspVersion(String s)
     {
-        // Simple parser that should be compliant with https://www.w3.org/TR/CSP3/#parse-serialized-policy
-        Map<String, String> cspMap = Arrays.stream(s.split(";"))
-            .map(String::trim)
-            .filter(line -> !line.isEmpty())
-            .map(line -> line.split("\\s+", 2))
-            .filter(parts -> parts.length == 2)
-            .collect(LabKeyCollectors.toCaseInsensitiveLinkedMap(parts -> parts[0], parts -> parts[1]));
-
-        String directive = "report-uri";
-        String reportUri = cspMap.get(directive);
-
-        if (reportUri != null)
+        Matcher matcher = CSP_VERSION_PATTERN.matcher(s);
+        if (matcher.find())
         {
-            try
-            {
-                ActionURL reportUrl =  new ActionURL(reportUri);
-                String cspVersion = reportUrl.getParameter("cspVersion");
+            _cspVersion = matcher.group(1);
 
-                if (null != cspVersion)
-                    _cspVersion = cspVersion;
-            }
-            catch (IllegalArgumentException e)
-            {
-                LOG.warn("Unable to parse {} URI", directive, e);
-            }
-        }
+            if (matcher.find())
+                LOG.warn("More than one cspVersion=XX assignment found; using the first one.");
 
-        LOG.debug("CspVersion: {}", getCspVersion());
+            LOG.debug("CspVersion: {}", getCspVersion());
+        }
     }
 
     @Override
@@ -277,7 +269,7 @@ private CspFilterSettings(ContentSecurityPolicyFilter filter, String baseServerU
             {
                 // Each filter adds its own "Reporting-Endpoints" header since we want to convey the correct version (eXX vs. rXX)
                 @SuppressWarnings("DataFlowIssue")
-                ActionURL violationUrl = PageFlowUtil.urlProvider(AdminUrls.class).getCspReportToURL(filter.getCspVersion());
+                ActionURL violationUrl = PageFlowUtil.urlProvider(AdminUrls.class).getCspReportToURL();
                 // Use an absolute URL so we always post to https:, even if the violating request uses http:
                 _reportingEndpointsHeaderValue = filter.getReportToEndpointName() + "=\"" + violationUrl.getURIString() + "\"";
 
@@ -406,6 +398,34 @@ public static boolean hasCsp(ContentSecurityPolicyType type)
         return CSP_FILTERS.get(type) != null;
     }
 
+    public static @NotNull String getCspVersion(@Nullable String disposition)
+    {
+        if (disposition != null)
+        {
+            ContentSecurityPolicyType type = ContentSecurityPolicyType.get(disposition);
+
+            if (type != null)
+            {
+                var filter = CSP_FILTERS.get(type);
+
+                if (null != filter)
+                {
+                    return filter.getCspVersion();
+                }
+                else
+                {
+                    LOG.error("Disposition {} doesn't match a configured CSP filter", disposition);
+                }
+            }
+            else
+            {
+                LOG.error("Bad disposition: {}", disposition);
+            }
+        }
+
+        return "Unknown";
+    }
+
     public static List<String> getMissingSubstitutions(ContentSecurityPolicyType type)
     {
         ContentSecurityPolicyFilter filter = CSP_FILTERS.get(type);
@@ -442,6 +462,32 @@ public static void registerMetricsProvider()
 
     public static class TestCase extends Assert
     {
+        @Test
+        public void testCspVersionExtraction()
+        {
+            testCspExtract("e14", "/* cspVersion=e14 */");
+            testCspExtract("r14", "/*cspVersion=r14 */");
+            testCspExtract("e15", "/*   cspVersion  =  e15 */");
+            testCspExtract("r15", "/* cspVersion=r15*/");
+            testCspExtract("e15", "/*   cspVersion    =    e15*/");
+            testCspExtract("e15", "/*   cspVersion    =    e15*/ /* cspVersion=XXX */");
+
+            testCspExtract("Unknown", "");
+            testCspExtract("Unknown", "     ");
+            testCspExtract("Unknown", "/* cspVersin=e14 */");
+            testCspExtract("Unknown", "/* cspVersion */");
+            testCspExtract("Unknown", "/* cspVersion= */");
+            testCspExtract("Unknown", "/* cspVersion=*/");
+            testCspExtract("Unknown", "/* cspVersion== */");
+        }
+
+        private void testCspExtract(String expected, String csp)
+        {
+            ContentSecurityPolicyFilter filter = new ContentSecurityPolicyFilter();
+            filter.extractCspVersion(csp);
+            assertEquals(expected, filter.getCspVersion());
+        }
+
         @Test
         public void testPolicyFiltering()
         {
@@ -461,7 +507,7 @@ public void testPolicyFiltering()
                         report-uri /* Whoa! */ /admin-contentsecuritypolicyreport.api?${CSP.REPORT.PARAMS} https://*;
                     """;
 
-            // Multi-line for readability, but notice that newlines are replaced before assignment
+            // Multi-line for readability, but notice that newlines are replaced when constructing the expected string
             String expected = """
                     default-src 'self' https: http: ;
                     connect-src 'self' http://www.labkey.org localhost:* ws: ${LABKEY.ALLOWED.CONNECTIONS} ;

assay/src/org/labkey/assay/AssayManager.java

@@ -378,10 +378,16 @@ public AssaySchema createSchema(User user, Container container, @Nullable Contai
 
     @Override
     public @NotNull List<ExpProtocol> getAssayProtocols(Container container)
+    {
+        return getAssayProtocols(container, false);
+    }
+
+    private @NotNull List<ExpProtocol> getAssayProtocols(Container container, boolean currentOnly)
     {
         List<ExpProtocol> allProtocols = new ArrayList<>();
 
-        for (Container containerInScope : container.getContainersFor(ContainerType.DataType.protocol))
+        Collection<Container> containerScopes = currentOnly ? List.of(container) : container.getContainersFor(ContainerType.DataType.protocol);
+        for (Container containerInScope : containerScopes)
         {
             List<ExpProtocol> ids = PROTOCOL_CACHE.get(containerInScope);
             allProtocols.addAll(ids);
@@ -622,14 +628,14 @@ public ExpExperiment findBatch(ExpRun run)
     }
 
     /**
-     * Creates a single document per assay design/folder combo, with some simple assay info (name, description), plus
-     * the names and comments from all the runs.
+     * Creates a single document per assay design, with some simple assay info (name, description).
+     * Note: the document for an assay protocol will just be associated with the protocol's container
      */
     @Override
     public void indexAssays(SearchService.TaskIndexingQueue queue)
     {
-        List<ExpProtocol> protocols = getAssayProtocols(queue.getContainer());
-
+        // GitHub Issue 895: for the assay protocol search document just user the current container's protocols
+        List<ExpProtocol> protocols = getAssayProtocols(queue.getContainer(), true);
         for (ExpProtocol protocol : protocols)
             indexAssay(queue, protocol);
     }
@@ -661,22 +667,6 @@ public void indexAssay(SearchService.TaskIndexingQueue queue, ExpProtocol protoc
         m.put(SearchService.PROPERTY.keywordsMed.toString(), keywords);
         m.put(SearchService.PROPERTY.categories.toString(), ASSAY_CATEGORY.getName());
 
-        ExperimentService.get().getExpRuns(c, protocol, null)
-            .forEach(run -> {
-                StringBuilder runKeywords = new StringBuilder();
-
-                runKeywords.append(" ");
-                runKeywords.append(run.getName());
-
-                if (null != run.getComments())
-                {
-                    runKeywords.append(" ");
-                    runKeywords.append(run.getComments());
-                }
-
-                body.append(runKeywords);
-            });
-
         String docId = protocol.getDocumentId();
         WebdavResource r = new SimpleDocumentResource(new Path(docId), docId, c.getEntityId(), "text/plain", body.toString(), assayBeginURL, createdBy, created, modifiedBy, modified, m);
         queue.addResource(r);
@@ -728,7 +718,7 @@ private void indexAssayBatches(SearchService.TaskIndexingQueue queue, ExpProtoco
     {
         if (shouldIndexProtocolBatches(protocol))
         {
-            for (ExpExperiment batch : protocol.getBatches())
+            for (ExpExperiment batch : protocol.getBatches(queue.getContainer()))
             {
                 if (modifiedSince == null || modifiedSince.before(batch.getModified()))
                     indexAssayBatch(queue, batch);

core/src/org/labkey/core/admin/AdminController.java

@@ -889,10 +889,16 @@ public ActionURL getSystemMaintenanceURL()
         }
 
         @Override
-        public ActionURL getCspReportToURL(@NotNull String cspVersion)
+        public ActionURL getCspReportToURL()
         {
-            return new ActionURL(ContentSecurityPolicyReportToAction.class, ContainerManager.getRoot())
-                .addParameter("cspVersion", cspVersion);
+            return new ActionURL(ContentSecurityPolicyReportToAction.class, ContainerManager.getRoot());
+        }
+
+        @Override
+        public ActionURL getAllowedExternalRedirectHostsURL()
+        {
+            return new ActionURL(AllowListAction.class, ContainerManager.getRoot())
+                .addParameter("type", AllowListType.Redirect.name());
         }
 
         public static ActionURL getDeprecatedFeaturesURL()
@@ -12285,6 +12291,7 @@ protected boolean handleOneReport(JSONObject jsonObj, HttpServletRequest request
                             if (!forwarded)
                             {
                                 jsonObj.put("labkeyVersion", AppProps.getInstance().getReleaseVersion());
+                                jsonObj.put("cspVersion", ContentSecurityPolicyFilter.getCspVersion(cspReport.optString("disposition", null)));
                                 User user = getUser();
                                 String email = null;
                                 // If the user is not logged in, we may still be able to snag the email address from our cookie
@@ -12299,9 +12306,6 @@ protected boolean handleOneReport(JSONObject jsonObj, HttpServletRequest request
                                 jsonObj.put("ip", ipAddress);
                                 if (isNotBlank(userAgent) && !jsonObj.has("user_agent"))
                                     jsonObj.put("user_agent", userAgent);
-                                String cspVersion = request.getParameter("cspVersion");
-                                if (null != cspVersion)
-                                    jsonObj.put("cspVersion", cspVersion);
                             }
 
                             var jsonStr = jsonObj.toString(2);