[bot] Merge 26.6 to develop (Conflicts)

assay/package.json

@@ -55,7 +55,7 @@
     }
   },
   "dependencies": {
-    "@labkey/components": "7.40.0"
+    "@labkey/components": "7.40.1"
   },
   "devDependencies": {
     "@labkey/build": "9.1.4",

core/package.json

@@ -57,7 +57,7 @@
     }
   },
   "dependencies": {
-    "@labkey/components": "7.40.0",
+    "@labkey/components": "7.40.1",
     "@labkey/themes": "1.9.3"
   },
   "devDependencies": {

core/src/client/AssayDesigner/AssayDesigner.tsx

@@ -136,8 +136,9 @@ class AssayDesigner extends React.Component<any, State> {
 
     navigate(defaultUrl: string) {
         this._dirty = false;
-
-        window.location.href = this.state.returnUrl || defaultUrl;
+        const redirectUrl = this.state.returnUrl || defaultUrl;
+        // TODO refactor this and other usages in platform/core/src/client to a helper safeRedirect() function from @labkey/components
+        window.location.href = ActionURL.buildURL('core', 'safeRedirect', undefined, { returnUrl: redirectUrl });
     }
 
     onCancel = (): void => {

core/src/client/DataClassDesigner/DataClassDesigner.tsx

@@ -66,9 +66,8 @@ class DataClassDesignerWrapper extends React.Component<any, State> {
 
     navigate(defaultUrl: string) {
         this._dirty = false;
-
-        const returnUrl = ActionURL.getReturnUrl();
-        window.location.href = returnUrl || defaultUrl;
+        const redirectUrl = ActionURL.getReturnUrl() || defaultUrl;
+        window.location.href = ActionURL.buildURL('core', 'safeRedirect', undefined, { returnUrl: redirectUrl });
     }
 
     onCancel = () => {

core/src/client/DatasetDesigner/DatasetDesigner.tsx

@@ -66,9 +66,8 @@ class DatasetDesigner extends PureComponent<any, State> {
 
     navigate(defaultUrl: string): void {
         this._dirty = false;
-
-        const returnUrl = ActionURL.getReturnUrl();
-        window.location.href = returnUrl || defaultUrl;
+        const redirectUrl = ActionURL.getReturnUrl() || defaultUrl;
+        window.location.href = ActionURL.buildURL('core', 'safeRedirect', undefined, { returnUrl: redirectUrl });
     }
 
     navigateOnComplete(model: DatasetModel): void {

core/src/client/DomainDesigner/DomainDesigner.tsx

@@ -161,9 +161,8 @@ class DomainDesigner extends React.PureComponent<any, Partial<IAppState>> {
 
     navigate = (): void => {
         this._dirty = false;
-
-        const returnUrl = ActionURL.getReturnUrl();
-        window.location.href = returnUrl || ActionURL.buildURL('project', 'begin', getServerContext().container.path);
+        const redirectUrl = ActionURL.getReturnUrl() || ActionURL.buildURL('project', 'begin', getServerContext().container.path);
+        window.location.href = ActionURL.buildURL('core', 'safeRedirect', undefined, { returnUrl: redirectUrl });
     };
 
     renderWarningConfirm() {

core/src/client/IssuesListDesigner/IssuesListDesigner.tsx

@@ -82,9 +82,8 @@ class IssuesListDesigner extends React.Component<{}, State> {
 
     navigate = (defaultUrl: string) => {
         this._dirty = false;
-
-        const returnUrl = ActionURL.getReturnUrl();
-        window.location.href = returnUrl || defaultUrl;
+        const redirectUrl = ActionURL.getReturnUrl() || defaultUrl;
+        window.location.href = ActionURL.buildURL('core', 'safeRedirect', undefined, { returnUrl: redirectUrl });
     };
 
     onComplete = (model: IssuesListDefModel) => {

core/src/client/ListDesigner/ListDesigner.tsx

@@ -103,8 +103,8 @@ export class ListDesigner extends React.Component<Props, State> {
 
     navigate = async (returnUrlProvider: () => Promise<string>, model?: ListModel): Promise<void> => {
         this._dirty = false;
-
-        window.location.href = this.getReturnUrl(model) ?? (await returnUrlProvider());
+        const redirectUrl = this.getReturnUrl(model) ?? (await returnUrlProvider());
+        window.location.href = ActionURL.buildURL('core', 'safeRedirect', undefined, { returnUrl: redirectUrl });
     };
 
     getReturnUrl = (model?: ListModel): string => {

core/src/client/SampleTypeDesigner/SampleTypeDesigner.tsx

@@ -144,9 +144,8 @@ class SampleTypeDesignerWrapper extends React.PureComponent<any, State> {
 
     navigate(defaultUrl: string) {
         this._dirty = false;
-
-        const returnUrl = ActionURL.getReturnUrl();
-        window.location.href = returnUrl || defaultUrl;
+        const redirectUrl = ActionURL.getReturnUrl() || defaultUrl;
+        window.location.href = ActionURL.buildURL('core', 'safeRedirect', undefined, { returnUrl: redirectUrl });
     }
 
     render() {

core/src/org/labkey/core/CoreController.java

@@ -35,7 +35,9 @@
 import org.labkey.api.action.ExportAction;
 import org.labkey.api.action.MutatingApiAction;
 import org.labkey.api.action.ReadOnlyApiAction;
+import org.labkey.api.action.ReturnUrlForm;
 import org.labkey.api.action.SimpleApiJsonForm;
+import org.labkey.api.action.SimpleRedirectAction;
 import org.labkey.api.action.SimpleViewAction;
 import org.labkey.api.action.SpringActionController;
 import org.labkey.api.admin.AbstractFolderContext.ExportType;
@@ -204,10 +206,6 @@
 
 import static org.labkey.api.view.template.WarningService.SESSION_WARNINGS_BANNER_KEY;
 
-/**
- * User: jeckels
- * Date: Jan 4, 2007
- */
 public class CoreController extends SpringActionController
 {
     private static final Map<Container, Content> _customStylesheetCache = new ConcurrentHashMap<>();
@@ -2908,4 +2906,20 @@ public void setProvider(String provider)
 
     }
 
+    // Called by various client components to ensure safe redirects, GitHub Issue #1023. This action redirects to
+    // local URLs only, never to an external site, even if the host is on the "Allowed External Redirect Hosts" list.
+    // Why is this safe? First, ActionURL is guaranteed to be a local URL (schema, host, and port are always taken
+    // from local AppProps, even if an absolute URL is requested via getURIString() or similar). Second,
+    // SimpleRedirectAction throws RedirectException which also guarantees local redirects (instances of that class
+    // always use getLocalURIString()).
+    @SuppressWarnings("unused")
+    @RequiresNoPermission
+    public static class SafeRedirectAction extends SimpleRedirectAction<ReturnUrlForm>
+    {
+        @Override
+        public ActionURL getRedirectURL(ReturnUrlForm form) throws Exception
+        {
+            return form.getReturnActionURL(AppProps.getInstance().getHomePageActionURL());
+        }
+    }
 }

core/src/org/labkey/core/analytics/AnalyticsServiceImpl.java

@@ -50,6 +50,7 @@ public class AnalyticsServiceImpl implements AnalyticsService
 {
     private static final String SEPARATOR = ",";
     private static final String GOOGLE_TAG_MANAGER_URL = "https://www.googletagmanager.com";
+    private static final String GOOGLE_URL = "https://www.google.com";
     private static final String ANALYTICS_CSP_KEY = AnalyticsServiceImpl.class.getName();
 
     public static AnalyticsServiceImpl get()
@@ -123,8 +124,9 @@ public void resetCSP()
 
         if (getTrackingStatus().contains(TrackingStatus.ga4FullUrl))
         {
-            ContentSecurityPolicyFilter.registerAllowedSources(ANALYTICS_CSP_KEY, Directive.Connection, "https://*.googletagmanager.com", "https://*.google-analytics.com", "https://*.analytics.google.com");
-            ContentSecurityPolicyFilter.registerAllowedSources(ANALYTICS_CSP_KEY, Directive.Image, "https://www.googletagmanager.com");
+            // Per https://developers.google.com/tag-platform/security/guides/csp (plus other variants we have seen in the wild)
+            ContentSecurityPolicyFilter.registerAllowedSources(ANALYTICS_CSP_KEY, Directive.Connection, "https://*.googletagmanager.com", "https://*.google-analytics.com", "https://*.analytics.google.com", GOOGLE_URL);
+            ContentSecurityPolicyFilter.registerAllowedSources(ANALYTICS_CSP_KEY, Directive.Image, GOOGLE_TAG_MANAGER_URL);
         }
     }
 

devtools/test/src/org/labkey/test/tests/devtools/AuthenticationProviderReorderTest.java

@@ -66,7 +66,7 @@ public void testReorderConfigurations()
 
         signOut();
         assertSsoLinkOrder(config_uncool, config_cool);
-        clickAndWait(Locator.button("Sign In"));
+        clickAndWait(Locator.linkWithText("Sign In"));
         assertSsoLinkOrder(config_uncool, config_cool);
         simpleSignIn();
 
@@ -79,7 +79,7 @@ public void testReorderConfigurations()
 
         signOut();
         assertSsoLinkOrder(config_cool, config_uncool);
-        clickAndWait(Locator.button("Sign In"));
+        clickAndWait(Locator.linkWithText("Sign In"));
         assertSsoLinkOrder(config_cool, config_uncool);
         simpleSignIn();
     }

experiment/package.json

@@ -13,7 +13,7 @@
     "test-integration": "cross-env NODE_ENV=test jest --ci --runInBand -c test/js/jest.config.integration.js"
   },
   "dependencies": {
-    "@labkey/components": "7.40.0"
+    "@labkey/components": "7.40.1"
   },
   "devDependencies": {
     "@labkey/build": "9.1.4",

pipeline/package.json

@@ -14,7 +14,7 @@
     "build-prod": "npm run clean && cross-env NODE_ENV=production PROD_SOURCE_MAP=source-map webpack --config node_modules/@labkey/build/webpack/prod.config.js --color --progress --profile"
   },
   "dependencies": {
-    "@labkey/components": "7.40.0"
+    "@labkey/components": "7.40.1"
   },
   "devDependencies": {
     "@labkey/build": "9.1.4",

query/src/org/labkey/query/sql/QIfDefined.java

@@ -48,7 +48,6 @@ void setQuerySelect(QuerySelect select)
     @Override
     public FieldKey getFieldKey()
     {
-        assert null != select;
         return ((QExpr)getFirstChild()).getFieldKey();
     }
 

study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java

@@ -27,6 +27,7 @@
 import org.labkey.test.TestFileUtils;
 import org.labkey.test.WebTestHelper;
 import org.labkey.test.categories.Daily;
+import org.labkey.test.components.DomainDesignerPage;
 import org.labkey.test.components.LookAndFeelScatterPlot;
 import org.labkey.test.components.LookAndFeelTimeChart;
 import org.labkey.test.components.domain.DomainFormPanel;
@@ -782,4 +783,22 @@ protected void clickViewDetailsLink(String reportName)
         waitForElement(link);
         clickAndWait(link);
     }
+
+    @Test // GitHub Issue #1023
+    public void testNoExternalReturnUrlRedirect() throws Exception
+    {
+        // Navigate to domain designer with an external returnUrl. The safeRedirect action
+        // should prevent external redirects, falling back to the local home page instead.
+        String domainDesignerUrl = WebTestHelper.buildURL("study", getProjectName() + "/" + getFolderName(), "defineDatasetType",
+                Map.of("returnUrl", "https://labkey.com"));
+        beginAt(domainDesignerUrl);
+        DomainDesignerPage domainDesignerPage = new DomainDesignerPage(getDriver());
+        domainDesignerPage.fieldsPanel();
+        domainDesignerPage.clickCancel();
+        String postCancelUrl = getDriver().getCurrentUrl();
+        assertFalse("Cancel with an external returnUrl should not navigate to an external site",
+                postCancelUrl.contains("labkey.com"));
+        assertTrue("Cancel with an external returnUrl should redirect to a local LabKey page instead of: " + postCancelUrl,
+                WebTestHelper.isTestServerUrl(postCancelUrl));
+    }
 }