diff --git a/src/org/labkey/test/TestProperties.java b/src/org/labkey/test/TestProperties.java index 34385b041f..83d56286fc 100644 --- a/src/org/labkey/test/TestProperties.java +++ b/src/org/labkey/test/TestProperties.java @@ -18,6 +18,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.SystemUtils; import org.labkey.serverapi.reader.Readers; +import org.labkey.test.util.CspLogUtil; import org.labkey.test.util.TestLogger; import org.openqa.selenium.Dimension; @@ -30,6 +31,7 @@ import java.time.LocalDateTime; import java.time.ZoneId; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -78,6 +80,7 @@ public abstract class TestProperties public static void load() { /* Force static block to run */ + CspLogUtil.init(); } public static boolean isTestCleanupSkipped() @@ -129,7 +132,7 @@ public static boolean isQueryCheckSkipped() public static boolean isCspCheckSkipped() { - return !getBooleanProperty("webtest.cspCheck", false); + return !getBooleanProperty("webtest.cspCheck", true); } public static boolean isNewWebDriverForEachTest() @@ -264,6 +267,10 @@ public static boolean isAssayProductFeatureAvailable() return isProductFeatureAvailable("assay"); } + /** + * Product features are assumed to be available unless the test environment (usually TeamCity) explicitly specifies + * that it is not. + */ public static boolean isProductFeatureAvailable(String feature) { return "true".equals(System.getProperty("webtest.productFeature." + feature.toLowerCase(), "true")); @@ -276,7 +283,7 @@ public static boolean ignoreDatabaseNotSupportedException() /** * Parses system property 'webtest.server.startup.timeout' to determine maximum allowed server startup time. - * If property is not defined or is not an integer, it defaults to 60 seconds. + * If property is not defined or is not an integer, it defaults to 120 seconds. * @return Maximum number of seconds to wait for server startup */ public static int getServerStartupTimeout() @@ -289,34 +296,48 @@ public static String getAdditionalPipelineTools() return System.getProperty("additional.pipeline.tools"); } + private static Map _optionalFeatures = null; public static Map getOptionalFeatures() { - Map features = new HashMap<>(); - - Properties props = System.getProperties(); - for (Map.Entry entry : props.entrySet()) + if (_optionalFeatures == null) { - String key = String.valueOf(entry.getKey()); - Boolean value = (entry.getValue() instanceof Boolean) - ? (Boolean)entry.getValue() - : Boolean.valueOf(String.valueOf(entry.getValue())); + Map features = new HashMap<>(); - String prefix = "webtest.experimental."; // Can be used with any optional feature flags; "experimental" is used for backward-compatibility purposes. - if (key.startsWith(prefix)) + Properties props = System.getProperties(); + for (Map.Entry entry : props.entrySet()) { - String feature = key.substring(prefix.length()); - features.put(feature, value); + String key = String.valueOf(entry.getKey()); + + String expPrefix = "webtest.experimental."; // "experimental" is accepted for backward-compatibility purposes. + String optPrefix = "webtest.optional."; // Preferred prefix + for (String prefix : List.of(expPrefix, optPrefix)) + { + if (key.startsWith(prefix)) + { + Boolean value = (entry.getValue() instanceof Boolean) + ? (Boolean) entry.getValue() + : Boolean.valueOf(String.valueOf(entry.getValue())); + String feature = key.substring(prefix.length()); + features.put(feature, value); + } + } } - } - return features; + _optionalFeatures = Collections.unmodifiableMap(features); + } + return _optionalFeatures; } + private static List debugLoggingPackages = null; public static List getDebugLoggingPackages() { - String prop = System.getProperty("webtest.debug.server.packages", ""); - String[] packages = prop.split("\\s*,\\s*"); - return Arrays.stream(packages).map(String::trim).filter(s -> !s.isEmpty()).collect(Collectors.toList()); + if (debugLoggingPackages == null) + { + String prop = System.getProperty("webtest.debug.server.packages", ""); + String[] packages = prop.split(","); + debugLoggingPackages = Arrays.stream(packages).map(String::trim).filter(s -> !s.isEmpty()).collect(Collectors.toList()); + } + return debugLoggingPackages; } private static File dumpDir = null; diff --git a/src/org/labkey/test/components/html/Table.java b/src/org/labkey/test/components/html/Table.java index 0f45a21ce0..17357346e2 100644 --- a/src/org/labkey/test/components/html/Table.java +++ b/src/org/labkey/test/components/html/Table.java @@ -88,6 +88,11 @@ public int getRowCount() return elementCache().getRows().size(); } + public List getRows() + { + return elementCache().getRows(); + } + /** * For a well formed html table get the text from the th elements in the thead. * diff --git a/src/org/labkey/test/pages/admin/ExternalSourcesPage.java b/src/org/labkey/test/pages/admin/ExternalSourcesPage.java index 4bcbf201cb..25d1da4db3 100644 --- a/src/org/labkey/test/pages/admin/ExternalSourcesPage.java +++ b/src/org/labkey/test/pages/admin/ExternalSourcesPage.java @@ -1,6 +1,8 @@ package org.labkey.test.pages.admin; +import org.junit.Assert; import org.labkey.test.Locator; +import org.labkey.test.Locators; import org.labkey.test.WebDriverWrapper; import org.labkey.test.WebTestHelper; import org.labkey.test.components.html.Input; @@ -9,18 +11,17 @@ import org.labkey.test.pages.LabKeyPage; import org.labkey.test.util.LogMethod; import org.labkey.test.util.LoggedParam; +import org.labkey.test.util.core.admin.CspConfigHelper.AllowedHost; +import org.openqa.selenium.NotFoundException; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.stream.Stream; /** - * Wraps `AdminController.ExternalSourceAction` + * Wraps `AdminController.ExternalSourcesAction` */ public class ExternalSourcesPage extends LabKeyPage { @@ -38,7 +39,7 @@ public static ExternalSourcesPage beginAt(WebDriverWrapper webDriverWrapper) @LogMethod public ExternalSourcesPage ensureHost(Directive directive, String host) { - if (!getExistingHosts().getOrDefault(directive, Collections.emptyList()).contains(host)) + if (!getExistingHosts().contains(new AllowedHost(directive, host))) { return addHost(directive, host); } @@ -57,34 +58,73 @@ public ExternalSourcesPage addHost(@LoggedParam Directive directive, @LoggedPara clickAndWait(elementCache().addButton); clearCache(); + assertNoLabKeyErrors(); return this; } - public Map> getExistingHosts() + @LogMethod (quiet = true) + public List addHostExpectingError(@LoggedParam Directive directive, @LoggedParam String host) { - Map> existingHosts = new HashMap<>(); + elementCache().directiveSelect.selectOption(directive); + elementCache().hostInput.set(host); - for (Map.Entry> entry : getExistingHostInputs().entrySet()) - { - existingHosts.put(entry.getKey(), entry.getValue().stream().map(Input::getValue).toList()); - } + clickAndWait(elementCache().addButton); + clearCache(); - return existingHosts; + List errorEls = Locators.labkeyError.findElements(elementCache()); + Assert.assertFalse("No errors found", errorEls.isEmpty()); + return getTexts(errorEls); } - public Map> getExistingHostInputs() + public ExternalSourcesPage editHost(Directive directive, String host, String newHost) + { + getExistingSourceRow(directive, host).getHostInput().set(newHost); + return this; + } + + public ExternalSourcesPage deleteHost(Directive directive, String host) + { + getExistingSourceRow(directive, host).clickDelete(); + return this; + } + + private ExistingSourceRow getExistingSourceRow(Directive directive, String host) + { + return elementCache().getExistingSourceRows().stream() + .filter(row -> row.getDirective().equals(directive) && row.getHost().equalsIgnoreCase(host)) + .findFirst().orElseThrow(() -> new NotFoundException("Host " + host + " does not exist for directive " + directive)); + } + + @LogMethod (quiet = true) + public ExternalSourcesPage saveChanges() + { + clickAndWait(elementCache().saveButton); + clearCache(); + + assertNoLabKeyErrors(); + return this; + } + + @LogMethod (quiet = true) + public List saveChangesExpectingError() { - List directiveColumn = elementCache().existingValuesTable.getColumnAsElement(1); - List hostsColumn = elementCache().existingValuesTable.getColumnAsElement(2); + clickAndWait(elementCache().saveButton); + clearCache(); + + List errorEls = Locators.labkeyError.findElements(elementCache()); + Assert.assertFalse("No errors found", errorEls.isEmpty()); + return getTexts(errorEls); + } - Map> existingHosts = new HashMap<>(); + public List getExistingHosts() + { + List existingHosts = new ArrayList<>(); - for (int i = 0; i < hostsColumn.size(); i++) + for (ExistingSourceRow row : elementCache().getExistingSourceRows()) { - WebElement directiveInput = Locator.tag("input").findElement(directiveColumn.get(i)); - Directive directive = Directive.valueOf(directiveInput.getDomAttribute("data-directive")); - Input hostInput = Input.Input(Locator.tag("input"), getDriver()).find(hostsColumn.get(i)); - existingHosts.computeIfAbsent(directive, d -> new ArrayList<>()).add(hostInput); + Directive directive = row.getDirective(); + String host = row.getHost(); + existingHosts.add(new AllowedHost(directive, host)); } return existingHosts; @@ -105,6 +145,59 @@ protected class ElementCache extends LabKeyPage.ElementCache final WebElement existingValuesForm = Locator.name("existingValues").findWhenNeeded(this); final Table existingValuesTable = new Table(getDriver(), Locator.byClass("labkey-data-region-legacy").findWhenNeeded(existingValuesForm), 0); + private List existingSourceRows; + + protected List getExistingSourceRows() + { + if (existingSourceRows == null) + { + existingSourceRows = existingValuesTable.getRows().stream().map(ExistingSourceRow::new).toList(); + } + return existingSourceRows; + } + + final WebElement saveButton = Locator.lkButton("Save").findWhenNeeded(existingValuesForm); + } + + protected class ExistingSourceRow + { + private final WebElement directiveInput; + private final Input hostInput; + private final WebElement deleteButton; + + private Directive directive; + + ExistingSourceRow(WebElement row) + { + directiveInput = Locator.tag("input").withAttributeContaining("name", "directive").findWhenNeeded(row); + hostInput = Input.Input(Locator.tag("input").withAttributeContaining("name", "host"), getDriver()).findWhenNeeded(row); + deleteButton = Locator.lkButton("Delete").findWhenNeeded(row); + } + + Directive getDirective() + { + if (directive == null) + { + directive = Directive.valueOf(directiveInput.getDomAttribute("data-directive")); + } + return directive; + } + + public Input getHostInput() + { + return hostInput; + } + + public String getHost() + { + return hostInput.get(); + } + + public void clickDelete() + { + clickAndWait(deleteButton); + clearCache(); + } } public enum Directive implements OptionSelect.SelectOption diff --git a/src/org/labkey/test/pages/core/admin/ShowAdminPage.java b/src/org/labkey/test/pages/core/admin/ShowAdminPage.java index 7417290227..16a591b25d 100644 --- a/src/org/labkey/test/pages/core/admin/ShowAdminPage.java +++ b/src/org/labkey/test/pages/core/admin/ShowAdminPage.java @@ -21,6 +21,7 @@ import org.labkey.test.components.DomainDesignerPage; import org.labkey.test.pages.ConfigureReportsAndScriptsPage; import org.labkey.test.pages.LabKeyPage; +import org.labkey.test.pages.admin.ExternalSourcesPage; import org.labkey.test.pages.compliance.ComplianceSettingsAccountsPage; import org.labkey.test.pages.core.login.LoginConfigurePage; import org.labkey.test.util.OptionalFeatureHelper; @@ -99,6 +100,13 @@ public ShowAuditLogPage clickAuditLog() return new ShowAuditLogPage(getDriver()); } + public ExternalSourcesPage clickAllowedExternalResourceHosts() + { + goToSettingsSection(); + clickAndWait(elementCache().externalResourceHostsLink); + return new ExternalSourcesPage(getDriver()); + } + public AllowedFileExtensionAdminPage clickAllowedFileExtensions() { goToSettingsSection(); @@ -278,7 +286,7 @@ protected ElementCache newElementCache() return new ElementCache(); } - protected class ElementCache extends LabKeyPage.ElementCache + protected class ElementCache extends LabKeyPage.ElementCache { protected WebElement sectionServerInfo = Locator.linkWithText("Server Information").findWhenNeeded(this); protected WebElement sectionSettingsLinks = Locator.linkWithText("Settings").findWhenNeeded(this); @@ -286,8 +294,9 @@ protected class ElementCache extends LabKeyPage.ElementCache protected WebElement sectionActiveUsers = Locator.linkWithText("Active Users").findWhenNeeded(this); protected WebElement analyticsSettingsLink = Locator.linkWithText("analytics settings").findWhenNeeded(this); - protected WebElement externalRedirectHostLink = Locator.linkWithText("allowed external redirect hosts").findElement(this); - protected WebElement allowedFileExtensionLink = Locator.linkWithText("allowed file extensions").findElement(this); + protected WebElement externalRedirectHostLink = Locator.linkWithText("allowed external redirect hosts").findWhenNeeded(this); + protected WebElement externalResourceHostsLink = Locator.linkWithText("allowed external resource hosts").findWhenNeeded(this); + protected WebElement allowedFileExtensionLink = Locator.linkWithText("allowed file extensions").findWhenNeeded(this); protected WebElement auditLogLink = Locator.linkWithText("audit log").findWhenNeeded(this); protected WebElement auditLogMaintenanceLink = Locator.linkWithText("Audit Log Maintenance").findWhenNeeded(this); protected WebElement authenticationLink = Locator.linkWithText("authentication").findWhenNeeded(this); diff --git a/src/org/labkey/test/tests/AbstractKnitrReportTest.java b/src/org/labkey/test/tests/AbstractKnitrReportTest.java index 1400ef6a54..d40ece66e7 100644 --- a/src/org/labkey/test/tests/AbstractKnitrReportTest.java +++ b/src/org/labkey/test/tests/AbstractKnitrReportTest.java @@ -17,21 +17,29 @@ import org.junit.Assume; import org.junit.BeforeClass; +import org.labkey.remoteapi.CommandException; import org.labkey.test.BaseWebDriverTest; import org.labkey.test.Locator; import org.labkey.test.TestFileUtils; import org.labkey.test.TestProperties; +import org.labkey.test.pages.admin.ExternalSourcesPage; +import org.labkey.test.pages.core.admin.logger.ManagerPage; import org.labkey.test.pages.reports.ManageViewsPage; import org.labkey.test.util.CodeMirrorHelper; +import org.labkey.test.util.Log4jUtils; import org.labkey.test.util.LogMethod; +import org.labkey.test.util.LoggedParam; import org.labkey.test.util.PortalHelper; import org.labkey.test.util.RReportHelper; +import org.labkey.test.util.core.admin.CspConfigHelper; import org.openqa.selenium.WebElement; import java.io.File; +import java.io.IOException; import java.nio.file.Path; import java.util.Arrays; import java.util.List; +import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -60,7 +68,7 @@ private static String readReport(final Path reportFile) } @BeforeClass - public static void initProject() + public static void initProject() throws Exception { AbstractKnitrReportTest init = getCurrentTest(); init.setupProject(); @@ -69,6 +77,18 @@ public static void initProject() @LogMethod protected void setupProject() { + Log4jUtils.setLogLevel("org.labkey.query.reports.ReportsController", ManagerPage.LoggingLevel.DEBUG); + try + { + new CspConfigHelper(this).setAllowedHosts(Map.of( + ExternalSourcesPage.Directive.Style, List.of("https://cdn.datatables.net"), + ExternalSourcesPage.Directive.Font, List.of("https://mathjax.rstudio.com"))); + } + catch (IOException | CommandException e) + { + throw new RuntimeException(e); + } + _rReportHelper.ensureRConfig(isDocker()); _containerHelper.createProject(getProjectName(), "Collaboration"); @@ -88,7 +108,8 @@ protected boolean isDocker() return false; } - protected String createKnitrReport(Path reportSourcePath, RReportHelper.ReportOption knitrOption) + @LogMethod + protected String createKnitrReport(@LoggedParam Path reportSourcePath, @LoggedParam RReportHelper.ReportOption knitrOption) { String reportSource = readReport(reportSourcePath); diff --git a/src/org/labkey/test/tests/CrawlerTest.java b/src/org/labkey/test/tests/CrawlerTest.java index f356e4ab76..bc10407e81 100644 --- a/src/org/labkey/test/tests/CrawlerTest.java +++ b/src/org/labkey/test/tests/CrawlerTest.java @@ -16,6 +16,7 @@ import org.labkey.test.util.Crawler; import org.labkey.test.util.CspLogUtil; import org.labkey.test.util.PermissionsHelper.MemberType; +import org.labkey.test.util.core.admin.CspConfigHelper; import org.labkey.test.util.selenium.WebDriverUtils; import org.openqa.selenium.UnhandledAlertException; @@ -30,7 +31,9 @@ public class CrawlerTest extends BaseWebDriverTest { private static final String MODULE_NAME = "CrawlerTest"; - private static final String USER = "injectiontester@labkey.injection.test"; + private static final String USER = "injectiontester@labkey.injection.test"; // required by 'injectJsp' page + + private final CspConfigHelper _cspConfigHelper = new CspConfigHelper(this); @Override protected void doCleanup(boolean afterTest) @@ -49,6 +52,7 @@ public static void setupProject() private void doSetup() { + CspConfigHelper.debugCspWarnings(); _containerHelper.createProject(getProjectName(), null); _userHelper.createUser(USER); new ApiPermissionsHelper(this).addMemberToRole(USER, "Reader", MemberType.user, getProjectName()); @@ -60,6 +64,8 @@ private void doSetup() @Test public void testCrawlerTest() throws Exception { + _cspConfigHelper.setEnforceCsp(false); + String safeParam = "OK!"; log("Verify vulnerable page requires specific user"); @@ -100,11 +106,27 @@ public void testCrawlerTest() throws Exception } } + @Test (expected = CspLogUtil.CspWarningDetectedException.class) + public void testEnforceCsp() throws Exception + { + _cspConfigHelper.setEnforceCsp(true); + + createDefaultConnection().impersonate(USER); + + log("Verify that page is not vulnerable when CSP is enforced"); + beginAt(getInjectUrl(Crawler.injectScriptBlock), 10_000); + + log("Verify that enforced CSP is also reported"); + CspLogUtil.checkNewCspWarnings(getArtifactCollector()); + } + @Test (expected = CspLogUtil.CspWarningDetectedException.class) public void testCspWarning() { Assume.assumeFalse("Can't test for CSP report", TestProperties.isCspCheckSkipped()); + _cspConfigHelper.setEnforceCsp(false); + beginAt(WebTestHelper.buildRelativeUrl(MODULE_NAME, getProjectName(), "cspWarning")); CspLogUtil.checkNewCspWarnings(getArtifactCollector()); } @@ -134,6 +156,8 @@ public void testExternalLink() throws Exception @Test public void testCrawler() throws Exception { + _cspConfigHelper.setEnforceCsp(false); + String safeParam = "OK!"; createDefaultConnection().impersonate(USER); diff --git a/src/org/labkey/test/tests/core/admin/CspResourceHostsTest.java b/src/org/labkey/test/tests/core/admin/CspResourceHostsTest.java new file mode 100644 index 0000000000..7b00b7fac4 --- /dev/null +++ b/src/org/labkey/test/tests/core/admin/CspResourceHostsTest.java @@ -0,0 +1,302 @@ +package org.labkey.test.tests.core.admin; + +import org.apache.hc.core5.http.HttpStatus; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.labkey.remoteapi.CommandException; +import org.labkey.test.BaseWebDriverTest; +import org.labkey.test.Locator; +import org.labkey.test.categories.Daily; +import org.labkey.test.components.html.SelectWrapper; +import org.labkey.test.pages.admin.ExternalSourcesPage; +import org.labkey.test.pages.admin.ExternalSourcesPage.Directive; +import org.labkey.test.pages.core.admin.ShowAdminPage; +import org.labkey.test.util.ApiPermissionsHelper; +import org.labkey.test.util.core.admin.CspConfigHelper; +import org.labkey.test.util.core.admin.CspConfigHelper.AllowedHost; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; +import static org.labkey.test.util.PermissionsHelper.MemberType.user; + +@Category({Daily.class}) +public class CspResourceHostsTest extends BaseWebDriverTest +{ + private static final String APP_ADMIN = "csp_app_admin@cspresourcehoststest.test"; + private static final String TROUBLESHOOTER = "csp_troubleshooter@cspresourcehoststest.test"; + + private final CspConfigHelper _cspConfigHelper = new CspConfigHelper(this); + + @Override + protected void doCleanup(boolean afterTest) + { + _userHelper.deleteUsers(afterTest, APP_ADMIN, TROUBLESHOOTER); + } + + @BeforeClass + public static void setupProject() + { + CspResourceHostsTest init = getCurrentTest(); + + init.doSetup(); + } + + private void doSetup() + { + _userHelper.createUser(APP_ADMIN); + _userHelper.createUser(TROUBLESHOOTER); + ApiPermissionsHelper apiPermissionsHelper = new ApiPermissionsHelper(this); + apiPermissionsHelper.addUserAsAppAdmin(APP_ADMIN); + apiPermissionsHelper.addMemberToRole(TROUBLESHOOTER, "Troubleshooter", user, null); + } + + @Before + public void preTest() throws Exception + { + _cspConfigHelper.clearAllowedHosts(); + } + + @Test + public void testTroubleshooterPermissions() throws Exception + { + Directive directive = Directive.Connection; + String host = "https://labkey.org"; + String add = "Add"; + String save = "Save"; + String delete = "Delete"; + String done = "Done"; + + ExternalSourcesPage externalSourcesPage = ExternalSourcesPage.beginAt(this); + externalSourcesPage.addHost(directive, host); + List expectedHosts = List.of(new AllowedHost(directive, host)); + + List buttons = getTexts(Locator.lkButton().findElements(getDriver())); + checker().verifyEqualsSorted("Form buttons", List.of(add, save, delete), buttons); + checker().verifyEquals("Defined directives", expectedHosts, externalSourcesPage.getExistingHosts()); + checker().screenShotIfNewError("site_admin_csp"); + + impersonate(TROUBLESHOOTER); + externalSourcesPage = ShowAdminPage.beginAt(this).clickAllowedExternalResourceHosts(); + + buttons = getTexts(Locator.lkButton().findElements(getDriver())); + checker().verifyEqualsSorted("Form buttons", List.of(done), buttons); + checker().verifyEquals("Defined directives", expectedHosts, externalSourcesPage.getExistingHosts()); + checker().screenShotIfNewError("troubleshooter_csp"); + + clickAndWait(Locator.lkButton(done)); + assertEquals("Done button destination", "/admin-showAdmin.view", getCurrentRelativeURL()); + + try + { + _cspConfigHelper.clearAllowedHosts(); + Assert.fail("Troubleshooter should not be able to change allowed hosts"); + } + catch (CommandException e) + { + if (e.getStatusCode() != HttpStatus.SC_FORBIDDEN) + throw e; + } + } + + @Test + public void testAddDuplicateHostsErrors() + { + String host1 = "https://labkey.org"; + List expectedDirectives = new ArrayList<>(); + + impersonate(APP_ADMIN); + ExternalSourcesPage externalSourcesPage = ExternalSourcesPage.beginAt(this); + + log("Verify that multiple directives can have the same host"); + for (Directive directive : Directive.values()) + { + expectedDirectives.add(new AllowedHost(directive, host1)); + externalSourcesPage.addHost(directive, " " + host1 + " "); + } + + log("Verify that each directive can't have duplicate entries that differ only by casing"); + for (Directive directive : Directive.values()) + { + String host1UpperCase = host1.toUpperCase(); + List errors = externalSourcesPage.addHostExpectingError(directive, " " + host1UpperCase + " "); + assertThat(errors).as("error count").hasSize(1); + assertThat(errors.get(0)).contains("Duplicate values are not allowed.", directive.name(), host1UpperCase); + } + + assertThat(externalSourcesPage.getExistingHosts()).as("Defined directives") + .containsExactlyInAnyOrderElementsOf(expectedDirectives); + } + + @Test + public void testEditDuplicateHostsErrors() + { + String host1 = "https://labkey.org"; + String host2 = "https://labkey.com"; + List expectedDirectives = new ArrayList<>(); + + impersonate(APP_ADMIN); + ExternalSourcesPage externalSourcesPage = ExternalSourcesPage.beginAt(this); + + log("Setup directives with multiple hosts"); + for (Directive directive : Directive.values()) + { + expectedDirectives.add(new AllowedHost(directive, host1)); + expectedDirectives.add(new AllowedHost(directive, host2)); + externalSourcesPage.addHost(directive, " " + host1 + " "); + externalSourcesPage.addHost(directive, " " + host2 + " "); + } + + log("Verify that directive can't be edited to have duplicate entries"); + for (Directive directive : Directive.values()) + { + String host1UpperCase = host1.toUpperCase(); + externalSourcesPage.editHost(directive, host2, " " + host1UpperCase + " "); + List errors = externalSourcesPage.saveChangesExpectingError(); + assertThat(errors).as("error count").hasSize(1); + assertThat(errors.get(0)).contains("Duplicate values are not allowed.", directive.name(), host1); + } + + assertThat(externalSourcesPage.getExistingHosts()).as("Defined directives") + .containsExactlyInAnyOrderElementsOf(expectedDirectives); + } + + @Test + public void testAddInvalidHostsErrors() + { + String host1 = "https://labkey.org"; + + impersonate(APP_ADMIN); + ExternalSourcesPage externalSourcesPage = ExternalSourcesPage.beginAt(this); + + log("Verify that each directive can't add invalid hosts"); + for (Directive directive : Directive.values()) + { + String badHost1 = host1 + ";"; + List errors = externalSourcesPage.addHostExpectingError(directive, badHost1); + assertThat(errors).as("error").containsExactly("Semicolons are not allowed in host names"); + } + + log("Verify that each directive can't add blank hosts"); + for (Directive directive : Directive.values()) + { + List errors = externalSourcesPage.addHostExpectingError(directive, ""); + assertThat(errors).as("error").containsExactly("Host must not be blank"); + } + + assertEquals("Defined directives", Collections.emptyList(), externalSourcesPage.getExistingHosts()); + } + + @Test + public void testEditInvalidHostsErrors() + { + String host = "https://labkey.org"; + List expectedDirectives = new ArrayList<>(); + + impersonate(APP_ADMIN); + ExternalSourcesPage externalSourcesPage = ExternalSourcesPage.beginAt(this); + + log("Setup directives with multiple hosts"); + for (Directive directive : Directive.values()) + { + expectedDirectives.add(new AllowedHost(directive, host)); + externalSourcesPage.addHost(directive, " " + host + " "); + } + + log("Verify that directive can't be edited to have semicolon"); + for (Directive directive : Directive.values()) + { + String host1UpperCase = host.toUpperCase(); + externalSourcesPage.editHost(directive, host, host1UpperCase + " ;"); + List errors = externalSourcesPage.saveChangesExpectingError(); + assertThat(errors).as("error").containsExactly("Semicolons are not allowed in host names"); + } + + log("Verify that directive can't be edited to be blank"); + for (Directive directive : Directive.values()) + { + externalSourcesPage.editHost(directive, host, ""); + List errors = externalSourcesPage.saveChangesExpectingError(); + assertThat(errors).as("error").containsExactly("Host must not be blank"); + } + + assertThat(externalSourcesPage.getExistingHosts()).as("Defined directives") + .containsExactlyInAnyOrderElementsOf(expectedDirectives); + } + + @Test + public void testDeleteAvailableHosts() + { + String host1 = "https://labkey.org"; + String host2 = "https://labkey.com"; + List expectedDirectives = new ArrayList<>(); + + impersonate(APP_ADMIN); + ExternalSourcesPage externalSourcesPage = ExternalSourcesPage.beginAt(this); + + log("Setup directives with multiple hosts in a random order"); + for (Directive directive : Directive.values()) + { + expectedDirectives.add(new AllowedHost(directive, host1)); + expectedDirectives.add(new AllowedHost(directive, host2)); + } + Collections.shuffle(expectedDirectives); + for (AllowedHost allowedHost : expectedDirectives) + { + externalSourcesPage.addHost(allowedHost.getDirective(), allowedHost.getHost()); + } + + assertThat(externalSourcesPage.getExistingHosts()).as("Defined directives") + .containsExactlyInAnyOrderElementsOf(expectedDirectives); + + log("Delete directives in a random order"); + Collections.shuffle(expectedDirectives); + Iterator iterator = expectedDirectives.iterator(); + while (iterator.hasNext()) + { + AllowedHost allowedHost = iterator.next(); + iterator.remove(); + externalSourcesPage.deleteHost(allowedHost.getDirective(), allowedHost.getHost()); + + assertThat(externalSourcesPage.getExistingHosts()).as("Defined directives") + .containsExactlyInAnyOrderElementsOf(expectedDirectives); + } + + assertThat(externalSourcesPage.getExistingHosts()) + .as("Defined directives after deleting all") + .isEmpty(); + } + + @Test + public void testAvailableDirectives() + { + List expectedDirectives = Arrays.stream(Directive.values()).map(Directive::name).toList(); + + ExternalSourcesPage.beginAt(this); + List availableDirectives = SelectWrapper.Select(Locator.id("newDirective")).find(getDriver()) + .getOptions().stream().map(el -> el.getDomProperty("value")).toList(); + + assertThat(availableDirectives).as("Available directives") + .containsExactlyInAnyOrderElementsOf(expectedDirectives); + } + + @Override + protected String getProjectName() + { + return null; + } + + @Override + public List getAssociatedModules() + { + return Arrays.asList(); + } +} diff --git a/src/org/labkey/test/util/CspLogUtil.java b/src/org/labkey/test/util/CspLogUtil.java index 4f7d79ceba..62cf9d042c 100644 --- a/src/org/labkey/test/util/CspLogUtil.java +++ b/src/org/labkey/test/util/CspLogUtil.java @@ -3,6 +3,7 @@ import org.apache.commons.collections4.MultiValuedMap; import org.apache.commons.collections4.multimap.HashSetValuedHashMap; import org.apache.commons.io.IOUtils; +import org.json.JSONObject; import org.junit.Assert; import org.labkey.serverapi.writer.PrintWriters; import org.labkey.test.TestFileUtils; @@ -16,17 +17,20 @@ import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Set; public class CspLogUtil { private static final List ignoredViolations = List.of( "/_rstudio/", - "/_rstudioReport/", - "/reports-createScriptReport.view?", - "/reports-runReport.view?" + "/_rstudioReport/" ); + // Issue 53226: reports-streamFile is blocked by object-src CSP directive + private static final Set ignoredDirectives = Set.of("object-src"); + private static final String logName = "csp-report.log"; private static final File logFile = new File(TestFileUtils.getServerLogDir(), logName); @@ -36,6 +40,25 @@ public class CspLogUtil private CspLogUtil() { } + public static void init() + { + if (lastModified == 0) + { + try + { + BasicFileAttributes logFileAttributes; + logFileAttributes = Files.readAttributes(logFile.toPath(), BasicFileAttributes.class); + + lastSize = logFileAttributes.size(); + lastModified = logFileAttributes.lastModifiedTime().toMillis(); + } + catch (IOException e) + { + lastModified = System.currentTimeMillis(); + } + } + } + public static void checkNewCspWarnings(ArtifactCollector artifactCollector) { if (TestProperties.isServerRemote() || missingLog) @@ -87,22 +110,19 @@ public static void checkNewCspWarnings(ArtifactCollector artifactCollector) boolean foundVioloation = false; MultiValuedMap violoations = new HashSetValuedHashMap<>(); + List cspReports = new ArrayList<>(); + StringBuilder sb = new StringBuilder(); for (String line : warningLines) { - String[] split = line.split("ContentSecurityPolicy warning on page: "); - if (split.length > 1) + if (!sb.isEmpty() || line.equals("{")) + { + sb.append(line); + } + if (line.equals("}")) { + cspReports.add(new CspReport(sb.toString())); + sb = new StringBuilder(); foundVioloation = true; - String url = split[1]; - if (ignoredViolations.stream().anyMatch(url::contains)) - { - TestLogger.warn("Ignoring CSP warning on page: " + url); - } - else - { - Crawler.ControllerActionId actionId = new Crawler.ControllerActionId(url); - violoations.put(actionId, url); - } } } @@ -111,6 +131,21 @@ public static void checkNewCspWarnings(ArtifactCollector artifactCollector) throw new AssertionError("Detected CSP violations but unable to parse log file: " + recentWarningsFile.getAbsolutePath()); } + for (CspReport cspReport : cspReports) + { + String url = cspReport.getDocumentUri(); + String violatedDirective = cspReport.getViolatedDirective(); + if (ignoredViolations.stream().anyMatch(url::contains) || ignoredDirectives.contains(violatedDirective)) + { + TestLogger.warn("Ignoring %s CSP warning on page: %s".formatted(violatedDirective, url)); + } + else + { + Crawler.ControllerActionId actionId = new Crawler.ControllerActionId(url); + violoations.put(actionId, cspReport.toString()); + } + } + if (!violoations.isEmpty()) { StringBuilder errorMessage = new StringBuilder() @@ -167,3 +202,32 @@ public CspWarningDetectedException(Object detailMessage) } } } + +class CspReport +{ + private final String _violatedDirective; + private final String _documentUri; + + CspReport(String reportStr) + { + JSONObject report = new JSONObject(reportStr).getJSONObject("csp-report"); + _violatedDirective = report.getString("violated-directive"); + _documentUri = report.getString("document-uri"); + } + + public String getViolatedDirective() + { + return _violatedDirective; + } + + public String getDocumentUri() + { + return _documentUri; + } + + @Override + public String toString() + { + return getViolatedDirective() + ": " + getDocumentUri(); + } +} diff --git a/src/org/labkey/test/util/OptionalFeatureHelper.java b/src/org/labkey/test/util/OptionalFeatureHelper.java index 09d1b54b90..6d89eabaed 100644 --- a/src/org/labkey/test/util/OptionalFeatureHelper.java +++ b/src/org/labkey/test/util/OptionalFeatureHelper.java @@ -28,6 +28,8 @@ public class OptionalFeatureHelper { + private static final Map initialValues = new HashMap<>(); + public static Boolean enableOptionalFeature(Connection cn, String feature) { return setOptionalFeature(cn, feature, true); @@ -56,7 +58,9 @@ public static Boolean setOptionalFeature(Connection cn, String feature, boolean CommandResponse r = command.execute(cn, null); Map response = r.getParsedData(); - return (Boolean)response.get("previouslyEnabled"); + Boolean previouslyEnabled = (Boolean) response.get("previouslyEnabled"); + initialValues.putIfAbsent(feature, previouslyEnabled); + return previouslyEnabled; } catch (IOException e) { @@ -68,6 +72,16 @@ public static Boolean setOptionalFeature(Connection cn, String feature, boolean } } + @LogMethod + public static void resetOptionalFeature(Connection cn, @LoggedParam String feature) + { + if (initialValues.containsKey(feature)) + { + setOptionalFeature(cn, feature, initialValues.get(feature)); + initialValues.remove(feature); + } + } + public static boolean isOptionalFeatureEnabled(Connection cn, String feature) { SimpleGetCommand command = new SimpleGetCommand("admin", "optionalFeature"); diff --git a/src/org/labkey/test/util/core/admin/CspConfigHelper.java b/src/org/labkey/test/util/core/admin/CspConfigHelper.java new file mode 100644 index 0000000000..a8ec0125ed --- /dev/null +++ b/src/org/labkey/test/util/core/admin/CspConfigHelper.java @@ -0,0 +1,115 @@ +package org.labkey.test.util.core.admin; + +import org.labkey.remoteapi.CommandException; +import org.labkey.remoteapi.Connection; +import org.labkey.remoteapi.SimplePostCommand; +import org.labkey.test.WebDriverWrapper; +import org.labkey.test.pages.admin.ExternalSourcesPage.Directive; +import org.labkey.test.pages.core.admin.logger.ManagerPage; +import org.labkey.test.util.Log4jUtils; +import org.labkey.test.util.OptionalFeatureHelper; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.function.Supplier; + +public class CspConfigHelper +{ + public static final String DISABLE_ENFORCE_CSP_OPTIONAL_FEATURE = "disableEnforceCsp"; + private final Supplier _connectionSupplier; + + public CspConfigHelper(Supplier connectionSupplier) + { + _connectionSupplier = connectionSupplier; + } + + public CspConfigHelper(Connection connection) + { + this(() -> connection); + } + + public CspConfigHelper(WebDriverWrapper driverWrapper) + { + this(driverWrapper::createDefaultConnection); + } + + private Connection getConnection() + { + return _connectionSupplier.get(); + } + + public void setAllowedHosts(Map> allowedHosts) throws IOException, CommandException + { + StringBuilder existingValues = new StringBuilder(); + for (Map.Entry> entry : allowedHosts.entrySet()) + { + Directive directive = entry.getKey(); + for (String value : entry.getValue()) + { + existingValues.append(directive.name()).append("|").append(value).append("\n"); + } + } + + SimplePostCommand postCommand = new SimplePostCommand("admin", "externalSources"); + postCommand.setParameters(Map.of( + "saveAll", true, + "existingValues", existingValues)); + postCommand.execute(getConnection(), null); + } + + public void clearAllowedHosts() throws IOException, CommandException + { + setAllowedHosts(Collections.emptyMap()); + } + + public void setEnforceCsp(boolean enforce) + { + Objects.requireNonNull(OptionalFeatureHelper.setOptionalFeature(getConnection(), DISABLE_ENFORCE_CSP_OPTIONAL_FEATURE, !enforce), () -> "Unable to configure enforce CSP."); + } + + public static void debugCspWarnings() + { + Log4jUtils.setLogLevel("org.labkey.core.admin.AdminController.ContentSecurityPolicyReportAction", ManagerPage.LoggingLevel.DEBUG); + } + + public static class AllowedHost + { + private final Directive _directive; + private final String _host; + + public AllowedHost(Directive directive, String host) + { + _directive = Objects.requireNonNull(directive); + _host = Objects.requireNonNull(host); + } + + public Directive getDirective() + { + return _directive; + } + + public String getHost() + { + return _host; + } + + @Override + public final boolean equals(Object o) + { + if (!(o instanceof AllowedHost that)) return false; + + return _directive == that._directive && _host.equals(that._host); + } + + @Override + public int hashCode() + { + int result = _directive.hashCode(); + result = 31 * result + _host.hashCode(); + return result; + } + } +} diff --git a/src/org/labkey/test/util/data/ColumnNameMapper.java b/src/org/labkey/test/util/data/ColumnNameMapper.java index e8160139e3..5cd85a1b5f 100644 --- a/src/org/labkey/test/util/data/ColumnNameMapper.java +++ b/src/org/labkey/test/util/data/ColumnNameMapper.java @@ -29,9 +29,8 @@ public ColumnNameMapper(Map mappings) public static ColumnNameMapper labelToName(Collection fields) { - return new ColumnNameMapper(label -> fields.stream() - .collect(Collectors.toMap(FieldDefinition::getEffectiveLabel, FieldDefinition::getName)) - .getOrDefault(label, label)); + Map labelToNameMap = fields.stream().collect(Collectors.toMap(FieldDefinition::getEffectiveLabel, FieldDefinition::getName)); + return new ColumnNameMapper(label -> labelToNameMap.getOrDefault(label, label)); } public static ColumnNameMapper nameToLabel(Collection fields) diff --git a/test.properties.template b/test.properties.template index 90aed9ef8b..6197e500ce 100644 --- a/test.properties.template +++ b/test.properties.template @@ -157,7 +157,7 @@ queryCheck=false ## Verify all custom views after each test class viewCheck=false ## Check for CSP warning after tests -webtest.cspCheck=false +#webtest.cspCheck=true #============================================================================== @@ -173,5 +173,5 @@ use.cloud.pipeline=false cloud.pipeline.bucket= ## List of loggers to enable DEBUG logging for. webtest.debug.server.packages= -## Enable/Disable experimental features. -#webtest.experimental.=[true|false] +## Enable/Disable optional features. +#webtest.optional.=[true|false]