diff --git a/api/src/org/labkey/api/admin/AdminUrls.java b/api/src/org/labkey/api/admin/AdminUrls.java index 4f2e30826d5..b22cefd042f 100644 --- a/api/src/org/labkey/api/admin/AdminUrls.java +++ b/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. diff --git a/api/src/org/labkey/api/exp/api/ExpProtocol.java b/api/src/org/labkey/api/exp/api/ExpProtocol.java index eb633711b42..41d5b91af3f 100644 --- a/api/src/org/labkey/api/exp/api/ExpProtocol.java +++ b/api/src/org/labkey/api/exp/api/ExpProtocol.java @@ -99,7 +99,7 @@ enum Status String getContact(); List getChildProtocols(); - List getBatches(); + List getBatches(@Nullable Container c); void setEntityId(String entityId); String getEntityId(); diff --git a/api/src/org/labkey/api/reports/ReportService.java b/api/src/org/labkey/api/reports/ReportService.java index f246d67da72..3fdba71398b 100644 --- a/api/src/org/labkey/api/reports/ReportService.java +++ b/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()); diff --git a/api/src/org/labkey/api/reports/report/r/RReport.java b/api/src/org/labkey/api/reports/report/r/RReport.java index be6e61a388a..83371fcb9b7 100644 --- a/api/src/org/labkey/api/reports/report/r/RReport.java +++ b/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 diff --git a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java index c28e8ebe84c..90a9b1c5437 100644 --- a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java +++ b/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 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 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} ; diff --git a/assay/src/org/labkey/assay/AssayManager.java b/assay/src/org/labkey/assay/AssayManager.java index 1fa842571d8..4d0302e937d 100644 --- a/assay/src/org/labkey/assay/AssayManager.java +++ b/assay/src/org/labkey/assay/AssayManager.java @@ -378,10 +378,16 @@ public AssaySchema createSchema(User user, Container container, @Nullable Contai @Override public @NotNull List getAssayProtocols(Container container) + { + return getAssayProtocols(container, false); + } + + private @NotNull List getAssayProtocols(Container container, boolean currentOnly) { List allProtocols = new ArrayList<>(); - for (Container containerInScope : container.getContainersFor(ContainerType.DataType.protocol)) + Collection containerScopes = currentOnly ? List.of(container) : container.getContainersFor(ContainerType.DataType.protocol); + for (Container containerInScope : containerScopes) { List 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 protocols = getAssayProtocols(queue.getContainer()); - + // GitHub Issue 895: for the assay protocol search document just user the current container's protocols + List 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); diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index fdeb3aaede3..dc5accedfed 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/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); diff --git a/core/test/src/org/labkey/test/tests/upgrade/EncryptionKeyUpgradeTest.java b/core/test/src/org/labkey/test/tests/upgrade/EncryptionKeyUpgradeTest.java index 65f403d7684..e5f985791fc 100644 --- a/core/test/src/org/labkey/test/tests/upgrade/EncryptionKeyUpgradeTest.java +++ b/core/test/src/org/labkey/test/tests/upgrade/EncryptionKeyUpgradeTest.java @@ -3,6 +3,8 @@ import org.junit.Test; import org.junit.experimental.categories.Category; import org.labkey.test.Locators; +import org.labkey.test.WebTestHelper; +import org.labkey.test.WebTestHelper.DatabaseType; import org.labkey.test.pages.ConfigureReportsAndScriptsPage; import org.labkey.test.pages.ConfigureReportsAndScriptsPage.EngineType; import org.labkey.test.pages.admin.RConfigurationPage; @@ -42,10 +44,13 @@ protected void doSetup() .setEngineOverrides(DUMMY_R_SERVE, DUMMY_R_SERVE) .save(); - // Set StatusCake api key - EditUpgradeMessagePage.beginAt(this) - .setStatusCakeApiKey("password") - .save(); + if (WebTestHelper.getDatabaseType() == DatabaseType.PostgreSQL) + { + // Set StatusCake api key + EditUpgradeMessagePage.beginAt(this) + .setStatusCakeApiKey("password") + .save(); + } } @Test @@ -64,9 +69,12 @@ public void testDummyRemoteREngine() @Test public void testStatusCakeApiKey() { - // Just loading this page can trigger an error if there was a problem with the encryption - assertEquals("StatusCake API key input should be present but blank", - "", EditUpgradeMessagePage.beginAt(this).getStatusCakeApiKey()); + if (WebTestHelper.getDatabaseType() == DatabaseType.PostgreSQL) + { + // Just loading this page can trigger an error if there was a problem with the encryption + assertEquals("StatusCake API key input should be present but blank", + "", EditUpgradeMessagePage.beginAt(this, null).getStatusCakeApiKey()); // Use the root container in case the '_mothership' project doesn't exist + } } @Override diff --git a/experiment/src/org/labkey/experiment/api/ExpProtocolImpl.java b/experiment/src/org/labkey/experiment/api/ExpProtocolImpl.java index 82f611ce76d..6b8a209c0bf 100644 --- a/experiment/src/org/labkey/experiment/api/ExpProtocolImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpProtocolImpl.java @@ -21,7 +21,6 @@ import org.labkey.api.assay.AssayProvider; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; -import org.labkey.api.data.Filter; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.SqlSelector; @@ -329,9 +328,12 @@ public List getChildProtocols() } @Override - public List getBatches() + public List getBatches(@Nullable Container c) { - Filter filter = new SimpleFilter(FieldKey.fromParts("BatchProtocolId"), getRowId()); + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("BatchProtocolId"), getRowId()); + // GitHub Issue 895: add param option to get just the assay batches for a specific container + if (c != null) + filter.addCondition(FieldKey.fromParts("container"), c); return ExpExperimentImpl.fromExperiments(new TableSelector(ExperimentServiceImpl.get().getTinfoExperiment(), filter, null).getArray(Experiment.class)); } diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 0758c8a156b..401357afcc1 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -4528,7 +4528,7 @@ public void deleteProtocolByRowIds(Container c, User user, @Nullable String audi for (ExpProtocol protocolToDelete : expProtocols) { - for (ExpExperiment batch : protocolToDelete.getBatches()) + for (ExpExperiment batch : protocolToDelete.getBatches(null)) { batch.delete(user); } diff --git a/query/src/org/labkey/query/QueryModule.java b/query/src/org/labkey/query/QueryModule.java index 255be49cbf0..0630786a1a2 100644 --- a/query/src/org/labkey/query/QueryModule.java +++ b/query/src/org/labkey/query/QueryModule.java @@ -31,6 +31,7 @@ import org.labkey.api.data.TableInfo; import org.labkey.api.data.views.DataViewService; import org.labkey.api.exp.property.PropertyService; +import org.labkey.api.mcp.McpService; import org.labkey.api.message.digest.DailyMessageDigest; import org.labkey.api.message.digest.ReportAndDatasetChangeDigestProvider; import org.labkey.api.migration.DatabaseMigrationService; @@ -40,7 +41,6 @@ import org.labkey.api.module.DefaultModule; import org.labkey.api.module.Module; import org.labkey.api.module.ModuleContext; -import org.labkey.api.mcp.McpService; import org.labkey.api.pipeline.PipelineService; import org.labkey.api.query.DefaultSchema; import org.labkey.api.query.JavaExportScriptFactory; @@ -77,6 +77,7 @@ import org.labkey.api.security.roles.PlatformDeveloperRole; import org.labkey.api.security.roles.Role; import org.labkey.api.security.roles.RoleManager; +import org.labkey.api.settings.OptionalFeatureFlag; import org.labkey.api.settings.OptionalFeatureService; import org.labkey.api.stats.AnalyticsProviderRegistry; import org.labkey.api.stats.SummaryStatisticRegistry; @@ -145,6 +146,7 @@ import java.util.function.Supplier; import static org.labkey.api.query.QueryService.USE_ROW_BY_ROW_UPDATE; +import static org.labkey.api.reports.ReportService.R_REPORT_CUSTOM_SHARING; public class QueryModule extends DefaultModule { @@ -346,6 +348,13 @@ public void doStartup(ModuleContext moduleContext) Role trustedAnalystRole = RoleManager.getRole("org.labkey.api.security.roles.TrustedAnalystRole"); if (null != trustedAnalystRole) trustedAnalystRole.addPermission(EditQueriesPermission.class); + + OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(R_REPORT_CUSTOM_SHARING, + "Restore custom R report sharing", + "Allows R reports to be shared on a per user basis. This option will be removed in LabKey Server 26.7.", + false, + false, + OptionalFeatureService.FeatureType.Deprecated)); } @Override